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>

crypto/x509/test/make_policy_certs.go

@@ -138,6 +138,15 @@ func main() {
 	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.

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-----

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-----

crypto/x509/x509_test.cc

@@ -5115,12 +5115,19 @@ TEST(X509Test, Policy) {
   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 @@ TEST(X509Test, Policy) {
                      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()});

sources.cmake

@@ -112,8 +112,10 @@ set(
   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