Sync pki to chromium eddbcb143c7462e0b8d60e859b96d678ca0c013c

This removes one more patch, and adapts import to deal with gmock from chrome
which is now included in boring.

Bug: chromium:1322914
Change-Id: I2a5957f741252941fea76205a21e98fd655f8cae
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63225
Commit-Queue: Adam Langley <agl@google.com>
Auto-Submit: Bob Beck <bbe@google.com>
Reviewed-by: Adam Langley <agl@google.com>
chromium-stable-with-bazel
Bob Beck 1 year ago committed by Boringssl LUCI CQ
parent 764e6a319b
commit d24a38200f
  1. 4
      pki/cert_issuer_source.h
  2. 17
      pki/certificate_policies.cc
  3. 9
      pki/certificate_policies.h
  4. 1
      pki/general_names.h
  5. 2
      pki/import_spec.json
  6. 5
      pki/parsed_certificate.cc
  7. 12
      pki/parsed_certificate.h
  8. 6
      pki/parsed_certificate_unittest.cc
  9. 23
      pki/patches/0001-Simplify-the-name-constraint-limit-to-resemble-Borin.patch
  10. 112
      pki/patches/0002-disable-path-builder-tests-with-unsupported-dependen.patch
  11. 40
      pki/path_builder.cc
  12. 3
      pki/path_builder.h
  13. 71
      pki/path_builder_unittest.cc
  14. 11
      pki/trust_store.h
  15. 6
      pki/trust_store_collection.cc
  16. 3
      pki/trust_store_collection.h
  17. 24
      pki/trust_store_collection_unittest.cc
  18. 4
      pki/trust_store_in_memory.cc
  19. 3
      pki/trust_store_in_memory.h
  20. 6
      pki/verify_certificate_chain.cc

@ -10,7 +10,6 @@
#include <vector>
#include "parsed_certificate.h"
namespace bssl {
@ -41,8 +40,7 @@ class OPENSSL_EXPORT CertIssuerSource {
// If no issuers are left then |issuers| will not be modified. This
// indicates that the issuers have been exhausted and GetNext() should
// not be called again.
virtual void GetNext(ParsedCertificateList* issuers,
void* debug_data) = 0;
virtual void GetNext(ParsedCertificateList* issuers) = 0;
};
virtual ~CertIssuerSource() = default;

@ -310,19 +310,22 @@ bool ParsePolicyConstraints(const der::Input& policy_constraints_tlv,
// InhibitAnyPolicy ::= SkipCerts
//
// SkipCerts ::= INTEGER (0..MAX)
bool ParseInhibitAnyPolicy(const der::Input& inhibit_any_policy_tlv,
uint8_t* num_certs) {
std::optional<uint8_t> ParseInhibitAnyPolicy(
const der::Input& inhibit_any_policy_tlv) {
der::Parser parser(inhibit_any_policy_tlv);
std::optional<uint8_t> num_certs = std::make_optional<uint8_t>();
// TODO(eroman): Surface reason for failure if length was longer than uint8.
if (!parser.ReadUint8(num_certs))
return false;
if (!parser.ReadUint8(&num_certs.value())) {
return std::nullopt;
}
// There should be no remaining data.
if (parser.HasMore())
return false;
if (parser.HasMore()) {
return std::nullopt;
}
return true;
return num_certs;
}
// From RFC 5280:

@ -114,11 +114,10 @@ struct ParsedPolicyConstraints {
const der::Input& policy_constraints_tlv,
ParsedPolicyConstraints* out);
// Parses an InhibitAnyPolicy as defined by RFC 5280. Returns true on success,
// and sets |num_certs|.
[[nodiscard]] OPENSSL_EXPORT bool ParseInhibitAnyPolicy(
const der::Input& inhibit_any_policy_tlv,
uint8_t* num_certs);
// Parses an InhibitAnyPolicy as defined by RFC 5280. Returns num certs on
// success, or empty if parser fails.
[[nodiscard]] OPENSSL_EXPORT std::optional<uint8_t> ParseInhibitAnyPolicy(
const der::Input& inhibit_any_policy_tlv);
struct ParsedPolicyMapping {
der::Input issuer_domain_policy;

@ -7,6 +7,7 @@
#include "fillins/openssl_util.h"
#include <memory>
#include <string_view>
#include <vector>

@ -94,7 +94,7 @@
{"match": "^#include \"testing/gtest/include/gtest/gtest.h\"",
"replace": "#include <gtest/gtest.h>"},
{"match": "^#include \"testing/gmock/include/gmock/gmock.h\"",
"replace": "#include <gtest/gtest.h>"},
"replace": "#include <gmock/gmock.h>"},
{"match": "^#include \"base/containers/span.h\"",
"replace": "#include <openssl/span.h>"},
{"match": "^#include \"third_party/abseil-cpp/absl/types/optional.h\"",

@ -243,9 +243,8 @@ std::shared_ptr<const ParsedCertificate> ParsedCertificate::Create(
// Inhibit Any Policy.
if (result->GetExtension(der::Input(kInhibitAnyPolicyOid), &extension)) {
result->has_inhibit_any_policy_ = true;
if (!ParseInhibitAnyPolicy(extension.value,
&result->inhibit_any_policy_)) {
result->inhibit_any_policy_ = ParseInhibitAnyPolicy(extension.value);
if (!result->inhibit_any_policy_) {
errors->AddError(kFailedParsingInhibitAnyPolicy);
return nullptr;
}

@ -224,13 +224,8 @@ class OPENSSL_EXPORT ParsedCertificate {
return policy_mappings_;
}
// Returns true if the certificate has a InhibitAnyPolicy extension.
bool has_inhibit_any_policy() const { return has_inhibit_any_policy_; }
// Returns the Inhibit Any Policy extension. Caller must check
// has_inhibit_any_policy() before accessing this.
uint8_t inhibit_any_policy() const {
BSSL_CHECK(has_inhibit_any_policy_);
// Returns the Inhibit Any Policy extension.
const std::optional<uint8_t>& inhibit_any_policy() const {
return inhibit_any_policy_;
}
@ -318,8 +313,7 @@ class OPENSSL_EXPORT ParsedCertificate {
std::vector<ParsedPolicyMapping> policy_mappings_;
// Inhibit Any Policy extension.
bool has_inhibit_any_policy_ = false;
uint8_t inhibit_any_policy_;
std::optional<uint8_t> inhibit_any_policy_;
// AuthorityKeyIdentifier extension.
std::optional<ParsedAuthorityKeyIdentifier> authority_key_identifier_;

@ -569,9 +569,9 @@ TEST(ParsedCertificateTest, InhibitAnyPolicy) {
ParsedExtension extension;
ASSERT_TRUE(cert->GetExtension(der::Input(kInhibitAnyPolicyOid), &extension));
uint8_t skip_count;
ASSERT_TRUE(ParseInhibitAnyPolicy(extension.value, &skip_count));
EXPECT_EQ(3, skip_count);
std::optional<uint8_t> skip_count = ParseInhibitAnyPolicy(extension.value);
ASSERT_TRUE(skip_count.has_value());
EXPECT_EQ(3, skip_count.value());
}
// Tests a subjectKeyIdentifier that is not an OCTET_STRING.

@ -1,8 +1,8 @@
From 78f16e55f9d5cae7cf4b4bcf45c4af53db231cd9 Mon Sep 17 00:00:00 2001
From be12ef3a1c4d74dfba98641b1d565451424c6aa2 Mon Sep 17 00:00:00 2001
From: Bob Beck <bbe@google.com>
Date: Wed, 17 May 2023 10:37:22 -0600
Subject: [PATCH 1/2] Simplify the name constraint limit to resemble
BoringSSL's Disable the name constraint manynames check for now
Subject: [PATCH] Simplify the name constraint limit to resemble BoringSSL's
Disable the name constraint manynames check for now
---
net/cert/pki/DEPS | 1 -
@ -33,19 +33,18 @@ Subject: [PATCH 1/2] Simplify the name constraint limit to resemble
delete mode 100644 net/data/verify_certificate_chain_unittest/many-names/ok-different-types-ips.test
diff --git a/net/cert/pki/DEPS b/net/cert/pki/DEPS
index e9d834e1b51fa..ecf528b6e10a5 100644
index cac7a02e057dc..e0bfd7b2c2045 100644
--- a/net/cert/pki/DEPS
+++ b/net/cert/pki/DEPS
@@ -6,7 +6,6 @@ include_rules = [
@@ -5,6 +5,5 @@ include_rules = [
"-base",
"+base/base_paths.h",
"+base/check.h",
"+base/files/file_util.h",
- "+base/numerics/clamped_math.h",
"+base/path_service.h",
"+base/supports_user_data.h",
]
diff --git a/net/cert/pki/name_constraints.cc b/net/cert/pki/name_constraints.cc
index b1f4aa44e54e1..06703db78fb5b 100644
index 123996d7c5d61..e09f4a6da74a9 100644
--- a/net/cert/pki/name_constraints.cc
+++ b/net/cert/pki/name_constraints.cc
@@ -8,7 +8,6 @@
@ -56,7 +55,7 @@ index b1f4aa44e54e1..06703db78fb5b 100644
#include "net/cert/pki/cert_errors.h"
#include "net/cert/pki/common_cert_errors.h"
#include "net/cert/pki/general_names.h"
@@ -343,32 +342,42 @@ void NameConstraints::IsPermittedCert(const der::Input& subject_rdn_sequence,
@@ -345,32 +344,42 @@ void NameConstraints::IsPermittedCert(const der::Input& subject_rdn_sequence,
CertErrors* errors) const {
// Checking NameConstraints is O(number_of_names * number_of_constraints).
// Impose a hard limit to mitigate the use of name constraints as a DoS
@ -122,7 +121,7 @@ index b1f4aa44e54e1..06703db78fb5b 100644
}
std::vector<std::string> subject_email_addresses_to_check;
@@ -380,15 +389,6 @@ void NameConstraints::IsPermittedCert(const der::Input& subject_rdn_sequence,
@@ -382,15 +391,6 @@ void NameConstraints::IsPermittedCert(const der::Input& subject_rdn_sequence,
errors->AddError(cert_errors::kNotPermittedByNameConstraints);
return;
}
@ -164,7 +163,7 @@ index a1af7849ebcc1..32a5be319d00d 100644
TYPED_TEST_P(VerifyCertificateChainSingleRootTest, TargetOnly) {
diff --git a/net/data/test_bundle_data.filelist b/net/data/test_bundle_data.filelist
index a33bdbe30a0eb..00c635befd873 100644
index 88fe51f7db6b5..849e421233ef8 100644
--- a/net/data/test_bundle_data.filelist
+++ b/net/data/test_bundle_data.filelist
@@ -753,12 +753,6 @@ data/verify_certificate_chain_unittest/key-rollover/rolloverchain.pem
@ -41105,5 +41104,5 @@ index efdbd5eaca095..56bc9c89c4c42 100644
Validity
Not Before: Oct 5 12:00:00 2021 GMT
--
2.41.0.694.ge786442a9b-goog
2.42.0.515.g380fc7ccd1-goog

@ -1,112 +0,0 @@
From 0c4866b5b94854983697ebc362efba8d05e5cb86 Mon Sep 17 00:00:00 2001
From: Bob Beck <bbe@google.com>
Date: Fri, 2 Jun 2023 11:08:50 +0200
Subject: [PATCH 2/2] disable path builder tests with unsupported dependencies
---
net/cert/pki/path_builder_unittest.cc | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/net/cert/pki/path_builder_unittest.cc b/net/cert/pki/path_builder_unittest.cc
index 2ee48ce3f5ecc..3a9ead52f0d54 100644
--- a/net/cert/pki/path_builder_unittest.cc
+++ b/net/cert/pki/path_builder_unittest.cc
@@ -32,6 +32,7 @@ namespace net {
namespace {
+#if !defined(_BORINGSSL_LIBPKI_)
using ::testing::_;
using ::testing::ElementsAre;
using ::testing::Exactly;
@@ -41,6 +42,7 @@ using ::testing::Return;
using ::testing::SaveArg;
using ::testing::SetArgPointee;
using ::testing::StrictMock;
+#endif // !_BORINGSSL_LIBPKI_
class TestPathBuilderDelegate : public SimplePathBuilderDelegate {
public:
@@ -159,6 +161,7 @@ class AsyncCertIssuerSourceStatic : public CertIssuerSource {
}
const void* kKey = &kKey;
+#if !defined(_BORINGSSL_LIBPKI_)
class TrustStoreThatStoresUserData : public TrustStore {
public:
class Data : public base::SupportsUserData::Data {
@@ -200,6 +203,7 @@ TEST(PathBuilderResultUserDataTest, ModifyUserDataInConstructor) {
ASSERT_TRUE(data);
EXPECT_EQ(1234, data->value);
}
+#endif // !_BORINGSSL_LIBPKI_
class PathBuilderMultiRootTest : public ::testing::Test {
public:
@@ -1564,6 +1568,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestDuplicateIntermediateAndRoot) {
EXPECT_EQ(newroot_->der_cert(), path.certs[2]->der_cert());
}
+#if !defined(_BORINGSSL_LIBPKI_)
class MockCertIssuerSourceRequest : public CertIssuerSource::Request {
public:
MOCK_METHOD2(GetNext, void(ParsedCertificateList*, base::SupportsUserData*));
@@ -1576,6 +1581,7 @@ class MockCertIssuerSource : public CertIssuerSource {
MOCK_METHOD2(AsyncGetIssuersOf,
void(const ParsedCertificate*, std::unique_ptr<Request>*));
};
+#endif // !_BORINGSSL_LIBPKI_
// Helper class to pass the Request to the PathBuilder when it calls
// AsyncGetIssuersOf. (GoogleMock has a ByMove helper, but it apparently can
@@ -1611,6 +1617,7 @@ class AppendCertToList {
std::shared_ptr<const ParsedCertificate> cert_;
};
+#if !defined(_BORINGSSL_LIBPKI_)
// Test that a single CertIssuerSource returning multiple async batches of
// issuers is handled correctly. Due to the StrictMocks, it also tests that path
// builder does not request issuers of certs that it shouldn't.
@@ -1780,6 +1787,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestDuplicateAsyncIntermediates) {
EXPECT_EQ(newintermediate_, path1.certs[1]);
EXPECT_EQ(newroot_, path1.certs[2]);
}
+#endif // !_BORINGSSL_LIBPKI_
class PathBuilderSimpleChainTest : public ::testing::Test {
public:
@@ -1934,6 +1942,7 @@ class CertPathBuilderDelegateBase : public SimplePathBuilderDelegate {
}
};
+#if !defined(_BORINGSSL_LIBPKI_)
class MockPathBuilderDelegate : public CertPathBuilderDelegateBase {
public:
MOCK_METHOD2(CheckPathAfterVerification,
@@ -1949,6 +1958,7 @@ TEST_F(PathBuilderCheckPathAfterVerificationTest, NoOpToValidPath) {
CertPathBuilder::Result result = RunPathBuilder(nullptr, &delegate);
EXPECT_TRUE(result.HasValidPath());
}
+#endif // !_BORINGSSL_LIBPKI_
DEFINE_CERT_ERROR_ID(kWarningFromDelegate, "Warning from delegate");
@@ -2000,6 +2010,7 @@ TEST_F(PathBuilderCheckPathAfterVerificationTest, AddsErrorToValidPath) {
EXPECT_TRUE(cert2_errors->ContainsError(kErrorFromDelegate));
}
+#if !defined(_BORINGSSL_LIBPKI_)
TEST_F(PathBuilderCheckPathAfterVerificationTest, NoopToAlreadyInvalidPath) {
StrictMock<MockPathBuilderDelegate> delegate;
// Just verify that the hook is called (on an invalid path).
@@ -2032,6 +2043,7 @@ TEST_F(PathBuilderCheckPathAfterVerificationTest, SetsDelegateData) {
EXPECT_EQ(0xB33F, data->value);
}
+#endif // !_BORINGSSL_LIBPKI_
TEST(PathBuilderPrioritizationTest, DatePrioritization) {
std::string test_dir =
--
2.41.0.694.ge786442a9b-goog

@ -166,12 +166,11 @@ int TrustAndKeyIdentifierMatchToOrder(const ParsedCertificate* target,
// which may be issuers of |cert|.
class CertIssuersIter {
public:
// Constructs the CertIssuersIter. |*cert_issuer_sources|, |*trust_store|,
// and |*debug_data| must be valid for the lifetime of the CertIssuersIter.
// Constructs the CertIssuersIter. |*cert_issuer_sources|, and
// |*trust_store| must be valid for the lifetime of the CertIssuersIter.
CertIssuersIter(std::shared_ptr<const ParsedCertificate> cert,
CertIssuerSources* cert_issuer_sources,
TrustStore* trust_store,
void* debug_data);
TrustStore* trust_store);
CertIssuersIter(const CertIssuersIter&) = delete;
CertIssuersIter& operator=(const CertIssuersIter&) = delete;
@ -239,19 +238,15 @@ class CertIssuersIter {
// cancelled if CertIssuersIter is destroyed.
std::vector<std::unique_ptr<CertIssuerSource::Request>>
pending_async_requests_;
void* debug_data_;
};
CertIssuersIter::CertIssuersIter(
std::shared_ptr<const ParsedCertificate> in_cert,
CertIssuerSources* cert_issuer_sources,
TrustStore* trust_store,
void* debug_data)
TrustStore* trust_store)
: cert_(std::move(in_cert)),
cert_issuer_sources_(cert_issuer_sources),
trust_store_(trust_store),
debug_data_(debug_data) {
trust_store_(trust_store) {
DVLOG(2) << "CertIssuersIter created for " << CertDebugString(cert());
}
@ -277,8 +272,7 @@ void CertIssuersIter::GetNextIssuer(IssuerEntry* out) {
while (!HasCurrentIssuer() &&
cur_async_request_ < pending_async_requests_.size()) {
ParsedCertificateList new_issuers;
pending_async_requests_[cur_async_request_]->GetNext(&new_issuers,
debug_data_);
pending_async_requests_[cur_async_request_]->GetNext(&new_issuers);
if (new_issuers.empty()) {
// Request is exhausted, no more results pending from that
// CertIssuerSource.
@ -317,7 +311,7 @@ void CertIssuersIter::AddIssuers(ParsedCertificateList new_issuers) {
// Look up the trust for this issuer.
IssuerEntry entry;
entry.cert = std::move(issuer);
entry.trust = trust_store_->GetTrust(entry.cert.get(), debug_data_);
entry.trust = trust_store_->GetTrust(entry.cert.get());
entry.trust_and_key_id_match_ordering = TrustAndKeyIdentifierMatchToOrder(
cert(), entry.cert.get(), entry.trust);
@ -472,8 +466,7 @@ const ParsedCertificate* CertPathBuilderResultPath::GetTrustedCert() const {
class CertPathIter {
public:
CertPathIter(std::shared_ptr<const ParsedCertificate> cert,
TrustStore* trust_store,
void* debug_data);
TrustStore* trust_store);
CertPathIter(const CertPathIter&) = delete;
CertPathIter& operator=(const CertPathIter&) = delete;
@ -512,18 +505,14 @@ class CertPathIter {
CertIssuerSources cert_issuer_sources_;
// The TrustStore for checking if a path ends in a trust anchor.
TrustStore* trust_store_;
void* debug_data_;
};
CertPathIter::CertPathIter(std::shared_ptr<const ParsedCertificate> cert,
TrustStore* trust_store,
void* debug_data)
: trust_store_(trust_store), debug_data_(debug_data) {
TrustStore* trust_store)
: trust_store_(trust_store) {
// Initialize |next_issuer_| to the target certificate.
next_issuer_.cert = std::move(cert);
next_issuer_.trust =
trust_store_->GetTrust(next_issuer_.cert.get(), debug_data_);
next_issuer_.trust = trust_store_->GetTrust(next_issuer_.cert.get());
}
void CertPathIter::AddCertIssuerSource(CertIssuerSource* cert_issuer_source) {
@ -684,8 +673,7 @@ bool CertPathIter::GetNextPath(ParsedCertificateList* out_certs,
}
cur_path_.Append(std::make_unique<CertIssuersIter>(
std::move(next_issuer_.cert), &cert_issuer_sources_, trust_store_,
debug_data_));
std::move(next_issuer_.cert), &cert_issuer_sources_, trust_store_));
next_issuer_ = IssuerEntry();
DVLOG(1) << "CertPathIter cur_path_ =\n" << cur_path_.PathDebugString();
// Continue descending the tree.
@ -752,9 +740,7 @@ CertPathBuilder::CertPathBuilder(
InitialPolicyMappingInhibit initial_policy_mapping_inhibit,
InitialAnyPolicyInhibit initial_any_policy_inhibit)
: cert_path_iter_(
std::make_unique<CertPathIter>(std::move(cert),
trust_store,
/*debug_data=*/&out_result_)),
std::make_unique<CertPathIter>(std::move(cert), trust_store)),
delegate_(delegate),
time_(time),
key_purpose_(key_purpose),

@ -10,7 +10,6 @@
#include <vector>
#include "cert_errors.h"
#include "parsed_certificate.h"
#include "trust_store.h"
@ -112,7 +111,7 @@ class OPENSSL_EXPORT CertPathBuilder {
public:
// Provides the overall result of path building. This includes the paths that
// were attempted.
struct OPENSSL_EXPORT Result {
struct OPENSSL_EXPORT Result {
Result();
Result(Result&&);

@ -22,7 +22,7 @@
#include "input.h"
#include "testdata/test_certificate_data.h"
#include <gtest/gtest.h>
#include <gmock/gmock.h>
#include <gtest/gtest.h>
#include <openssl/pool.h>
@ -32,7 +32,6 @@ namespace bssl {
namespace {
#if !defined(_BORINGSSL_LIBPKI_)
using ::testing::_;
using ::testing::ElementsAre;
using ::testing::Exactly;
@ -42,7 +41,6 @@ using ::testing::Return;
using ::testing::SaveArg;
using ::testing::SetArgPointee;
using ::testing::StrictMock;
#endif // !_BORINGSSL_LIBPKI_
class TestPathBuilderDelegate : public SimplePathBuilderDelegate {
public:
@ -87,8 +85,7 @@ class AsyncCertIssuerSourceStatic : public CertIssuerSource {
~StaticAsyncRequest() override = default;
void GetNext(ParsedCertificateList* out_certs,
void* debug_data) override {
void GetNext(ParsedCertificateList* out_certs) override {
if (issuers_iter_ != issuers_.end())
out_certs->push_back(std::move(*issuers_iter_++));
}
@ -160,51 +157,6 @@ class AsyncCertIssuerSourceStatic : public CertIssuerSource {
return ::testing::AssertionSuccess();
}
const void* kKey = &kKey;
#if !defined(_BORINGSSL_LIBPKI_)
class TrustStoreThatStoresUserData : public TrustStore {
public:
class Data ::Data {
public:
explicit Data(int value) : value(value) {}
int value = 0;
};
// TrustStore implementation:
void SyncGetIssuersOf(const ParsedCertificate* cert,
ParsedCertificateList* issuers) override {}
CertificateTrust GetTrust(const ParsedCertificate* cert,
void* debug_data) override {
debug_data->SetUserData(kKey, std::make_unique<Data>(1234));
return CertificateTrust::ForUnspecified();
}
};
TEST(PathBuilderResultUserDataTest, ModifyUserDataInConstructor) {
std::shared_ptr<const ParsedCertificate> a_by_b;
ASSERT_TRUE(ReadTestCert("multi-root-A-by-B.pem", &a_by_b));
SimplePathBuilderDelegate delegate(
1024, SimplePathBuilderDelegate::DigestPolicy::kWeakAllowSha1);
der::GeneralizedTime verify_time = {2017, 3, 1, 0, 0, 0};
TrustStoreThatStoresUserData trust_store;
// |trust_store| will unconditionally store user data in the
// CertPathBuilder::Result. This ensures that the Result object has been
// initialized before the first GetTrust call occurs (otherwise the test will
// crash or fail on ASAN bots).
CertPathBuilder path_builder(
a_by_b, &trust_store, &delegate, verify_time, KeyPurpose::ANY_EKU,
InitialExplicitPolicy::kFalse, {der::Input(kAnyPolicyOid)},
InitialPolicyMappingInhibit::kFalse, InitialAnyPolicyInhibit::kFalse);
CertPathBuilder::Result result = path_builder.Run();
auto* data = static_cast<TrustStoreThatStoresUserData::Data*>(
result.GetUserData(kKey));
ASSERT_TRUE(data);
EXPECT_EQ(1234, data->value);
}
#endif // !_BORINGSSL_LIBPKI_
class PathBuilderMultiRootTest : public ::testing::Test {
public:
PathBuilderMultiRootTest()
@ -1568,10 +1520,9 @@ TEST_F(PathBuilderKeyRolloverTest, TestDuplicateIntermediateAndRoot) {
EXPECT_EQ(newroot_->der_cert(), path.certs[2]->der_cert());
}
#if !defined(_BORINGSSL_LIBPKI_)
class MockCertIssuerSourceRequest : public CertIssuerSource::Request {
public:
MOCK_METHOD2(GetNext, void(ParsedCertificateList*, void*));
MOCK_METHOD1(GetNext, void(ParsedCertificateList*));
};
class MockCertIssuerSource : public CertIssuerSource {
@ -1581,7 +1532,6 @@ class MockCertIssuerSource : public CertIssuerSource {
MOCK_METHOD2(AsyncGetIssuersOf,
void(const ParsedCertificate*, std::unique_ptr<Request>*));
};
#endif // !_BORINGSSL_LIBPKI_
// Helper class to pass the Request to the PathBuilder when it calls
// AsyncGetIssuersOf. (GoogleMock has a ByMove helper, but it apparently can
@ -1608,16 +1558,12 @@ class AppendCertToList {
const std::shared_ptr<const ParsedCertificate>& cert)
: cert_(cert) {}
void operator()(ParsedCertificateList* out,
void* debug_data) {
out->push_back(cert_);
}
void operator()(ParsedCertificateList* out) { out->push_back(cert_); }
private:
std::shared_ptr<const ParsedCertificate> cert_;
};
#if !defined(_BORINGSSL_LIBPKI_)
// Test that a single CertIssuerSource returning multiple async batches of
// issuers is handled correctly. Due to the StrictMocks, it also tests that path
// builder does not request issuers of certs that it shouldn't.
@ -1650,7 +1596,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestMultipleAsyncIssuersFromSingleSource) {
.WillOnce(Invoke(&req_mover, &CertIssuerSourceRequestMover::MoveIt));
}
EXPECT_CALL(*target_issuers_req, GetNext(_, _))
EXPECT_CALL(*target_issuers_req, GetNext(_))
// First async batch: return oldintermediate_.
.WillOnce(Invoke(AppendCertToList(oldintermediate_)))
// Second async batch: return newintermediate_.
@ -1737,7 +1683,7 @@ TEST_F(PathBuilderKeyRolloverTest, TestDuplicateAsyncIntermediates) {
oldintermediate_->der_cert().Length(), nullptr)),
{}, nullptr));
EXPECT_CALL(*target_issuers_req, GetNext(_, _))
EXPECT_CALL(*target_issuers_req, GetNext(_))
// First async batch: return oldintermediate_.
.WillOnce(Invoke(AppendCertToList(oldintermediate_)))
// Second async batch: return a different copy of oldintermediate_ again.
@ -1787,7 +1733,6 @@ TEST_F(PathBuilderKeyRolloverTest, TestDuplicateAsyncIntermediates) {
EXPECT_EQ(newintermediate_, path1.certs[1]);
EXPECT_EQ(newroot_, path1.certs[2]);
}
#endif // !_BORINGSSL_LIBPKI_
class PathBuilderSimpleChainTest : public ::testing::Test {
public:
@ -1942,7 +1887,6 @@ class CertPathBuilderDelegateBase : public SimplePathBuilderDelegate {
}
};
#if !defined(_BORINGSSL_LIBPKI_)
class MockPathBuilderDelegate : public CertPathBuilderDelegateBase {
public:
MOCK_METHOD2(CheckPathAfterVerification,
@ -1958,7 +1902,6 @@ TEST_F(PathBuilderCheckPathAfterVerificationTest, NoOpToValidPath) {
CertPathBuilder::Result result = RunPathBuilder(nullptr, &delegate);
EXPECT_TRUE(result.HasValidPath());
}
#endif // !_BORINGSSL_LIBPKI_
DEFINE_CERT_ERROR_ID(kWarningFromDelegate, "Warning from delegate");
@ -2010,7 +1953,6 @@ TEST_F(PathBuilderCheckPathAfterVerificationTest, AddsErrorToValidPath) {
EXPECT_TRUE(cert2_errors->ContainsError(kErrorFromDelegate));
}
#if !defined(_BORINGSSL_LIBPKI_)
TEST_F(PathBuilderCheckPathAfterVerificationTest, NoopToAlreadyInvalidPath) {
StrictMock<MockPathBuilderDelegate> delegate;
// Just verify that the hook is called (on an invalid path).
@ -2043,7 +1985,6 @@ TEST_F(PathBuilderCheckPathAfterVerificationTest, SetsDelegateData) {
EXPECT_EQ(0xB33F, data->value);
}
#endif // !_BORINGSSL_LIBPKI_
TEST(PathBuilderPrioritizationTest, DatePrioritization) {
std::string test_dir =

@ -7,7 +7,6 @@
#include "fillins/openssl_util.h"
#include "cert_issuer_source.h"
#include "parsed_certificate.h"
#include <optional>
@ -132,17 +131,9 @@ class OPENSSL_EXPORT TrustStore : public CertIssuerSource {
TrustStore& operator=(const TrustStore&) = delete;
// Returns the trusted of |cert|, which must be non-null.
//
// Optionally, if |debug_data| is non-null, debug information may be added
// (any added Data must implement the Clone method.) The same |debug_data|
// object may be passed to multiple GetTrust calls for a single verification,
// so implementations should check whether they already added data with a
// certain key and update it instead of overwriting it.
virtual CertificateTrust GetTrust(const ParsedCertificate* cert,
void* debug_data) = 0;
virtual CertificateTrust GetTrust(const ParsedCertificate* cert) = 0;
// Disable async issuers for TrustStore, as it isn't needed.
// TODO(mattm): Pass debug_data here too.
void AsyncGetIssuersOf(const ParsedCertificate* cert,
std::unique_ptr<Request>* out_req) final;
};

@ -23,14 +23,12 @@ void TrustStoreCollection::SyncGetIssuersOf(const ParsedCertificate* cert,
}
}
CertificateTrust TrustStoreCollection::GetTrust(
const ParsedCertificate* cert,
void* debug_data) {
CertificateTrust TrustStoreCollection::GetTrust(const ParsedCertificate* cert) {
// The current aggregate result.
CertificateTrust result = CertificateTrust::ForUnspecified();
for (auto* store : stores_) {
CertificateTrust cur_trust = store->GetTrust(cert, debug_data);
CertificateTrust cur_trust = store->GetTrust(cert);
// * If any stores distrust the certificate, consider it untrusted.
// * If multiple stores consider it trusted, use the trust result from the

@ -32,8 +32,7 @@ class OPENSSL_EXPORT TrustStoreCollection : public TrustStore {
// TrustStore implementation:
void SyncGetIssuersOf(const ParsedCertificate* cert,
ParsedCertificateList* issuers) override;
CertificateTrust GetTrust(const ParsedCertificate* cert,
void* debug_data) override;
CertificateTrust GetTrust(const ParsedCertificate* cert) override;
private:
std::vector<TrustStore*> stores_;

@ -76,13 +76,12 @@ TEST_F(TrustStoreCollectionTest, OneStore) {
EXPECT_EQ(newroot_.get(), issuers[0].get());
// newroot_ is trusted.
CertificateTrust trust =
collection.GetTrust(newroot_.get(), /*debug_data=*/nullptr);
CertificateTrust trust = collection.GetTrust(newroot_.get());
EXPECT_EQ(CertificateTrust::ForTrustAnchor().ToDebugString(),
trust.ToDebugString());
// oldroot_ is not.
trust = collection.GetTrust(oldroot_.get(), /*debug_data=*/nullptr);
trust = collection.GetTrust(oldroot_.get());
EXPECT_EQ(CertificateTrust::ForUnspecified().ToDebugString(),
trust.ToDebugString());
}
@ -105,13 +104,12 @@ TEST_F(TrustStoreCollectionTest, OutputVectorsAppendedTo) {
EXPECT_EQ(newroot_.get(), issuers[3].get());
// newroot_ is trusted.
CertificateTrust trust =
collection.GetTrust(newroot_.get(), /*debug_data=*/nullptr);
CertificateTrust trust = collection.GetTrust(newroot_.get());
EXPECT_EQ(CertificateTrust::ForTrustAnchor().ToDebugString(),
trust.ToDebugString());
// newrootrollover_ is not.
trust = collection.GetTrust(newrootrollover_.get(), /*debug_data=*/nullptr);
trust = collection.GetTrust(newrootrollover_.get());
EXPECT_EQ(CertificateTrust::ForUnspecified().ToDebugString(),
trust.ToDebugString());
}
@ -134,18 +132,17 @@ TEST_F(TrustStoreCollectionTest, TwoStores) {
EXPECT_EQ(oldroot_.get(), issuers[1].get());
// newroot_ is trusted.
CertificateTrust trust =
collection.GetTrust(newroot_.get(), /*debug_data=*/nullptr);
CertificateTrust trust = collection.GetTrust(newroot_.get());
EXPECT_EQ(CertificateTrust::ForTrustAnchor().ToDebugString(),
trust.ToDebugString());
// oldroot_ is trusted.
trust = collection.GetTrust(oldroot_.get(), /*debug_data=*/nullptr);
trust = collection.GetTrust(oldroot_.get());
EXPECT_EQ(CertificateTrust::ForTrustAnchor().ToDebugString(),
trust.ToDebugString());
// newrootrollover_ is not.
trust = collection.GetTrust(newrootrollover_.get(), /*debug_data=*/nullptr);
trust = collection.GetTrust(newrootrollover_.get());
EXPECT_EQ(CertificateTrust::ForUnspecified().ToDebugString(),
trust.ToDebugString());
}
@ -171,18 +168,17 @@ TEST_F(TrustStoreCollectionTest, DistrustTakesPriority) {
collection.AddTrustStore(&in_memory2);
// newroot_ is distrusted..
CertificateTrust trust =
collection.GetTrust(newroot_.get(), /*debug_data=*/nullptr);
CertificateTrust trust = collection.GetTrust(newroot_.get());
EXPECT_EQ(CertificateTrust::ForDistrusted().ToDebugString(),
trust.ToDebugString());
// oldintermediate_ is distrusted.
trust = collection.GetTrust(oldintermediate_.get(), /*debug_data=*/nullptr);
trust = collection.GetTrust(oldintermediate_.get());
EXPECT_EQ(CertificateTrust::ForDistrusted().ToDebugString(),
trust.ToDebugString());
// newrootrollover_ is unspecified.
trust = collection.GetTrust(newrootrollover_.get(), /*debug_data=*/nullptr);
trust = collection.GetTrust(newrootrollover_.get());
EXPECT_EQ(CertificateTrust::ForUnspecified().ToDebugString(),
trust.ToDebugString());
}

@ -52,9 +52,7 @@ void TrustStoreInMemory::SyncGetIssuersOf(const ParsedCertificate* cert,
issuers->push_back(it->second.cert);
}
CertificateTrust TrustStoreInMemory::GetTrust(
const ParsedCertificate* cert,
void* debug_data) {
CertificateTrust TrustStoreInMemory::GetTrust(const ParsedCertificate* cert) {
const Entry* entry = GetEntry(cert);
return entry ? entry->trust : CertificateTrust::ForUnspecified();
}

@ -62,8 +62,7 @@ class OPENSSL_EXPORT TrustStoreInMemory : public TrustStore {
// TrustStore implementation:
void SyncGetIssuersOf(const ParsedCertificate* cert,
ParsedCertificateList* issuers) override;
CertificateTrust GetTrust(const ParsedCertificate* cert,
void* debug_data) override;
CertificateTrust GetTrust(const ParsedCertificate* cert) override;
// Returns true if the trust store contains the given ParsedCertificate
// (matches by DER).

@ -998,9 +998,9 @@ void PathVerifier::ApplyPolicyConstraints(const ParsedCertificate& cert) {
// (j) If the inhibitAnyPolicy extension is included in the
// certificate and is less than inhibit_anyPolicy, set
// inhibit_anyPolicy to the value of inhibitAnyPolicy.
if (cert.has_inhibit_any_policy() &&
cert.inhibit_any_policy() < inhibit_any_policy_) {
inhibit_any_policy_ = cert.inhibit_any_policy();
if (cert.inhibit_any_policy() &&
cert.inhibit_any_policy().value() < inhibit_any_policy_) {
inhibit_any_policy_ = cert.inhibit_any_policy().value();
}
}

Loading…
Cancel
Save