diff options
author | tb <> | 2021-02-25 17:29:22 +0000 |
---|---|---|
committer | tb <> | 2021-02-25 17:29:22 +0000 |
commit | 7227be70483e3ee10d48bca3182b402fe54a1d4d (patch) | |
tree | d1962c3b5f81151f136e89663cedd20e033ce721 | |
parent | f5ad8c746a9589134ceaa336268d6a41efef12d5 (diff) | |
download | openbsd-7227be70483e3ee10d48bca3182b402fe54a1d4d.tar.gz openbsd-7227be70483e3ee10d48bca3182b402fe54a1d4d.tar.bz2 openbsd-7227be70483e3ee10d48bca3182b402fe54a1d4d.zip |
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
-rw-r--r-- | src/lib/libcrypto/x509/x509_vfy.c | 16 |
1 files changed, 10 insertions, 6 deletions
diff --git a/src/lib/libcrypto/x509/x509_vfy.c b/src/lib/libcrypto/x509/x509_vfy.c index 1c2d03b9b6..9577040d9d 100644 --- a/src/lib/libcrypto/x509/x509_vfy.c +++ b/src/lib/libcrypto/x509/x509_vfy.c | |||
@@ -1,4 +1,4 @@ | |||
1 | /* $OpenBSD: x509_vfy.c,v 1.85 2021/02/11 04:56:43 tb Exp $ */ | 1 | /* $OpenBSD: x509_vfy.c,v 1.86 2021/02/25 17:29:22 tb Exp $ */ |
2 | /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) | 2 | /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) |
3 | * All rights reserved. | 3 | * All rights reserved. |
4 | * | 4 | * |
@@ -240,12 +240,13 @@ x509_vfy_check_id(X509_STORE_CTX *ctx) { | |||
240 | * Oooooooh.. | 240 | * Oooooooh.. |
241 | */ | 241 | */ |
242 | static int | 242 | static int |
243 | X509_verify_cert_legacy_build_chain(X509_STORE_CTX *ctx, int *bad) | 243 | X509_verify_cert_legacy_build_chain(X509_STORE_CTX *ctx, int *bad, int *out_ok) |
244 | { | 244 | { |
245 | X509 *x, *xtmp, *xtmp2, *chain_ss = NULL; | 245 | X509 *x, *xtmp, *xtmp2, *chain_ss = NULL; |
246 | int bad_chain = 0; | 246 | int bad_chain = 0; |
247 | X509_VERIFY_PARAM *param = ctx->param; | 247 | X509_VERIFY_PARAM *param = ctx->param; |
248 | int depth, i, ok = 0; | 248 | int ok = 0, ret = 0; |
249 | int depth, i; | ||
249 | int num, j, retry, trust; | 250 | int num, j, retry, trust; |
250 | int (*cb) (int xok, X509_STORE_CTX *xctx); | 251 | int (*cb) (int xok, X509_STORE_CTX *xctx); |
251 | STACK_OF(X509) *sktmp = NULL; | 252 | STACK_OF(X509) *sktmp = NULL; |
@@ -517,11 +518,15 @@ X509_verify_cert_legacy_build_chain(X509_STORE_CTX *ctx, int *bad) | |||
517 | if (!ok) | 518 | if (!ok) |
518 | goto end; | 519 | goto end; |
519 | } | 520 | } |
521 | |||
522 | ret = 1; | ||
520 | end: | 523 | end: |
521 | sk_X509_free(sktmp); | 524 | sk_X509_free(sktmp); |
522 | X509_free(chain_ss); | 525 | X509_free(chain_ss); |
523 | *bad = bad_chain; | 526 | *bad = bad_chain; |
524 | return ok; | 527 | *out_ok = ok; |
528 | |||
529 | return ret; | ||
525 | } | 530 | } |
526 | 531 | ||
527 | static int | 532 | static int |
@@ -531,8 +536,7 @@ X509_verify_cert_legacy(X509_STORE_CTX *ctx) | |||
531 | 536 | ||
532 | ctx->error = X509_V_OK; /* Initialize to OK */ | 537 | ctx->error = X509_V_OK; /* Initialize to OK */ |
533 | 538 | ||
534 | ok = X509_verify_cert_legacy_build_chain(ctx, &bad_chain); | 539 | if (!X509_verify_cert_legacy_build_chain(ctx, &bad_chain, &ok)) |
535 | if (!ok) | ||
536 | goto end; | 540 | goto end; |
537 | 541 | ||
538 | /* We have the chain complete: now we need to check its purpose */ | 542 | /* We have the chain complete: now we need to check its purpose */ |