From 1e257a891fd88e84b90d02b361a8a8f34ffc013b Mon Sep 17 00:00:00 2001 From: Viktor Dukhovni Date: Wed, 2 Sep 2026 18:53:50 +1000 Subject: [PATCH] Defer computation of relative CRLDP names These are derived just-in-time, during any actual CRL processing. Fixes CVE-2026-35189 Reviewed-by: Bob Beck Reviewed-by: Norbert Pocs Merge-date: Tue Sep 29 11:15:59 2026 --- crypto/x509/v3_crld.c | 42 +++++++++++++--------- crypto/x509/v3_purp.c | 45 ++++++++---------------- crypto/x509/x509_vfy.c | 80 ++++++++++++++++++++++++++++++++++++------ include/crypto/x509.h | 3 ++ 4 files changed, 112 insertions(+), 58 deletions(-) diff --git a/crypto/x509/v3_crld.c b/crypto/x509/v3_crld.c index 2391e32eba..b20128fedc 100644 --- a/crypto/x509/v3_crld.c +++ b/crypto/x509/v3_crld.c @@ -532,31 +532,41 @@ static int i2r_object(const X509V3_EXT_METHOD *method, void *oid, BIO *bp, return 1; } -/* Append any nameRelativeToCRLIssuer in dpn to iname, set in dpn->dpname */ -int DIST_POINT_set_dpname(DIST_POINT_NAME *dpn, const X509_NAME *iname) +/* + * Return a new X509_NAME consisting of iname with the nameRelativeToCRLIssuer + * fragment of dpn appended, with its DER encoding already cached. + * dpn must be a relative name (type 1). Returns NULL on error. + */ +X509_NAME *ossl_dist_point_name_full(const DIST_POINT_NAME *dpn, + const X509_NAME *iname) { int i; - STACK_OF(X509_NAME_ENTRY) *frag; + STACK_OF(X509_NAME_ENTRY) *frag = dpn->name.relativename; X509_NAME_ENTRY *ne; + X509_NAME *dpname = X509_NAME_dup(iname); - if (dpn == NULL || dpn->type != 1) - return 1; - frag = dpn->name.relativename; - X509_NAME_free(dpn->dpname); /* just in case it was already set */ - dpn->dpname = X509_NAME_dup(iname); - if (dpn->dpname == NULL) - return 0; + if (dpname == NULL) + return NULL; for (i = 0; i < sk_X509_NAME_ENTRY_num(frag); i++) { ne = sk_X509_NAME_ENTRY_value(frag, i); - if (!X509_NAME_add_entry(dpn->dpname, ne, -1, i ? 0 : 1)) + if (!X509_NAME_add_entry(dpname, ne, -1, i ? 0 : 1)) goto err; } /* generate cached encoding of name */ - if (i2d_X509_NAME(dpn->dpname, NULL) >= 0) - return 1; + if (i2d_X509_NAME(dpname, NULL) >= 0) + return dpname; err: - X509_NAME_free(dpn->dpname); - dpn->dpname = NULL; - return 0; + X509_NAME_free(dpname); + return NULL; +} + +/* Append any nameRelativeToCRLIssuer in dpn to iname, set in dpn->dpname */ +int DIST_POINT_set_dpname(DIST_POINT_NAME *dpn, const X509_NAME *iname) +{ + if (dpn == NULL || dpn->type != 1) + return 1; + X509_NAME_free(dpn->dpname); /* just in case it was already set */ + dpn->dpname = ossl_dist_point_name_full(dpn, iname); + return dpn->dpname != NULL; } diff --git a/crypto/x509/v3_purp.c b/crypto/x509/v3_purp.c index 8b51ea109a..0c8ce78d3a 100644 --- a/crypto/x509/v3_purp.c +++ b/crypto/x509/v3_purp.c @@ -306,12 +306,17 @@ int X509_supported_extension(X509_EXTENSION *ex) return 0; } -/* Returns 1 on success, 0 if x is invalid, -1 on (internal) error. */ -static int setup_dp(const X509 *x, DIST_POINT *dp) +/* + * The full name of a nameRelativeToCRLIssuer distribution point is not + * computed here. Doing so for every parsed certificate cost an + * X509_NAME_dup() of the issuer name per relative distribution point, + * which a certificate with many such entries could turn into hundreds of + * megabytes of heap on a plain TLS handshake, while the result is only + * needed when a CRL is actually being matched against the certificate. + * That name is now built on demand in the CRL checking code instead. + */ +static int setup_dp(DIST_POINT *dp) { - const X509_NAME *iname = NULL; - int i; - if (dp->distpoint == NULL && sk_GENERAL_NAME_num(dp->CRLissuer) <= 0) { ERR_raise(ERR_LIB_X509, X509_R_INVALID_DISTPOINT); return 0; @@ -325,30 +330,10 @@ static int setup_dp(const X509 *x, DIST_POINT *dp) } else { dp->dp_reasons = CRLDP_ALL_REASONS; } - if (dp->distpoint == NULL || dp->distpoint->type != 1) - return 1; - - /* Handle name fragment given by nameRelativeToCRLIssuer */ - /* - * Note that the below way of determining iname is not really compliant - * with https://tools.ietf.org/html/rfc5280#section-4.2.1.13 - * According to it, sk_GENERAL_NAME_num(dp->CRLissuer) MUST be <= 1 - * and any CRLissuer could be of type different to GEN_DIRNAME. - */ - 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) { - iname = gen->d.directoryName; - break; - } - } - if (iname == NULL) - iname = X509_get_issuer_name(x); - return DIST_POINT_set_dpname(dp->distpoint, iname) ? 1 : -1; + return 1; } -/* Return 1 on success, 0 if x is invalid, -1 on (internal) error. */ +/* Return 1 on success, 0 on error. */ static int setup_crldp(X509 *x) { int i; @@ -358,10 +343,8 @@ static int setup_crldp(X509 *x) return 0; for (i = 0; i < sk_DIST_POINT_num(x->crldp); i++) { - int res = setup_dp(x, sk_DIST_POINT_value(x->crldp, i)); - - if (res < 1) - return res; + if (!setup_dp(sk_DIST_POINT_value(x->crldp, i))) + return 0; } return 1; } diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c index 23118b7f11..bd91f6f887 100644 --- a/crypto/x509/x509_vfy.c +++ b/crypto/x509/x509_vfy.c @@ -1445,16 +1445,59 @@ static int check_crl_chain(X509_STORE_CTX *ctx, return X509_cmp(cert_ta, crl_ta) == 0; } +/* + * Return the full name of the certificate CRL distribution point dp, whose + * distpoint is a nameRelativeToCRLIssuer fragment: the CRL issuer name with + * the fragment appended. The CRL issuer is the directoryName in + * dp->CRLissuer if there is one, else the issuer of the certificate. + * + * The result is a fresh X509_NAME owned by the caller. It is deliberately + * not stored in dp->distpoint->dpname: once its extension cache has been + * published a certificate is shared between threads without locking, and + * computing the name here rather than when the certificate is parsed keeps + * a certificate with many relative distribution points from costing a copy + * of the issuer name per entry on every parse. Returns NULL on error. + */ +static X509_NAME *crldp_full_name(const X509 *x, const DIST_POINT *dp) +{ + const X509_NAME *iname = NULL; + int i; + + /* + * Note that the below way of determining iname is not really compliant + * with https://tools.ietf.org/html/rfc5280#section-4.2.1.13 + * According to it, sk_GENERAL_NAME_num(dp->CRLissuer) MUST be <= 1 + * and any CRLissuer could be of type different to GEN_DIRNAME. + */ + 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) { + iname = gen->d.directoryName; + break; + } + } + if (iname == NULL) + iname = X509_get_issuer_name(x); + return ossl_dist_point_name_full(dp->distpoint, iname); +} + /*- * Check for match between two dist point names: three separate cases. * 1. 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. + * + * a is the certificate's distribution point name and b the CRL's issuing + * distribution point name. When a is a relative name, aname is its full + * name as built by crldp_full_name(); a->dpname itself is not consulted. + * For b the full name is the cached b->dpname set when the CRL was parsed. */ -static int idp_check_dp(DIST_POINT_NAME *a, DIST_POINT_NAME *b) +static int idp_check_dp(DIST_POINT_NAME *a, const X509_NAME *aname, + DIST_POINT_NAME *b) { - X509_NAME *nm = NULL; + const X509_NAME *nm = NULL; GENERAL_NAMES *gens = NULL; GENERAL_NAME *gena, *genb; int i, j; @@ -1462,16 +1505,16 @@ static int idp_check_dp(DIST_POINT_NAME *a, DIST_POINT_NAME *b) if (a == NULL || b == NULL) return 1; if (a->type == 1) { - if (a->dpname == NULL) + if (aname == NULL) return 0; /* Case 1: two X509_NAME */ if (b->type == 1) { if (b->dpname == NULL) return 0; - return X509_NAME_cmp(a->dpname, b->dpname) == 0; + return X509_NAME_cmp(aname, b->dpname) == 0; } /* Case 2: set name and GENERAL_NAMES appropriately */ - nm = a->dpname; + nm = aname; gens = b->name.fullname; } else if (b->type == 1) { if (b->dpname == NULL) @@ -1544,13 +1587,28 @@ static int crl_crldp_check(X509 *x, X509_CRL *crl, int crl_score, *preasons = crl->idp_reasons; for (i = 0; i < sk_DIST_POINT_num(x->crldp); i++) { DIST_POINT *dp = sk_DIST_POINT_value(x->crldp, i); + X509_NAME *dpname = NULL; + int match; - if (crldp_check_crlissuer(dp, crl, crl_score)) { - if (crl->idp == NULL - || idp_check_dp(dp->distpoint, crl->idp->distpoint)) { - *preasons &= dp->dp_reasons; - return 1; - } + if (!crldp_check_crlissuer(dp, crl, crl_score)) + continue; + if (crl->idp == NULL) { + match = 1; + } else { + /* + * A relative distribution point name is only comparable in + * full form. Build it for this one comparison and discard it; + * if that fails the entry simply does not match. + */ + if (dp->distpoint != NULL && dp->distpoint->type == 1 + && (dpname = crldp_full_name(x, dp)) == NULL) + continue; + match = idp_check_dp(dp->distpoint, dpname, crl->idp->distpoint); + X509_NAME_free(dpname); + } + if (match) { + *preasons &= dp->dp_reasons; + return 1; } } return (crl->idp == NULL || crl->idp->distpoint == NULL) diff --git a/include/crypto/x509.h b/include/crypto/x509.h index 5a5f450356..7b14653ada 100644 --- a/include/crypto/x509.h +++ b/include/crypto/x509.h @@ -14,6 +14,7 @@ #include "internal/refcount.h" #include #include +#include #include #include "crypto/types.h" @@ -314,6 +315,8 @@ int ossl_a2i_ipadd(unsigned char *ipout, const char *ipasc); int ossl_x509_set1_time(int *modified, ASN1_TIME **ptm, const ASN1_TIME *tm); int ossl_x509_print_ex_brief(BIO *bio, X509 *cert, unsigned long neg_cflags); int ossl_x509v3_cache_extensions(X509 *x); +X509_NAME *ossl_dist_point_name_full(const struct DIST_POINT_NAME_st *dpn, + const X509_NAME *iname); int ossl_x509_init_sig_info(X509 *x); int ossl_x509_set0_libctx(X509 *x, OSSL_LIB_CTX *libctx, const char *propq); -- 2.55.0