Add tests for rejecting duplicate policy OIDs.
Change-Id: I738490d70208885481f638273d663140444c3a76
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/55825
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/x509/test/make_policy_certs.go b/crypto/x509/test/make_policy_certs.go
index 18a6ffe..a4da0a2 100644
--- a/crypto/x509/test/make_policy_certs.go
+++ b/crypto/x509/test/make_policy_certs.go
@@ -138,6 +138,15 @@
intermediateInvalid.template.ExtraExtensions = []pkix.Extension{{Id: certificatePoliciesOID, Value: []byte("INVALID")}}
mustGenerateCertificate("policy_intermediate_invalid.pem", &intermediateInvalid, &root)
+ // Duplicates are not allowed in certificatePolicies.
+ leafDuplicate := leaf
+ leafDuplicate.template.PolicyIdentifiers = []asn1.ObjectIdentifier{testOID1, testOID2, testOID2}
+ mustGenerateCertificate("policy_leaf_duplicate.pem", &leafDuplicate, &root)
+
+ intermediateDuplicate := intermediate
+ intermediateDuplicate.template.PolicyIdentifiers = []asn1.ObjectIdentifier{testOID1, testOID2, testOID2}
+ mustGenerateCertificate("policy_intermediate_duplicate.pem", &intermediateDuplicate, &root)
+
// TODO(davidben): Generate more certificates to test policy validation more
// extensively, including an intermediate with constraints. For now this
// just tests the basic case.
diff --git a/crypto/x509/test/policy_intermediate_duplicate.pem b/crypto/x509/test/policy_intermediate_duplicate.pem
new file mode 100644
index 0000000..af0fe89
--- /dev/null
+++ b/crypto/x509/test/policy_intermediate_duplicate.pem
@@ -0,0 +1,12 @@
+-----BEGIN CERTIFICATE-----
+MIIBuzCCAWKgAwIBAgIBAjAKBggqhkjOPQQDAjAWMRQwEgYDVQQDEwtQb2xpY3kg
+Um9vdDAgFw0wMDAxMDEwMDAwMDBaGA8yMTAwMDEwMTAwMDAwMFowHjEcMBoGA1UE
+AxMTUG9saWN5IEludGVybWVkaWF0ZTBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IA
+BOI6fKiM3jFLkLyAn88cvlw4SwxuygRjopP3FFBKHyUQvh3VVvfqSpSCSmp50Qia
+jQ6Dg7CTpVZVVH+bguT7JTCjgZYwgZMwDgYDVR0PAQH/BAQDAgIEMBMGA1UdJQQM
+MAoGCCsGAQUFBwMBMA8GA1UdEwEB/wQFMAMBAf8wHQYDVR0OBBYEFJDS9/4O7qhr
+CIRhwsXrPVBagG2uMDwGA1UdIAQ1MDMwDwYNKoZIhvcSBAGEtwkCATAPBg0qhkiG
+9xIEAYS3CQICMA8GDSqGSIb3EgQBhLcJAgIwCgYIKoZIzj0EAwIDRwAwRAIgCnvy
+K47AK/Ve/rzcFSm1fcjFg9UwZoTvOAhZU/xpfLgCIFV4vHl6jsGq9rPs4KblSsIY
+VBjAjG2AYkH0Lq+O4LjO
+-----END CERTIFICATE-----
diff --git a/crypto/x509/test/policy_leaf_duplicate.pem b/crypto/x509/test/policy_leaf_duplicate.pem
new file mode 100644
index 0000000..80fce22
--- /dev/null
+++ b/crypto/x509/test/policy_leaf_duplicate.pem
@@ -0,0 +1,12 @@
+-----BEGIN CERTIFICATE-----
+MIIBsjCCAVigAwIBAgIBAzAKBggqhkjOPQQDAjAWMRQwEgYDVQQDEwtQb2xpY3kg
+Um9vdDAgFw0wMDAxMDEwMDAwMDBaGA8yMTAwMDEwMTAwMDAwMFowGjEYMBYGA1UE
+AxMPd3d3LmV4YW1wbGUuY29tMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEkSrY
+vFVtkZJmvirfY0JDDYrZQrNJecPLt0ksJux2URL5nAQiQY1SERGnEaiNLpoc0dle
+TS8wQT/cjw/wPgoeV6OBkDCBjTAOBgNVHQ8BAf8EBAMCAgQwEwYDVR0lBAwwCgYI
+KwYBBQUHAwEwDAYDVR0TAQH/BAIwADAaBgNVHREEEzARgg93d3cuZXhhbXBsZS5j
+b20wPAYDVR0gBDUwMzAPBg0qhkiG9xIEAYS3CQIBMA8GDSqGSIb3EgQBhLcJAgIw
+DwYNKoZIhvcSBAGEtwkCAjAKBggqhkjOPQQDAgNIADBFAiEA3MEtsp3pypprhmPB
+UbMC7FwvK+YZI5qo5dDRGUu0H6QCIEbUDagJc0qNdvZ4H//E/cvqb8dH6UmmIXVX
+/WMkIJt2
+-----END CERTIFICATE-----
diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc
index a69b284..5dca616 100644
--- a/crypto/x509/x509_test.cc
+++ b/crypto/x509/x509_test.cc
@@ -5115,12 +5115,19 @@
bssl::UniquePtr<X509> intermediate_invalid(CertFromPEM(
GetTestData("crypto/x509/test/policy_intermediate_invalid.pem").c_str()));
ASSERT_TRUE(intermediate_invalid);
+ bssl::UniquePtr<X509> intermediate_duplicate(CertFromPEM(
+ GetTestData("crypto/x509/test/policy_intermediate_duplicate.pem")
+ .c_str()));
+ ASSERT_TRUE(intermediate_duplicate);
bssl::UniquePtr<X509> leaf(
CertFromPEM(GetTestData("crypto/x509/test/policy_leaf.pem").c_str()));
ASSERT_TRUE(leaf);
bssl::UniquePtr<X509> leaf_invalid(CertFromPEM(
GetTestData("crypto/x509/test/policy_leaf_invalid.pem").c_str()));
ASSERT_TRUE(leaf_invalid);
+ bssl::UniquePtr<X509> leaf_duplicate(CertFromPEM(
+ GetTestData("crypto/x509/test/policy_leaf_duplicate.pem").c_str()));
+ ASSERT_TRUE(leaf_duplicate);
// By default, OpenSSL does not check policies, so even syntax errors in the
// certificatePolicies extension go unnoticed. (This is probably not
@@ -5168,17 +5175,31 @@
set_policies(param, {oid1.get(), oid3.get()});
}));
- // The policy extension in the intermediate cannot be parsed.
+ // The policy extension cannot be parsed.
EXPECT_EQ(X509_V_ERR_INVALID_POLICY_EXTENSION,
Verify(leaf.get(), {root.get()}, {intermediate_invalid.get()},
/*crls=*/{}, X509_V_FLAG_EXPLICIT_POLICY,
[&](X509_VERIFY_PARAM *param) {
set_policies(param, {oid1.get()});
}));
+ EXPECT_EQ(X509_V_ERR_INVALID_POLICY_EXTENSION,
+ Verify(leaf_invalid.get(), {root.get()}, {intermediate.get()},
+ /*crls=*/{}, X509_V_FLAG_EXPLICIT_POLICY,
+ [&](X509_VERIFY_PARAM *param) {
+ set_policies(param, {oid1.get()});
+ }));
+
+ // There is a duplicate policy in the policy extension.
+ EXPECT_EQ(X509_V_ERR_INVALID_POLICY_EXTENSION,
+ Verify(leaf.get(), {root.get()}, {intermediate_duplicate.get()},
+ /*crls=*/{}, X509_V_FLAG_EXPLICIT_POLICY,
+ [&](X509_VERIFY_PARAM *param) {
+ set_policies(param, {oid1.get()});
+ }));
// The policy extension in the leaf cannot be parsed.
EXPECT_EQ(X509_V_ERR_INVALID_POLICY_EXTENSION,
- Verify(leaf_invalid.get(), {root.get()}, {intermediate.get()},
+ Verify(leaf_duplicate.get(), {root.get()}, {intermediate.get()},
/*crls=*/{}, X509_V_FLAG_EXPLICIT_POLICY,
[&](X509_VERIFY_PARAM *param) {
set_policies(param, {oid1.get()});
diff --git a/sources.cmake b/sources.cmake
index 69a1f64..46788fa 100644
--- a/sources.cmake
+++ b/sources.cmake
@@ -112,8 +112,10 @@
crypto/x509/test/many_names2.pem
crypto/x509/test/many_names3.pem
crypto/x509/test/policy_root.pem
+ crypto/x509/test/policy_intermediate_duplicate.pem
crypto/x509/test/policy_intermediate_invalid.pem
crypto/x509/test/policy_intermediate.pem
+ crypto/x509/test/policy_leaf_duplicate.pem
crypto/x509/test/policy_leaf_invalid.pem
crypto/x509/test/policy_leaf.pem
crypto/x509/test/pss_sha1_explicit.pem
