From 54bac6bdfd50d98b8613596f93c4487044459d87 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Blin?=
<sebastien.blin@savoirfairelinux.com>
Date: Fri, 11 Aug 2023 14:43:58 -0400
Subject: [PATCH] Revert "jamiaccount: do not open swarm channels with banned
devices"
This reverts commit eb641097e3bae1f0705452d26ac5372a8239217e.
Reason for revert: no patch in dhtnet
Change-Id: I11d0ea6bca9b570073e0d41c31144572cecbd6f8
---
contrib/src/dhtnet/rules.mak | 1 -
contrib/src/dhtnet/test.patch | 43 --------
src/jamidht/account_manager.cpp | 2 +-
src/jamidht/jamiaccount.cpp | 2 +-
test/unitTest/certstore.cpp | 25 -----
.../conversation/conversationRequest.cpp | 99 -------------------
6 files changed, 2 insertions(+), 170 deletions(-)
delete mode 100644 contrib/src/dhtnet/test.patch
diff --git a/contrib/src/dhtnet/rules.mak b/contrib/src/dhtnet/rules.mak
index b158b70d1a..5f62ffd7d4 100644
--- a/contrib/src/dhtnet/rules.mak
+++ b/contrib/src/dhtnet/rules.mak
@@ -20,7 +20,6 @@ dhtnet: dhtnet-$(DHTNET_VERSION).tar.gz
mkdir -p $(UNPACK_DIR)
$(UNPACK) -C $(UNPACK_DIR)
$(APPLY) $(SRC)/dhtnet/libjami-testable.patch
- $(APPLY) $(SRC)/dhtnet/test.patch
$(MOVE)
.dhtnet: dhtnet toolchain.cmake .sum-dhtnet
diff --git a/contrib/src/dhtnet/test.patch b/contrib/src/dhtnet/test.patch
deleted file mode 100644
index 63ec14e2a6..0000000000
--- a/contrib/src/dhtnet/test.patch
+++ /dev/null
@@ -1,43 +0,0 @@
- src/security/certstore.cpp | 28 +++++++++++++++++++++-------
- 1 file changed, 21 insertions(+), 7 deletions(-)
-
-diff --git a/src/security/certstore.cpp b/src/security/certstore.cpp
-index 9b6bb96..2a3fa8a 100644
---- a/src/security/certstore.cpp
-+++ b/src/security/certstore.cpp
-@@ -591,14 +591,28 @@ TrustStore::PermissionStatus
- TrustStore::getCertificateStatus(const std::string& cert_id) const
- {
- std::lock_guard<std::recursive_mutex> lk(mutex_);
-- auto s = certStatus_.find(cert_id);
-- if (s == std::end(certStatus_)) {
-- auto us = unknownCertStatus_.find(cert_id);
-- if (us == std::end(unknownCertStatus_))
-- return PermissionStatus::UNDEFINED;
-- return us->second.allowed ? PermissionStatus::ALLOWED : PermissionStatus::BANNED;
-+ auto cert = certStore_.getCertificate(cert_id);
-+ if (!cert)
-+ return PermissionStatus::UNDEFINED;
-+ auto banned = false;
-+ auto unknown = true;
-+ while (cert) {
-+ auto s = certStatus_.find(cert->getId().toString());
-+ unknown &= s == std::end(certStatus_);
-+ if (s != std::end(certStatus_) && !s->second.second.allowed) {
-+ return PermissionStatus::BANNED;
-+ } else {
-+ auto us = unknownCertStatus_.find(cert->getId().toString());
-+ unknown &= us == std::end(unknownCertStatus_);
-+ if (!us->second.allowed)
-+ return PermissionStatus::BANNED;
-+ }
-+ if (cert->getUID() == cert->getIssuerUID())
-+ break;
-+ cert = certStore_.getCertificate(cert->getIssuerUID());
- }
-- return s->second.second.allowed ? PermissionStatus::ALLOWED : PermissionStatus::BANNED;
-+
-+ return unknown ? PermissionStatus::UNDEFINED : PermissionStatus::ALLOWED;
- }
-
- std::vector<std::string>
diff --git a/src/jamidht/account_manager.cpp b/src/jamidht/account_manager.cpp
index 47512cd36b..3559ca1fa9 100644
--- a/src/jamidht/account_manager.cpp
+++ b/src/jamidht/account_manager.cpp
@@ -112,7 +112,7 @@ AccountManager::loadIdentity(const std::string& accountId,
const std::string& key_path,
const std::string& key_pwd) const
{
- // Return to avoid unnecessary log if certificate or key is missing. Example case: when
+ // Return to avoid unnecessary log if certificate or key is missing. Example case: when
// importing an account when the certificate has not been unpacked from the archive.
if (crt_path.empty() or key_path.empty())
return {};
diff --git a/src/jamidht/jamiaccount.cpp b/src/jamidht/jamiaccount.cpp
index 1cbd04cff1..ea0fc7483a 100644
--- a/src/jamidht/jamiaccount.cpp
+++ b/src/jamidht/jamiaccount.cpp
@@ -2166,7 +2166,7 @@ JamiAccount::convModule()
if (!shared)
return;
std::lock_guard<std::mutex> lkCM(shared->connManagerMtx_);
- if (!shared->connectionManager_ || shared->convModule()->isBanned(convId, deviceId)) {
+ if (!shared->connectionManager_) {
Manager::instance().ioContext()->post([cb] { cb({}); });
return;
}
diff --git a/test/unitTest/certstore.cpp b/test/unitTest/certstore.cpp
index c6f1f31477..a16f5320c1 100644
--- a/test/unitTest/certstore.cpp
+++ b/test/unitTest/certstore.cpp
@@ -56,12 +56,10 @@ public:
private:
void trustStoreTest();
void getCertificateWithSplitted();
- void testBannedParent();
CPPUNIT_TEST_SUITE(CertStoreTest);
CPPUNIT_TEST(trustStoreTest);
CPPUNIT_TEST(getCertificateWithSplitted);
- CPPUNIT_TEST(testBannedParent);
CPPUNIT_TEST_SUITE_END();
};
@@ -216,29 +214,6 @@ CertStoreTest::getCertificateWithSplitted()
&& fullCert->issuer->issuer->getUID() == caCert->getUID());
}
-void
-CertStoreTest::testBannedParent()
-{
- auto aliceAccount = Manager::instance().getAccount<JamiAccount>(aliceId);
-
- auto ca = dht::crypto::generateIdentity("test CA");
- auto account = dht::crypto::generateIdentity("test account", ca, 4096, true);
- auto device = dht::crypto::generateIdentity("test device", account);
- auto device2 = dht::crypto::generateIdentity("test device 2", account);
- auto id = ca.second->getId().toString();
- auto pinned = aliceAccount->certStore().getPinnedCertificates();
- CPPUNIT_ASSERT(std::find_if(pinned.begin(), pinned.end(), [&](auto v) { return v == id; })
- == pinned.end());
-
- // Ban account
- aliceAccount->setCertificateStatus(account.second, dhtnet::tls::TrustStore::PermissionStatus::BANNED);
- CPPUNIT_ASSERT(aliceAccount->accountManager()->getCertificateStatus(account.second->getId().toString())
- == dhtnet::tls::TrustStore::PermissionStatus::BANNED);
- CPPUNIT_ASSERT(not aliceAccount->accountManager()->isAllowed(*account.second));
- CPPUNIT_ASSERT(not aliceAccount->accountManager()->isAllowed(*device2.second));
- CPPUNIT_ASSERT(not aliceAccount->accountManager()->isAllowed(*device.second));
-}
-
} // namespace test
} // namespace jami
diff --git a/test/unitTest/conversation/conversationRequest.cpp b/test/unitTest/conversation/conversationRequest.cpp
index bc69699fa2..09b7fee580 100644
--- a/test/unitTest/conversation/conversationRequest.cpp
+++ b/test/unitTest/conversation/conversationRequest.cpp
@@ -68,7 +68,6 @@ public:
void testRemoveSelfDoesntRemoveConversation();
void testRemoveConversationUpdateContactDetails();
void testBanContact();
- void testBanContactRestartAccount();
void testBanContactRemoveTrustRequest();
void testAddOfflineContactThenConnect();
void testDeclineTrustRequestDoNotGenerateAnother();
@@ -98,7 +97,6 @@ private:
CPPUNIT_TEST(testRemoveSelfDoesntRemoveConversation);
CPPUNIT_TEST(testRemoveConversationUpdateContactDetails);
CPPUNIT_TEST(testBanContact);
- CPPUNIT_TEST(testBanContactRestartAccount);
CPPUNIT_TEST(testBanContactRemoveTrustRequest);
CPPUNIT_TEST(testAddOfflineContactThenConnect);
CPPUNIT_TEST(testDeclineTrustRequestDoNotGenerateAnother);
@@ -987,103 +985,6 @@ ConversationRequestTest::testBanContact()
CPPUNIT_ASSERT(fileutils::isDirectory(repoPath));
}
-
-void
-ConversationRequestTest::testBanContactRestartAccount()
-{
- auto aliceAccount = Manager::instance().getAccount<JamiAccount>(aliceId);
- auto bobAccount = Manager::instance().getAccount<JamiAccount>(bobId);
- auto bobUri = bobAccount->getUsername();
- auto aliceUri = aliceAccount->getUsername();
- std::mutex mtx;
- std::unique_lock<std::mutex> lk {mtx};
- std::condition_variable cv;
- std::map<std::string, std::shared_ptr<libjami::CallbackWrapperBase>> confHandlers;
- bool conversationReady = false, requestReceived = false, memberMessageGenerated = false;
- std::string convId = "";
- confHandlers.insert(
- libjami::exportable_callback<libjami::ConfigurationSignal::IncomingTrustRequest>(
- [&](const std::string& account_id,
- const std::string& /*from*/,
- const std::string& /*conversationId*/,
- const std::vector<uint8_t>& /*payload*/,
- time_t /*received*/) {
- if (account_id == bobId)
- requestReceived = true;
- cv.notify_one();
- }));
- confHandlers.insert(libjami::exportable_callback<libjami::ConversationSignal::ConversationReady>(
- [&](const std::string& accountId, const std::string& conversationId) {
- if (accountId == aliceId) {
- convId = conversationId;
- } else if (accountId == bobId) {
- conversationReady = true;
- }
- cv.notify_one();
- }));
- confHandlers.insert(libjami::exportable_callback<libjami::ConversationSignal::MessageReceived>(
- [&](const std::string& accountId,
- const std::string& conversationId,
- std::map<std::string, std::string> message) {
- if (accountId == aliceId && conversationId == convId && message["type"] == "member") {
- memberMessageGenerated = true;
- }
- cv.notify_one();
- }));
- bool contactRemoved = false;
- confHandlers.insert(libjami::exportable_callback<libjami::ConfigurationSignal::ContactRemoved>(
- [&](const std::string& accountId, const std::string& uri, bool) {
- if (accountId == bobId && uri == aliceUri) {
- contactRemoved = true;
- }
- cv.notify_one();
- }));
- auto bobConnected = false;
- confHandlers.insert(
- libjami::exportable_callback<libjami::ConfigurationSignal::VolatileDetailsChanged>(
- [&](const std::string&, const std::map<std::string, std::string>&) {
- auto details = bobAccount->getVolatileAccountDetails();
- auto daemonStatus = details[libjami::Account::ConfProperties::Registration::STATUS];
- if (daemonStatus == "REGISTERED") {
- bobConnected = true;
- } else if (daemonStatus == "UNREGISTERED") {
- bobConnected = false;
- }
- cv.notify_one();
- }));
- libjami::registerSignalHandlers(confHandlers);
-
- aliceAccount->addContact(bobUri);
- aliceAccount->sendTrustRequest(bobUri, {});
- CPPUNIT_ASSERT(cv.wait_for(lk, 5s, [&]() { return !convId.empty(); }));
- // Check created files
- CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return requestReceived; }));
- memberMessageGenerated = false;
- CPPUNIT_ASSERT(bobAccount->acceptTrustRequest(aliceUri));
- CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return memberMessageGenerated; }));
-
- memberMessageGenerated = false;
- bobAccount->removeContact(aliceUri, true);
- CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return contactRemoved; }));
- std::this_thread::sleep_for(10s); // Wait a bit to ensure that everything is updated
- bobConnected = true;
- Manager::instance().sendRegister(bobId, false);
- CPPUNIT_ASSERT(cv.wait_for(lk, 30s, [&]() { return !bobConnected; }));
- std::this_thread::sleep_for(5s); // Wait a bit to ensure that everything is updated
-
- // Connect bob, it will trigger the bootstrap
- auto statusBootstrap = Conversation::BootstrapStatus::SUCCESS;
- // alice is banned so bootstrap MUST fail
- bobAccount->convModule()->onBootstrapStatus(
- [&](std::string /*convId*/, Conversation::BootstrapStatus status) {
- statusBootstrap = status;
- cv.notify_one();
- });
- Manager::instance().sendRegister(bobId, true);
- CPPUNIT_ASSERT(cv.wait_for(lk, 60s, [&]() { return statusBootstrap == Conversation::BootstrapStatus::FAILED; }));
-
-}
-
void
ConversationRequestTest::testBanContactRemoveTrustRequest()
{
--
GitLab