From c909900ab543b9dad667f7f9e3272e4cf6894a24 Mon Sep 17 00:00:00 2001 From: Hubert Chao Date: Wed, 6 Dec 2023 18:55:23 +0000 Subject: [PATCH] Add distrust by SPKI to TrustStoreInMemory Change-Id: I9dcb1ef1218ece2678688abe7459fb2d1dcb8854 Bug: chromium:1477317 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64308 Reviewed-by: Bob Beck Commit-Queue: Bob Beck --- pki/trust_store_in_memory.cc | 10 +++ pki/trust_store_in_memory.h | 10 +++ pki/trust_store_in_memory_unittest.cc | 110 ++++++++++++++++++++++++++ sources.cmake | 1 + 4 files changed, 131 insertions(+) create mode 100644 pki/trust_store_in_memory_unittest.cc diff --git a/pki/trust_store_in_memory.cc b/pki/trust_store_in_memory.cc index 47fe6d466..3da1b8dba 100644 --- a/pki/trust_store_in_memory.cc +++ b/pki/trust_store_in_memory.cc @@ -36,6 +36,10 @@ void TrustStoreInMemory::AddDistrustedCertificateForTest( AddCertificate(std::move(cert), CertificateTrust::ForDistrusted()); } +void TrustStoreInMemory::AddDistrustedCertificateBySPKI(std::string spki) { + distrusted_spkis_.insert(std::move(spki)); +} + void TrustStoreInMemory::AddCertificateWithUnspecifiedTrust( std::shared_ptr cert) { AddCertificate(std::move(cert), CertificateTrust::ForUnspecified()); @@ -50,6 +54,12 @@ void TrustStoreInMemory::SyncGetIssuersOf(const ParsedCertificate *cert, } CertificateTrust TrustStoreInMemory::GetTrust(const ParsedCertificate *cert) { + // Check SPKI distrust first. + if (distrusted_spkis_.find(cert->tbs().spki_tlv.AsString()) != + distrusted_spkis_.end()) { + return CertificateTrust::ForDistrusted(); + } + const Entry *entry = GetEntry(cert); return entry ? entry->trust : CertificateTrust::ForUnspecified(); } diff --git a/pki/trust_store_in_memory.h b/pki/trust_store_in_memory.h index 4fea8d1a1..59ae3a43e 100644 --- a/pki/trust_store_in_memory.h +++ b/pki/trust_store_in_memory.h @@ -6,6 +6,7 @@ #define BSSL_PKI_TRUST_STORE_IN_MEMORY_H_ #include +#include #include @@ -55,6 +56,12 @@ class OPENSSL_EXPORT TrustStoreInMemory : public TrustStore { void AddDistrustedCertificateForTest( std::shared_ptr cert); + // Distrusts the provided SPKI. This will override any other trust (e.g. if a + // certificate is passed into AddTrustAnchor() and the certificate's SPKI is + // passed into AddDistrustedCertificateBySPKI(), GetTrust() will return + // CertificateTrust::ForDistrusted()). + void AddDistrustedCertificateBySPKI(std::string spki); + // Adds a certificate to the store, that is neither trusted nor untrusted. void AddCertificateWithUnspecifiedTrust( std::shared_ptr cert); @@ -81,6 +88,9 @@ class OPENSSL_EXPORT TrustStoreInMemory : public TrustStore { // Multimap from normalized subject -> Entry. std::unordered_multimap entries_; + // Set of distrusted SPKIs. + std::set distrusted_spkis_; + // Returns the `Entry` matching `cert`, or `nullptr` if not in the trust // store. const Entry *GetEntry(const ParsedCertificate *cert) const; diff --git a/pki/trust_store_in_memory_unittest.cc b/pki/trust_store_in_memory_unittest.cc new file mode 100644 index 000000000..92c3bb8e4 --- /dev/null +++ b/pki/trust_store_in_memory_unittest.cc @@ -0,0 +1,110 @@ +/* Copyright (c) 2023, Google Inc. + * + * Permission to use, copy, modify, and/or distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY + * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION + * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ + +#include "trust_store_in_memory.h" + +#include +#include "test_helpers.h" + +namespace bssl { +namespace { + +class TrustStoreInMemoryTest : public testing::Test { + public: + void SetUp() override { + ParsedCertificateList chain; + ASSERT_TRUE(ReadCertChainFromFile( + "testdata/verify_certificate_chain_unittest/key-rollover/oldchain.pem", + &chain)); + + ASSERT_EQ(3U, chain.size()); + target_ = chain[0]; + oldintermediate_ = chain[1]; + oldroot_ = chain[2]; + ASSERT_TRUE(target_); + ASSERT_TRUE(oldintermediate_); + ASSERT_TRUE(oldroot_); + + ASSERT_TRUE( + ReadCertChainFromFile("testdata/verify_certificate_chain_unittest/" + "key-rollover/longrolloverchain.pem", + &chain)); + + ASSERT_EQ(5U, chain.size()); + newintermediate_ = chain[1]; + newroot_ = chain[2]; + newrootrollover_ = chain[3]; + ASSERT_TRUE(newintermediate_); + ASSERT_TRUE(newroot_); + ASSERT_TRUE(newrootrollover_); + } + + protected: + std::shared_ptr oldroot_; + std::shared_ptr newroot_; + std::shared_ptr newrootrollover_; + + std::shared_ptr target_; + std::shared_ptr oldintermediate_; + std::shared_ptr newintermediate_; +}; + +TEST_F(TrustStoreInMemoryTest, OneRootTrusted) { + TrustStoreInMemory in_memory; + in_memory.AddTrustAnchor(newroot_); + + // newroot_ is trusted. + CertificateTrust trust = in_memory.GetTrust(newroot_.get()); + EXPECT_EQ(CertificateTrust::ForTrustAnchor().ToDebugString(), + trust.ToDebugString()); + + // oldroot_ is not. + trust = in_memory.GetTrust(oldroot_.get()); + EXPECT_EQ(CertificateTrust::ForUnspecified().ToDebugString(), + trust.ToDebugString()); +} + +TEST_F(TrustStoreInMemoryTest, DistrustBySPKI) { + TrustStoreInMemory in_memory; + in_memory.AddDistrustedCertificateBySPKI(newroot_->tbs().spki_tlv.AsString()); + + // newroot_ is distrusted. + CertificateTrust trust = in_memory.GetTrust(newroot_.get()); + EXPECT_EQ(CertificateTrust::ForDistrusted().ToDebugString(), + trust.ToDebugString()); + + // oldroot_ is unspecified. + trust = in_memory.GetTrust(oldroot_.get()); + EXPECT_EQ(CertificateTrust::ForUnspecified().ToDebugString(), + trust.ToDebugString()); + + // newrootrollover_ is also distrusted because it has the same key. + trust = in_memory.GetTrust(newrootrollover_.get()); + EXPECT_EQ(CertificateTrust::ForDistrusted().ToDebugString(), + trust.ToDebugString()); +} + +TEST_F(TrustStoreInMemoryTest, DistrustBySPKIOverridesTrust) { + TrustStoreInMemory in_memory; + in_memory.AddTrustAnchor(newroot_); + in_memory.AddDistrustedCertificateBySPKI(newroot_->tbs().spki_tlv.AsString()); + + // newroot_ is distrusted. + CertificateTrust trust = in_memory.GetTrust(newroot_.get()); + EXPECT_EQ(CertificateTrust::ForDistrusted().ToDebugString(), + trust.ToDebugString()); +} + +} // namespace +} // namespace bssl diff --git a/sources.cmake b/sources.cmake index f738b61d5..8edf805a3 100644 --- a/sources.cmake +++ b/sources.cmake @@ -414,6 +414,7 @@ set( pki/string_util_unittest.cc pki/test_helpers.cc pki/trust_store_collection_unittest.cc + pki/trust_store_in_memory_unittest.cc pki/verify_certificate_chain_pkits_unittest.cc pki/verify_certificate_chain_unittest.cc pki/verify_name_match_unittest.cc