From 8abc792a54aef98765d1d524494dffd2b8d626d2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Blin?=
 <sebastien.blin@savoirfairelinux.com>
Date: Fri, 13 Aug 2021 09:14:03 -0400
Subject: [PATCH] certstore: make getCertificate always construct the complete
 cert

Because the certstore can contains a certificate chain splitted
in different parts (eg, in a swarm the device cert is in /devices
and the rest in /members), the certstore should be able to return
the whole certificate.
Moreover, at some places we do the assumption that the cert is
complete (AccountManager::foundPeerDevice which uses cert->issuer)
getCertificate() should only return the certificate if complete.

Change-Id: I1ff2006cb4f63694016e38e5b4a72b6a287c2345
GitLab: https://git.jami.net/savoirfairelinux/ring-project/-/issues/1282
---
 src/security/certstore.cpp  | 31 ++++++++++++++++++++++++-------
 src/security/certstore.h    |  5 ++++-
 test/unitTest/certstore.cpp | 27 +++++++++++++++++++++++++++
 3 files changed, 55 insertions(+), 8 deletions(-)

diff --git a/src/security/certstore.cpp b/src/security/certstore.cpp
index c2ff3563d9..ec338e4098 100644
--- a/src/security/certstore.cpp
+++ b/src/security/certstore.cpp
@@ -143,15 +143,32 @@ CertificateStore::getPinnedCertificates() const
 }
 
 std::shared_ptr<crypto::Certificate>
-CertificateStore::getCertificate(const std::string& k) const
-{
+CertificateStore::getCertificate(const std::string& k)
+{
+    auto getCertificate_ = [this](const std::string& k) -> std::shared_ptr<crypto::Certificate> {
+        auto cit = certs_.find(k);
+        if (cit == certs_.cend())
+            return {};
+        return cit->second;
+    };
     std::unique_lock<std::mutex> l(lock_);
-
-    auto cit = certs_.find(k);
-    if (cit == certs_.cend()) {
-        return {};
+    auto crt = getCertificate_(k);
+    // Check if certificate is complete
+    // If the certificate has been splitted, reconstruct it
+    auto top_issuer = crt;
+    while (top_issuer && top_issuer->getUID() != top_issuer->getIssuerUID()) {
+        if (top_issuer->issuer) {
+            top_issuer = top_issuer->issuer;
+        } else if (auto cert = getCertificate_(top_issuer->getIssuerUID())) {
+            top_issuer->issuer = cert;
+            top_issuer = cert;
+        } else {
+            // In this case, a certificate was not found
+            JAMI_WARN("Incomplete certificate detected %s", k.c_str());
+            break;
+        }
     }
-    return cit->second;
+    return crt;
 }
 
 std::shared_ptr<crypto::Certificate>
diff --git a/src/security/certstore.h b/src/security/certstore.h
index 0057d00aaa..d76f5de12d 100644
--- a/src/security/certstore.h
+++ b/src/security/certstore.h
@@ -53,7 +53,10 @@ public:
     CertificateStore();
 
     std::vector<std::string> getPinnedCertificates() const;
-    std::shared_ptr<crypto::Certificate> getCertificate(const std::string& cert_id) const;
+    /**
+     * Return certificate (with full chain)
+     */
+    std::shared_ptr<crypto::Certificate> getCertificate(const std::string& cert_id);
 
     std::shared_ptr<crypto::Certificate> findCertificateByName(
         const std::string& name, crypto::NameType type = crypto::NameType::UNKNOWN) const;
diff --git a/test/unitTest/certstore.cpp b/test/unitTest/certstore.cpp
index 6e59541e2d..f2fcf21043 100644
--- a/test/unitTest/certstore.cpp
+++ b/test/unitTest/certstore.cpp
@@ -36,9 +36,11 @@ public:
 
 private:
     void trustStoreTest();
+    void getCertificateWithSplitted();
 
     CPPUNIT_TEST_SUITE(CertStoreTest);
     CPPUNIT_TEST(trustStoreTest);
+    CPPUNIT_TEST(getCertificateWithSplitted);
     CPPUNIT_TEST_SUITE_END();
 };
 
@@ -157,6 +159,31 @@ CertStoreTest::trustStoreTest()
                    == 0);
 }
 
+void
+CertStoreTest::getCertificateWithSplitted()
+{
+    jami::tls::TrustStore trustStore;
+    auto& certStore = jami::tls::CertificateStore::instance();
+
+    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 caCert = std::make_shared<dht::crypto::Certificate>(ca.second->toString(false));
+    auto accountCert = std::make_shared<dht::crypto::Certificate>(account.second->toString(false));
+    auto devicePartialCert = std::make_shared<dht::crypto::Certificate>(
+        device.second->toString(false));
+
+    certStore.pinCertificate(caCert);
+    certStore.pinCertificate(accountCert);
+    certStore.pinCertificate(devicePartialCert);
+
+    auto fullCert = certStore.getCertificate(device.second->getId().toString());
+    CPPUNIT_ASSERT(fullCert->issuer && fullCert->issuer->getUID() == accountCert->getUID());
+    CPPUNIT_ASSERT(fullCert->issuer->issuer
+                   && fullCert->issuer->issuer->getUID() == caCert->getUID());
+}
+
 } // namespace test
 } // namespace jami
 
-- 
GitLab