From 56baf78d85019db5e758a31073e62d693df35556 Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@google.com>
Date: Sun, 12 Nov 2023 13:48:29 -0500
Subject: [PATCH 01/13] Remove EXFLAG_FRESHEST

Update-Note: Though exported, this was an internal flag to the delta CRL
implementation. Remove it.

Bug: 601
Change-Id: Ic7f99da94391aea861fd7ea9ad79a3fb66cc649e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63930
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
(cherry picked from commit 3e28180d3a337a524c07c9301a967aaa739a10ec)
---
 crypto/x509/x_crl.c      | 18 +++++-------------
 crypto/x509v3/v3_purp.c  |  3 ---
 include/openssl/x509v3.h |  1 -
 3 files changed, 5 insertions(+), 17 deletions(-)

diff --git a/crypto/x509/x_crl.c b/crypto/x509/x_crl.c
index 8bab2b0569..86c5921344 100644
--- a/crypto/x509/x_crl.c
+++ b/crypto/x509/x_crl.c
@@ -190,8 +190,6 @@ static int crl_set_issuers(X509_CRL *crl) {
 static int crl_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it,
                   void *exarg) {
   X509_CRL *crl = (X509_CRL *)*pval;
-  STACK_OF(X509_EXTENSION) *exts;
-  X509_EXTENSION *ext;
   size_t idx;
   int i;
 
@@ -266,20 +264,14 @@ static int crl_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it,
       // this in a flag. We only currently handle IDP so anything else
       // critical sets the flag. This code accesses the X509_CRL structure
       // directly: applications shouldn't do this.
-
-      exts = crl->crl->extensions;
-
+      const STACK_OF(X509_EXTENSION) *exts = crl->crl->extensions;
       for (idx = 0; idx < sk_X509_EXTENSION_num(exts); idx++) {
-        int nid;
-        ext = sk_X509_EXTENSION_value(exts, idx);
-        nid = OBJ_obj2nid(X509_EXTENSION_get_object(ext));
-        if (nid == NID_freshest_crl) {
-          crl->flags |= EXFLAG_FRESHEST;
-        }
+        const X509_EXTENSION *ext = sk_X509_EXTENSION_value(exts, idx);
+        int nid = OBJ_obj2nid(X509_EXTENSION_get_object(ext));
         if (X509_EXTENSION_get_critical(ext)) {
           // We handle IDP and deltas
-          if ((nid == NID_issuing_distribution_point) ||
-              (nid == NID_authority_key_identifier) || (nid == NID_delta_crl)) {
+          if (nid == NID_issuing_distribution_point ||
+              nid == NID_authority_key_identifier || nid == NID_delta_crl) {
             continue;
           }
           crl->flags |= EXFLAG_CRITICAL;
diff --git a/crypto/x509v3/v3_purp.c b/crypto/x509v3/v3_purp.c
index c25a8c26a2..36dbdb9e90 100644
--- a/crypto/x509v3/v3_purp.c
+++ b/crypto/x509v3/v3_purp.c
@@ -554,9 +554,6 @@ int x509v3_cache_extensions(X509 *x) {
 
   for (j = 0; j < X509_get_ext_count(x); j++) {
     const X509_EXTENSION *ex = X509_get_ext(x, j);
-    if (OBJ_obj2nid(X509_EXTENSION_get_object(ex)) == NID_freshest_crl) {
-      x->ex_flags |= EXFLAG_FRESHEST;
-    }
     if (!X509_EXTENSION_get_critical(ex)) {
       continue;
     }
diff --git a/include/openssl/x509v3.h b/include/openssl/x509v3.h
index 529cd6025a..c55b421874 100644
--- a/include/openssl/x509v3.h
+++ b/include/openssl/x509v3.h
@@ -349,7 +349,6 @@ struct ISSUING_DIST_POINT_st {
 #define EXFLAG_SET 0x100
 #define EXFLAG_CRITICAL 0x200
 
-#define EXFLAG_FRESHEST 0x1000
 // Self signed
 #define EXFLAG_SS 0x2000
 

From 03c2d85928ae367c1a3cf914ccc451cb43c756a5 Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@google.com>
Date: Sun, 12 Nov 2023 19:33:53 -0500
Subject: [PATCH 02/13] Don't parse delta CRL and CRL number extensions

These extensions were only parsed for delta CRL support. Instead, ignore
CRL numbers and treat critical delta CRL extensions as unrecognized
critical extensions. This trims a pile of dead, untested code from the
verifier.

Update-Note: While this is broadly a no-op, this may change behavior
slightly at the edges. Invalid CRL number extensions will now be ignored
instead of treated as a parse error. A delta CRL that incorrectly marks
its delta CRL extension as non-critical will be interpreted as a normal
CRL. (This is the expected behavior for an implementation which does not
implement delta CRLs. Extensions like this are supposed to be marked
critical.)

Bug: 601
Change-Id: Iece9a8acce83a7c59e129499517fc8eb0d048e98
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63931
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
(cherry picked from commit 0c4481340ba40d1aa06f2e5dce8f9cee5d6d5b31)
---
 crypto/x509/internal.h |  3 --
 crypto/x509/x509_vfy.c | 64 +++++++++++++++++-------------------------
 crypto/x509/x_crl.c    | 22 +--------------
 3 files changed, 27 insertions(+), 62 deletions(-)

diff --git a/crypto/x509/internal.h b/crypto/x509/internal.h
index 420e61cd67..4d2619c35a 100644
--- a/crypto/x509/internal.h
+++ b/crypto/x509/internal.h
@@ -219,9 +219,6 @@ struct X509_crl_st {
   // Convenient breakdown of IDP
   int idp_flags;
   int idp_reasons;
-  // CRL and base CRL numbers for delta processing
-  ASN1_INTEGER *crl_number;
-  ASN1_INTEGER *base_crl_number;
   unsigned char crl_hash[SHA256_DIGEST_LENGTH];
   STACK_OF(GENERAL_NAMES) *issuers;
 } /* X509_CRL */;
diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index a26fdcce1a..3ec3366bdc 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -1058,12 +1058,6 @@ static int get_crl_score(X509_STORE_CTX *ctx, X509 **pissuer,
   if (crl->idp_flags & (IDP_INDIRECT | IDP_REASONS)) {
     return 0;
   }
-  if (crl->base_crl_number) {
-    // Don't process deltas at this stage
-    //
-    // TODO(crbug.com/boringssl/601): Clean up remnants of delta CRL support.
-    return 0;
-  }
   // If issuer name doesn't match certificate need indirect CRL
   if (X509_NAME_cmp(X509_get_issuer_name(x), X509_CRL_get_issuer(crl))) {
     if (!(crl->idp_flags & IDP_INDIRECT)) {
@@ -1318,9 +1312,7 @@ static int crl_crldp_check(X509 *x, X509_CRL *crl, int crl_score,
   return 0;
 }
 
-// Retrieve CRL corresponding to current certificate. If deltas enabled try
-// to find a delta CRL too
-
+// Retrieve CRL corresponding to current certificate.
 static int get_crl_delta(X509_STORE_CTX *ctx, X509_CRL **pcrl, X509_CRL **pdcrl,
                          X509 *x) {
   int ok;
@@ -1338,7 +1330,6 @@ static int get_crl_delta(X509_STORE_CTX *ctx, X509_CRL **pcrl, X509_CRL **pdcrl,
   }
 
   // Lookup CRLs from store
-
   skcrl = ctx->lookup_crls(ctx, nm);
 
   // If no CRLs found and a near match from get_crl_sk use that
@@ -1394,38 +1385,27 @@ static int check_crl(X509_STORE_CTX *ctx, X509_CRL *crl) {
   }
 
   if (issuer) {
-    // Skip most tests for deltas because they have already been done
-    if (!crl->base_crl_number) {
-      // Check for cRLSign bit if keyUsage present
-      if ((issuer->ex_flags & EXFLAG_KUSAGE) &&
-          !(issuer->ex_kusage & KU_CRL_SIGN)) {
-        ctx->error = X509_V_ERR_KEYUSAGE_NO_CRL_SIGN;
-        ok = ctx->verify_cb(0, ctx);
-        if (!ok) {
-          goto err;
-        }
-      }
-
-      if (!(ctx->current_crl_score & CRL_SCORE_SCOPE)) {
-        ctx->error = X509_V_ERR_DIFFERENT_CRL_SCOPE;
-        ok = ctx->verify_cb(0, ctx);
-        if (!ok) {
-          goto err;
-        }
+    // Check for cRLSign bit if keyUsage present
+    if ((issuer->ex_flags & EXFLAG_KUSAGE) &&
+        !(issuer->ex_kusage & KU_CRL_SIGN)) {
+      ctx->error = X509_V_ERR_KEYUSAGE_NO_CRL_SIGN;
+      ok = ctx->verify_cb(0, ctx);
+      if (!ok) {
+        goto err;
       }
+    }
 
-      if (!(ctx->current_crl_score & CRL_SCORE_SAME_PATH)) {
-        if (check_crl_path(ctx, ctx->current_issuer) <= 0) {
-          ctx->error = X509_V_ERR_CRL_PATH_VALIDATION_ERROR;
-          ok = ctx->verify_cb(0, ctx);
-          if (!ok) {
-            goto err;
-          }
-        }
+    if (!(ctx->current_crl_score & CRL_SCORE_SCOPE)) {
+      ctx->error = X509_V_ERR_DIFFERENT_CRL_SCOPE;
+      ok = ctx->verify_cb(0, ctx);
+      if (!ok) {
+        goto err;
       }
+    }
 
-      if (crl->idp_flags & IDP_INVALID) {
-        ctx->error = X509_V_ERR_INVALID_EXTENSION;
+    if (!(ctx->current_crl_score & CRL_SCORE_SAME_PATH)) {
+      if (check_crl_path(ctx, ctx->current_issuer) <= 0) {
+        ctx->error = X509_V_ERR_CRL_PATH_VALIDATION_ERROR;
         ok = ctx->verify_cb(0, ctx);
         if (!ok) {
           goto err;
@@ -1433,6 +1413,14 @@ static int check_crl(X509_STORE_CTX *ctx, X509_CRL *crl) {
       }
     }
 
+    if (crl->idp_flags & IDP_INVALID) {
+      ctx->error = X509_V_ERR_INVALID_EXTENSION;
+      ok = ctx->verify_cb(0, ctx);
+      if (!ok) {
+        goto err;
+      }
+    }
+
     if (!(ctx->current_crl_score & CRL_SCORE_TIME)) {
       ok = check_crl_time(ctx, crl, 1);
       if (!ok) {
diff --git a/crypto/x509/x_crl.c b/crypto/x509/x_crl.c
index 86c5921344..2975424e21 100644
--- a/crypto/x509/x_crl.c
+++ b/crypto/x509/x_crl.c
@@ -201,8 +201,6 @@ static int crl_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it,
       crl->idp_flags = 0;
       crl->idp_reasons = CRLDP_ALL_REASONS;
       crl->issuers = NULL;
-      crl->crl_number = NULL;
-      crl->base_crl_number = NULL;
       break;
 
     case ASN1_OP_D2I_POST: {
@@ -245,21 +243,6 @@ static int crl_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it,
         return 0;
       }
 
-      crl->crl_number = X509_CRL_get_ext_d2i(crl, NID_crl_number, &i, NULL);
-      if (crl->crl_number == NULL && i != -1) {
-        return 0;
-      }
-
-      crl->base_crl_number = X509_CRL_get_ext_d2i(crl, NID_delta_crl, &i, NULL);
-      if (crl->base_crl_number == NULL && i != -1) {
-        return 0;
-      }
-      // Delta CRLs must have CRL number
-      if (crl->base_crl_number && !crl->crl_number) {
-        OPENSSL_PUT_ERROR(X509, X509_R_DELTA_CRL_WITHOUT_CRL_NUMBER);
-        return 0;
-      }
-
       // See if we have any unhandled critical CRL extensions and indicate
       // this in a flag. We only currently handle IDP so anything else
       // critical sets the flag. This code accesses the X509_CRL structure
@@ -269,9 +252,8 @@ static int crl_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it,
         const X509_EXTENSION *ext = sk_X509_EXTENSION_value(exts, idx);
         int nid = OBJ_obj2nid(X509_EXTENSION_get_object(ext));
         if (X509_EXTENSION_get_critical(ext)) {
-          // We handle IDP and deltas
           if (nid == NID_issuing_distribution_point ||
-              nid == NID_authority_key_identifier || nid == NID_delta_crl) {
+              nid == NID_authority_key_identifier) {
             continue;
           }
           crl->flags |= EXFLAG_CRITICAL;
@@ -289,8 +271,6 @@ static int crl_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it,
     case ASN1_OP_FREE_POST:
       AUTHORITY_KEYID_free(crl->akid);
       ISSUING_DIST_POINT_free(crl->idp);
-      ASN1_INTEGER_free(crl->crl_number);
-      ASN1_INTEGER_free(crl->base_crl_number);
       sk_GENERAL_NAMES_pop_free(crl->issuers, GENERAL_NAMES_free);
       break;
   }

From 92a7f9f8234ca8a84547f6873656f7f9aa89aa31 Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@google.com>
Date: Sun, 12 Nov 2023 14:04:00 -0500
Subject: [PATCH 03/13] Remove dcrl output parameter in CRL lookup logic

This is now always NULL.

Bug: 601
Change-Id: I030b41fd65973ca13b96c426962430ea8a6ae198
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63932
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
(cherry picked from commit d9a58a171a15a469b37b00e8b5f3c6a098a1297f)
---
 crypto/x509/x509_vfy.c | 64 ++++++++++++------------------------------
 1 file changed, 18 insertions(+), 46 deletions(-)

diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index 3ec3366bdc..b1713b4414 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -126,8 +126,7 @@ static int check_policy(X509_STORE_CTX *ctx);
 
 static int get_crl_score(X509_STORE_CTX *ctx, X509 **pissuer,
                          unsigned int *preasons, X509_CRL *crl, X509 *x);
-static int get_crl_delta(X509_STORE_CTX *ctx, X509_CRL **pcrl, X509_CRL **pdcrl,
-                         X509 *x);
+static int get_crl(X509_STORE_CTX *ctx, X509_CRL **pcrl, X509 *x);
 static void crl_akid_check(X509_STORE_CTX *ctx, X509_CRL *crl, X509 **pissuer,
                            int *pcrl_score);
 static int crl_crldp_check(X509 *x, X509_CRL *crl, int crl_score,
@@ -838,7 +837,7 @@ static int check_revocation(X509_STORE_CTX *ctx) {
 }
 
 static int check_cert(X509_STORE_CTX *ctx) {
-  X509_CRL *crl = NULL, *dcrl = NULL;
+  X509_CRL *crl = NULL;
   X509 *x;
   int ok = 0, cnum;
   unsigned int last_reasons;
@@ -854,7 +853,7 @@ static int check_cert(X509_STORE_CTX *ctx) {
     if (ctx->get_crl) {
       ok = ctx->get_crl(ctx, &crl, x);
     } else {
-      ok = get_crl_delta(ctx, &crl, &dcrl, x);
+      ok = get_crl(ctx, &crl, x);
     }
     // If error looking up CRL, nothing we can do except notify callback
     if (!ok) {
@@ -868,31 +867,13 @@ static int check_cert(X509_STORE_CTX *ctx) {
       goto err;
     }
 
-    if (dcrl) {
-      ok = ctx->check_crl(ctx, dcrl);
-      if (!ok) {
-        goto err;
-      }
-      ok = ctx->cert_crl(ctx, dcrl, x);
-      if (!ok) {
-        goto err;
-      }
-    } else {
-      ok = 1;
-    }
-
-    // Don't look in full CRL if delta reason is removefromCRL
-    if (ok != 2) {
-      ok = ctx->cert_crl(ctx, crl, x);
-      if (!ok) {
-        goto err;
-      }
+    ok = ctx->cert_crl(ctx, crl, x);
+    if (!ok) {
+      goto err;
     }
 
     X509_CRL_free(crl);
-    X509_CRL_free(dcrl);
     crl = NULL;
-    dcrl = NULL;
     // If reasons not updated we wont get anywhere by another iteration,
     // so exit loop.
     if (last_reasons == ctx->current_reasons) {
@@ -901,10 +882,9 @@ static int check_cert(X509_STORE_CTX *ctx) {
       goto err;
     }
   }
+
 err:
   X509_CRL_free(crl);
-  X509_CRL_free(dcrl);
-
   ctx->current_crl = NULL;
   return ok;
 }
@@ -978,19 +958,18 @@ static int check_crl_time(X509_STORE_CTX *ctx, X509_CRL *crl, int notify) {
   return 1;
 }
 
-static int get_crl_sk(X509_STORE_CTX *ctx, X509_CRL **pcrl, X509_CRL **pdcrl,
-                      X509 **pissuer, int *pscore, unsigned int *preasons,
+static int get_crl_sk(X509_STORE_CTX *ctx, X509_CRL **pcrl, X509 **pissuer,
+                      int *pscore, unsigned *preasons,
                       STACK_OF(X509_CRL) *crls) {
   int crl_score, best_score = *pscore;
-  size_t i;
-  unsigned int reasons, best_reasons = 0;
+  unsigned best_reasons = 0;
   X509 *x = ctx->current_cert;
-  X509_CRL *crl, *best_crl = NULL;
+  X509_CRL *best_crl = NULL;
   X509 *crl_issuer = NULL, *best_crl_issuer = NULL;
 
-  for (i = 0; i < sk_X509_CRL_num(crls); i++) {
-    crl = sk_X509_CRL_value(crls, i);
-    reasons = *preasons;
+  for (size_t i = 0; i < sk_X509_CRL_num(crls); i++) {
+    X509_CRL *crl = sk_X509_CRL_value(crls, i);
+    unsigned reasons = *preasons;
     crl_score = get_crl_score(ctx, &crl_issuer, &reasons, crl, x);
     if (crl_score < best_score || crl_score == 0) {
       continue;
@@ -1023,11 +1002,6 @@ static int get_crl_sk(X509_STORE_CTX *ctx, X509_CRL **pcrl, X509_CRL **pdcrl,
     *pscore = best_score;
     *preasons = best_reasons;
     X509_CRL_up_ref(best_crl);
-    // TODO(crbug.com/boringssl/601): Remove remnants of delta CRL support.
-    if (*pdcrl) {
-      X509_CRL_free(*pdcrl);
-      *pdcrl = NULL;
-    }
   }
 
   if (best_score >= CRL_SCORE_VALID) {
@@ -1313,17 +1287,16 @@ static int crl_crldp_check(X509 *x, X509_CRL *crl, int crl_score,
 }
 
 // Retrieve CRL corresponding to current certificate.
-static int get_crl_delta(X509_STORE_CTX *ctx, X509_CRL **pcrl, X509_CRL **pdcrl,
-                         X509 *x) {
+static int get_crl(X509_STORE_CTX *ctx, X509_CRL **pcrl, X509 *x) {
   int ok;
   X509 *issuer = NULL;
   int crl_score = 0;
   unsigned int reasons;
-  X509_CRL *crl = NULL, *dcrl = NULL;
+  X509_CRL *crl = NULL;
   STACK_OF(X509_CRL) *skcrl;
   X509_NAME *nm = X509_get_issuer_name(x);
   reasons = ctx->current_reasons;
-  ok = get_crl_sk(ctx, &crl, &dcrl, &issuer, &crl_score, &reasons, ctx->crls);
+  ok = get_crl_sk(ctx, &crl, &issuer, &crl_score, &reasons, ctx->crls);
 
   if (ok) {
     goto done;
@@ -1337,7 +1310,7 @@ static int get_crl_delta(X509_STORE_CTX *ctx, X509_CRL **pcrl, X509_CRL **pdcrl,
     goto done;
   }
 
-  get_crl_sk(ctx, &crl, &dcrl, &issuer, &crl_score, &reasons, skcrl);
+  get_crl_sk(ctx, &crl, &issuer, &crl_score, &reasons, skcrl);
 
   sk_X509_CRL_pop_free(skcrl, X509_CRL_free);
 
@@ -1349,7 +1322,6 @@ static int get_crl_delta(X509_STORE_CTX *ctx, X509_CRL **pcrl, X509_CRL **pdcrl,
     ctx->current_crl_score = crl_score;
     ctx->current_reasons = reasons;
     *pcrl = crl;
-    *pdcrl = dcrl;
     return 1;
   }
 

From bbe336264d3709f1ea1c25af279133b9aaee13aa Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@google.com>
Date: Sun, 12 Nov 2023 14:05:24 -0500
Subject: [PATCH 04/13] Remove removedFromCRL handling

This is part of delta CRLs, where OpenSSL would return 2 instead of 1 to
signal a removedFromCRL entry. The logic that caught that was removed in
the preceeding CL.

Bug: 601
Change-Id: I5ac0b32a864b09d303198ca99ecbfc1219906e36
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63933
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
(cherry picked from commit 9177d65315147771026640d9af4c101435eb737d)
---
 crypto/x509/x509_vfy.c | 6 +-----
 crypto/x509/x_crl.c    | 3 ---
 include/openssl/x509.h | 8 ++------
 3 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index b1713b4414..259d1f29c9 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -1444,12 +1444,8 @@ static int cert_crl(X509_STORE_CTX *ctx, X509_CRL *crl, X509 *x) {
       return 0;
     }
   }
-  // Look for serial number of certificate in CRL If found make sure reason
-  // is not removeFromCRL.
+  // Look for serial number of certificate in CRL.
   if (X509_CRL_get0_by_cert(crl, &rev, x)) {
-    if (rev->reason == CRL_REASON_REMOVE_FROM_CRL) {
-      return 2;
-    }
     ctx->error = X509_V_ERR_CERT_REVOKED;
     ok = ctx->verify_cb(0, ctx);
     if (!ok) {
diff --git a/crypto/x509/x_crl.c b/crypto/x509/x_crl.c
index 2975424e21..4e325e3bee 100644
--- a/crypto/x509/x_crl.c
+++ b/crypto/x509/x_crl.c
@@ -440,9 +440,6 @@ static int crl_lookup(X509_CRL *crl, X509_REVOKED **ret,
       if (ret) {
         *ret = rev;
       }
-      if (rev->reason == CRL_REASON_REMOVE_FROM_CRL) {
-        return 2;
-      }
       return 1;
     }
   }
diff --git a/include/openssl/x509.h b/include/openssl/x509.h
index 22868eebcf..adcd66455a 100644
--- a/include/openssl/x509.h
+++ b/include/openssl/x509.h
@@ -578,9 +578,8 @@ OPENSSL_EXPORT const ASN1_TIME *X509_CRL_get0_nextUpdate(const X509_CRL *crl);
 OPENSSL_EXPORT X509_NAME *X509_CRL_get_issuer(const X509_CRL *crl);
 
 // X509_CRL_get0_by_serial finds the entry in |crl| whose serial number is
-// |serial|. If found, it sets |*out| to the entry. It then returns two if the
-// reason code is removeFromCRL and one if it was revoked. If not found, it
-// returns zero.
+// |serial|. If found, it sets |*out| to the entry and returns one. If not
+// found, it returns zero.
 //
 // On success, |*out| continues to be owned by |crl|. It is an error to free or
 // otherwise modify |*out|.
@@ -588,9 +587,6 @@ OPENSSL_EXPORT X509_NAME *X509_CRL_get_issuer(const X509_CRL *crl);
 // TODO(crbug.com/boringssl/600): Ideally |crl| would be const. It is broadly
 // thread-safe, but changes the order of entries in |crl|. It cannot be called
 // concurrently with |i2d_X509_CRL|.
-//
-// TODO(crbug.com/boringssl/601): removeFromCRL is part of delta CRLs. Remove
-// this special case.
 OPENSSL_EXPORT int X509_CRL_get0_by_serial(X509_CRL *crl, X509_REVOKED **out,
                                            const ASN1_INTEGER *serial);
 

From 2025859f4d4896dc5e100d30fa607d36573cc923 Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@google.com>
Date: Sun, 12 Nov 2023 14:09:43 -0500
Subject: [PATCH 05/13] Remove the redundant idp_reasons field

This is part of onlySomeReasons handling, which was removed with
extended CRL support. Demonstrating that the code to be removed is a
no-op takes a few steps, so I'm splitting this into a couple CLs for
ease of review.

First, every CRL either has the IDP_REASONS set, or idp_reasons is
CRLDP_ALL_REASONS. The only thing that reads idp_reasons is
crl_crldp_check, which is run after get_crl_score skips all IDP_REASONS
certificates. Thus, the field can be assumed to be CRLDP_ALL_REASONS.

Bug: 601
Change-Id: I4d41b5665d35db10dd9752e47553ae90f46f1e44
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63934
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
(cherry picked from commit 39cc892c73d6c3faf2e604c44509f132c232f24c)
---
 crypto/x509/internal.h |  1 -
 crypto/x509/x509_vfy.c |  2 +-
 crypto/x509/x_crl.c    | 11 +----------
 3 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/crypto/x509/internal.h b/crypto/x509/internal.h
index 4d2619c35a..588d2d85dc 100644
--- a/crypto/x509/internal.h
+++ b/crypto/x509/internal.h
@@ -218,7 +218,6 @@ struct X509_crl_st {
   ISSUING_DIST_POINT *idp;
   // Convenient breakdown of IDP
   int idp_flags;
-  int idp_reasons;
   unsigned char crl_hash[SHA256_DIGEST_LENGTH];
   STACK_OF(GENERAL_NAMES) *issuers;
 } /* X509_CRL */;
diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index 259d1f29c9..fe9be4a37f 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -1269,7 +1269,7 @@ static int crl_crldp_check(X509 *x, X509_CRL *crl, int crl_score,
       return 0;
     }
   }
-  *preasons = crl->idp_reasons;
+  *preasons = CRLDP_ALL_REASONS;
   for (i = 0; i < sk_DIST_POINT_num(x->crldp); i++) {
     DIST_POINT *dp = sk_DIST_POINT_value(x->crldp, i);
     if (crldp_check_crlissuer(dp, crl, crl_score)) {
diff --git a/crypto/x509/x_crl.c b/crypto/x509/x_crl.c
index 4e325e3bee..79f31f0359 100644
--- a/crypto/x509/x_crl.c
+++ b/crypto/x509/x_crl.c
@@ -190,7 +190,6 @@ static int crl_set_issuers(X509_CRL *crl) {
 static int crl_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it,
                   void *exarg) {
   X509_CRL *crl = (X509_CRL *)*pval;
-  size_t idx;
   int i;
 
   switch (operation) {
@@ -199,7 +198,6 @@ static int crl_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it,
       crl->akid = NULL;
       crl->flags = 0;
       crl->idp_flags = 0;
-      crl->idp_reasons = CRLDP_ALL_REASONS;
       crl->issuers = NULL;
       break;
 
@@ -248,7 +246,7 @@ static int crl_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it,
       // critical sets the flag. This code accesses the X509_CRL structure
       // directly: applications shouldn't do this.
       const STACK_OF(X509_EXTENSION) *exts = crl->crl->extensions;
-      for (idx = 0; idx < sk_X509_EXTENSION_num(exts); idx++) {
+      for (size_t idx = 0; idx < sk_X509_EXTENSION_num(exts); idx++) {
         const X509_EXTENSION *ext = sk_X509_EXTENSION_value(exts, idx);
         int nid = OBJ_obj2nid(X509_EXTENSION_get_object(ext));
         if (X509_EXTENSION_get_critical(ext)) {
@@ -306,13 +304,6 @@ static int setup_idp(X509_CRL *crl, ISSUING_DIST_POINT *idp) {
 
   if (idp->onlysomereasons) {
     crl->idp_flags |= IDP_REASONS;
-    if (idp->onlysomereasons->length > 0) {
-      crl->idp_reasons = idp->onlysomereasons->data[0];
-    }
-    if (idp->onlysomereasons->length > 1) {
-      crl->idp_reasons |= (idp->onlysomereasons->data[1] << 8);
-    }
-    crl->idp_reasons &= CRLDP_ALL_REASONS;
   }
 
   return DIST_POINT_set_dpname(idp->distpoint, X509_CRL_get_issuer(crl));

From cf0ad79fcc94c9b2d9568d13efe822ac082f1734 Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@google.com>
Date: Sun, 12 Nov 2023 14:28:51 -0500
Subject: [PATCH 06/13] Don't process DistributionPoints with a reasons field

This isn't *quite* a no-op, but it's the other half of removing support
for partitioned CRLs. The distribution point's reasons field and the
CRL's onlySomeReasons field are masked together to determine which
reasons we have covered so far. This is used in some complex logic from
RFC 5280, section 6.3.3 to loop through a bunch of CRLs before
determining that we've covered evertything.

OpenSSL's "extended CRL" feature skipped all CRLs with an
onlySomeReasons field, but did not condition on the distribution point,
so the loop was still active in some cases.

The new verifier from Chromium doesn't support either. If the
distribution point has a reasons field, we ignore it. Align with
Chromium. Now the reasons field is always the special all-reasons value.

As part of this, this removed the dp_reasons field from DIST_POINT. This
is a public struct, but was unused outside of cl/581761514, so we can
stop computing it.

Update-Note: See above.

Bug: 601
Change-Id: I9b0a9766b281d3486874e1b6d4d415a51e50ba59
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63935
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
(cherry picked from commit 580c04109e8a63d08582b3d948cf54849371a73e)
---
 crypto/x509/x509_vfy.c   | 64 ++++++++++++++--------------------------
 crypto/x509v3/v3_purp.c  | 16 ++--------
 include/openssl/x509v3.h |  1 -
 3 files changed, 24 insertions(+), 57 deletions(-)

diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index fe9be4a37f..5cd77c4220 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -124,13 +124,12 @@ static int check_revocation(X509_STORE_CTX *ctx);
 static int check_cert(X509_STORE_CTX *ctx);
 static int check_policy(X509_STORE_CTX *ctx);
 
-static int get_crl_score(X509_STORE_CTX *ctx, X509 **pissuer,
-                         unsigned int *preasons, X509_CRL *crl, X509 *x);
+static int get_crl_score(X509_STORE_CTX *ctx, X509 **pissuer, X509_CRL *crl,
+                         X509 *x);
 static int get_crl(X509_STORE_CTX *ctx, X509_CRL **pcrl, X509 *x);
 static void crl_akid_check(X509_STORE_CTX *ctx, X509_CRL *crl, X509 **pissuer,
                            int *pcrl_score);
-static int crl_crldp_check(X509 *x, X509_CRL *crl, int crl_score,
-                           unsigned int *preasons);
+static int crl_crldp_check(X509 *x, X509_CRL *crl, int crl_score);
 static int check_crl_path(X509_STORE_CTX *ctx, X509 *x);
 static int check_crl_chain(X509_STORE_CTX *ctx, STACK_OF(X509) *cert_path,
                            STACK_OF(X509) *crl_path);
@@ -959,18 +958,15 @@ static int check_crl_time(X509_STORE_CTX *ctx, X509_CRL *crl, int notify) {
 }
 
 static int get_crl_sk(X509_STORE_CTX *ctx, X509_CRL **pcrl, X509 **pissuer,
-                      int *pscore, unsigned *preasons,
-                      STACK_OF(X509_CRL) *crls) {
+                      int *pscore, STACK_OF(X509_CRL) *crls) {
   int crl_score, best_score = *pscore;
-  unsigned best_reasons = 0;
   X509 *x = ctx->current_cert;
   X509_CRL *best_crl = NULL;
   X509 *crl_issuer = NULL, *best_crl_issuer = NULL;
 
   for (size_t i = 0; i < sk_X509_CRL_num(crls); i++) {
     X509_CRL *crl = sk_X509_CRL_value(crls, i);
-    unsigned reasons = *preasons;
-    crl_score = get_crl_score(ctx, &crl_issuer, &reasons, crl, x);
+    crl_score = get_crl_score(ctx, &crl_issuer, crl, x);
     if (crl_score < best_score || crl_score == 0) {
       continue;
     }
@@ -990,7 +986,6 @@ static int get_crl_sk(X509_STORE_CTX *ctx, X509_CRL **pcrl, X509 **pissuer,
     best_crl = crl;
     best_crl_issuer = crl_issuer;
     best_score = crl_score;
-    best_reasons = reasons;
   }
 
   if (best_crl) {
@@ -1000,7 +995,6 @@ static int get_crl_sk(X509_STORE_CTX *ctx, X509_CRL **pcrl, X509 **pissuer,
     *pcrl = best_crl;
     *pissuer = best_crl_issuer;
     *pscore = best_score;
-    *preasons = best_reasons;
     X509_CRL_up_ref(best_crl);
   }
 
@@ -1013,14 +1007,10 @@ static int get_crl_sk(X509_STORE_CTX *ctx, X509_CRL **pcrl, X509 **pissuer,
 
 // For a given CRL return how suitable it is for the supplied certificate
 // 'x'. The return value is a mask of several criteria. If the issuer is not
-// the certificate issuer this is returned in *pissuer. The reasons mask is
-// also used to determine if the CRL is suitable: if no new reasons the CRL
-// is rejected, otherwise reasons is updated.
-
-static int get_crl_score(X509_STORE_CTX *ctx, X509 **pissuer,
-                         unsigned int *preasons, X509_CRL *crl, X509 *x) {
+// the certificate issuer this is returned in *pissuer.
+static int get_crl_score(X509_STORE_CTX *ctx, X509 **pissuer, X509_CRL *crl,
+                         X509 *x) {
   int crl_score = 0;
-  unsigned int tmp_reasons = *preasons, crl_reasons;
 
   // First see if we can reject CRL straight away
 
@@ -1060,18 +1050,10 @@ static int get_crl_score(X509_STORE_CTX *ctx, X509 **pissuer,
   }
 
   // Check cert for matching CRL distribution points
-
-  if (crl_crldp_check(x, crl, crl_score, &crl_reasons)) {
-    // If no new reasons reject
-    if (!(crl_reasons & ~tmp_reasons)) {
-      return 0;
-    }
-    tmp_reasons |= crl_reasons;
+  if (crl_crldp_check(x, crl, crl_score)) {
     crl_score |= CRL_SCORE_SCOPE;
   }
 
-  *preasons = tmp_reasons;
-
   return crl_score;
 }
 
@@ -1254,9 +1236,7 @@ static int crldp_check_crlissuer(DIST_POINT *dp, X509_CRL *crl, int crl_score) {
 
 // Check CRLDP and IDP
 
-static int crl_crldp_check(X509 *x, X509_CRL *crl, int crl_score,
-                           unsigned int *preasons) {
-  size_t i;
+static int crl_crldp_check(X509 *x, X509_CRL *crl, int crl_score) {
   if (crl->idp_flags & IDP_ONLYATTR) {
     return 0;
   }
@@ -1269,14 +1249,15 @@ static int crl_crldp_check(X509 *x, X509_CRL *crl, int crl_score,
       return 0;
     }
   }
-  *preasons = CRLDP_ALL_REASONS;
-  for (i = 0; i < sk_DIST_POINT_num(x->crldp); i++) {
+  for (size_t i = 0; i < sk_DIST_POINT_num(x->crldp); i++) {
     DIST_POINT *dp = sk_DIST_POINT_value(x->crldp, i);
-    if (crldp_check_crlissuer(dp, crl, crl_score)) {
-      if (!crl->idp || idp_check_dp(dp->distpoint, crl->idp->distpoint)) {
-        *preasons &= dp->dp_reasons;
-        return 1;
-      }
+    // Skip distribution points with a reasons field. We do not support CRLs
+    // partitioned by reason code. RFC 5280 requires CAs include at least one
+    // DistributionPoint that covers all reasons.
+    if (dp->reasons != NULL &&  //
+        crldp_check_crlissuer(dp, crl, crl_score) &&
+        (!crl->idp || idp_check_dp(dp->distpoint, crl->idp->distpoint))) {
+      return 1;
     }
   }
   if ((!crl->idp || !crl->idp->distpoint) &&
@@ -1291,12 +1272,10 @@ static int get_crl(X509_STORE_CTX *ctx, X509_CRL **pcrl, X509 *x) {
   int ok;
   X509 *issuer = NULL;
   int crl_score = 0;
-  unsigned int reasons;
   X509_CRL *crl = NULL;
   STACK_OF(X509_CRL) *skcrl;
   X509_NAME *nm = X509_get_issuer_name(x);
-  reasons = ctx->current_reasons;
-  ok = get_crl_sk(ctx, &crl, &issuer, &crl_score, &reasons, ctx->crls);
+  ok = get_crl_sk(ctx, &crl, &issuer, &crl_score, ctx->crls);
 
   if (ok) {
     goto done;
@@ -1310,7 +1289,7 @@ static int get_crl(X509_STORE_CTX *ctx, X509_CRL **pcrl, X509 *x) {
     goto done;
   }
 
-  get_crl_sk(ctx, &crl, &issuer, &crl_score, &reasons, skcrl);
+  get_crl_sk(ctx, &crl, &issuer, &crl_score, skcrl);
 
   sk_X509_CRL_pop_free(skcrl, X509_CRL_free);
 
@@ -1320,7 +1299,8 @@ static int get_crl(X509_STORE_CTX *ctx, X509_CRL **pcrl, X509 *x) {
   if (crl) {
     ctx->current_issuer = issuer;
     ctx->current_crl_score = crl_score;
-    ctx->current_reasons = reasons;
+    // TODO(crbug.com/boringssl/601): Clean up remnants of partitioned CRLs.
+    ctx->current_reasons = CRLDP_ALL_REASONS;
     *pcrl = crl;
     return 1;
   }
diff --git a/crypto/x509v3/v3_purp.c b/crypto/x509v3/v3_purp.c
index 36dbdb9e90..1696225f53 100644
--- a/crypto/x509v3/v3_purp.c
+++ b/crypto/x509v3/v3_purp.c
@@ -351,23 +351,11 @@ int X509_supported_extension(const X509_EXTENSION *ex) {
 }
 
 static int setup_dp(X509 *x, DIST_POINT *dp) {
-  X509_NAME *iname = NULL;
-  size_t i;
-  if (dp->reasons) {
-    if (dp->reasons->length > 0) {
-      dp->dp_reasons = dp->reasons->data[0];
-    }
-    if (dp->reasons->length > 1) {
-      dp->dp_reasons |= (dp->reasons->data[1] << 8);
-    }
-    dp->dp_reasons &= CRLDP_ALL_REASONS;
-  } else {
-    dp->dp_reasons = CRLDP_ALL_REASONS;
-  }
   if (!dp->distpoint || (dp->distpoint->type != 1)) {
     return 1;
   }
-  for (i = 0; i < sk_GENERAL_NAME_num(dp->CRLissuer); i++) {
+  X509_NAME *iname = NULL;
+  for (size_t i = 0; i < sk_GENERAL_NAME_num(dp->CRLissuer); i++) {
     GENERAL_NAME *gen = sk_GENERAL_NAME_value(dp->CRLissuer, i);
     if (gen->type == GEN_DIRNAME) {
       iname = gen->d.directoryName;
diff --git a/include/openssl/x509v3.h b/include/openssl/x509v3.h
index c55b421874..547e09645d 100644
--- a/include/openssl/x509v3.h
+++ b/include/openssl/x509v3.h
@@ -237,7 +237,6 @@ struct DIST_POINT_st {
   DIST_POINT_NAME *distpoint;
   ASN1_BIT_STRING *reasons;
   GENERAL_NAMES *CRLissuer;
-  int dp_reasons;
 };
 
 typedef STACK_OF(DIST_POINT) CRL_DIST_POINTS;

From f672ed5b44c37e681cee19ed93f6f7fc34c0337e Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@google.com>
Date: Sun, 12 Nov 2023 14:42:35 -0500
Subject: [PATCH 07/13] Remove the now no-op CRL reasons loop

Now that we only process CRLs that cover all reasons:

1. A successful get_crl will always set current_reasons to
   CRLDP_ALL_REASONS.

2. The last_reasons == current_reasons will never happen.

3. The loop always makes exactly one iteration.

A footnote on point 1: it is also possible for the caller to override
get_crl. In that case, the caller's get_crl was previously responsible
for setting current_reasons, but there was no way to do so. In reality,
that callback was actually impossible to use correctly. See
https://github.com/openssl/openssl/issues/21679 and
https://github.com/openssl/openssl/issues/10211.

I previously attempted to remove those first, but gRPC did not notice it
was unusable and use it anyway. Instead, they're suppressing
X509_V_ERR_UNABLE_TO_GET_CRL via the callback, which is probably working
around the bug in their get_crl implementation.

Later, when we tackle the callback, we'll probably need to unwind the
gRPC mess and, in the process, add a X509_STORE_CTX_set_current_reasons
for them to call for OpenSSL compatibility. For now, this change has the
side effect of removing the need for them to call that.

Bug: 601
Change-Id: Icc5c0fb195d9f66991d0e560911f304e82afa5fd
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63936
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
(cherry picked from commit 8cae3d04ed7264ae99024caff4da79e9a796b915)
---
 crypto/x509/internal.h |  1 -
 crypto/x509/x509_vfy.c | 63 +++++++++++++++---------------------------
 2 files changed, 22 insertions(+), 42 deletions(-)

diff --git a/crypto/x509/internal.h b/crypto/x509/internal.h
index 588d2d85dc..56e050aecd 100644
--- a/crypto/x509/internal.h
+++ b/crypto/x509/internal.h
@@ -349,7 +349,6 @@ struct x509_store_ctx_st {
   X509_CRL *current_crl;  // current CRL
 
   int current_crl_score;         // score of current CRL
-  unsigned int current_reasons;  // Reason mask
 
   X509_STORE_CTX *parent;  // For CRL path validation: parent context
 
diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index 5cd77c4220..f20212c3ac 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -837,49 +837,33 @@ static int check_revocation(X509_STORE_CTX *ctx) {
 
 static int check_cert(X509_STORE_CTX *ctx) {
   X509_CRL *crl = NULL;
-  X509 *x;
-  int ok = 0, cnum;
-  unsigned int last_reasons;
-  cnum = ctx->error_depth;
-  x = sk_X509_value(ctx->chain, cnum);
+  int ok = 0, cnum = ctx->error_depth;
+  X509 *x = sk_X509_value(ctx->chain, cnum);
   ctx->current_cert = x;
   ctx->current_issuer = NULL;
   ctx->current_crl_score = 0;
-  ctx->current_reasons = 0;
-  while (ctx->current_reasons != CRLDP_ALL_REASONS) {
-    last_reasons = ctx->current_reasons;
-    // Try to retrieve relevant CRL
-    if (ctx->get_crl) {
-      ok = ctx->get_crl(ctx, &crl, x);
-    } else {
-      ok = get_crl(ctx, &crl, x);
-    }
-    // If error looking up CRL, nothing we can do except notify callback
-    if (!ok) {
-      ctx->error = X509_V_ERR_UNABLE_TO_GET_CRL;
-      ok = ctx->verify_cb(0, ctx);
-      goto err;
-    }
-    ctx->current_crl = crl;
-    ok = ctx->check_crl(ctx, crl);
-    if (!ok) {
-      goto err;
-    }
 
-    ok = ctx->cert_crl(ctx, crl, x);
-    if (!ok) {
-      goto err;
-    }
+  // Try to retrieve relevant CRL
+  if (ctx->get_crl) {
+    ok = ctx->get_crl(ctx, &crl, x);
+  } else {
+    ok = get_crl(ctx, &crl, x);
+  }
+  // If error looking up CRL, nothing we can do except notify callback
+  if (!ok) {
+    ctx->error = X509_V_ERR_UNABLE_TO_GET_CRL;
+    ok = ctx->verify_cb(0, ctx);
+    goto err;
+  }
+  ctx->current_crl = crl;
+  ok = ctx->check_crl(ctx, crl);
+  if (!ok) {
+    goto err;
+  }
 
-    X509_CRL_free(crl);
-    crl = NULL;
-    // If reasons not updated we wont get anywhere by another iteration,
-    // so exit loop.
-    if (last_reasons == ctx->current_reasons) {
-      ctx->error = X509_V_ERR_UNABLE_TO_GET_CRL;
-      ok = ctx->verify_cb(0, ctx);
-      goto err;
-    }
+  ok = ctx->cert_crl(ctx, crl, x);
+  if (!ok) {
+    goto err;
   }
 
 err:
@@ -1276,7 +1260,6 @@ static int get_crl(X509_STORE_CTX *ctx, X509_CRL **pcrl, X509 *x) {
   STACK_OF(X509_CRL) *skcrl;
   X509_NAME *nm = X509_get_issuer_name(x);
   ok = get_crl_sk(ctx, &crl, &issuer, &crl_score, ctx->crls);
-
   if (ok) {
     goto done;
   }
@@ -1299,8 +1282,6 @@ static int get_crl(X509_STORE_CTX *ctx, X509_CRL **pcrl, X509 *x) {
   if (crl) {
     ctx->current_issuer = issuer;
     ctx->current_crl_score = crl_score;
-    // TODO(crbug.com/boringssl/601): Clean up remnants of partitioned CRLs.
-    ctx->current_reasons = CRLDP_ALL_REASONS;
     *pcrl = crl;
     return 1;
   }

From 2d2b9fdfc4d0abfce502d1744590383309e07ca3 Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@google.com>
Date: Sun, 12 Nov 2023 14:51:23 -0500
Subject: [PATCH 08/13] Remove the delta CRL special case on expiry

We never set CRL_SCORE_TIME_DELTA.

Bug: 601
Change-Id: Ic7497492565bda9f3e9d7091f5e16b81e111cfa1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63937
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
(cherry picked from commit 9bed3737639dfaf1394cf0518e3de896a0390f0e)
---
 crypto/x509/x509_vfy.c | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index f20212c3ac..2987db91f3 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -77,42 +77,30 @@ static CRYPTO_EX_DATA_CLASS g_ex_data_class =
 // CRL score values
 
 // No unhandled critical extensions
-
 #define CRL_SCORE_NOCRITICAL 0x100
 
 // certificate is within CRL scope
-
 #define CRL_SCORE_SCOPE 0x080
 
 // CRL times valid
-
 #define CRL_SCORE_TIME 0x040
 
 // Issuer name matches certificate
-
 #define CRL_SCORE_ISSUER_NAME 0x020
 
 // If this score or above CRL is probably valid
-
 #define CRL_SCORE_VALID \
   (CRL_SCORE_NOCRITICAL | CRL_SCORE_TIME | CRL_SCORE_SCOPE)
 
 // CRL issuer is certificate issuer
-
 #define CRL_SCORE_ISSUER_CERT 0x018
 
 // CRL issuer is on certificate path
-
 #define CRL_SCORE_SAME_PATH 0x008
 
 // CRL issuer matches CRL AKID
-
 #define CRL_SCORE_AKID 0x004
 
-// Have a delta CRL with valid times
-
-#define CRL_SCORE_TIME_DELTA 0x002
-
 static int null_callback(int ok, X509_STORE_CTX *e);
 static int check_issued(X509_STORE_CTX *ctx, X509 *x, X509 *issuer);
 static X509 *find_issuer(X509_STORE_CTX *ctx, STACK_OF(X509) *sk, X509 *x);
@@ -922,8 +910,7 @@ static int check_crl_time(X509_STORE_CTX *ctx, X509_CRL *crl, int notify) {
         return 0;
       }
     }
-    // Ignore expiry of base CRL is delta is valid
-    if ((i < 0) && !(ctx->current_crl_score & CRL_SCORE_TIME_DELTA)) {
+    if (i < 0) {
       if (!notify) {
         return 0;
       }

From f4e80c700f2bb6d468d972bb7e938e64919b5682 Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@google.com>
Date: Sun, 12 Nov 2023 15:00:31 -0500
Subject: [PATCH 09/13] Remove some remnants of indirect CRLs in CRL matching

We'll never accept CRLs where the issuers don't match. That means
CRL_SCORE_ISSUER_NAME is always set, so we can remove code that
conditions on it.

Update-Note: This also makes a corresponding distribution point change
to ignore distribution points with a CRLissuer field. Before, we would
check for it to match the CRL issuer, but this field is only meant to be
used with indirect CRLs (RFC 5280, section 6.3.3, step b.1). The old
code didn't include this, so I think it isn't *quite* a no-op on some
invalid DP/CRL pairs, but it matches the new verifier from Chromium.

Bug: 601
Change-Id: Ibe409b88cae1c2b78b3924e61270884d6e0eb436
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63938
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
(cherry picked from commit 54eaf6ba8c88ac4ce8847e7fd5cb33c6c59e92bc)
---
 crypto/x509/x509_vfy.c | 63 +++++++++++++++---------------------------
 1 file changed, 22 insertions(+), 41 deletions(-)

diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index 2987db91f3..d034a22bfd 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -993,14 +993,11 @@ static int get_crl_score(X509_STORE_CTX *ctx, X509 **pissuer, X509_CRL *crl,
   if (crl->idp_flags & (IDP_INDIRECT | IDP_REASONS)) {
     return 0;
   }
-  // If issuer name doesn't match certificate need indirect CRL
+  // We do not support indirect CRLs, so the issuer names must match.
   if (X509_NAME_cmp(X509_get_issuer_name(x), X509_CRL_get_issuer(crl))) {
-    if (!(crl->idp_flags & IDP_INDIRECT)) {
-      return 0;
-    }
-  } else {
-    crl_score |= CRL_SCORE_ISSUER_NAME;
+    return 0;
   }
+  crl_score |= CRL_SCORE_ISSUER_NAME;
 
   if (!(crl->flags & EXFLAG_CRITICAL)) {
     crl_score |= CRL_SCORE_NOCRITICAL;
@@ -1041,11 +1038,9 @@ static void crl_akid_check(X509_STORE_CTX *ctx, X509_CRL *crl, X509 **pissuer,
   crl_issuer = sk_X509_value(ctx->chain, cidx);
 
   if (X509_check_akid(crl_issuer, crl->akid) == X509_V_OK) {
-    if (*pcrl_score & CRL_SCORE_ISSUER_NAME) {
-      *pcrl_score |= CRL_SCORE_AKID | CRL_SCORE_ISSUER_CERT;
-      *pissuer = crl_issuer;
-      return;
-    }
+    *pcrl_score |= CRL_SCORE_AKID | CRL_SCORE_ISSUER_CERT;
+    *pissuer = crl_issuer;
+    return;
   }
 
   for (cidx++; cidx < (int)sk_X509_num(ctx->chain); cidx++) {
@@ -1186,27 +1181,7 @@ static int idp_check_dp(DIST_POINT_NAME *a, DIST_POINT_NAME *b) {
   return 0;
 }
 
-static int crldp_check_crlissuer(DIST_POINT *dp, X509_CRL *crl, int crl_score) {
-  size_t i;
-  X509_NAME *nm = X509_CRL_get_issuer(crl);
-  // If no CRLissuer return is successful iff don't need a match
-  if (!dp->CRLissuer) {
-    return !!(crl_score & CRL_SCORE_ISSUER_NAME);
-  }
-  for (i = 0; i < sk_GENERAL_NAME_num(dp->CRLissuer); i++) {
-    GENERAL_NAME *gen = sk_GENERAL_NAME_value(dp->CRLissuer, i);
-    if (gen->type != GEN_DIRNAME) {
-      continue;
-    }
-    if (!X509_NAME_cmp(gen->d.directoryName, nm)) {
-      return 1;
-    }
-  }
-  return 0;
-}
-
 // Check CRLDP and IDP
-
 static int crl_crldp_check(X509 *x, X509_CRL *crl, int crl_score) {
   if (crl->idp_flags & IDP_ONLYATTR) {
     return 0;
@@ -1222,20 +1197,26 @@ static int crl_crldp_check(X509 *x, X509_CRL *crl, int crl_score) {
   }
   for (size_t i = 0; i < sk_DIST_POINT_num(x->crldp); i++) {
     DIST_POINT *dp = sk_DIST_POINT_value(x->crldp, i);
-    // Skip distribution points with a reasons field. We do not support CRLs
-    // partitioned by reason code. RFC 5280 requires CAs include at least one
-    // DistributionPoint that covers all reasons.
-    if (dp->reasons != NULL &&  //
-        crldp_check_crlissuer(dp, crl, crl_score) &&
+    // Skip distribution points with a reasons field or a CRL issuer:
+    //
+    // We do not support CRLs partitioned by reason code. RFC 5280 requires CAs
+    // include at least one DistributionPoint that covers all reasons.
+    //
+    // 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))) {
       return 1;
     }
   }
-  if ((!crl->idp || !crl->idp->distpoint) &&
-      (crl_score & CRL_SCORE_ISSUER_NAME)) {
-    return 1;
-  }
-  return 0;
+
+  // If the CRL does not specify an issuing distribution point, allow it to
+  // match anything.
+  //
+  // TODO(davidben): Does this match RFC 5280? It's hard to follow because RFC
+  // 5280 starts from distribution points, while this starts from CRLs.
+  return !crl->idp || !crl->idp->distpoint;
 }
 
 // Retrieve CRL corresponding to current certificate.

From 048aeb22d0462af1805e7f4bddc1039cdebba5ab Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@google.com>
Date: Sun, 12 Nov 2023 15:14:16 -0500
Subject: [PATCH 10/13] Unexport the idp_flags constants

Avoid polluting the global namespace a bit. The idp_flags field itself
is private, so it is impossible for a caller to do anything useful with
it.

Change-Id: I5853e72a3f37a93c75fb73ed5d7ca467b9ba01a3
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63939
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
(cherry picked from commit d53bbba11f164a275659d48356e992537e07774e)
---
 crypto/x509/internal.h   | 16 ++++++++++++++++
 include/openssl/x509v3.h | 18 ------------------
 2 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/crypto/x509/internal.h b/crypto/x509/internal.h
index 56e050aecd..4eea501e90 100644
--- a/crypto/x509/internal.h
+++ b/crypto/x509/internal.h
@@ -206,6 +206,22 @@ typedef struct {
 // an |X509_NAME|.
 DECLARE_ASN1_FUNCTIONS(X509_CRL_INFO)
 
+// Values in idp_flags field
+// IDP present
+#define IDP_PRESENT 0x1
+// IDP values inconsistent
+#define IDP_INVALID 0x2
+// onlyuser true
+#define IDP_ONLYUSER 0x4
+// onlyCA true
+#define IDP_ONLYCA 0x8
+// onlyattr true
+#define IDP_ONLYATTR 0x10
+// indirectCRL true
+#define IDP_INDIRECT 0x20
+// onlysomereasons present
+#define IDP_REASONS 0x40
+
 struct X509_crl_st {
   // actual signature
   X509_CRL_INFO *crl;
diff --git a/include/openssl/x509v3.h b/include/openssl/x509v3.h
index 547e09645d..9007b3c5fc 100644
--- a/include/openssl/x509v3.h
+++ b/include/openssl/x509v3.h
@@ -315,24 +315,6 @@ struct ISSUING_DIST_POINT_st {
   int onlyattr;
 };
 
-// Values in idp_flags field
-// IDP present
-#define IDP_PRESENT 0x1
-// IDP values inconsistent
-#define IDP_INVALID 0x2
-// onlyuser true
-#define IDP_ONLYUSER 0x4
-// onlyCA true
-#define IDP_ONLYCA 0x8
-// onlyattr true
-#define IDP_ONLYATTR 0x10
-// indirectCRL true
-#define IDP_INDIRECT 0x20
-// onlysomereasons present
-#define IDP_REASONS 0x40
-
-
-
 // X509_PURPOSE stuff
 
 #define EXFLAG_BCONS 0x1

From caf27e5d107c51f42032a286b5dd9b1d929ebd0c Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@google.com>
Date: Sun, 12 Nov 2023 15:15:26 -0500
Subject: [PATCH 11/13] Use the ASN1_BOOLEAN typedef in ISSUING_DIST_POINT

ASN1_BOOLEAN is int, so this is a no-op. But they're parsed as
ASN1_FBOOLEAN (boolean with default false), so we should use the
typedef.

Also leave some TODOs for future cleanup opportunities.

Change-Id: I7b060e7530f0ef1afb3818c043727feb6fd4fcbf
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63940
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
(cherry picked from commit 33d6e8170aaaa24b019e730051f163ea62c9b7c1)
---
 crypto/x509/x_crl.c      | 11 ++++++++++-
 include/openssl/x509v3.h |  8 ++++----
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/crypto/x509/x_crl.c b/crypto/x509/x_crl.c
index 79f31f0359..1533a59fdb 100644
--- a/crypto/x509/x_crl.c
+++ b/crypto/x509/x_crl.c
@@ -276,7 +276,9 @@ static int crl_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it,
 }
 
 // Convert IDP into a more convenient form
-
+//
+// TODO(davidben): Each of these flags are already booleans, so this is not
+// really more convenient. We can probably remove |idp_flags|.
 static int setup_idp(X509_CRL *crl, ISSUING_DIST_POINT *idp) {
   int idp_only = 0;
   // Set various flags according to IDP
@@ -294,6 +296,11 @@ static int setup_idp(X509_CRL *crl, ISSUING_DIST_POINT *idp) {
     crl->idp_flags |= IDP_ONLYATTR;
   }
 
+  // Per RFC 5280, section 5.2.5, at most one of onlyContainsUserCerts,
+  // onlyContainsCACerts, and onlyContainsAttributeCerts may be true.
+  //
+  // TODO(crbug.com/boringssl/443): Move this check to the |ISSUING_DIST_POINT|
+  // parser.
   if (idp_only > 1) {
     crl->idp_flags |= IDP_INVALID;
   }
@@ -306,6 +313,8 @@ static int setup_idp(X509_CRL *crl, ISSUING_DIST_POINT *idp) {
     crl->idp_flags |= IDP_REASONS;
   }
 
+  // TODO(davidben): The new verifier does not support nameRelativeToCRLIssuer.
+  // Remove this?
   return DIST_POINT_set_dpname(idp->distpoint, X509_CRL_get_issuer(crl));
 }
 
diff --git a/include/openssl/x509v3.h b/include/openssl/x509v3.h
index 9007b3c5fc..22533b0da5 100644
--- a/include/openssl/x509v3.h
+++ b/include/openssl/x509v3.h
@@ -308,11 +308,11 @@ typedef struct POLICY_CONSTRAINTS_st {
 
 struct ISSUING_DIST_POINT_st {
   DIST_POINT_NAME *distpoint;
-  int onlyuser;
-  int onlyCA;
+  ASN1_BOOLEAN onlyuser;
+  ASN1_BOOLEAN onlyCA;
   ASN1_BIT_STRING *onlysomereasons;
-  int indirectCRL;
-  int onlyattr;
+  ASN1_BOOLEAN indirectCRL;
+  ASN1_BOOLEAN onlyattr;
 };
 
 // X509_PURPOSE stuff

From 1b799d6078ad94acff45d310104dcee54f79bc74 Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@google.com>
Date: Sun, 12 Nov 2023 15:33:10 -0500
Subject: [PATCH 12/13] Remove support for the certificateIssuer CRL entry
 extension

This is part of indirect CRLs, though it wasn't correctly conditioned on
them. All CRL entries are now expected to relate to the CRL issuer. This
removes the very complicated delta encoding this extension uses. (This
extension applies to not just the current CRL entry, but all subsequent
CRL entries that don't override it. This also means sorting a CRL is
destructive.)

Bug: 601
Change-Id: I388baee64719007648fb3b5defea82a4661735d2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63941
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
(cherry picked from commit ae40ad9b2191723bee9d7beac343dd54afea81e3)
---
 crypto/x509/internal.h |  3 --
 crypto/x509/x_crl.c    | 89 ++++++------------------------------------
 2 files changed, 13 insertions(+), 79 deletions(-)

diff --git a/crypto/x509/internal.h b/crypto/x509/internal.h
index 4eea501e90..aefe2a041b 100644
--- a/crypto/x509/internal.h
+++ b/crypto/x509/internal.h
@@ -185,8 +185,6 @@ struct x509_revoked_st {
   ASN1_INTEGER *serialNumber;
   ASN1_TIME *revocationDate;
   STACK_OF(X509_EXTENSION) /* optional */ *extensions;
-  // Set up if indirect CRL
-  STACK_OF(GENERAL_NAME) *issuer;
   // Revocation reason
   int reason;
 } /* X509_REVOKED */;
@@ -235,7 +233,6 @@ struct X509_crl_st {
   // Convenient breakdown of IDP
   int idp_flags;
   unsigned char crl_hash[SHA256_DIGEST_LENGTH];
-  STACK_OF(GENERAL_NAMES) *issuers;
 } /* X509_CRL */;
 
 struct X509_VERIFY_PARAM_st {
diff --git a/crypto/x509/x_crl.c b/crypto/x509/x_crl.c
index 1533a59fdb..fe78c0c6ed 100644
--- a/crypto/x509/x_crl.c
+++ b/crypto/x509/x_crl.c
@@ -115,45 +115,15 @@ ASN1_SEQUENCE_enc(X509_CRL_INFO, enc, crl_inf_cb) = {
     ASN1_EXP_SEQUENCE_OF_OPT(X509_CRL_INFO, extensions, X509_EXTENSION, 0),
 } ASN1_SEQUENCE_END_enc(X509_CRL_INFO, X509_CRL_INFO)
 
-// Set CRL entry issuer according to CRL certificate issuer extension. Check
-// for unhandled critical CRL entry extensions.
-
-static int crl_set_issuers(X509_CRL *crl) {
-  size_t i, k;
-  int j;
-  GENERAL_NAMES *gens, *gtmp;
-  STACK_OF(X509_REVOKED) *revoked;
-
-  revoked = X509_CRL_get_REVOKED(crl);
-
-  gens = NULL;
-  for (i = 0; i < sk_X509_REVOKED_num(revoked); i++) {
+static int crl_parse_entry_extensions(X509_CRL *crl) {
+  STACK_OF(X509_REVOKED) *revoked = X509_CRL_get_REVOKED(crl);
+  for (size_t i = 0; i < sk_X509_REVOKED_num(revoked); i++) {
     X509_REVOKED *rev = sk_X509_REVOKED_value(revoked, i);
-    STACK_OF(X509_EXTENSION) *exts;
-    ASN1_ENUMERATED *reason;
-    X509_EXTENSION *ext;
-    gtmp = X509_REVOKED_get_ext_d2i(rev, NID_certificate_issuer, &j, NULL);
-    if (!gtmp && (j != -1)) {
-      crl->flags |= EXFLAG_INVALID;
-      return 1;
-    }
-
-    if (gtmp) {
-      gens = gtmp;
-      if (!crl->issuers) {
-        crl->issuers = sk_GENERAL_NAMES_new_null();
-        if (!crl->issuers) {
-          return 0;
-        }
-      }
-      if (!sk_GENERAL_NAMES_push(crl->issuers, gtmp)) {
-        return 0;
-      }
-    }
-    rev->issuer = gens;
 
-    reason = X509_REVOKED_get_ext_d2i(rev, NID_crl_reason, &j, NULL);
-    if (!reason && (j != -1)) {
+    int crit;
+    ASN1_ENUMERATED *reason =
+        X509_REVOKED_get_ext_d2i(rev, NID_crl_reason, &crit, NULL);
+    if (!reason && crit != -1) {
       crl->flags |= EXFLAG_INVALID;
       return 1;
     }
@@ -165,17 +135,11 @@ static int crl_set_issuers(X509_CRL *crl) {
       rev->reason = CRL_REASON_NONE;
     }
 
-    // Check for critical CRL entry extensions
-
-    exts = rev->extensions;
-
-    for (k = 0; k < sk_X509_EXTENSION_num(exts); k++) {
-      ext = sk_X509_EXTENSION_value(exts, k);
+    // We do not support any critical CRL entry extensions.
+    const STACK_OF(X509_EXTENSION) *exts = rev->extensions;
+    for (size_t j = 0; j < sk_X509_EXTENSION_num(exts); j++) {
+      const X509_EXTENSION *ext = sk_X509_EXTENSION_value(exts, j);
       if (X509_EXTENSION_get_critical(ext)) {
-        if (OBJ_obj2nid(X509_EXTENSION_get_object(ext)) ==
-            NID_certificate_issuer) {
-          continue;
-        }
         crl->flags |= EXFLAG_CRITICAL;
         break;
       }
@@ -198,7 +162,6 @@ static int crl_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it,
       crl->akid = NULL;
       crl->flags = 0;
       crl->idp_flags = 0;
-      crl->issuers = NULL;
       break;
 
     case ASN1_OP_D2I_POST: {
@@ -259,7 +222,7 @@ static int crl_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it,
         }
       }
 
-      if (!crl_set_issuers(crl)) {
+      if (!crl_parse_entry_extensions(crl)) {
         return 0;
       }
 
@@ -269,7 +232,6 @@ static int crl_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it,
     case ASN1_OP_FREE_POST:
       AUTHORITY_KEYID_free(crl->akid);
       ISSUING_DIST_POINT_free(crl->idp);
-      sk_GENERAL_NAMES_pop_free(crl->issuers, GENERAL_NAMES_free);
       break;
   }
   return 1;
@@ -374,32 +336,7 @@ int X509_CRL_get0_by_cert(X509_CRL *crl, X509_REVOKED **ret, X509 *x) {
 
 static int crl_revoked_issuer_match(X509_CRL *crl, X509_NAME *nm,
                                     X509_REVOKED *rev) {
-  size_t i;
-
-  if (!rev->issuer) {
-    if (!nm) {
-      return 1;
-    }
-    if (!X509_NAME_cmp(nm, X509_CRL_get_issuer(crl))) {
-      return 1;
-    }
-    return 0;
-  }
-
-  if (!nm) {
-    nm = X509_CRL_get_issuer(crl);
-  }
-
-  for (i = 0; i < sk_GENERAL_NAME_num(rev->issuer); i++) {
-    GENERAL_NAME *gen = sk_GENERAL_NAME_value(rev->issuer, i);
-    if (gen->type != GEN_DIRNAME) {
-      continue;
-    }
-    if (!X509_NAME_cmp(nm, gen->d.directoryName)) {
-      return 1;
-    }
-  }
-  return 0;
+  return nm == NULL || X509_NAME_cmp(nm, X509_CRL_get_issuer(crl)) == 0;
 }
 
 static struct CRYPTO_STATIC_MUTEX g_crl_sort_lock = CRYPTO_STATIC_MUTEX_INIT;

From 1ae65acafb680a7b3abb9d8fe1d2846000937562 Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@google.com>
Date: Sun, 12 Nov 2023 15:51:49 -0500
Subject: [PATCH 13/13] Remove no longer reachable CRL path validation code

crl_akid_check now always, on success, leaves CRL_SCORE_SAME_PATH in the
score. (Note CRL_SCORE_ISSUER_CERT contains CRL_SCORE_SAME_PATH.) This
means the recursive validation logic in check_crl_path never runs, and
we can remove a very worrying re-entrant call in the validator. The CRL
must be issued by either the issuer, or some ancestor in the chain.
(This also matches the behavior of the new validator.)

Bug: 601
Change-Id: Ie5c0feb5bb5ade3bfd49e338a637196fce29fd2a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/63942
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
(cherry picked from commit a4851dd8c67a2b311403d67270e02e7296d31157)
---
 crypto/x509/internal.h |   2 -
 crypto/x509/x509_vfy.c | 120 ++++++-----------------------------------
 include/openssl/x509.h |   6 ++-
 3 files changed, 21 insertions(+), 107 deletions(-)

diff --git a/crypto/x509/internal.h b/crypto/x509/internal.h
index aefe2a041b..5de3ed8daa 100644
--- a/crypto/x509/internal.h
+++ b/crypto/x509/internal.h
@@ -363,8 +363,6 @@ struct x509_store_ctx_st {
 
   int current_crl_score;         // score of current CRL
 
-  X509_STORE_CTX *parent;  // For CRL path validation: parent context
-
   CRYPTO_EX_DATA ex_data;
 } /* X509_STORE_CTX */;
 
diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
index d034a22bfd..2213a139dd 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -115,12 +115,9 @@ static int check_policy(X509_STORE_CTX *ctx);
 static int get_crl_score(X509_STORE_CTX *ctx, X509 **pissuer, X509_CRL *crl,
                          X509 *x);
 static int get_crl(X509_STORE_CTX *ctx, X509_CRL **pcrl, X509 *x);
-static void crl_akid_check(X509_STORE_CTX *ctx, X509_CRL *crl, X509 **pissuer,
-                           int *pcrl_score);
+static int crl_akid_check(X509_STORE_CTX *ctx, X509_CRL *crl, X509 **pissuer,
+                          int *pcrl_score);
 static int crl_crldp_check(X509 *x, X509_CRL *crl, int crl_score);
-static int check_crl_path(X509_STORE_CTX *ctx, X509 *x);
-static int check_crl_chain(X509_STORE_CTX *ctx, STACK_OF(X509) *cert_path,
-                           STACK_OF(X509) *crl_path);
 
 static int internal_verify(X509_STORE_CTX *ctx);
 
@@ -545,10 +542,7 @@ static int get_issuer_sk(X509 **issuer, X509_STORE_CTX *ctx, X509 *x) {
 
 static int check_chain_extensions(X509_STORE_CTX *ctx) {
   int ok = 0, plen = 0;
-
-  // If |ctx->parent| is set, this is CRL path validation.
-  int purpose =
-      ctx->parent == NULL ? ctx->param->purpose : X509_PURPOSE_CRL_SIGN;
+  int purpose = ctx->param->purpose;
 
   // Check all untrusted certificates
   for (int i = 0; i < ctx->last_untrusted; i++) {
@@ -807,10 +801,6 @@ static int check_revocation(X509_STORE_CTX *ctx) {
   if (ctx->param->flags & X509_V_FLAG_CRL_CHECK_ALL) {
     last = (int)sk_X509_num(ctx->chain) - 1;
   } else {
-    // If checking CRL paths this isn't the EE certificate
-    if (ctx->parent) {
-      return 1;
-    }
     last = 0;
   }
   for (int i = 0; i <= last; i++) {
@@ -861,7 +851,6 @@ static int check_cert(X509_STORE_CTX *ctx) {
 }
 
 // Check CRL times against values in X509_STORE_CTX
-
 static int check_crl_time(X509_STORE_CTX *ctx, X509_CRL *crl, int notify) {
   if (ctx->param->flags & X509_V_FLAG_NO_CHECK_TIME) {
     return 1;
@@ -1009,11 +998,8 @@ static int get_crl_score(X509_STORE_CTX *ctx, X509 **pissuer, X509_CRL *crl,
   }
 
   // Check authority key ID and locate certificate issuer
-  crl_akid_check(ctx, crl, pissuer, &crl_score);
-
-  // If we can't locate certificate issuer at this point forget it
-
-  if (!(crl_score & CRL_SCORE_AKID)) {
+  if (!crl_akid_check(ctx, crl, pissuer, &crl_score)) {
+    // If we can't locate certificate issuer at this point forget it
     return 0;
   }
 
@@ -1025,8 +1011,8 @@ static int get_crl_score(X509_STORE_CTX *ctx, X509 **pissuer, X509_CRL *crl,
   return crl_score;
 }
 
-static void crl_akid_check(X509_STORE_CTX *ctx, X509_CRL *crl, X509 **pissuer,
-                           int *pcrl_score) {
+static int crl_akid_check(X509_STORE_CTX *ctx, X509_CRL *crl, X509 **pissuer,
+                          int *pcrl_score) {
   X509 *crl_issuer = NULL;
   X509_NAME *cnm = X509_CRL_get_issuer(crl);
   int cidx = ctx->error_depth;
@@ -1040,7 +1026,7 @@ static void crl_akid_check(X509_STORE_CTX *ctx, X509_CRL *crl, X509 **pissuer,
   if (X509_check_akid(crl_issuer, crl->akid) == X509_V_OK) {
     *pcrl_score |= CRL_SCORE_AKID | CRL_SCORE_ISSUER_CERT;
     *pissuer = crl_issuer;
-    return;
+    return 1;
   }
 
   for (cidx++; cidx < (int)sk_X509_num(ctx->chain); cidx++) {
@@ -1051,64 +1037,10 @@ static void crl_akid_check(X509_STORE_CTX *ctx, X509_CRL *crl, X509 **pissuer,
     if (X509_check_akid(crl_issuer, crl->akid) == X509_V_OK) {
       *pcrl_score |= CRL_SCORE_AKID | CRL_SCORE_SAME_PATH;
       *pissuer = crl_issuer;
-      return;
+      return 1;
     }
   }
-}
-
-// Check the path of a CRL issuer certificate. This creates a new
-// X509_STORE_CTX and populates it with most of the parameters from the
-// parent. This could be optimised somewhat since a lot of path checking will
-// be duplicated by the parent, but this will rarely be used in practice.
-
-static int check_crl_path(X509_STORE_CTX *ctx, X509 *x) {
-  X509_STORE_CTX crl_ctx;
-  int ret;
-  // Don't allow recursive CRL path validation
-  if (ctx->parent) {
-    return 0;
-  }
-  if (!X509_STORE_CTX_init(&crl_ctx, ctx->ctx, x, ctx->untrusted)) {
-    return -1;
-  }
 
-  crl_ctx.crls = ctx->crls;
-  // Copy verify params across
-  X509_STORE_CTX_set0_param(&crl_ctx, ctx->param);
-
-  crl_ctx.parent = ctx;
-  crl_ctx.verify_cb = ctx->verify_cb;
-
-  // Verify CRL issuer
-  ret = X509_verify_cert(&crl_ctx);
-
-  if (ret <= 0) {
-    goto err;
-  }
-
-  // Check chain is acceptable
-
-  ret = check_crl_chain(ctx, ctx->chain, crl_ctx.chain);
-err:
-  X509_STORE_CTX_cleanup(&crl_ctx);
-  return ret;
-}
-
-// RFC 3280 says nothing about the relationship between CRL path and
-// certificate path, which could lead to situations where a certificate could
-// be revoked or validated by a CA not authorised to do so. RFC 5280 is more
-// strict and states that the two paths must end in the same trust anchor,
-// though some discussions remain... until this is resolved we use the
-// RFC 5280 version
-
-static int check_crl_chain(X509_STORE_CTX *ctx, STACK_OF(X509) *cert_path,
-                           STACK_OF(X509) *crl_path) {
-  X509 *cert_ta, *crl_ta;
-  cert_ta = sk_X509_value(cert_path, sk_X509_num(cert_path) - 1);
-  crl_ta = sk_X509_value(crl_path, sk_X509_num(crl_path) - 1);
-  if (!X509_cmp(cert_ta, crl_ta)) {
-    return 1;
-  }
   return 0;
 }
 
@@ -1116,7 +1048,6 @@ static int check_crl_chain(X509_STORE_CTX *ctx, STACK_OF(X509) *cert_path,
 // Both are relative names and compare X509_NAME types. 2. One full, one
 // relative. Compare X509_NAME to GENERAL_NAMES. 3. Both are full names and
 // compare two GENERAL_NAMES. 4. One is NULL: automatic match.
-
 static int idp_check_dp(DIST_POINT_NAME *a, DIST_POINT_NAME *b) {
   X509_NAME *nm = NULL;
   GENERAL_NAMES *gens = NULL;
@@ -1304,16 +1235,6 @@ static int check_crl(X509_STORE_CTX *ctx, X509_CRL *crl) {
       }
     }
 
-    if (!(ctx->current_crl_score & CRL_SCORE_SAME_PATH)) {
-      if (check_crl_path(ctx, ctx->current_issuer) <= 0) {
-        ctx->error = X509_V_ERR_CRL_PATH_VALIDATION_ERROR;
-        ok = ctx->verify_cb(0, ctx);
-        if (!ok) {
-          goto err;
-        }
-      }
-    }
-
     if (crl->idp_flags & IDP_INVALID) {
       ctx->error = X509_V_ERR_INVALID_EXTENSION;
       ok = ctx->verify_cb(0, ctx);
@@ -1386,10 +1307,6 @@ static int cert_crl(X509_STORE_CTX *ctx, X509_CRL *crl, X509 *x) {
 }
 
 static int check_policy(X509_STORE_CTX *ctx) {
-  if (ctx->parent) {
-    return 1;
-  }
-
   X509 *current_cert = NULL;
   int ret = X509_policy_check(ctx->chain, ctx->param->policies,
                               ctx->param->flags, &current_cert);
@@ -1658,7 +1575,10 @@ X509_CRL *X509_STORE_CTX_get0_current_crl(X509_STORE_CTX *ctx) {
 }
 
 X509_STORE_CTX *X509_STORE_CTX_get0_parent_ctx(X509_STORE_CTX *ctx) {
-  return ctx->parent;
+  // In OpenSSL, an |X509_STORE_CTX| sometimes has a parent context during CRL
+  // path validation for indirect CRLs. We require the CRL to be issued
+  // somewhere along the certificate path, so this is always NULL.
+  return NULL;
 }
 
 void X509_STORE_CTX_set_cert(X509_STORE_CTX *ctx, X509 *x) { ctx->cert = x; }
@@ -1885,16 +1805,10 @@ void X509_STORE_CTX_cleanup(X509_STORE_CTX *ctx) {
     ctx->cleanup(ctx);
     ctx->cleanup = NULL;
   }
-  if (ctx->param != NULL) {
-    if (ctx->parent == NULL) {
-      X509_VERIFY_PARAM_free(ctx->param);
-    }
-    ctx->param = NULL;
-  }
-  if (ctx->chain != NULL) {
-    sk_X509_pop_free(ctx->chain, X509_free);
-    ctx->chain = NULL;
-  }
+  X509_VERIFY_PARAM_free(ctx->param);
+  ctx->param = NULL;
+  sk_X509_pop_free(ctx->chain, X509_free);
+  ctx->chain = NULL;
   CRYPTO_free_ex_data(&g_ex_data_class, ctx, &(ctx->ex_data));
   OPENSSL_memset(&ctx->ex_data, 0, sizeof(CRYPTO_EX_DATA));
 }
diff --git a/include/openssl/x509.h b/include/openssl/x509.h
index adcd66455a..5f7ea65dec 100644
--- a/include/openssl/x509.h
+++ b/include/openssl/x509.h
@@ -2392,6 +2392,10 @@ OPENSSL_EXPORT int X509_NAME_get_text_by_OBJ(const X509_NAME *name,
 OPENSSL_EXPORT int X509_NAME_get_text_by_NID(const X509_NAME *name, int nid,
                                              char *buf, int len);
 
+// X509_STORE_CTX_get0_parent_ctx returns NULL.
+OPENSSL_EXPORT X509_STORE_CTX *X509_STORE_CTX_get0_parent_ctx(
+    X509_STORE_CTX *ctx);
+
 
 // Private structures.
 
@@ -3053,8 +3057,6 @@ OPENSSL_EXPORT int X509_STORE_CTX_get_error_depth(X509_STORE_CTX *ctx);
 OPENSSL_EXPORT X509 *X509_STORE_CTX_get_current_cert(X509_STORE_CTX *ctx);
 OPENSSL_EXPORT X509 *X509_STORE_CTX_get0_current_issuer(X509_STORE_CTX *ctx);
 OPENSSL_EXPORT X509_CRL *X509_STORE_CTX_get0_current_crl(X509_STORE_CTX *ctx);
-OPENSSL_EXPORT X509_STORE_CTX *X509_STORE_CTX_get0_parent_ctx(
-    X509_STORE_CTX *ctx);
 
 // X509_STORE_CTX_get_chain returns the pointer to the verified chain if
 // verification was successful. If unsuccessful it may return null or a partial