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>
diff --git a/crypto/x509/internal.h b/crypto/x509/internal.h
index 408698b..72d0f9b 100644
--- a/crypto/x509/internal.h
+++ b/crypto/x509/internal.h
@@ -219,9 +219,6 @@
// 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 f203da9..89e95aa 100644
--- a/crypto/x509/x509_vfy.c
+++ b/crypto/x509/x509_vfy.c
@@ -1046,12 +1046,6 @@
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)) {
@@ -1306,9 +1300,7 @@
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;
@@ -1326,7 +1318,6 @@
}
// Lookup CRLs from store
-
skcrl = ctx->lookup_crls(ctx, nm);
// If no CRLs found and a near match from get_crl_sk use that
@@ -1382,42 +1373,39 @@
}
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;
+ // 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;
+ }
+ }
+
+ 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 (!(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);
- if (!ok) {
- goto err;
- }
+ if (crl->idp_flags & IDP_INVALID) {
+ ctx->error = X509_V_ERR_INVALID_EXTENSION;
+ ok = ctx->verify_cb(0, ctx);
+ if (!ok) {
+ goto err;
}
}
diff --git a/crypto/x509/x_crl.c b/crypto/x509/x_crl.c
index 227867b..37aed82 100644
--- a/crypto/x509/x_crl.c
+++ b/crypto/x509/x_crl.c
@@ -201,8 +201,6 @@
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 @@
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 @@
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 @@
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;
}
