From 77da1e57a26be2f7238e94b154addfc65dcc6445 Mon Sep 17 00:00:00 2001 From: tb Date: Thu, 25 Feb 2021 17:29:22 +0000 Subject: [PATCH] Fix two bugs in the legacy verifier To integrate the new X.509 verifier, X509_verify_cert() was refactored. The code building chains in the legacy verifier was split into a separate function. The first bug is that its return value was treated as a Boolean although it wasn't. Second, the return alone is not enough to decide whether to carry on the validation or not. Slightly rearrange things to restore the behavior of the legacy verifier prior to this refactoring. Issue found and test case provided by Anton Borowka and jan. ok jan jsing --- lib/libcrypto/x509/x509_vfy.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/libcrypto/x509/x509_vfy.c b/lib/libcrypto/x509/x509_vfy.c index 1c2d03b9b63..9577040d9d5 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.85 2021/02/11 04:56:43 tb Exp $ */ +/* $OpenBSD: x509_vfy.c,v 1.86 2021/02/25 17:29:22 tb Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -240,12 +240,13 @@ x509_vfy_check_id(X509_STORE_CTX *ctx) { * Oooooooh.. */ static int -X509_verify_cert_legacy_build_chain(X509_STORE_CTX *ctx, int *bad) +X509_verify_cert_legacy_build_chain(X509_STORE_CTX *ctx, int *bad, int *out_ok) { X509 *x, *xtmp, *xtmp2, *chain_ss = NULL; int bad_chain = 0; X509_VERIFY_PARAM *param = ctx->param; - int depth, i, ok = 0; + int ok = 0, ret = 0; + int depth, i; int num, j, retry, trust; int (*cb) (int xok, X509_STORE_CTX *xctx); STACK_OF(X509) *sktmp = NULL; @@ -517,11 +518,15 @@ X509_verify_cert_legacy_build_chain(X509_STORE_CTX *ctx, int *bad) if (!ok) goto end; } + + ret = 1; end: sk_X509_free(sktmp); X509_free(chain_ss); *bad = bad_chain; - return ok; + *out_ok = ok; + + return ret; } static int @@ -531,8 +536,7 @@ X509_verify_cert_legacy(X509_STORE_CTX *ctx) ctx->error = X509_V_OK; /* Initialize to OK */ - ok = X509_verify_cert_legacy_build_chain(ctx, &bad_chain); - if (!ok) + if (!X509_verify_cert_legacy_build_chain(ctx, &bad_chain, &ok)) goto end; /* We have the chain complete: now we need to check its purpose */ -- 2.20.1