Allow the delegate to indicate it wishes to accept PreCertificates when building chains. Change-Id: I500b4a01e62c020548a88b5cc61db3bfaba4e06b Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65767 Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: Bob Beck <bbe@google.com>
diff --git a/pki/parse_certificate.h b/pki/parse_certificate.h index cd38c65..81e267f 100644 --- a/pki/parse_certificate.h +++ b/pki/parse_certificate.h
@@ -415,6 +415,14 @@ // In dotted notation: 2.5.29.31 inline constexpr uint8_t kCrlDistributionPointsOid[] = {0x55, 0x1d, 0x1f}; +// From RFC 6962: +// +// critical poison extension. +// +// In dotted notation 1.3.6.1.4.1.11129.2.4.3 +inline constexpr uint8_t kCtPoisonOid[] = {0x2B, 0x06, 0x01, 0x04, 0x01, + 0xD6, 0x79, 0x02, 0x04, 0x03}; + // From // https://learn.microsoft.com/en-us/windows/win32/seccertenroll/supported-extensions#msapplicationpolicies //
diff --git a/pki/path_builder_unittest.cc b/pki/path_builder_unittest.cc index 7747c34..3bdeec9 100644 --- a/pki/path_builder_unittest.cc +++ b/pki/path_builder_unittest.cc
@@ -54,9 +54,18 @@ MockSignatureVerifyCache *GetMockVerifyCache() { return &cache_; } - private: + void AllowPrecert() { allow_precertificate_ = true; } + + void DisallowPrecert() { allow_precertificate_ = false; } + + bool AcceptPreCertificates() override { + return allow_precertificate_; + } + +private: bool deadline_is_expired_ = false; bool use_signature_cache_ = false; + bool allow_precertificate_ = false; MockSignatureVerifyCache cache_; }; @@ -907,6 +916,46 @@ EXPECT_EQ(c_by_d_, valid_path->certs[2]); } +TEST_F(PathBuilderMultiRootTest, TestPreCertificate) { + + std::string test_dir = + "testdata/path_builder_unittest/precertificate/"; + std::shared_ptr<const ParsedCertificate> root1 = + ReadCertFromFile(test_dir + "root.pem"); + ASSERT_TRUE(root1); + std::shared_ptr<const ParsedCertificate> target = + ReadCertFromFile(test_dir + "precertificate.pem"); + ASSERT_TRUE(target); + + der::GeneralizedTime precert_time = {2023, 10, 1, 0, 0, 0}; + + TrustStoreInMemory trust_store; + trust_store.AddTrustAnchor(root1); + + // PreCertificate should be rejected by default. + EXPECT_FALSE(delegate_.AcceptPreCertificates()); + CertPathBuilder path_builder( + target, &trust_store, &delegate_, precert_time, KeyPurpose::ANY_EKU, + initial_explicit_policy_, user_initial_policy_set_, + initial_policy_mapping_inhibit_, initial_any_policy_inhibit_); + auto result = path_builder.Run(); + ASSERT_EQ(1U, result.paths.size()); + ASSERT_FALSE(result.paths[0]->IsValid()) + << result.paths[0]->errors.ToDebugString(result.paths[0]->certs); + + // PreCertificate should be accepted if configured. + delegate_.AllowPrecert(); + EXPECT_TRUE(delegate_.AcceptPreCertificates()); + CertPathBuilder path_builder2( + target, &trust_store, &delegate_, precert_time, KeyPurpose::ANY_EKU, + initial_explicit_policy_, user_initial_policy_set_, + initial_policy_mapping_inhibit_, initial_any_policy_inhibit_); + auto result2 = path_builder2.Run(); + ASSERT_EQ(1U, result2.paths.size()); + ASSERT_TRUE(result2.paths[0]->IsValid()) + << result2.paths[0]->errors.ToDebugString(result.paths[0]->certs); +} + class PathBuilderKeyRolloverTest : public ::testing::Test { public: PathBuilderKeyRolloverTest()
diff --git a/pki/simple_path_builder_delegate.cc b/pki/simple_path_builder_delegate.cc index a168513..f219477 100644 --- a/pki/simple_path_builder_delegate.cc +++ b/pki/simple_path_builder_delegate.cc
@@ -55,6 +55,8 @@ bool SimplePathBuilderDelegate::IsDebugLogEnabled() { return false; } +bool SimplePathBuilderDelegate::AcceptPreCertificates() { return false; } + void SimplePathBuilderDelegate::DebugLog(std::string_view msg) {} SignatureVerifyCache *SimplePathBuilderDelegate::GetVerifyCache() {
diff --git a/pki/simple_path_builder_delegate.h b/pki/simple_path_builder_delegate.h index 4c36b09..376782b 100644 --- a/pki/simple_path_builder_delegate.h +++ b/pki/simple_path_builder_delegate.h
@@ -70,6 +70,9 @@ // No-op implementation. void DebugLog(std::string_view msg) override; + // No-op implementation. + bool AcceptPreCertificates() override; + private: const size_t min_rsa_modulus_length_bits_; const DigestPolicy digest_policy_;
diff --git a/pki/testdata/path_builder_unittest/precertificate/precertificate.pem b/pki/testdata/path_builder_unittest/precertificate/precertificate.pem new file mode 100644 index 0000000..23b9900 --- /dev/null +++ b/pki/testdata/path_builder_unittest/precertificate/precertificate.pem
@@ -0,0 +1,27 @@ +-----BEGIN CERTIFICATE----- +MIIEfzCCA2egAwIBAgIQDIMDs7tv6W4RteOR0LnF9DANBgkqhkiG9w0BAQsFADBG +MQswCQYDVQQGEwJVUzEiMCAGA1UEChMZR29vZ2xlIFRydXN0IFNlcnZpY2VzIExM +QzETMBEGA1UEAxMKR1RTIENBIDFQNTAeFw0yMzA5MTQwMDU5NTFaFw0yMzEyMTMw +MDU5NTBaMBkxFzAVBgNVBAMTDnJmYy1lZGl0b3Iub3JnMIIBIjANBgkqhkiG9w0B +AQEFAAOCAQ8AMIIBCgKCAQEA2jNKh7UQ/v36Qz/KV+QLBYwxI9+kzv3t6363d63V +3EkhIPTLZF8wQ7kfztmFKUuMcLcvEpY1jwxyBioGJN1b9QaxlLCsK8WvEyxnR+nq +pR1h+j+zceZIEZoDVcS2GIr7LoFtFUeGidQRYBRf9Wu6bRMaT2fHaI6FcZECH7Bn +TSe2e62BFejah3pA91+oVlnI9EjKtMQOGEaoyKjrKswVaWcFiUWKcvGnjygkYWRb +6jBUhGINamkCgrtAblHrwB0ym9VENj06fpnWbkxFtU6GmmRplHag5npRwWI439+9 +QNtqj6PgMsSMBLj7ljq8GlY81Y6TZTZ2F06b0NLkRdjetwIDAQABo4IBlDCCAZAw +DgYDVR0PAQH/BAQDAgWgMBMGA1UdJQQMMAoGCCsGAQUFBwMBMAwGA1UdEwEB/wQC +MAAwHQYDVR0OBBYEFGMbD3hNq55udW3jdGHpx+v2rzbWMB8GA1UdIwQYMBaAFNX8 +ng3fHsrdCJeXbivFX8Ur9ey4MHgGCCsGAQUFBwEBBGwwajA1BggrBgEFBQcwAYYp +aHR0cDovL29jc3AucGtpLmdvb2cvcy9ndHMxcDUvZDJ5N2JUcElndEkwMQYIKwYB +BQUHMAKGJWh0dHA6Ly9wa2kuZ29vZy9yZXBvL2NlcnRzL2d0czFwNS5kZXIwKwYD +VR0RBCQwIoIOcmZjLWVkaXRvci5vcmeCECoucmZjLWVkaXRvci5vcmcwIQYDVR0g +BBowGDAIBgZngQwBAgEwDAYKKwYBBAHWeQIFAzA8BgNVHR8ENTAzMDGgL6Athito +dHRwOi8vY3Jscy5wa2kuZ29vZy9ndHMxcDUvZWV4a0MyUEp4YXcuY3JsMBMGCisG +AQQB1nkCBAMBAf8EAgUAMA0GCSqGSIb3DQEBCwUAA4IBAQAP3c64wNkNq6Nq7HrV +OzaygYS0dZ9gC8EuJsBe591AkPO8/B8y2kyPFRSHaG7IfRDDSyb9KqgtbknBDbHh +GWDF7CS6sid8ulT1kwDd8HKBVCfd37EODrHmzfJhtxMfEcB7FGjxHjcNZ7g5A4K1 +ph0AXHym+hPB0Jz/0MzDMeFpFp7llib3vAYfXz6bS4xYcBMqPmJV+okpHYjF4UXZ +0JIeMMSlypw5FgoMcsMydFW5X1KzoeKwSDhsVet03AM14QqlKv9p9u4MPzzVxYuW +syhj72JMl1YA01MZ06Org8SaLwUtKu0/kq4mysCNA3GYkKwUQIod/ilW65l6dZoS ++lRn +-----END CERTIFICATE-----
diff --git a/pki/testdata/path_builder_unittest/precertificate/root.pem b/pki/testdata/path_builder_unittest/precertificate/root.pem new file mode 100644 index 0000000..ab573a0 --- /dev/null +++ b/pki/testdata/path_builder_unittest/precertificate/root.pem
@@ -0,0 +1,32 @@ +-----BEGIN CERTIFICATE----- +MIIFjDCCA3SgAwIBAgINAgO8UKMnU/CRgCLt8TANBgkqhkiG9w0BAQsFADBHMQsw +CQYDVQQGEwJVUzEiMCAGA1UEChMZR29vZ2xlIFRydXN0IFNlcnZpY2VzIExMQzEU +MBIGA1UEAxMLR1RTIFJvb3QgUjEwHhcNMjAwODEzMDAwMDQyWhcNMjcwOTMwMDAw +MDQyWjBGMQswCQYDVQQGEwJVUzEiMCAGA1UEChMZR29vZ2xlIFRydXN0IFNlcnZp +Y2VzIExMQzETMBEGA1UEAxMKR1RTIENBIDFQNTCCASIwDQYJKoZIhvcNAQEBBQAD +ggEPADCCAQoCggEBALOC8CSMvy2Hr7LZp676yrpE1ls+/rL3smUW3N4Q6E8tEFha +KIaHoe5qs6DZdU9/oVIBi1WoSlsGSMg2EiWrifnyI1+dYGX5XNq+OuhcbX2c0IQY +hTDNTpvsPNiz4ZbU88ULZduPsHTL9h7zePGslcXdc8MxiIGvdKpv/QzjBZXwxRBP +ZWP6oK/GGD3Fod+XedcFibMwsHSuPZIQa4wVd90LBFf7gQPd6iI01eVWsvDEjUGx +wwLbYuyA0P921IbkBBq2tgwrYnF92a/Z8V76wB7KoBlcVfCA0SoMB4aQnzXjKCtb +7yPIox2kozru/oPcgkwlsE3FUa2em9NbhMIaWukCAwEAAaOCAXYwggFyMA4GA1Ud +DwEB/wQEAwIBhjAdBgNVHSUEFjAUBggrBgEFBQcDAQYIKwYBBQUHAwIwEgYDVR0T +AQH/BAgwBgEB/wIBADAdBgNVHQ4EFgQU1fyeDd8eyt0Il5duK8VfxSv17LgwHwYD +VR0jBBgwFoAU5K8rJnEaK0gnhS9SZizv8IkTcT4waAYIKwYBBQUHAQEEXDBaMCYG +CCsGAQUFBzABhhpodHRwOi8vb2NzcC5wa2kuZ29vZy9ndHNyMTAwBggrBgEFBQcw +AoYkaHR0cDovL3BraS5nb29nL3JlcG8vY2VydHMvZ3RzcjEuZGVyMDQGA1UdHwQt +MCswKaAnoCWGI2h0dHA6Ly9jcmwucGtpLmdvb2cvZ3RzcjEvZ3RzcjEuY3JsME0G +A1UdIARGMEQwOAYKKwYBBAHWeQIFAzAqMCgGCCsGAQUFBwIBFhxodHRwczovL3Br +aS5nb29nL3JlcG9zaXRvcnkvMAgGBmeBDAECATANBgkqhkiG9w0BAQsFAAOCAgEA +bGMn7iPf5VJoTYFmkYXffWXlWzcxCCayB12avrHKAbmtv5139lEd15jFC0mhe6HX +02jlRA+LujbdQoJ30o3d9T/768gHmJPuWtC1Pd5LHC2MTex+jHv+TkD98LSzWQIQ +UVzjwCv9twZIUX4JXj8P3Kf+l+d5xQ5EiXjFaVkpoJo6SDYpppSTVS24R7XplrWf +B82mqz4yisCGg8XBQcifLzWODcAHeuGsyWW1y4qn3XHYYWU5hKwyPvd6NvFWn1ep +QW1akKfbOup1gAxjC2l0bwdMFfM3KKUZpG719iDNY7J+xCsJdYna0Twuck82GqGe +RNDNm6YjCD+XoaeeWqX3CZStXXZdKFbRGmZRUQd73j2wyO8weiQtvrizhvZL9/C1 +T//Oxvn2PyonCA8JPiNax+NCLXo25D2YlmA5mOrR22Mq63gJsU4hs463zj6S8ZVc +pDnQwCvIUxX10i+CzQZ0Z5mQdzcKly3FHB700FvpFePqAgnIE9cTcGW/+4ibWiW+ +dwnhp2pOEXW5Hk3xABtqZnmOw27YbaIiom0F+yzy8VDloNHYnzV9/HCrWSoC8b6w +0/H4zRK5aiWQW+OFIOb12stAHBk0IANhd7p/SA9JCynr52Fkx2PRR+sc4e6URu85 +c8zuTyuN3PtYp7NlIJmVuftVb9eWbpQ99HqSjmMd320= +-----END CERTIFICATE-----
diff --git a/pki/verify_certificate_chain.cc b/pki/verify_certificate_chain.cc index 1d3d0c0..c42f757 100644 --- a/pki/verify_certificate_chain.cc +++ b/pki/verify_certificate_chain.cc
@@ -84,9 +84,13 @@ // Adds errors to |errors| if the certificate contains unconsumed _critical_ // extensions. void VerifyNoUnconsumedCriticalExtensions(const ParsedCertificate &cert, - CertErrors *errors) { + CertErrors *errors, + bool allow_precertificate) { for (const auto &it : cert.extensions()) { const ParsedExtension &extension = it.second; + if (allow_precertificate && extension.oid == der::Input(kCtPoisonOid)) { + continue; + } if (extension.critical && !IsHandledCriticalExtension(extension, cert)) { errors->AddError(cert_errors::kUnconsumedCriticalExtension, CreateCertErrorParams2Der("oid", extension.oid, "value", @@ -659,7 +663,7 @@ // Procedure". It does processing for the final certificate (the target cert). void WrapUp(const ParsedCertificate &cert, KeyPurpose required_key_purpose, const std::set<der::Input> &user_initial_policy_set, - CertErrors *errors); + bool allow_precertificate, CertErrors *errors); // Enforces trust anchor constraints compatibile with RFC 5937. // @@ -1165,7 +1169,8 @@ // the certificate. Process any other recognized non-critical // extension present in the certificate that is relevant to path // processing. - VerifyNoUnconsumedCriticalExtensions(cert, errors); + VerifyNoUnconsumedCriticalExtensions(cert, errors, + delegate_->AcceptPreCertificates()); } // Checks if the target certificate has the CA bit set. If it does, add @@ -1196,7 +1201,8 @@ void PathVerifier::WrapUp(const ParsedCertificate &cert, KeyPurpose required_key_purpose, const std::set<der::Input> &user_initial_policy_set, - CertErrors *errors) { + bool allow_precertificate, + CertErrors * errors) { // From RFC 5280 section 6.1.5: // (a) If explicit_policy is not 0, decrement explicit_policy by 1. if (explicit_policy_ > 0) { @@ -1224,7 +1230,7 @@ // // Note that this is duplicated by PrepareForNextCertificate() so as to // directly match the procedures in RFC 5280's section 6.1. - VerifyNoUnconsumedCriticalExtensions(cert, errors); + VerifyNoUnconsumedCriticalExtensions(cert, errors, allow_precertificate); // This calculates the intersection from RFC 5280 section 6.1.5 step g, as // well as applying the deferred recursive node that were skipped earlier in @@ -1325,7 +1331,8 @@ // Extensions may be marked critical or not critical. When trust anchor // constraints are enforced, clients MUST reject certification paths // containing a trust anchor with unrecognized critical extensions. - VerifyNoUnconsumedCriticalExtensions(cert, errors); + VerifyNoUnconsumedCriticalExtensions(cert, errors, + /*allow_precertificate=*/false); } void PathVerifier::ProcessRootCertificate(const ParsedCertificate &cert, @@ -1427,7 +1434,8 @@ // Checking for unknown critical extensions matches Windows, but is stricter // than the Mac verifier. - VerifyNoUnconsumedCriticalExtensions(cert, errors); + VerifyNoUnconsumedCriticalExtensions(cert, errors, + /*allow_precertificate=*/false); } bssl::UniquePtr<EVP_PKEY> PathVerifier::ParseAndCheckPublicKey( @@ -1572,7 +1580,8 @@ if (!is_target_cert) { PrepareForNextCertificate(cert, cert_errors); } else { - WrapUp(cert, required_key_purpose, user_initial_policy_set, cert_errors); + WrapUp(cert, required_key_purpose, user_initial_policy_set, + delegate->AcceptPreCertificates(), cert_errors); } }
diff --git a/pki/verify_certificate_chain.h b/pki/verify_certificate_chain.h index 718eceb..b747fac 100644 --- a/pki/verify_certificate_chain.h +++ b/pki/verify_certificate_chain.h
@@ -74,6 +74,13 @@ // is no verification cache. virtual SignatureVerifyCache *GetVerifyCache() = 0; + // This is called to determine if PreCertificates should be accepted, for the + // purpose of validating issued PreCertificates in a path. Most callers should + // return false here. This should never return true for TLS certificate + // validation. If this function returns true the CT precertificate poison + // extension will not prevent the certificate from being validated. + virtual bool AcceptPreCertificates() = 0; + virtual ~VerifyCertificateChainDelegate(); };
diff --git a/sources.cmake b/sources.cmake index c65afb9..6afc739 100644 --- a/sources.cmake +++ b/sources.cmake
@@ -1430,6 +1430,8 @@ pki/testdata/path_builder_unittest/multi-root-D-by-D.pem pki/testdata/path_builder_unittest/multi-root-E-by-E.pem pki/testdata/path_builder_unittest/multi-root-F-by-E.pem + pki/testdata/path_builder_unittest/precertificate/precertificate.pem + pki/testdata/path_builder_unittest/precertificate/root.pem pki/testdata/path_builder_unittest/self_issued_prioritization/generate-certs.py pki/testdata/path_builder_unittest/self_issued_prioritization/keys/Root1.key pki/testdata/path_builder_unittest/self_issued_prioritization/keys/Root2.key