From 38f65f6847d3431d74d2d89e5b375c725260e4c4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Blin?=
 <sebastien.blin@savoirfairelinux.com>
Date: Fri, 21 Apr 2023 11:00:56 -0400
Subject: [PATCH] conversation_module: fix banned detection

If a member is removed from a conversation, we want to check this,
not only the device. Else the connection may be accepted for an
unwanted operation.

Change-Id: Ib417ffb5ed711fdcb5c629b78abf5d1c10d7c73d
---
 src/jamidht/conversation_channel_handler.cpp |  4 ++--
 src/jamidht/conversation_module.cpp          | 20 +++++++++++++++-----
 src/jamidht/conversation_module.h            |  6 +++---
 src/jamidht/jamiaccount.cpp                  |  2 +-
 src/jamidht/swarm/swarm_channel_handler.cpp  |  4 ++--
 5 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/src/jamidht/conversation_channel_handler.cpp b/src/jamidht/conversation_channel_handler.cpp
index ca0405cb4a..6c295de031 100644
--- a/src/jamidht/conversation_channel_handler.cpp
+++ b/src/jamidht/conversation_channel_handler.cpp
@@ -56,8 +56,8 @@ ConversationChannelHandler::onRequest(const std::shared_ptr<dht::crypto::Certifi
 
     if (auto acc = account_.lock())
         if (auto convModule = acc->convModule()) {
-            auto res = !convModule->isBannedDevice(conversationId,
-                                                   cert->issuer->getLongId().toString());
+            auto res = !convModule->isBanned(conversationId, cert->issuer->getId().toString());
+            res &= !convModule->isBanned(conversationId, cert->getLongId().toString());
             return res;
         }
     return false;
diff --git a/src/jamidht/conversation_module.cpp b/src/jamidht/conversation_module.cpp
index 84f0e06cd5..99a44d41cf 100644
--- a/src/jamidht/conversation_module.cpp
+++ b/src/jamidht/conversation_module.cpp
@@ -2219,12 +2219,22 @@ ConversationModule::conversationVCard(const std::string& conversationId) const
 }
 
 bool
-ConversationModule::isBannedDevice(const std::string& convId, const std::string& deviceId) const
+ConversationModule::isBanned(const std::string& convId, const std::string& uri) const
 {
-    std::unique_lock<std::mutex> lk(pimpl_->conversationsMtx_);
-    auto conversation = pimpl_->conversations_.find(convId);
-    return conversation == pimpl_->conversations_.end() || !conversation->second
-           || conversation->second->isBanned(deviceId);
+    {
+        std::unique_lock<std::mutex> lk(pimpl_->conversationsMtx_);
+        auto conversation = pimpl_->conversations_.find(convId);
+        if (conversation == pimpl_->conversations_.end() || !conversation->second)
+            return true;
+        if (conversation->second->mode() != ConversationMode::ONE_TO_ONE) {
+            return conversation->second->isBanned(uri);
+        }
+    }
+    // If 1:1 we check the certificate status
+    if (auto acc = pimpl_->account_.lock()) {
+        return acc->accountManager()->getCertificateStatus(uri) == tls::TrustStore::PermissionStatus::BANNED;
+    }
+    return true;
 }
 
 void
diff --git a/src/jamidht/conversation_module.h b/src/jamidht/conversation_module.h
index 9b7d68fa58..59d28770c0 100644
--- a/src/jamidht/conversation_module.h
+++ b/src/jamidht/conversation_module.h
@@ -380,11 +380,11 @@ public:
     std::vector<uint8_t> conversationVCard(const std::string& conversationId) const;
 
     /**
-     * Return if a device is banned from a conversation
+     * Return if a device or member is banned from a conversation
      * @param convId
-     * @param deviceId
+     * @param uri
      */
-    bool isBannedDevice(const std::string& convId, const std::string& deviceId) const;
+    bool isBanned(const std::string& convId, const std::string& uri) const;
 
     // Remove swarm
     /**
diff --git a/src/jamidht/jamiaccount.cpp b/src/jamidht/jamiaccount.cpp
index 9837ef551e..8611a9cac3 100644
--- a/src/jamidht/jamiaccount.cpp
+++ b/src/jamidht/jamiaccount.cpp
@@ -2038,7 +2038,7 @@ JamiAccount::doRegister_()
                     }
 
                     // Check if pull from banned device
-                    if (convModule()->isBannedDevice(conversationId, remoteDevice)) {
+                    if (convModule()->isBanned(conversationId, remoteDevice)) {
                         JAMI_WARNING(
                             "[Account {:s}] Git server requested for conversation {:s}, but the "
                             "device is "
diff --git a/src/jamidht/swarm/swarm_channel_handler.cpp b/src/jamidht/swarm/swarm_channel_handler.cpp
index 625795899a..3608c4170d 100644
--- a/src/jamidht/swarm/swarm_channel_handler.cpp
+++ b/src/jamidht/swarm/swarm_channel_handler.cpp
@@ -57,8 +57,8 @@ SwarmChannelHandler::onRequest(const std::shared_ptr<dht::crypto::Certificate>&
     auto conversationId = name.substr(sep + 1);
     if (auto acc = account_.lock())
         if (auto convModule = acc->convModule()) {
-            auto res = !convModule->isBannedDevice(conversationId,
-                                                   cert->issuer->getLongId().toString());
+            auto res = !convModule->isBanned(conversationId, cert->issuer->getId().toString());
+            res &= !convModule->isBanned(conversationId, cert->getLongId().toString());
             return res;
         }
     return false;
-- 
GitLab