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
