From: Dr. David von Oheimb Date: Sat, 9 Jul 2022 08:23:33 +0000 (+0200) Subject: x509_vfy.c and x509_lu.c: refactor find_issuer(), X509_STORE_CTX_get1_issuer(), etc. X-Git-Tag: openssl-3.5.0-alpha1~901 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9ca66fc2731a7e76415282a0a8a6b60f0169b156;p=thirdparty%2Fopenssl.git x509_vfy.c and x509_lu.c: refactor find_issuer(), X509_STORE_CTX_get1_issuer(), etc. Reviewed-by: Hugo Landau Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/18762) --- diff --git a/crypto/x509/x509_lu.c b/crypto/x509/x509_lu.c index 29b30ad6b2c..b76cb082098 100644 --- a/crypto/x509/x509_lu.c +++ b/crypto/x509/x509_lu.c @@ -367,11 +367,13 @@ static int ossl_x509_store_ctx_get_by_subject(const X509_STORE_CTX *ctx, if (tmp == NULL) return 0; } - if (!X509_OBJECT_up_ref_count(tmp)) - return -1; - ret->type = tmp->type; - ret->data = tmp->data; + if (ret != NULL) { + if (!X509_OBJECT_up_ref_count(tmp)) + return -1; + ret->type = tmp->type; + ret->data = tmp->data; + } return 1; } @@ -647,7 +649,10 @@ STACK_OF(X509) *X509_STORE_get1_all_certs(X509_STORE *store) return NULL; } -/* Returns NULL on internal/fatal error, empty stack if not found */ +/*- + * Collect from |ctx->store| all certs with subject matching |nm|. + * Returns NULL on internal/fatal error, empty stack if not found. + */ STACK_OF(X509) *X509_STORE_CTX_get1_certs(X509_STORE_CTX *ctx, const X509_NAME *nm) { @@ -670,29 +675,18 @@ STACK_OF(X509) *X509_STORE_CTX_get1_certs(X509_STORE_CTX *ctx, * Nothing found in cache: do lookup to possibly add new objects to * cache */ - X509_OBJECT *xobj = X509_OBJECT_new(); - X509_STORE_unlock(store); - if (xobj == NULL) - return NULL; - i = ossl_x509_store_ctx_get_by_subject(ctx, X509_LU_X509, nm, xobj); - if (i <= 0) { - X509_OBJECT_free(xobj); + i = ossl_x509_store_ctx_get_by_subject(ctx, X509_LU_X509, nm, NULL); + if (i <= 0) return i < 0 ? NULL : sk_X509_new_null(); - } - X509_OBJECT_free(xobj); if (!X509_STORE_lock(store)) return NULL; sk_X509_OBJECT_sort(store->objs); idx = x509_object_idx_cnt(store->objs, X509_LU_X509, nm, &cnt); - if (idx < 0) { - sk = sk_X509_new_null(); - goto end; - } } sk = sk_X509_new_null(); - if (sk == NULL) + if (idx < 0 || sk == NULL) goto end; for (i = 0; i < cnt; i++, idx++) { obj = sk_X509_OBJECT_value(store->objs, idx); @@ -713,21 +707,16 @@ STACK_OF(X509_CRL) *X509_STORE_CTX_get1_crls(const X509_STORE_CTX *ctx, const X509_NAME *nm) { int i = 1, idx, cnt; - STACK_OF(X509_CRL) *sk = sk_X509_CRL_new_null(); + STACK_OF(X509_CRL) *sk; X509_CRL *x; - X509_OBJECT *obj, *xobj = X509_OBJECT_new(); + X509_OBJECT *obj; X509_STORE *store = ctx->store; /* Always do lookup to possibly add new CRLs to cache */ - if (sk == NULL - || xobj == NULL - || (i = ossl_x509_store_ctx_get_by_subject(ctx, X509_LU_CRL, - nm, xobj)) < 0) { - X509_OBJECT_free(xobj); - sk_X509_CRL_free(sk); + i = ossl_x509_store_ctx_get_by_subject(ctx, X509_LU_CRL, nm, NULL); + if (i < 0) return NULL; - } - X509_OBJECT_free(xobj); + sk = sk_X509_CRL_new_null(); if (i == 0) return sk; if (!X509_STORE_lock(store)) { @@ -789,91 +778,6 @@ X509_OBJECT *X509_OBJECT_retrieve_match(STACK_OF(X509_OBJECT) *h, return NULL; } -/*- - * Try to get issuer cert from |ctx->store| matching the subject name of |x|. - * Prefer the first non-expired one, else take the most recently expired one. - * - * Return values are: - * 1 lookup successful. - * 0 certificate not found. - * -1 some other error. - */ -int X509_STORE_CTX_get1_issuer(X509 **issuer, X509_STORE_CTX *ctx, X509 *x) -{ - const X509_NAME *xn; - X509_OBJECT *obj = X509_OBJECT_new(), *pobj = NULL; - X509_STORE *store = ctx->store; - int i, ok, idx, ret, nmatch = 0; - - if (obj == NULL) - return -1; - *issuer = NULL; - xn = X509_get_issuer_name(x); - ok = ossl_x509_store_ctx_get_by_subject(ctx, X509_LU_X509, xn, obj); - if (ok != 1) { - X509_OBJECT_free(obj); - return ok; - } - /* If certificate matches and is currently valid all OK */ - if (ctx->check_issued(ctx, x, obj->data.x509)) { - if (ossl_x509_check_cert_time(ctx, obj->data.x509, -1)) { - *issuer = obj->data.x509; - /* |*issuer| has taken over the cert reference from |obj| */ - obj->type = X509_LU_NONE; - X509_OBJECT_free(obj); - return 1; - } - } - X509_OBJECT_free(obj); - - /* - * Due to limitations of the API this can only retrieve a single cert. - * However it will fill the cache with all matching certificates, - * so we can examine the cache for all matches. - */ - if (store == NULL) - return 0; - - /* Find index of first currently valid cert accepted by 'check_issued' */ - ret = 0; - if (!X509_STORE_lock(store)) - return 0; - - sk_X509_OBJECT_sort(store->objs); - idx = x509_object_idx_cnt(store->objs, X509_LU_X509, xn, &nmatch); - if (idx != -1) { /* should be true as we've had at least one match */ - /* Look through all matching certs for suitable issuer */ - for (i = idx; i < idx + nmatch; i++) { - pobj = sk_X509_OBJECT_value(store->objs, i); - /* See if we've run past the matches */ - if (pobj->type != X509_LU_X509) - break; - if (ctx->check_issued(ctx, x, pobj->data.x509)) { - ret = 1; - /* If times check fine, exit with match, else keep looking. */ - if (ossl_x509_check_cert_time(ctx, pobj->data.x509, -1)) { - *issuer = pobj->data.x509; - break; - } - /* - * Leave the so far most recently expired match in *issuer - * so we return nearest match if no certificate time is OK. - */ - if (*issuer == NULL - || ASN1_TIME_compare(X509_get0_notAfter(pobj->data.x509), - X509_get0_notAfter(*issuer)) > 0) - *issuer = pobj->data.x509; - } - } - } - if (*issuer != NULL && !X509_up_ref(*issuer)) { - *issuer = NULL; - ret = -1; - } - X509_STORE_unlock(store); - return ret; -} - int X509_STORE_set_flags(X509_STORE *xs, unsigned long flags) { return X509_VERIFY_PARAM_set_flags(xs->param, flags); diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c index 8257b431ea5..8cccf570215 100644 --- a/crypto/x509/x509_vfy.c +++ b/crypto/x509/x509_vfy.c @@ -50,7 +50,6 @@ static int dane_verify(X509_STORE_CTX *ctx); static int dane_verify_rpk(X509_STORE_CTX *ctx); 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); static int check_extensions(X509_STORE_CTX *ctx); static int check_name_constraints(X509_STORE_CTX *ctx); static int check_id(X509_STORE_CTX *ctx); @@ -58,7 +57,6 @@ static int check_trust(X509_STORE_CTX *ctx, int num_untrusted); 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_issuer_sk(X509 **issuer, X509_STORE_CTX *ctx, X509 *x); static int check_dane_issuer(X509_STORE_CTX *ctx, int depth); static int check_cert_key_level(X509_STORE_CTX *ctx, X509 *cert); static int check_key_level(X509_STORE_CTX *ctx, EVP_PKEY *pkey); @@ -377,30 +375,64 @@ static int sk_X509_contains(STACK_OF(X509) *sk, X509 *cert) return 0; } -/* - * Find in given STACK_OF(X509) |sk| an issuer cert (if any) of given cert |x|. - * The issuer must not yet be in |ctx->chain|, yet allowing the exception that - * |x| is self-issued and |ctx->chain| has just one element. - * Prefer the first non-expired one, else take the most recently expired one. +/*- + * Find in |sk| an issuer cert of cert |x| accepted by |ctx->check_issued|. + * If no_dup, the issuer must not yet be in |ctx->chain|, yet allowing the + * exception that |x| is self-issued and |ctx->chain| has just one element. + * Prefer the first match with suitable validity period or latest expiration. */ -static X509 *find_issuer(X509_STORE_CTX *ctx, STACK_OF(X509) *sk, X509 *x) +static X509 *get0_best_issuer_sk(X509_STORE_CTX *ctx, int trusted, + int no_dup, STACK_OF(X509) *sk, X509 *x) { int i; - X509 *issuer, *rv = NULL; + X509 *candidate, *issuer = NULL; for (i = 0; i < sk_X509_num(sk); i++) { - issuer = sk_X509_value(sk, i); - if (ctx->check_issued(ctx, x, issuer) - && (((x->ex_flags & EXFLAG_SI) != 0 && sk_X509_num(ctx->chain) == 1) - || !sk_X509_contains(ctx->chain, issuer))) { - if (ossl_x509_check_cert_time(ctx, issuer, -1)) - return issuer; - if (rv == NULL || ASN1_TIME_compare(X509_get0_notAfter(issuer), - X509_get0_notAfter(rv)) > 0) - rv = issuer; + candidate = sk_X509_value(sk, i); + if (no_dup + && !((x->ex_flags & EXFLAG_SI) != 0 && sk_X509_num(ctx->chain) == 1) + && sk_X509_contains(ctx->chain, candidate)) + continue; + if (ctx->check_issued(ctx, x, candidate)) { + if (!trusted) { /* do not check key usage for trust anchors */ + if (ossl_x509_signing_allowed(candidate, x) != X509_V_OK) + continue; + } + if (ossl_x509_check_cert_time(ctx, candidate, -1)) + return candidate; + /* + * Leave in *issuer the first match that has the latest expiration + * date so we return nearest match if no certificate time is OK. + */ + if (issuer == NULL + || ASN1_TIME_compare(X509_get0_notAfter(candidate), + X509_get0_notAfter(issuer)) > 0) + issuer = candidate; } } - return rv; + return issuer; +} + +/*- + * Try to get issuer cert from |ctx->store| accepted by |ctx->check_issued|. + * + * Return values are: + * 1 lookup successful. + * 0 certificate not found. + * -1 some other error. + */ +int X509_STORE_CTX_get1_issuer(X509 **issuer, X509_STORE_CTX *ctx, X509 *x) +{ + STACK_OF(X509) *certs = X509_STORE_CTX_get1_certs(ctx, X509_get_issuer_name(x)); + int ret = 0; + + if (certs == NULL) + return -1; + *issuer = get0_best_issuer_sk(ctx, 1 /* trusted */, 0, certs, x); + if (*issuer != NULL) + ret = X509_up_ref(*issuer) ? 1 : -1; + OSSL_STACK_OF_X509_free(certs); + return ret; } /* Check that the given certificate |x| is issued by the certificate |issuer| */ @@ -421,9 +453,10 @@ static int check_issued(ossl_unused X509_STORE_CTX *ctx, X509 *x, X509 *issuer) * Alternative get_issuer method: look up from a STACK_OF(X509) in other_ctx. * Returns -1 on internal error. */ -static int get_issuer_sk(X509 **issuer, X509_STORE_CTX *ctx, X509 *x) +static int get1_best_issuer_other_sk(X509 **issuer, X509_STORE_CTX *ctx, X509 *x) { - *issuer = find_issuer(ctx, ctx->other_ctx, x); + *issuer = get0_best_issuer_sk(ctx, 1 /* trusted */, 1 /* no_dup */, + ctx->other_ctx, x); if (*issuer == NULL) return 0; return X509_up_ref(*issuer) ? 1 : -1; @@ -2570,7 +2603,7 @@ int X509_STORE_CTX_init(X509_STORE_CTX *ctx, X509_STORE *store, X509 *x509, void X509_STORE_CTX_set0_trusted_stack(X509_STORE_CTX *ctx, STACK_OF(X509) *sk) { ctx->other_ctx = sk; - ctx->get_issuer = get_issuer_sk; + ctx->get_issuer = get1_best_issuer_other_sk; ctx->lookup_certs = lookup_certs_sk; } @@ -3458,7 +3491,7 @@ static int build_chain(X509_STORE_CTX *ctx) goto int_err; curr = sk_X509_value(ctx->chain, num - 1); issuer = (X509_self_signed(curr, 0) > 0 || num > max_depth) ? - NULL : find_issuer(ctx, sk_untrusted, curr); + NULL : get0_best_issuer_sk(ctx, 0, 1, sk_untrusted, curr); if (issuer == NULL) { /* * Once we have reached a self-signed cert or num > max_depth