diff --git a/contrib/src/dhtnet/rules.mak b/contrib/src/dhtnet/rules.mak index 5f62ffd7d4423968fbce6ae3a7114d9bfe0b7221..b158b70d1a324a7525f3c0b5e4eca35fce8feaa0 100644 --- a/contrib/src/dhtnet/rules.mak +++ b/contrib/src/dhtnet/rules.mak @@ -20,6 +20,7 @@ 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 new file mode 100644 index 0000000000000000000000000000000000000000..63ec14e2a6e75296c30b8db3ff202f54fb197fc1 --- /dev/null +++ b/contrib/src/dhtnet/test.patch @@ -0,0 +1,43 @@ + 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 3559ca1fa9643c2f4636d679b81fc26288446761..47512cd36bfc6be0cfcae2c6a23de2fea2a725c6 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 ea0fc7483a984ad629f3ce0001d827a9849e7286..1cbd04cff1c59c734e525233b1639903c430b8a9 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_) { + if (!shared->connectionManager_ || shared->convModule()->isBanned(convId, deviceId)) { Manager::instance().ioContext()->post([cb] { cb({}); }); return; } diff --git a/test/unitTest/certstore.cpp b/test/unitTest/certstore.cpp index a16f5320c1e7fd04b6ebb3c9fae82f66a9bf5971..c6f1f31477234456ed55ef9f7b1cae298130e2f5 100644 --- a/test/unitTest/certstore.cpp +++ b/test/unitTest/certstore.cpp @@ -56,10 +56,12 @@ 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(); }; @@ -214,6 +216,29 @@ 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 09b7fee5806bd3ef40b4f3efdd65bfcc78620169..bc69699fa2f0585f2f99c2d3d396e23e9531fc7d 100644 --- a/test/unitTest/conversation/conversationRequest.cpp +++ b/test/unitTest/conversation/conversationRequest.cpp @@ -68,6 +68,7 @@ public: void testRemoveSelfDoesntRemoveConversation(); void testRemoveConversationUpdateContactDetails(); void testBanContact(); + void testBanContactRestartAccount(); void testBanContactRemoveTrustRequest(); void testAddOfflineContactThenConnect(); void testDeclineTrustRequestDoNotGenerateAnother(); @@ -97,6 +98,7 @@ private: CPPUNIT_TEST(testRemoveSelfDoesntRemoveConversation); CPPUNIT_TEST(testRemoveConversationUpdateContactDetails); CPPUNIT_TEST(testBanContact); + CPPUNIT_TEST(testBanContactRestartAccount); CPPUNIT_TEST(testBanContactRemoveTrustRequest); CPPUNIT_TEST(testAddOfflineContactThenConnect); CPPUNIT_TEST(testDeclineTrustRequestDoNotGenerateAnother); @@ -985,6 +987,103 @@ 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() {