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>
diff --git a/pki/cert_issuer_source.h b/pki/cert_issuer_source.h
index 53687d7..ba37d45 100644
--- a/pki/cert_issuer_source.h
+++ b/pki/cert_issuer_source.h
@@ -10,7 +10,6 @@
#include <vector>
-
#include "parsed_certificate.h"
namespace bssl {
@@ -41,8 +40,7 @@
// 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;
diff --git a/pki/certificate_policies.cc b/pki/certificate_policies.cc
index 0586975..3716a4b 100644
--- a/pki/certificate_policies.cc
+++ b/pki/certificate_policies.cc
@@ -310,19 +310,22 @@
// 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:
diff --git a/pki/certificate_policies.h b/pki/certificate_policies.h
index 60767c6..73f39b7 100644
--- a/pki/certificate_policies.h
+++ b/pki/certificate_policies.h
@@ -114,11 +114,10 @@
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;
diff --git a/pki/general_names.h b/pki/general_names.h
index 75833b0..38643d1 100644
--- a/pki/general_names.h
+++ b/pki/general_names.h
@@ -7,6 +7,7 @@
#include "fillins/openssl_util.h"
#include <memory>
+#include <string_view>
#include <vector>
diff --git a/pki/import_spec.json b/pki/import_spec.json
index 0594c3b..198af1b 100644
--- a/pki/import_spec.json
+++ b/pki/import_spec.json
@@ -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\"",
diff --git a/pki/parsed_certificate.cc b/pki/parsed_certificate.cc
index 028bdad..1c94830 100644
--- a/pki/parsed_certificate.cc
+++ b/pki/parsed_certificate.cc
@@ -243,9 +243,8 @@
// 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;
}
diff --git a/pki/parsed_certificate.h b/pki/parsed_certificate.h
index 16b5132..3f872f0 100644
--- a/pki/parsed_certificate.h
+++ b/pki/parsed_certificate.h
@@ -224,13 +224,8 @@
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 @@
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_;
diff --git a/pki/parsed_certificate_unittest.cc b/pki/parsed_certificate_unittest.cc
index 7c3ba7f..b3ee6a0 100644
--- a/pki/parsed_certificate_unittest.cc
+++ b/pki/parsed_certificate_unittest.cc
@@ -569,9 +569,9 @@
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.
diff --git a/pki/patches/0001-Simplify-the-name-constraint-limit-to-resemble-Borin.patch b/pki/patches/0001-Simplify-the-name-constraint-limit-to-resemble-Borin.patch
index e786b3a..f823b54 100644
--- a/pki/patches/0001-Simplify-the-name-constraint-limit-to-resemble-Borin.patch
+++ b/pki/patches/0001-Simplify-the-name-constraint-limit-to-resemble-Borin.patch
@@ -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 @@
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 @@
#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 @@
}
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 @@
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 @@
Validity
Not Before: Oct 5 12:00:00 2021 GMT
--
-2.41.0.694.ge786442a9b-goog
+2.42.0.515.g380fc7ccd1-goog
diff --git a/pki/patches/0002-disable-path-builder-tests-with-unsupported-dependen.patch b/pki/patches/0002-disable-path-builder-tests-with-unsupported-dependen.patch
deleted file mode 100644
index 6abbe75..0000000
--- a/pki/patches/0002-disable-path-builder-tests-with-unsupported-dependen.patch
+++ /dev/null
@@ -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
-
diff --git a/pki/path_builder.cc b/pki/path_builder.cc
index 2162e18..affaf25 100644
--- a/pki/path_builder.cc
+++ b/pki/path_builder.cc
@@ -166,12 +166,11 @@
// 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 @@
// 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 @@
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 @@
// 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 @@
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 @@
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 @@
}
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 @@
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),
diff --git a/pki/path_builder.h b/pki/path_builder.h
index f5b8a91..ea11b43 100644
--- a/pki/path_builder.h
+++ b/pki/path_builder.h
@@ -10,7 +10,6 @@
#include <vector>
-
#include "cert_errors.h"
#include "parsed_certificate.h"
#include "trust_store.h"
@@ -112,7 +111,7 @@
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&&);
diff --git a/pki/path_builder_unittest.cc b/pki/path_builder_unittest.cc
index da8aa6e..ef7ffca 100644
--- a/pki/path_builder_unittest.cc
+++ b/pki/path_builder_unittest.cc
@@ -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 {
-#if !defined(_BORINGSSL_LIBPKI_)
using ::testing::_;
using ::testing::ElementsAre;
using ::testing::Exactly;
@@ -42,7 +41,6 @@
using ::testing::SaveArg;
using ::testing::SetArgPointee;
using ::testing::StrictMock;
-#endif // !_BORINGSSL_LIBPKI_
class TestPathBuilderDelegate : public SimplePathBuilderDelegate {
public:
@@ -87,8 +85,7 @@
~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 @@
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 @@
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 @@
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 @@
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 @@
.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 @@
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 @@
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 @@
}
};
-#if !defined(_BORINGSSL_LIBPKI_)
class MockPathBuilderDelegate : public CertPathBuilderDelegateBase {
public:
MOCK_METHOD2(CheckPathAfterVerification,
@@ -1958,7 +1902,6 @@
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 @@
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 @@
EXPECT_EQ(0xB33F, data->value);
}
-#endif // !_BORINGSSL_LIBPKI_
TEST(PathBuilderPrioritizationTest, DatePrioritization) {
std::string test_dir =
diff --git a/pki/trust_store.h b/pki/trust_store.h
index 2f8eeb9..bd8ccea 100644
--- a/pki/trust_store.h
+++ b/pki/trust_store.h
@@ -7,7 +7,6 @@
#include "fillins/openssl_util.h"
-
#include "cert_issuer_source.h"
#include "parsed_certificate.h"
#include <optional>
@@ -132,17 +131,9 @@
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;
};
diff --git a/pki/trust_store_collection.cc b/pki/trust_store_collection.cc
index b248fac..27dd060 100644
--- a/pki/trust_store_collection.cc
+++ b/pki/trust_store_collection.cc
@@ -23,14 +23,12 @@
}
}
-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
diff --git a/pki/trust_store_collection.h b/pki/trust_store_collection.h
index e92ad2c..239d5e0 100644
--- a/pki/trust_store_collection.h
+++ b/pki/trust_store_collection.h
@@ -32,8 +32,7 @@
// 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_;
diff --git a/pki/trust_store_collection_unittest.cc b/pki/trust_store_collection_unittest.cc
index 6d52726..80923ac 100644
--- a/pki/trust_store_collection_unittest.cc
+++ b/pki/trust_store_collection_unittest.cc
@@ -76,13 +76,12 @@
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 @@
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 @@
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 @@
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());
}
diff --git a/pki/trust_store_in_memory.cc b/pki/trust_store_in_memory.cc
index d85f86b..07cb17a 100644
--- a/pki/trust_store_in_memory.cc
+++ b/pki/trust_store_in_memory.cc
@@ -52,9 +52,7 @@
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();
}
diff --git a/pki/trust_store_in_memory.h b/pki/trust_store_in_memory.h
index 033c1d1..e770510 100644
--- a/pki/trust_store_in_memory.h
+++ b/pki/trust_store_in_memory.h
@@ -62,8 +62,7 @@
// 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).
diff --git a/pki/verify_certificate_chain.cc b/pki/verify_certificate_chain.cc
index e9302ca..c7eb4ce 100644
--- a/pki/verify_certificate_chain.cc
+++ b/pki/verify_certificate_chain.cc
@@ -998,9 +998,9 @@
// (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();
}
}
