deduplicate certificates in find_all_certs

2 files changed, 221 insertions, 213 deletions

diff --git a/src/lib/x509/certstor_system_windows/certstor_windows.cpp b/src/lib/x509/certstor_system_windows/certstor_windows.cpp
index cde6dd962..086dc82a3 100644
--- a/src/lib/x509/certstor_system_windows/certstor_windows.cpp
+++ b/src/lib/x509/certstor_system_windows/certstor_windows.cpp
@@ -18,12 +18,32 @@
 #include <Windows.h>
 #include <Wincrypt.h>
 
-
 namespace {
+
 const std::array<std::string, 4> cert_store_names{"MY", "Root", "Trust", "CA"};
-}
 
-namespace Botan {
+HCERTSTORE openCertStore(const std::string& cert_store_name)
+   {
+   auto store = CertOpenSystemStore(0, cert_store_name.c_str());
+   if(!store)
+      {
+      throw Botan::Internal_Error(
+         "failed to open windows certificate store '" + cert_store_name +
+         "' (Error Code: " +
+         std::to_string(::GetLastError()) + ")");
+      }
+   return store;
+   }
+
+bool already_contains_certificate(
+   const std::vector<std::shared_ptr<const Botan::X509_Certificate>>& certs, std::shared_ptr<Botan::X509_Certificate> cert)
+   {
+   return std::any_of(certs.begin(), certs.end(),
+                      [&](std::shared_ptr<const Botan::X509_Certificate> c)
+      {
+      return *c == *cert;
+      });
+   }
 
 /**
  * Abstract RAII wrapper for PCCERT_CONTEXT and HCERTSTORE
@@ -34,195 +54,183 @@ namespace Botan {
  */
 template<class T>
 class Handle_Guard
-{
-public:
-    Handle_Guard(T context)
-        : m_context(context)
-    {
-    }
-
-    Handle_Guard(const Handle_Guard<T>& rhs) = delete;
-    Handle_Guard(Handle_Guard<T>&& rhs) :
-        m_context(std::move(rhs.m_context))
-    {
-        rhs.m_context = nullptr;
-    }
-
-    ~Handle_Guard() {
-        close<T>();
-    }
-
-    operator bool() const {
-        return m_context != nullptr;
-    }
-
-    bool assign(T context)
-    {
-        m_context = context;
-        return m_context != nullptr;
-    }
-
-    T& get() {
-        return m_context;
-    }
-
-    const T& get() const {
-        return m_context;
-    }
-
-    T operator->() {
-        return m_context;
-    }
-
-private:
-    template<class T>
-    void close() {
-        static_assert(false, "Handle_Guard is not available for this type");
-    }
-
-    template<>
-    void close<PCCERT_CONTEXT> () {
-        if(m_context)
-        {
+   {
+   public:
+      Handle_Guard(T context)
+         : m_context(context)
+         {
+         }
+
+      Handle_Guard(const Handle_Guard<T>& rhs) = delete;
+      Handle_Guard(Handle_Guard<T>&& rhs) :
+         m_context(std::move(rhs.m_context))
+         {
+         rhs.m_context = nullptr;
+         }
+
+      ~Handle_Guard()
+         {
+         close<T>();
+         }
+
+      operator bool() const
+         {
+         return m_context != nullptr;
+         }
+
+      bool assign(T context)
+         {
+         m_context = context;
+         return m_context != nullptr;
+         }
+
+      T& get()
+         {
+         return m_context;
+         }
+
+      const T& get() const
+         {
+         return m_context;
+         }
+
+      T operator->()
+         {
+         return m_context;
+         }
+
+   private:
+      template<class T>
+      void close()
+         {
+         static_assert(false, "Handle_Guard is not available for this type");
+         }
+
+      template<>
+      void close<PCCERT_CONTEXT> ()
+         {
+         if(m_context)
+            {
             CertFreeCertificateContext(m_context);
-        }
-    }
+            }
+         }
 
-    template<>
-    void close<HCERTSTORE> () {
-        if(m_context)
-        {
+      template<>
+      void close<HCERTSTORE> ()
+         {
+         if(m_context)
+            {
             CertCloseStore(m_context, 0);
-        }
-    }
-
-    T m_context;
-};
-
-Certificate_Store_Windows::Certificate_Store_Windows()
-{}
+            }
+         }
 
-HCERTSTORE openCertStore(const std::string& cert_store_name)
-{
-    auto store = CertOpenSystemStore(0, cert_store_name.c_str());
-    if (!store) {
-        throw Internal_Error(
-            "failed to open windows certificate store '" + cert_store_name +
-            "' (Error Code: " +
-            std::to_string(::GetLastError()) + ")");
-    }
-    return store;
+      T m_context;
+   };
 }
 
-bool already_contains_key_id(
-    const std::vector<std::shared_ptr<const X509_Certificate>>& certs, const std::vector<uint8_t>& key_id)
-{
-    return std::any_of(certs.begin(), certs.end(),
-    [&](std::shared_ptr<const X509_Certificate> c) {
-        return c->subject_key_id() == key_id;
-    });
-}
+namespace Botan {
+Certificate_Store_Windows::Certificate_Store_Windows() {}
 
 std::vector<X509_DN> Certificate_Store_Windows::all_subjects() const
-{
-    std::vector<X509_DN> subject_dns;
-    for (auto &store_name : ::cert_store_names)
-    {
-        Handle_Guard<HCERTSTORE> windows_cert_store = openCertStore(store_name.c_str());
-        Handle_Guard<PCCERT_CONTEXT> cert_context = nullptr;
-
-        // Handle_Guard::assign exchanges the underlying pointer. No RAII is needed here, because the Windows API takes care of
-        // freeing the previous context.
-        while(cert_context.assign(CertEnumCertificatesInStore(windows_cert_store.get(), cert_context.get())))
-        {
-            if (cert_context) {
-                X509_Certificate cert(cert_context->pbCertEncoded, cert_context->cbCertEncoded);
-                subject_dns.push_back(cert.subject_dn());
-            }
-        }
-    }
-
-    return subject_dns;
-}
+   {
+   std::vector<X509_DN> subject_dns;
+   for(auto& store_name : ::cert_store_names)
+      {
+      Handle_Guard<HCERTSTORE> windows_cert_store = openCertStore(store_name.c_str());
+      Handle_Guard<PCCERT_CONTEXT> cert_context = nullptr;
+
+      // Handle_Guard::assign exchanges the underlying pointer. No RAII is needed here, because the Windows API takes care of
+      // freeing the previous context.
+      while(cert_context.assign(CertEnumCertificatesInStore(windows_cert_store.get(), cert_context.get())))
+         {
+         X509_Certificate cert(cert_context->pbCertEncoded, cert_context->cbCertEncoded);
+         subject_dns.push_back(cert.subject_dn());
+         }
+      }
+
+   return subject_dns;
+   }
 
 std::shared_ptr<const X509_Certificate>
-Certificate_Store_Windows::find_cert(const Botan::X509_DN &          subject_dn,
-                                     const std::vector<uint8_t> &key_id) const
-{
-    const auto certs = find_all_certs(subject_dn, key_id);
-    return certs.empty() ? nullptr : certs.front();
-}
+Certificate_Store_Windows::find_cert(const Botan::X509_DN&           subject_dn,
+                                     const std::vector<uint8_t>& key_id) const
+   {
+   const auto certs = find_all_certs(subject_dn, key_id);
+   return certs.empty() ? nullptr : certs.front();
+   }
 
 std::vector<std::shared_ptr<const X509_Certificate>> Certificate_Store_Windows::find_all_certs(
-            const X509_DN& subject_dn,
-            const std::vector<uint8_t>& key_id) const
-{
-    std::vector<uint8_t> dn_data;
-    DER_Encoder encoder(dn_data);
-    subject_dn.encode_into(encoder);
-
-    CERT_NAME_BLOB blob;
-    blob.cbData = static_cast<DWORD>(dn_data.size());
-    blob.pbData = reinterpret_cast<BYTE*>(dn_data.data());
-
-    std::vector<std::shared_ptr<const X509_Certificate>> certs;
-    for (auto &store_name : ::cert_store_names)
-    {
-        Handle_Guard<HCERTSTORE> windows_cert_store = openCertStore(store_name);
-        Handle_Guard<PCCERT_CONTEXT> cert_context = nullptr;
-        while(cert_context.assign(CertFindCertificateInStore(
-                                      windows_cert_store.get(), PKCS_7_ASN_ENCODING | X509_ASN_ENCODING,
-                                      NULL, CERT_FIND_SUBJECT_NAME,
-                                      &blob, cert_context.get())))
-        {
-            auto cert = std::make_shared<X509_Certificate>(cert_context->pbCertEncoded, cert_context->cbCertEncoded);
-            if (key_id.empty() || (cert->subject_key_id() == key_id && !already_contains_key_id(certs, key_id)))
+         const X509_DN& subject_dn,
+         const std::vector<uint8_t>& key_id) const
+   {
+   std::vector<uint8_t> dn_data;
+   DER_Encoder encoder(dn_data);
+   subject_dn.encode_into(encoder);
+
+   CERT_NAME_BLOB blob;
+   blob.cbData = static_cast<DWORD>(dn_data.size());
+   blob.pbData = reinterpret_cast<BYTE*>(dn_data.data());
+
+   std::vector<std::shared_ptr<const X509_Certificate>> certs;
+   for(auto& store_name : ::cert_store_names)
+      {
+      Handle_Guard<HCERTSTORE> windows_cert_store = openCertStore(store_name);
+      Handle_Guard<PCCERT_CONTEXT> cert_context = nullptr;
+      while(cert_context.assign(CertFindCertificateInStore(
+                                   windows_cert_store.get(), PKCS_7_ASN_ENCODING | X509_ASN_ENCODING,
+                                   NULL, CERT_FIND_SUBJECT_NAME,
+                                   &blob, cert_context.get())))
+         {
+         auto cert = std::make_shared<X509_Certificate>(cert_context->pbCertEncoded, cert_context->cbCertEncoded);
+         if(!already_contains_certificate(certs, cert) && (key_id.empty() || cert->subject_key_id() == key_id))
             {
-                certs.push_back(cert);
+            certs.push_back(cert);
             }
-        }
-    }
-    return certs;
-}
+         }
+      }
+   return certs;
+   }
 
 std::shared_ptr<const Botan::X509_Certificate>
 Certificate_Store_Windows::find_cert_by_pubkey_sha1(
-    const std::vector<uint8_t> &key_hash) const
-{
-    if(key_hash.size() != 20)
-    {
-        throw Invalid_Argument("Certificate_Store_Windows::find_cert_by_pubkey_sha1 invalid hash");
-    }
-
-    CRYPT_HASH_BLOB blob;
-    blob.cbData = static_cast<DWORD>(key_hash.size());
-    blob.pbData = const_cast<BYTE*>(key_hash.data());
-
-    for (auto &store_name : ::cert_store_names) {
-        Handle_Guard<HCERTSTORE> windows_cert_store = openCertStore(store_name);
-        Handle_Guard<PCCERT_CONTEXT> cert_context = CertFindCertificateInStore(
-                    windows_cert_store.get(), PKCS_7_ASN_ENCODING | X509_ASN_ENCODING,
-                    0, CERT_FIND_KEY_IDENTIFIER,
-                    &blob, nullptr);
-
-        if (cert_context) {
-            return std::make_shared<X509_Certificate>(cert_context->pbCertEncoded, cert_context->cbCertEncoded);
-        }
-    }
-
-    return nullptr;
-}
+   const std::vector<uint8_t>& key_hash) const
+   {
+   if(key_hash.size() != 20)
+      {
+      throw Invalid_Argument("Certificate_Store_Windows::find_cert_by_pubkey_sha1 invalid hash");
+      }
+
+   CRYPT_HASH_BLOB blob;
+   blob.cbData = static_cast<DWORD>(key_hash.size());
+   blob.pbData = const_cast<BYTE*>(key_hash.data());
+
+   for(auto& store_name : ::cert_store_names)
+      {
+      Handle_Guard<HCERTSTORE> windows_cert_store = openCertStore(store_name);
+      Handle_Guard<PCCERT_CONTEXT> cert_context = CertFindCertificateInStore(
+               windows_cert_store.get(), PKCS_7_ASN_ENCODING | X509_ASN_ENCODING,
+               0, CERT_FIND_KEY_IDENTIFIER,
+               &blob, nullptr);
+
+      if(cert_context)
+         {
+         return std::make_shared<X509_Certificate>(cert_context->pbCertEncoded, cert_context->cbCertEncoded);
+         }
+      }
+
+   return nullptr;
+   }
 
 std::shared_ptr<const X509_Certificate>
 Certificate_Store_Windows::find_cert_by_raw_subject_dn_sha256(const std::vector<uint8_t>& subject_hash) const
-{
-    BOTAN_UNUSED(subject_hash);
-    throw Not_Implemented("Certificate_Store_Windows::find_cert_by_raw_subject_dn_sha256");
-}
+   {
+   BOTAN_UNUSED(subject_hash);
+   throw Not_Implemented("Certificate_Store_Windows::find_cert_by_raw_subject_dn_sha256");
+   }
 
 std::shared_ptr<const X509_CRL> Certificate_Store_Windows::find_crl_for(const X509_Certificate& subject) const
-{
-    BOTAN_UNUSED(subject);
-    throw Not_Implemented("Certificate_Store_Windows::find_crl_for");
-}
+   {
+   BOTAN_UNUSED(subject);
+   throw Not_Implemented("Certificate_Store_Windows::find_crl_for");
+   }
 }
diff --git a/src/lib/x509/certstor_system_windows/certstor_windows.h b/src/lib/x509/certstor_system_windows/certstor_windows.h
index 9013b1b48..15a642b04 100644
--- a/src/lib/x509/certstor_system_windows/certstor_windows.h
+++ b/src/lib/x509/certstor_system_windows/certstor_windows.h
@@ -16,54 +16,54 @@ namespace Botan {
 * Certificate Store that is backed by a file of PEMs of trusted CAs.
 */
 class BOTAN_PUBLIC_API(2, 11) Certificate_Store_Windows final : public Certificate_Store
-{
-public:
-    Certificate_Store_Windows();
+   {
+   public:
+      Certificate_Store_Windows();
 
-    Certificate_Store_Windows(const Certificate_Store_Windows&) = default;
-    Certificate_Store_Windows(Certificate_Store_Windows&&) = default;
-    Certificate_Store_Windows& operator=(const Certificate_Store_Windows&) = default;
-    Certificate_Store_Windows& operator=(Certificate_Store_Windows&&) = default;
+      Certificate_Store_Windows(const Certificate_Store_Windows&) = default;
+      Certificate_Store_Windows(Certificate_Store_Windows&&) = default;
+      Certificate_Store_Windows& operator=(const Certificate_Store_Windows&) = default;
+      Certificate_Store_Windows& operator=(Certificate_Store_Windows&&) = default;
 
-    /**
-    * @return DNs for all certificates managed by the store
-    */
-    std::vector<X509_DN> all_subjects() const override;
+      /**
+      * @return DNs for all certificates managed by the store
+      */
+      std::vector<X509_DN> all_subjects() const override;
 
-    /**
-    * Find a certificate by Subject DN and (optionally) key identifier
-    * @return the first certificate that matches
-    */
-    std::shared_ptr<const X509_Certificate> find_cert(
-        const X509_DN& subject_dn,
-        const std::vector<uint8_t>& key_id) const override;
+      /**
+      * Find a certificate by Subject DN and (optionally) key identifier
+      * @return the first certificate that matches
+      */
+      std::shared_ptr<const X509_Certificate> find_cert(
+         const X509_DN& subject_dn,
+         const std::vector<uint8_t>& key_id) const override;
 
-    /**
-    * Find all certificates with a given Subject DN.
-    * Subject DN and even the key identifier might not be unique.
-    */
-    std::vector<std::shared_ptr<const X509_Certificate>> find_all_certs(
-                const X509_DN& subject_dn, const std::vector<uint8_t>& key_id) const override;
+      /**
+      * Find all certificates with a given Subject DN.
+      * Subject DN and even the key identifier might not be unique.
+      */
+      std::vector<std::shared_ptr<const X509_Certificate>> find_all_certs(
+               const X509_DN& subject_dn, const std::vector<uint8_t>& key_id) const override;
 
-    /**
-    * Find a certificate by searching for one with a matching SHA-1 hash of
-    * public key.
-    * @return a matching certificate or nullptr otherwise
-    */
-    std::shared_ptr<const X509_Certificate>
-    find_cert_by_pubkey_sha1(const std::vector<uint8_t>& key_hash) const override;
+      /**
+      * Find a certificate by searching for one with a matching SHA-1 hash of
+      * public key.
+      * @return a matching certificate or nullptr otherwise
+      */
+      std::shared_ptr<const X509_Certificate>
+      find_cert_by_pubkey_sha1(const std::vector<uint8_t>& key_hash) const override;
 
-    /**
-     * @throws Botan::Not_Implemented
-     */
-    std::shared_ptr<const X509_Certificate>
-    find_cert_by_raw_subject_dn_sha256(const std::vector<uint8_t>& subject_hash) const override;
+      /**
+       * @throws Botan::Not_Implemented
+       */
+      std::shared_ptr<const X509_Certificate>
+      find_cert_by_raw_subject_dn_sha256(const std::vector<uint8_t>& subject_hash) const override;
 
-    /**
-     * TODO
-     */
-    std::shared_ptr<const X509_CRL> find_crl_for(const X509_Certificate& subject) const override;
-};
+      /**
+       * TODO
+       */
+      std::shared_ptr<const X509_CRL> find_crl_for(const X509_Certificate& subject) const override;
+   };
 }
 
 #endif