commit 5386d908a663a424d154cbae7f76f2266c60c2fe
author Bob Beck <bbe@google.com> Tue Apr 08 23:48:17 2025 +0000
committer Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Thu Apr 10 10:02:39 2025 -0700
tree be84e65675e7c6d68fb97300a3e06239477ca6d4
parent 5219abf59b071d2b7c454e5827a368d4db97691f

Fix backward check in crl_crldp_check

I missed this on review and stared at it a few times
before realizing because the "score" code is pretty obtuse.

If we don't have reasons, and we don't have a CRLissuer, we will
have already checked that the signer of the CRL matches the issuer
of the cert (The "Otherwise" part of step b.1), and so we want
to continue to do step b.2. If there is an issuing distribution point
in the crl check to see if one of the names in it matches the names in the
distribution point in the cert.

Fixed: 400486977
Bug: 409778435

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

crypto/x509/x509_vfy.cc

diff --git a/crypto/x509/x509_vfy.cc b/crypto/x509/x509_vfy.cc
index 03f4612..92f154c 100644
--- a/crypto/x509/x509_vfy.cc
+++ b/crypto/x509/x509_vfy.cc
@@ -1020,6 +1020,8 @@
 
 // Check CRLDP and IDP
 static int crl_crldp_check(X509 *x, X509_CRL *crl, int crl_score) {
+  // TODO(bbe): crbug.com/409778435 Make tests for the corner cases we hit
+  // here so that we stay correct for RFC 5280 6.3.3 steps b.1 and b.2
   if (crl->idp_flags & IDP_ONLYATTR) {
     return 0;
   }
@@ -1041,9 +1043,14 @@
     //
     // We also do not support indirect CRLs, and a CRL issuer can only match
     // indirect CRLs (RFC 5280, section 6.3.3, step b.1).
-    // support.
-    if (dp->reasons != NULL && dp->CRLissuer != NULL &&
-        (!crl->idp || idp_check_dp(dp->distpoint, crl->idp->distpoint))) {
+    if (dp->reasons != NULL || dp->CRLissuer != NULL) {
+      continue;
+    }
+    // At this point we have already checked that the CRL issuer matches
+    // the certificate issuer (and set CRL_SCORE_ISSUER_NAME);
+
+    // RFC 5280 Section 6.3.3 step b.2
+    if (!crl->idp || idp_check_dp(dp->distpoint, crl->idp->distpoint)){
       return 1;
     }
   }