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>
diff --git a/crypto/x509/internal.h b/crypto/x509/internal.h
index 7de539f..36a244b 100644
--- a/crypto/x509/internal.h
+++ b/crypto/x509/internal.h
@@ -185,8 +185,6 @@
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 @@
// 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 f4f5efe..758fccb 100644
--- a/crypto/x509/x_crl.c
+++ b/crypto/x509/x_crl.c
@@ -115,45 +115,15 @@
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 @@
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 @@
crl->akid = NULL;
crl->flags = 0;
crl->idp_flags = 0;
- crl->issuers = NULL;
break;
case ASN1_OP_D2I_POST: {
@@ -259,7 +222,7 @@
}
}
- if (!crl_set_issuers(crl)) {
+ if (!crl_parse_entry_extensions(crl)) {
return 0;
}
@@ -269,7 +232,6 @@
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 @@
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 CRYPTO_MUTEX g_crl_sort_lock = CRYPTO_MUTEX_INIT;
