From: beck Date: Fri, 3 Sep 2021 08:58:53 +0000 (+0000) Subject: Call the callback on success in new verifier in a compatible way X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=89ae9cb0e3f28a4dd8735bd4d06dcc3f8a5b9291;p=openbsd Call the callback on success in new verifier in a compatible way when we succeed with a chain, and ensure we do not call the callback twice when the caller doesn't expect it. A refactor of the end of the legacy verify code in x509_vfy is probably overdue, but this should be done based on a piece that works. the important bit here is this allows the perl regression tests in tree to pass. Changes the previously committed regress tests to test the success case callbacks to be known to pass. ok bluhm@ tb@ --- diff --git a/lib/libcrypto/x509/x509_internal.h b/lib/libcrypto/x509/x509_internal.h index 7d3250d0632..8891aecb135 100644 --- a/lib/libcrypto/x509/x509_internal.h +++ b/lib/libcrypto/x509/x509_internal.h @@ -1,4 +1,4 @@ -/* $OpenBSD: x509_internal.h,v 1.11 2021/08/28 15:22:42 beck Exp $ */ +/* $OpenBSD: x509_internal.h,v 1.12 2021/09/03 08:58:53 beck Exp $ */ /* * Copyright (c) 2020 Bob Beck * @@ -90,6 +90,7 @@ int x509_vfy_check_revocation(X509_STORE_CTX *ctx); int x509_vfy_check_policy(X509_STORE_CTX *ctx); int x509_vfy_check_trust(X509_STORE_CTX *ctx); int x509_vfy_check_chain_extensions(X509_STORE_CTX *ctx); +int x509_vfy_callback_indicate_success(X509_STORE_CTX *ctx); void x509v3_cache_extensions(X509 *x); X509 *x509_vfy_lookup_cert_match(X509_STORE_CTX *ctx, X509 *x); diff --git a/lib/libcrypto/x509/x509_verify.c b/lib/libcrypto/x509/x509_verify.c index 39371ef0384..2ec53f6fc8a 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.47 2021/08/30 08:59:33 beck Exp $ */ +/* $OpenBSD: x509_verify.c,v 1.48 2021/09/03 08:58:53 beck Exp $ */ /* * Copyright (c) 2020-2021 Bob Beck * @@ -383,6 +383,7 @@ x509_verify_ctx_validate_legacy_chain(struct x509_verify_ctx *ctx, return 0; chain->cert_errors[ctx->xsc->error_depth] = ctx->xsc->error; + ctx->error_depth = ctx->xsc->error_depth; } return ret; @@ -537,10 +538,11 @@ x509_verify_consider_candidate(struct x509_verify_ctx *ctx, X509 *cert, x509_verify_chain_free(new_chain); return 0; } - if (x509_verify_cert_error(ctx, candidate, depth, X509_V_OK, 1)) { - (void) x509_verify_ctx_add_chain(ctx, new_chain); - goto done; + if (!x509_verify_ctx_add_chain(ctx, new_chain)) { + x509_verify_chain_free(new_chain); + return 0; } + goto done; } x509_verify_build_chains(ctx, candidate, new_chain, full_chain); @@ -596,8 +598,15 @@ x509_verify_build_chains(struct x509_verify_ctx *ctx, X509 *cert, return; count = ctx->chains_count; + ctx->error = X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY; ctx->error_depth = depth; + + if (ctx->saved_error != 0) + ctx->error = ctx->saved_error; + if (ctx->saved_error_depth != 0) + ctx->error_depth = ctx->saved_error_depth; + if (ctx->xsc != NULL) { /* * Long ago experiments at Muppet labs resulted in a @@ -663,8 +672,6 @@ x509_verify_build_chains(struct x509_verify_ctx *ctx, X509 *cert, } else if (ctx->error_depth == depth) { if (!x509_verify_ctx_set_xsc_chain(ctx, current_chain, 0, 0)) return; - (void) x509_verify_cert_error(ctx, cert, depth, - ctx->error, 0); } } @@ -1131,9 +1138,12 @@ x509_verify(struct x509_verify_ctx *ctx, X509 *leaf, char *name) } do { retry_chain_build = 0; - if (x509_verify_ctx_cert_is_root(ctx, leaf, full_chain)) - x509_verify_ctx_add_chain(ctx, current_chain); - else { + if (x509_verify_ctx_cert_is_root(ctx, leaf, full_chain)) { + if (!x509_verify_ctx_add_chain(ctx, current_chain)) { + x509_verify_chain_free(current_chain); + goto err; + } + } else { x509_verify_build_chains(ctx, leaf, current_chain, full_chain); if (full_chain && ctx->chains_count == 0) { @@ -1189,8 +1199,24 @@ x509_verify(struct x509_verify_ctx *ctx, X509 *leaf, char *name) if (!x509_verify_ctx_set_xsc_chain(ctx, ctx->chains[0], 1, 1)) goto err; + ctx->xsc->error = X509_V_OK; + /* + * Call the callback indicating success up our already + * verified chain. The callback could still tell us to + * fail. + */ + if(!x509_vfy_callback_indicate_success(ctx->xsc)) + goto err; + } else { + /* + * We had a failure, indicate the failure, but + * allow the callback to override at depth 0 + */ + if (ctx->xsc->verify_cb(0, ctx->xsc)) { + ctx->xsc->error = X509_V_OK; + return 1; + } } - return ctx->xsc->verify_cb(ctx->chains_count > 0, ctx->xsc); } return (ctx->chains_count); diff --git a/lib/libcrypto/x509/x509_vfy.c b/lib/libcrypto/x509/x509_vfy.c index a161b330aec..2f69017e969 100644 --- a/lib/libcrypto/x509/x509_vfy.c +++ b/lib/libcrypto/x509/x509_vfy.c @@ -1,4 +1,4 @@ -/* $OpenBSD: x509_vfy.c,v 1.88 2021/08/28 15:22:42 beck Exp $ */ +/* $OpenBSD: x509_vfy.c,v 1.89 2021/09/03 08:58:53 beck Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -1879,7 +1879,7 @@ x509_check_cert_time(X509_STORE_CTX *ctx, X509 *x, int depth) } static int -internal_verify(X509_STORE_CTX *ctx) +x509_vfy_internal_verify(X509_STORE_CTX *ctx, int chain_verified) { int n = sk_X509_num(ctx->chain) - 1; X509 *xi = sk_X509_value(ctx->chain, n); @@ -1915,8 +1915,8 @@ internal_verify(X509_STORE_CTX *ctx) * certificate and its depth (rather than the depth of * the subject). */ - if (xs != xi || - (ctx->param->flags & X509_V_FLAG_CHECK_SS_SIGNATURE)) { + if (!chain_verified && ( xs != xi || + (ctx->param->flags & X509_V_FLAG_CHECK_SS_SIGNATURE))) { EVP_PKEY *pkey; if ((pkey = X509_get_pubkey(xi)) == NULL) { if (!verify_cb_cert(ctx, xi, xi != xs ? n+1 : n, @@ -1933,7 +1933,7 @@ internal_verify(X509_STORE_CTX *ctx) } check_cert: /* Calls verify callback as needed */ - if (!x509_check_cert_time(ctx, xs, n)) + if (!chain_verified && !x509_check_cert_time(ctx, xs, n)) return 0; /* @@ -1954,6 +1954,18 @@ check_cert: return 1; } +static int +internal_verify(X509_STORE_CTX *ctx) +{ + return x509_vfy_internal_verify(ctx, 0); +} + +int +x509_vfy_callback_indicate_success(X509_STORE_CTX *ctx) +{ + return x509_vfy_internal_verify(ctx, 1); +} + int X509_cmp_current_time(const ASN1_TIME *ctm) { diff --git a/regress/lib/libcrypto/x509/Makefile b/regress/lib/libcrypto/x509/Makefile index b05bf0bc66c..b5cf333a295 100644 --- a/regress/lib/libcrypto/x509/Makefile +++ b/regress/lib/libcrypto/x509/Makefile @@ -1,4 +1,4 @@ -# $OpenBSD: Makefile,v 1.7 2021/09/01 08:12:15 beck Exp $ +# $OpenBSD: Makefile,v 1.8 2021/09/03 08:58:53 beck Exp $ PROGS = constraints verify x509attribute x509name callback LDADD= -Wl,-Bstatic -lcrypto -Wl,-Bdynamic @@ -14,8 +14,6 @@ REGRESS_TARGETS += regress-x509attribute REGRESS_TARGETS += regress-x509name REGRESS_TARGETS += regress-callback -REGRESS_EXPECTED_FAILURES += regress-callback - CLEANFILES+= x509name.result callbackout .if make(clean) || make(cleandir)