Merge pull request #5510 from randombit/jack/certstore-search

Improve search and indexing in Certificate_Store implementations

Author: randombit

src/bogo_shim/bogo_shim.cpp

@@ -1512,7 +1512,7 @@ class Shim_Callbacks final : public Botan::TLS::Callbacks {
 
          if(!cert_chain.empty() && cert_chain.front().is_self_signed()) {
             for(auto* const roots : trusted_roots) {
-               if(roots->certificate_known(cert_chain.front())) {
+               if(roots->contains(cert_chain.front())) {
                   shim_log("Trusting self-signed certificate");
                   return;
                }

src/lib/x509/certstor.cpp

@@ -9,8 +9,8 @@
 #include <botan/certstor.h>
 
 #include <botan/asn1_time.h>
+#include <botan/assert.h>
 #include <botan/data_src.h>
-#include <botan/hash.h>
 #include <botan/pkix_types.h>
 #include <botan/internal/filesystem.h>
 #include <algorithm>
@@ -19,7 +19,11 @@ namespace Botan {
 
 Certificate_Store::~Certificate_Store() = default;
 
-bool Certificate_Store::certificate_known(const X509_Certificate& searching) const {
+bool Certificate_Store::certificate_known(const X509_Certificate& cert) const {
+   return contains(cert);
+}
+
+bool Certificate_Store::contains(const X509_Certificate& searching) const {
    for(const auto& cert : find_all_certs(searching.subject_dn(), searching.subject_key_id())) {
       if(cert == searching) {
          return true;
@@ -46,13 +50,13 @@ std::optional<X509_CRL> Certificate_Store::find_crl_for(const X509_Certificate&
 }
 
 void Certificate_Store_In_Memory::add_certificate(const X509_Certificate& cert) {
-   for(const auto& c : m_certs) {
-      if(c == cert) {
-         return;
-      }
+   const auto tag = cert.tag();
+   if(!m_cert_tags.contains(tag)) {
+      m_cert_tags.insert(tag);
+      const size_t idx = m_certs.size();
+      m_certs.push_back(cert);
+      m_dn_to_indices[cert.subject_dn()].push_back(idx);
    }
-
-   m_certs.push_back(cert);
 }
 
 std::vector<X509_DN> Certificate_Store_In_Memory::all_subjects() const {
@@ -66,19 +70,23 @@ std::vector<X509_DN> Certificate_Store_In_Memory::all_subjects() const {
 
 std::optional<X509_Certificate> Certificate_Store_In_Memory::find_cert(const X509_DN& subject_dn,
                                                                        const std::vector<uint8_t>& key_id) const {
-   for(const auto& cert : m_certs) {
-      // Only compare key ids if set in both call and in the cert
+   const auto it = m_dn_to_indices.find(subject_dn);
+   if(it == m_dn_to_indices.end()) {
+      return std::nullopt;
+   }
+
+   for(const size_t idx : it->second) {
+      const auto& cert = m_certs[idx];
+      BOTAN_ASSERT_NOMSG(cert.subject_dn() == subject_dn);
+
       if(!key_id.empty()) {
          const std::vector<uint8_t>& skid = cert.subject_key_id();
-
          if(!skid.empty() && skid != key_id) {  // no match
             continue;
          }
       }
 
-      if(cert.subject_dn() == subject_dn) {
-         return cert;
-      }
+      return cert;
    }
 
    return std::nullopt;
@@ -88,18 +96,23 @@ std::vector<X509_Certificate> Certificate_Store_In_Memory::find_all_certs(const
                                                                           const std::vector<uint8_t>& key_id) const {
    std::vector<X509_Certificate> matches;
 
-   for(const auto& cert : m_certs) {
+   const auto it = m_dn_to_indices.find(subject_dn);
+   if(it == m_dn_to_indices.end()) {
+      return matches;
+   }
+
+   for(const size_t idx : it->second) {
+      const auto& cert = m_certs[idx];
+      BOTAN_ASSERT_NOMSG(cert.subject_dn() == subject_dn);
+
       if(!key_id.empty()) {
          const std::vector<uint8_t>& skid = cert.subject_key_id();
-
          if(!skid.empty() && skid != key_id) {  // no match
             continue;
          }
       }
 
-      if(cert.subject_dn() == subject_dn) {
-         matches.push_back(cert);
-      }
+      matches.push_back(cert);
    }
 
    return matches;
@@ -111,11 +124,8 @@ std::optional<X509_Certificate> Certificate_Store_In_Memory::find_cert_by_pubkey
       throw Invalid_Argument("Certificate_Store_In_Memory::find_cert_by_pubkey_sha1 invalid hash");
    }
 
-   auto hash = HashFunction::create_or_throw("SHA-1");
-
    for(const auto& cert : m_certs) {
-      hash->update(cert.subject_public_key_bitstring());
-      if(key_hash == hash->final_stdvec()) {  //final_stdvec also clears the hash to initial state
+      if(key_hash == cert.subject_public_key_bitstring_sha1()) {
          return cert;
       }
    }
@@ -129,11 +139,8 @@ std::optional<X509_Certificate> Certificate_Store_In_Memory::find_cert_by_raw_su
       throw Invalid_Argument("Certificate_Store_In_Memory::find_cert_by_raw_subject_dn_sha256 invalid hash");
    }
 
-   auto hash = HashFunction::create_or_throw("SHA-256");
-
    for(const auto& cert : m_certs) {
-      hash->update(cert.raw_subject_dn());
-      if(subject_hash == hash->final_stdvec()) {  //final_stdvec also clears the hash to initial state
+      if(subject_hash == cert.raw_subject_dn_sha256()) {
          return cert;
       }
    }
@@ -190,6 +197,10 @@ std::optional<X509_CRL> Certificate_Store_In_Memory::find_crl_for(const X509_Cer
    return {};
 }
 
+bool Certificate_Store_In_Memory::contains(const X509_Certificate& cert) const {
+   return m_cert_tags.contains(cert.tag());
+}
+
 Certificate_Store_In_Memory::Certificate_Store_In_Memory(const X509_Certificate& cert) {
    add_certificate(cert);
 }
@@ -216,8 +227,7 @@ Certificate_Store_In_Memory::Certificate_Store_In_Memory(std::string_view dir) {
          DataSource_Stream src(cert_file, true);
          while(!src.end_of_data()) {
             try {
-               const X509_Certificate cert(src);
-               m_certs.push_back(cert);
+               add_certificate(X509_Certificate(src));
             } catch(std::exception&) {
                // stop searching for other certificate at first exception
                break;

src/lib/x509/certstor.h

@@ -8,9 +8,12 @@
 #ifndef BOTAN_CERT_STORE_H_
 #define BOTAN_CERT_STORE_H_
 
+#include <botan/pkix_types.h>
 #include <botan/x509_crl.h>
 #include <botan/x509cert.h>
+#include <map>
 #include <optional>
+#include <set>
 
 namespace Botan {
 
@@ -76,6 +79,13 @@ class BOTAN_PUBLIC_API(2, 0) Certificate_Store /* NOLINT(*-special-member-functi
       /**
       * @return whether this certificate is contained within the store
       * @param cert certificate to be searched
+      *
+      * Default implementation uses find_all_certs
+      */
+      virtual bool contains(const X509_Certificate& cert) const;
+
+      /**
+      * Old version of contains
       */
       bool certificate_known(const X509_Certificate& cert) const;
 
@@ -155,9 +165,12 @@ class BOTAN_PUBLIC_API(2, 0) Certificate_Store_In_Memory final : public Certific
       */
       std::optional<X509_CRL> find_crl_for(const X509_Certificate& subject) const override;
 
+      bool contains(const X509_Certificate& cert) const override;
+
    private:
-      // TODO: Add indexing on the DN and key id to avoid linear search
       std::vector<X509_Certificate> m_certs;
+      std::set<X509_Certificate::Tag> m_cert_tags;
+      std::map<X509_DN, std::vector<size_t>> m_dn_to_indices;
       std::vector<X509_CRL> m_crls;
 };
 

src/lib/x509/certstor_flatfile/certstor_flatfile.cpp

@@ -55,11 +55,17 @@ Flatfile_Certificate_Store::Flatfile_Certificate_Store(std::string_view file, bo
       * but we cannot fix the trust store. So instead just ignore any such certificate.
       */
       if(cert.is_self_signed() && cert.is_CA_cert()) {
-         m_all_subjects.push_back(cert.subject_dn());
-         m_dn_to_cert[cert.subject_dn()].push_back(cert);
-         m_pubkey_sha1_to_cert.emplace(cert.subject_public_key_bitstring_sha1(), cert);
-         m_subject_dn_sha256_to_cert.emplace(cert.raw_subject_dn_sha256(), cert);
-         m_issuer_dn_to_cert[cert.issuer_dn()].push_back(cert);
+         const auto tag = cert.tag();
+
+         // dedup
+         if(!m_cert_tags.contains(tag)) {
+            m_cert_tags.insert(tag);
+            m_all_subjects.push_back(cert.subject_dn());
+            m_dn_to_cert[cert.subject_dn()].push_back(cert);
+            m_pubkey_sha1_to_cert.emplace(cert.subject_public_key_bitstring_sha1(), cert);
+            m_subject_dn_sha256_to_cert.emplace(cert.raw_subject_dn_sha256(), cert);
+            m_issuer_dn_to_cert[cert.issuer_dn()].push_back(cert);
+         }
       } else if(!ignore_non_ca) {
          throw Invalid_Argument("Flatfile_Certificate_Store received non CA cert " + cert.subject_dn().to_string());
       }
@@ -74,6 +80,10 @@ std::vector<X509_DN> Flatfile_Certificate_Store::all_subjects() const {
    return m_all_subjects;
 }
 
+bool Flatfile_Certificate_Store::contains(const X509_Certificate& cert) const {
+   return m_cert_tags.contains(cert.tag());
+}
+
 std::vector<X509_Certificate> Flatfile_Certificate_Store::find_all_certs(const X509_DN& subject_dn,
                                                                          const std::vector<uint8_t>& key_id) const {
    if(!m_dn_to_cert.contains(subject_dn)) {

src/lib/x509/certstor_flatfile/certstor_flatfile.h

@@ -13,6 +13,7 @@
 #include <botan/pkix_types.h>
 
 #include <map>
+#include <set>
 #include <vector>
 
 namespace Botan {
@@ -68,8 +69,11 @@ class BOTAN_PUBLIC_API(2, 11) Flatfile_Certificate_Store final : public Certific
        */
       std::optional<X509_CRL> find_crl_for(const X509_Certificate& subject) const override;
 
+      bool contains(const X509_Certificate& cert) const override;
+
    private:
       std::vector<X509_DN> m_all_subjects;
+      std::set<X509_Certificate::Tag> m_cert_tags;
       std::map<X509_DN, std::vector<X509_Certificate>> m_dn_to_cert;
       std::map<std::vector<uint8_t>, std::optional<X509_Certificate>> m_pubkey_sha1_to_cert;
       std::map<std::vector<uint8_t>, std::optional<X509_Certificate>> m_subject_dn_sha256_to_cert;

src/lib/x509/certstor_sql/certstor_sql.cpp

@@ -165,6 +165,12 @@ bool Certificate_Store_In_SQL::insert_cert(const X509_Certificate& cert) {
    return true;
 }
 
+bool Certificate_Store_In_SQL::contains(const X509_Certificate& cert) const {
+   auto stmt = m_database->new_statement("SELECT 1 FROM " + m_prefix + "certificates WHERE fingerprint == ?1");
+   stmt->bind(1, cert.fingerprint("SHA-256"));
+   return stmt->step();
+}
+
 bool Certificate_Store_In_SQL::remove_cert(const X509_Certificate& cert) {
    if(!find_cert(cert.subject_dn(), cert.subject_key_id())) {
       return false;

src/lib/x509/certstor_sql/certstor_sql.h

@@ -108,6 +108,8 @@ class BOTAN_PUBLIC_API(2, 0) Certificate_Store_In_SQL : public Certificate_Store
       */
       std::optional<X509_CRL> find_crl_for(const X509_Certificate& issuer) const override;
 
+      bool contains(const X509_Certificate& cert) const override;
+
    private:
       RandomNumberGenerator& m_rng;
       std::shared_ptr<SQL_Database> m_database;

src/lib/x509/x509cert.cpp

@@ -634,14 +634,14 @@ std::unique_ptr<Public_Key> X509_Certificate::load_subject_public_key() const {
    return this->subject_public_key();
 }
 
-std::vector<uint8_t> X509_Certificate::raw_issuer_dn_sha256() const {
+const std::vector<uint8_t>& X509_Certificate::raw_issuer_dn_sha256() const {
    if(data().m_issuer_dn_bits_sha256.empty()) {
       throw Encoding_Error("X509_Certificate::raw_issuer_dn_sha256 called but SHA-256 disabled in build");
    }
    return data().m_issuer_dn_bits_sha256;
 }
 
-std::vector<uint8_t> X509_Certificate::raw_subject_dn_sha256() const {
+const std::vector<uint8_t>& X509_Certificate::raw_subject_dn_sha256() const {
    if(data().m_subject_dn_bits_sha256.empty()) {
       throw Encoding_Error("X509_Certificate::raw_subject_dn_sha256 called but SHA-256 disabled in build");
    }

src/lib/x509/x509cert.h

@@ -126,7 +126,7 @@ class BOTAN_PUBLIC_API(2, 0) X509_Certificate : public X509_Object {
       /**
       * SHA-256 of Raw issuer DN
       */
-      std::vector<uint8_t> raw_issuer_dn_sha256() const;
+      const std::vector<uint8_t>& raw_issuer_dn_sha256() const;
 
       /**
       * Raw subject DN
@@ -136,7 +136,7 @@ class BOTAN_PUBLIC_API(2, 0) X509_Certificate : public X509_Object {
       /**
       * SHA-256 of Raw subject DN
       */
-      std::vector<uint8_t> raw_subject_dn_sha256() const;
+      const std::vector<uint8_t>& raw_subject_dn_sha256() const;
 
       /**
       * Get the notBefore of the certificate as X509_Time

src/lib/x509/x509path.cpp

@@ -272,7 +272,7 @@ Certificate_Status_Code verify_ocsp_signing_cert(const X509_Certificate& signing
    //
    //    1. Matches a local configuration of OCSP signing authority
    //       for the certificate in question, or
-   if(restrictions.trusted_ocsp_responders()->certificate_known(signing_cert)) {
+   if(restrictions.trusted_ocsp_responders()->contains(signing_cert)) {
       return Certificate_Status_Code::OK;
    }
 
@@ -691,7 +691,7 @@ Certificate_Status_Code PKIX::build_all_certificate_paths(std::vector<std::vecto
 
    auto cert_in_any_trusted_store = [&](const X509_Certificate& cert) {
       return std::ranges::any_of(trusted_certstores,
-                                 [&](const Certificate_Store* store) { return store->certificate_known(cert); });
+                                 [&](const Certificate_Store* store) { return store->contains(cert); });
    };
 
    /*