commit fca688f26b939db9c9981204373cecbd108b5d6c
author David Benjamin <davidben@google.com> Wed Mar 29 14:00:20 2023 +0900
committer Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Mar 29 15:18:36 2023 +0000
tree 0f03f880be1511c64f890de933837cf446b10757
parent 678bae408369d614e0777bc5cd5a380dac35ed59

Fix policy validation when the user policy set is NULL

OpenSSL interprets NULL and empty lists as {anyPolicy}. I intended to
implement this, but didn't quite get the NULL case right. Fix this and
add a test.

Change-Id: I50dbf02695f424697e28a6e0ec4fd50b2822f44f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58425
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>

crypto/x509/internal.h

diff --git a/crypto/x509/internal.h b/crypto/x509/internal.h
index 45c3fcc..1937aa1 100644
--- a/crypto/x509/internal.h
+++ b/crypto/x509/internal.h
@@ -404,7 +404,8 @@
 // Path-building functions.
 
 // X509_policy_check checks certificate policies in |certs|. |user_policies| is
-// the user-initial-policy-set. |flags| is a set of |X509_V_FLAG_*| values to
+// the user-initial-policy-set. If |user_policies| is NULL or empty, it is
+// interpreted as anyPolicy. |flags| is a set of |X509_V_FLAG_*| values to
 // apply. It returns |X509_V_OK| on success and |X509_V_ERR_*| on error. It
 // additionally sets |*out_current_cert| to the certificate where the error
 // occurred. If the function succeeded, or the error applies to the entire

crypto/x509/policy.c

diff --git a/crypto/x509/policy.c b/crypto/x509/policy.c
index abcc586..b0c2712 100644
--- a/crypto/x509/policy.c
+++ b/crypto/x509/policy.c
@@ -579,7 +579,7 @@
 // evaluation.
 static int has_explicit_policy(STACK_OF(X509_POLICY_LEVEL) *levels,
                                const STACK_OF(ASN1_OBJECT) *user_policies) {
-  assert(sk_ASN1_OBJECT_is_sorted(user_policies));
+  assert(user_policies == NULL || sk_ASN1_OBJECT_is_sorted(user_policies));
 
   // Step (g.i). If the policy graph is empty, the intersection is empty.
   size_t num_levels = sk_X509_POLICY_LEVEL_num(levels);
@@ -763,12 +763,14 @@
   // is only necessary to check if the user-constrained-policy-set is not empty.
   if (explicit_policy == 0) {
     // Build a sorted copy of |user_policies| for more efficient lookup.
-    user_policies_sorted = sk_ASN1_OBJECT_dup(user_policies);
-    if (user_policies_sorted == NULL) {
-      goto err;
+    if (user_policies != NULL) {
+      user_policies_sorted = sk_ASN1_OBJECT_dup(user_policies);
+      if (user_policies_sorted == NULL) {
+        goto err;
+      }
+      sk_ASN1_OBJECT_set_cmp_func(user_policies_sorted, asn1_object_cmp);
+      sk_ASN1_OBJECT_sort(user_policies_sorted);
     }
-    sk_ASN1_OBJECT_set_cmp_func(user_policies_sorted, asn1_object_cmp);
-    sk_ASN1_OBJECT_sort(user_policies_sorted);
 
     if (!has_explicit_policy(levels, user_policies_sorted)) {
       ret = X509_V_ERR_NO_EXPLICIT_POLICY;

crypto/x509/x509_test.cc

diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc
index 653af39..03f5d19 100644
--- a/crypto/x509/x509_test.cc
+++ b/crypto/x509/x509_test.cc
@@ -5358,6 +5358,8 @@
   };
 
   // The chain is good for |oid1| and |oid2|, but not |oid3|.
+  EXPECT_EQ(X509_V_OK, Verify(leaf.get(), {root.get()}, {intermediate.get()},
+                              /*crls=*/{}, X509_V_FLAG_EXPLICIT_POLICY));
   EXPECT_EQ(X509_V_OK,
             Verify(leaf.get(), {root.get()}, {intermediate.get()}, /*crls=*/{},
                    X509_V_FLAG_EXPLICIT_POLICY, [&](X509_VERIFY_PARAM *param) {
@@ -5442,6 +5444,9 @@
                    }));
 
   // A leaf can also set requireExplicitPolicy.
+  EXPECT_EQ(X509_V_OK,
+            Verify(leaf_require.get(), {root.get()}, {intermediate.get()},
+                   /*crls=*/{}, X509_V_FLAG_POLICY_CHECK));
   EXPECT_EQ(X509_V_OK, Verify(leaf_require.get(), {root.get()},
                               {intermediate.get()}, /*crls=*/{},
                               /*flags=*/0, [&](X509_VERIFY_PARAM *param) {