Repository: randombit/botan
Pull request #1936 (2019-05-07 09:24:09)

use map of vectors instead of multimap in flatfile certstor
There appeared to be a problem using the std::multimap::equal_range function for retrieving certificates to a given subject DN. Using the ca-certificates.crt from the Linux Travis Agent, `find_all_certs` for a subject_dn and no key_id returned wrong results.
The reason for that is still unclear to me, but using a map of vectors mitigates the problem.

This should also fix the tests for #1931 


Patch:
diff --git a/src/lib/x509/certstor_flatfile/certstor_flatfile.cpp b/src/lib/x509/certstor_flatfile/certstor_flatfile.cpp
index 24cea1d40..90feecd24 100644
--- a/src/lib/x509/certstor_flatfile/certstor_flatfile.cpp
+++ b/src/lib/x509/certstor_flatfile/certstor_flatfile.cpp
@@ -60,7 +60,7 @@ Flatfile_Certificate_Store::Flatfile_Certificate_Store(const std::string& file,
       if(cert->is_self_signed() && cert->is_CA_cert())
          {
          m_all_subjects.push_back(cert->subject_dn());
-         m_dn_to_cert.emplace(cert->subject_dn(), cert);
+         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);
          }
@@ -94,24 +94,23 @@ std::vector<std::shared_ptr<const X509_Certificate>> Flatfile_Certificate_Store:
          const X509_DN& subject_dn,
          const std::vector<uint8_t>& key_id) const
    {
-   const auto found_range = m_dn_to_cert.equal_range(subject_dn);
-
-   if(found_range.first == m_dn_to_cert.end())
-      {
-      return {};
-      }
-
    std::vector<std::shared_ptr<const X509_Certificate>> found_certs;
-
-   for(auto i = found_range.first; i != found_range.second; ++i)
+   try
       {
-      const std::shared_ptr<const X509_Certificate> cert = i->second;
+      const auto certs = m_dn_to_cert.at(subject_dn);
 
-      if(key_id.empty() || key_id == cert->subject_key_id())
+      for(auto cert : certs)
          {
-         found_certs.push_back(cert);
+         if(key_id.empty() || key_id == cert->subject_key_id())
+            {
+            found_certs.push_back(cert);
+            }
          }
       }
+   catch(const std::out_of_range&)
+      {
+      return {};
+      }
 
    return found_certs;
    }
diff --git a/src/lib/x509/certstor_flatfile/certstor_flatfile.h b/src/lib/x509/certstor_flatfile/certstor_flatfile.h
index a778e045e..9c0949bcd 100644
--- a/src/lib/x509/certstor_flatfile/certstor_flatfile.h
+++ b/src/lib/x509/certstor_flatfile/certstor_flatfile.h
@@ -76,7 +76,7 @@ class BOTAN_PUBLIC_API(2, 11) Flatfile_Certificate_Store final : public Certific
 
    private:
       std::vector<X509_DN> m_all_subjects;
-      std::multimap<X509_DN, std::shared_ptr<const X509_Certificate>> m_dn_to_cert;
+      std::map<X509_DN, std::vector<std::shared_ptr<const X509_Certificate>>> m_dn_to_cert;
       std::map<std::vector<uint8_t>, std::shared_ptr<const X509_Certificate>> m_pubkey_sha1_to_cert;
       std::map<std::vector<uint8_t>, std::shared_ptr<const X509_Certificate>> m_subject_dn_sha256_to_cert;
    };


Test patch:
diff --git a/src/tests/test_certstor_flatfile.cpp b/src/tests/test_certstor_flatfile.cpp
index 2c24128c8..29d2543a4 100644
--- a/src/tests/test_certstor_flatfile.cpp
+++ b/src/tests/test_certstor_flatfile.cpp
@@ -110,6 +110,35 @@ Test::Result find_cert_by_subject_dn()
    return result;
    }
 
+Test::Result find_cert_by_utf8_subject_dn()
+   {
+   Test::Result result("Flatfile Certificate Store - Find Certificate by UTF8 subject DN");
+
+   try
+      {
+      auto dn = get_utf8_dn();
+
+      result.start_timer();
+      Botan::Flatfile_Certificate_Store certstore(get_valid_ca_bundle_path());
+      auto cert = certstore.find_cert(dn, std::vector<uint8_t>());
+
+      result.end_timer();
+
+      if(result.test_not_null("found certificate", cert.get()))
+         {
+         auto cns = cert->subject_dn().get_attribute("CN");
+         result.test_is_eq("exactly one CN", cns.size(), size_t(1));
+         result.test_eq("CN", cns.front(), "D-TRUST Root Class 3 CA 2 EV 2009");
+         }
+      }
+   catch(std::exception& e)
+      {
+      result.test_failure(e.what());
+      }
+
+   return result;
+   }
+
 Test::Result find_cert_by_subject_dn_and_key_id()
    {
    Test::Result result("Flatfile Certificate Store - Find Certificate by subject DN and key ID");
@@ -259,6 +288,7 @@ class Certstor_Flatfile_Tests final : public Test
          results.push_back(open_certificate_store());
          results.push_back(find_certificate_by_pubkey_sha1());
          results.push_back(find_cert_by_subject_dn());
+         results.push_back(find_cert_by_utf8_subject_dn());
          results.push_back(find_cert_by_subject_dn_and_key_id());
          results.push_back(find_certs_by_subject_dn_and_key_id());
          results.push_back(find_all_subjects());
diff --git a/src/tests/test_certstor_utils.cpp b/src/tests/test_certstor_utils.cpp
index 9a0a141d9..007271cc8 100644
--- a/src/tests/test_certstor_utils.cpp
+++ b/src/tests/test_certstor_utils.cpp
@@ -33,6 +33,16 @@ Botan::X509_DN get_dn()
                   "4341205833");
    }
 
+Botan::X509_DN get_utf8_dn()
+   {
+   // ASN.1 encoded subject DN of "D-TRUST Root Class 3 CA 2 EV 2009"
+   // This DN contains UTF8-encoded strings
+   // expires on 05. November 2029 at 8:50:46 UTC
+   return read_dn("3050310B300906035504061302444531153013060355040A0C0C442D54727"
+                  "5737420476D6248312A302806035504030C21442D545255535420526F6F74"
+                  "20436C617373203320434120322045562032303039");
+   }
+
 std::vector<uint8_t> get_key_id()
    {
    // this is the same as the public key SHA1
diff --git a/src/tests/test_certstor_utils.h b/src/tests/test_certstor_utils.h
index f86e87596..d3b26f52e 100644
--- a/src/tests/test_certstor_utils.h
+++ b/src/tests/test_certstor_utils.h
@@ -23,6 +23,8 @@ Botan::X509_DN read_dn(const std::string hex);
 
 Botan::X509_DN get_dn();
 
+Botan::X509_DN get_utf8_dn();
+
 std::vector<uint8_t> get_key_id();
 
 Botan::X509_DN get_unknown_dn();