From ad8616e328386996ac0c754338c98e3df36d6aaf Mon Sep 17 00:00:00 2001 From: beck Date: Wed, 18 Aug 2021 15:10:46 +0000 Subject: [PATCH] Refactor the legacy chain validation from the chain adding code into its own function, in preparation for subesquent change. No functional change. ok tb@ --- lib/libcrypto/x509/x509_verify.c | 122 ++++++++++++++++++------------- 1 file changed, 70 insertions(+), 52 deletions(-) diff --git a/lib/libcrypto/x509/x509_verify.c b/lib/libcrypto/x509/x509_verify.c index 18d395d2737..dd053ad8122 100644 --- a/lib/libcrypto/x509/x509_verify.c +++ b/lib/libcrypto/x509/x509_verify.c @@ -1,4 +1,4 @@ -/* $OpenBSD: x509_verify.c,v 1.39 2021/07/12 15:12:38 beck Exp $ */ +/* $OpenBSD: x509_verify.c,v 1.40 2021/08/18 15:10:46 beck Exp $ */ /* * Copyright (c) 2020-2021 Bob Beck * @@ -307,6 +307,71 @@ x509_verify_ctx_restore_xsc_error(struct x509_verify_ctx *ctx) return 1; } +/* Perform legacy style validation of a chain */ +static int +x509_verify_ctx_validate_legacy_chain(struct x509_verify_ctx *ctx, + struct x509_verify_chain *chain, size_t depth) +{ + int ret = 0; + + if (ctx->xsc == NULL) + return 1; + + /* + * If we have a legacy xsc, choose a validated chain, and + * apply the extensions, revocation, and policy checks just + * like the legacy code did. We do this here instead of as + * building the chains to more easily support the callback and + * the bewildering array of VERIFY_PARAM knobs that are there + * for the fiddling. + */ + + /* These may be set in one of the following calls. */ + ctx->xsc->error = X509_V_OK; + ctx->xsc->error_depth = 0; + + if (!x509_verify_ctx_set_xsc_chain(ctx, chain, 0, 1)) + goto err; + + /* + * XXX currently this duplicates some work done in chain + * build, but we keep it here until we have feature parity + */ + if (!x509_vfy_check_chain_extensions(ctx->xsc)) + goto err; + + if (!x509_constraints_chain(ctx->xsc->chain, + &ctx->xsc->error, &ctx->xsc->error_depth)) { + X509 *cert = sk_X509_value(ctx->xsc->chain, depth); + if (!x509_verify_cert_error(ctx, cert, + ctx->xsc->error_depth, ctx->xsc->error, 0)) + goto err; + } + + if (!x509_vfy_check_revocation(ctx->xsc)) + goto err; + + if (!x509_vfy_check_policy(ctx->xsc)) + goto err; + + ret = 1; + + err: + /* + * The above checks may have set ctx->xsc->error and + * ctx->xsc->error_depth - save these for later on. + */ + if (ctx->xsc->error != X509_V_OK) { + if (ctx->xsc->error_depth < 0 || + ctx->xsc->error_depth >= X509_VERIFY_MAX_CHAIN_CERTS) + return 0; + chain->cert_errors[ctx->xsc->error_depth] = + ctx->xsc->error; + } + + return ret; +} + /* Add a validated chain to our list of valid chains */ static int x509_verify_ctx_add_chain(struct x509_verify_ctx *ctx, @@ -328,59 +393,12 @@ x509_verify_ctx_add_chain(struct x509_verify_ctx *ctx, X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY) chain->cert_errors[depth] = X509_V_OK; - /* - * If we have a legacy xsc, choose a validated chain, - * and apply the extensions, revocation, and policy checks - * just like the legacy code did. We do this here instead - * of as building the chains to more easily support the - * callback and the bewildering array of VERIFY_PARAM - * knobs that are there for the fiddling. - */ - if (ctx->xsc != NULL) { - /* These may be set in one of the following calls. */ - ctx->xsc->error = X509_V_OK; - ctx->xsc->error_depth = 0; - - if (!x509_verify_ctx_set_xsc_chain(ctx, chain, 0, 1)) - return 0; - - /* - * XXX currently this duplicates some work done - * in chain build, but we keep it here until - * we have feature parity - */ - if (!x509_vfy_check_chain_extensions(ctx->xsc)) - return 0; - - if (!x509_constraints_chain(ctx->xsc->chain, - &ctx->xsc->error, &ctx->xsc->error_depth)) { - X509 *cert = sk_X509_value(ctx->xsc->chain, depth); - if (!x509_verify_cert_error(ctx, cert, - ctx->xsc->error_depth, ctx->xsc->error, 0)) - return 0; - } - - if (!x509_vfy_check_revocation(ctx->xsc)) - return 0; - - if (!x509_vfy_check_policy(ctx->xsc)) - return 0; + if (!x509_verify_ctx_validate_legacy_chain(ctx, chain, depth)) + return 0; - /* - * The above checks may have set ctx->xsc->error and - * ctx->xsc->error_depth - save these for later on. - */ - if (ctx->xsc->error != X509_V_OK) { - if (ctx->xsc->error_depth < 0 || - ctx->xsc->error_depth >= X509_VERIFY_MAX_CHAIN_CERTS) - return 0; - chain->cert_errors[ctx->xsc->error_depth] = - ctx->xsc->error; - } - } /* - * no xsc means we are being called from the non-legacy API, - * extensions and purpose are dealt with as the chain is built. + * In the non-legacy code, extensions and purpose are dealt + * with as the chain is built. * * The non-legacy api returns multiple chains but does not do * any revocation checking (it must be done by the caller on -- 2.20.1