Fix inhibitPolicyMapping in the new policy tree code. There was a typo and inhibitPolicyMapping updated the wrong value. With this fixed, we pass the PKITS tests (as imported into Chromium). Change-Id: I3b80eb56561ae5ae88023fa639d697a9f1757b21 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/56205 Commit-Queue: David Benjamin <davidben@google.com> Auto-Submit: David Benjamin <davidben@google.com> Reviewed-by: Bob Beck <bbe@google.com> Commit-Queue: Bob Beck <bbe@google.com>
diff --git a/crypto/x509/policy.c b/crypto/x509/policy.c index 5f9fe9c..393cb84 100644 --- a/crypto/x509/policy.c +++ b/crypto/x509/policy.c
@@ -563,7 +563,7 @@ } int ok = apply_skip_certs(constraints->requireExplicitPolicy, explicit_policy) && - apply_skip_certs(constraints->inhibitPolicyMapping, inhibit_any_policy); + apply_skip_certs(constraints->inhibitPolicyMapping, policy_mapping); POLICY_CONSTRAINTS_free(constraints); if (!ok) { return 0;
diff --git a/crypto/x509/test/make_policy_certs.go b/crypto/x509/test/make_policy_certs.go index 8d3a11d..c57d973 100644 --- a/crypto/x509/test/make_policy_certs.go +++ b/crypto/x509/test/make_policy_certs.go
@@ -103,6 +103,20 @@ panic(err) } + root2 := templateAndKey{ + template: x509.Certificate{ + SerialNumber: new(big.Int).SetInt64(1), + Subject: pkix.Name{CommonName: "Policy Root 2"}, + NotBefore: notBefore, + NotAfter: notAfter, + BasicConstraintsValid: true, + IsCA: true, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + KeyUsage: x509.KeyUsageCertSign, + SignatureAlgorithm: x509.ECDSAWithSHA256, + }, + key: rootKey, + } root := templateAndKey{ template: x509.Certificate{ SerialNumber: new(big.Int).SetInt64(1), @@ -154,6 +168,10 @@ mustGenerateCertificate("policy_intermediate.pem", &intermediate, &root) mustGenerateCertificate("policy_leaf.pem", &leaf, &intermediate) + // root2 is used for tests that need a longer chain, using a Root/Root2 + // cross-sign as one of the certificates. + mustGenerateCertificate("policy_root2.pem", &root2, &root2) + // Introduce syntax errors in the leaf and intermediate. leafInvalid := leaf leafInvalid.template.PolicyIdentifiers = nil @@ -284,9 +302,18 @@ leafSingle.template.PolicyIdentifiers = []asn1.ObjectIdentifier{testOID5} mustGenerateCertificate("policy_leaf_oid5.pem", &leafSingle, &intermediate) - // TODO(davidben): Generate more certificates to test policy validation more - // extensively, including an intermediate with constraints. For now this - // just tests the basic case. + // Make version of Root, signed by Root 2, with policy mapping inhibited. + // This can be combined with intermediateMapped to test the combination. + b = cryptobyte.NewBuilder(nil) + b.AddASN1(cbasn1.SEQUENCE, func(seq *cryptobyte.Builder) { + seq.AddASN1Int64WithTag(0, cbasn1.Tag(1).ContextSpecific()) + }) + inhibitPolicyMapping0 := b.BytesOrPanic() + + inhibitMapping := root + inhibitMapping.template.PolicyIdentifiers = []asn1.ObjectIdentifier{anyPolicyOID} + inhibitMapping.template.ExtraExtensions = []pkix.Extension{{Id: policyConstraintsOID, Value: inhibitPolicyMapping0}} + mustGenerateCertificate("policy_root_cross_inhibit_mapping.pem", &inhibitMapping, &root2) } const leafKeyPEM = `-----BEGIN PRIVATE KEY-----
diff --git a/crypto/x509/test/policy_root2.pem b/crypto/x509/test/policy_root2.pem new file mode 100644 index 0000000..1350035 --- /dev/null +++ b/crypto/x509/test/policy_root2.pem
@@ -0,0 +1,10 @@ +-----BEGIN CERTIFICATE----- +MIIBeDCCAR6gAwIBAgIBATAKBggqhkjOPQQDAjAYMRYwFAYDVQQDEw1Qb2xpY3kg +Um9vdCAyMCAXDTAwMDEwMTAwMDAwMFoYDzIxMDAwMTAxMDAwMDAwWjAYMRYwFAYD +VQQDEw1Qb2xpY3kgUm9vdCAyMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEJnal +2JdRr2O8t43E0yujFVyEK+OE6goUWCOiHlfbSAeoyLDmPkKJdW5PMf+wORRjp1Fh +VSxADxD0icD53Y6JbaNXMFUwDgYDVR0PAQH/BAQDAgIEMBMGA1UdJQQMMAoGCCsG +AQUFBwMBMA8GA1UdEwEB/wQFMAMBAf8wHQYDVR0OBBYEFNBp56Afsnjet1jLYZ4e +lIdRx3USMAoGCCqGSM49BAMCA0gAMEUCIQDm9rw9ODVtJUPBn2lWoK8s7ElbyY4/ +Gc2thHR50UUzbgIgKRenEDhKiBR6cGC77RaIiaaafW8b7HMd7obuZdDU/58= +-----END CERTIFICATE-----
diff --git a/crypto/x509/test/policy_root_cross_inhibit_mapping.pem b/crypto/x509/test/policy_root_cross_inhibit_mapping.pem new file mode 100644 index 0000000..9273a53 --- /dev/null +++ b/crypto/x509/test/policy_root_cross_inhibit_mapping.pem
@@ -0,0 +1,11 @@ +-----BEGIN CERTIFICATE----- +MIIBljCCAT2gAwIBAgIBATAKBggqhkjOPQQDAjAYMRYwFAYDVQQDEw1Qb2xpY3kg +Um9vdCAyMCAXDTAwMDEwMTAwMDAwMFoYDzIxMDAwMTAxMDAwMDAwWjAWMRQwEgYD +VQQDEwtQb2xpY3kgUm9vdDBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABCZ2pdiX +Ua9jvLeNxNMroxVchCvjhOoKFFgjoh5X20gHqMiw5j5CiXVuTzH/sDkUY6dRYVUs +QA8Q9InA+d2OiW2jeDB2MA4GA1UdDwEB/wQEAwICBDATBgNVHSUEDDAKBggrBgEF +BQcDATAPBgNVHRMBAf8EBTADAQH/MB0GA1UdDgQWBBTQaeegH7J43rdYy2GeHpSH +Ucd1EjARBgNVHSAECjAIMAYGBFUdIAAwDAYDVR0kBAUwA4EBADAKBggqhkjOPQQD +AgNHADBEAiBzR3JGEf9PITYuiXTx+vx9gXji5idGsVog9wRUbY98wwIgVVeYNQQb +x+RN2wYp3kmm8iswUOrqiI6J4PSzT8CYP8Q= +-----END CERTIFICATE-----
diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc index e4af13e..32b2483 100644 --- a/crypto/x509/x509_test.cc +++ b/crypto/x509/x509_test.cc
@@ -5115,6 +5115,13 @@ bssl::UniquePtr<X509> root( CertFromPEM(GetTestData("crypto/x509/test/policy_root.pem").c_str())); ASSERT_TRUE(root); + bssl::UniquePtr<X509> root_cross_inhibit_mapping(CertFromPEM( + GetTestData("crypto/x509/test/policy_root_cross_inhibit_mapping.pem") + .c_str())); + ASSERT_TRUE(root_cross_inhibit_mapping); + bssl::UniquePtr<X509> root2( + CertFromPEM(GetTestData("crypto/x509/test/policy_root2.pem").c_str())); + ASSERT_TRUE(root2); bssl::UniquePtr<X509> intermediate(CertFromPEM( GetTestData("crypto/x509/test/policy_intermediate.pem").c_str())); ASSERT_TRUE(intermediate); @@ -5516,6 +5523,25 @@ [&](X509_VERIFY_PARAM *param) { set_policies(param, {oid4.get()}); })); + + // Policy mapping can be inhibited, either by the caller or a certificate in + // the chain, in which case mapped policies are unassertable (apart from some + // anyPolicy edge cases). + EXPECT_EQ( + X509_V_ERR_NO_EXPLICIT_POLICY, + Verify(leaf_oid1.get(), {root.get()}, {intermediate_mapped_oid3.get()}, + /*crls=*/{}, X509_V_FLAG_EXPLICIT_POLICY | X509_V_FLAG_INHIBIT_MAP, + [&](X509_VERIFY_PARAM *param) { + set_policies(param, {oid3.get()}); + })); + EXPECT_EQ( + X509_V_ERR_NO_EXPLICIT_POLICY, + Verify(leaf_oid1.get(), {root2.get()}, + {intermediate_mapped_oid3.get(), root_cross_inhibit_mapping.get()}, + /*crls=*/{}, X509_V_FLAG_EXPLICIT_POLICY, + [&](X509_VERIFY_PARAM *param) { + set_policies(param, {oid3.get()}); + })); } #if defined(OPENSSL_THREADS)
diff --git a/sources.cmake b/sources.cmake index 881cb15..021e920 100644 --- a/sources.cmake +++ b/sources.cmake
@@ -134,7 +134,9 @@ crypto/x509/test/policy_leaf_require.pem crypto/x509/test/policy_leaf_require1.pem crypto/x509/test/policy_leaf.pem + crypto/x509/test/policy_root_cross_inhibit_mapping.pem crypto/x509/test/policy_root.pem + crypto/x509/test/policy_root2.pem crypto/x509/test/pss_sha1_explicit.pem crypto/x509/test/pss_sha1_mgf1_syntax_error.pem crypto/x509/test/pss_sha1.pem