From 90ef40ae9d614b7e8df22be569d2596374073170 Mon Sep 17 00:00:00 2001 From: jsing <> Date: Tue, 5 Jan 2021 16:45:59 +0000 Subject: Gracefully handle root certificates being both trusted and untrusted. When a certificate (namely a root) is specified as both a trusted and untrusted certificate, the new verifier will find multiple chains - the first being back to the trusted root certificate and a second via the root that is untrusted, followed by the trusted root certificate. This situation can be triggered by a server that (unnecessarily) includes the root certificate in its certificate list. While this validates correctly (using the first chain), it means that we encounter a failure while building the second chain due to the root certificate already being in the chain. When this occurs we call the verify callback indicating a bad certificate. Some sensitive software (including bacula and icinga), treat this single bad chain callback as terminal, even though we successfully verify the certificate. Avoid this problem by simply dumping the chain if we encounter a situation where the certificate is already in the chain and also a trusted root - we'll have already picked up the trusted root as a shorter path. Issue with icinga2 initially reported by Theodore Wynnychenko. Fix tested by sthen@ for both bacula and icinga2. ok tb@ --- src/lib/libcrypto/x509/x509_internal.h | 3 ++- src/lib/libcrypto/x509/x509_verify.c | 17 ++++++++++++++--- 2 files changed, 16 insertions(+), 4 deletions(-) (limited to 'src/lib') diff --git a/src/lib/libcrypto/x509/x509_internal.h b/src/lib/libcrypto/x509/x509_internal.h index 2f2fe47a8f..1ede7b6bad 100644 --- a/src/lib/libcrypto/x509/x509_internal.h +++ b/src/lib/libcrypto/x509/x509_internal.h @@ -1,4 +1,4 @@ -/* $OpenBSD: x509_internal.h,v 1.5 2020/11/18 17:00:59 tb Exp $ */ +/* $OpenBSD: x509_internal.h,v 1.6 2021/01/05 16:45:59 jsing Exp $ */ /* * Copyright (c) 2020 Bob Beck * @@ -65,6 +65,7 @@ struct x509_verify_ctx { X509_STORE_CTX *xsc; struct x509_verify_chain **chains; /* Validated chains */ size_t chains_count; + int dump_chain; /* Dump current chain without erroring */ STACK_OF(X509) *roots; /* Trusted roots for this validation */ STACK_OF(X509) *intermediates; /* Intermediates provided by peer */ time_t *check_time; /* Time for validity checks */ diff --git a/src/lib/libcrypto/x509/x509_verify.c b/src/lib/libcrypto/x509/x509_verify.c index 88a7ef034d..a5b41afb85 100644 --- a/src/lib/libcrypto/x509/x509_verify.c +++ b/src/lib/libcrypto/x509/x509_verify.c @@ -1,4 +1,4 @@ -/* $OpenBSD: x509_verify.c,v 1.25 2020/12/16 18:46:29 tb Exp $ */ +/* $OpenBSD: x509_verify.c,v 1.26 2021/01/05 16:45:59 jsing Exp $ */ /* * Copyright (c) 2020 Bob Beck * @@ -381,8 +381,18 @@ x509_verify_consider_candidate(struct x509_verify_ctx *ctx, X509 *cert, /* Fail if the certificate is already in the chain */ for (i = 0; i < sk_X509_num(current_chain->certs); i++) { if (X509_cmp(sk_X509_value(current_chain->certs, i), - candidate) == 0) + candidate) == 0) { + if (is_root_cert) { + /* + * Someone made a boo-boo and put their root + * in with their intermediates - handle this + * gracefully as we'll have already picked + * this up as a shorter chain. + */ + ctx->dump_chain = 1; + } return 0; + } } if (ctx->sig_checks++ > X509_VERIFY_MAX_SIGCHECKS) { @@ -475,6 +485,7 @@ x509_verify_build_chains(struct x509_verify_ctx *ctx, X509 *cert, return; count = ctx->chains_count; + ctx->dump_chain = 0; ctx->error = X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY; ctx->error_depth = depth; if (ctx->xsc != NULL) { @@ -528,7 +539,7 @@ x509_verify_build_chains(struct x509_verify_ctx *ctx, X509 *cert, ctx->xsc->current_cert = cert; (void) ctx->xsc->verify_cb(1, ctx->xsc); } - } else if (ctx->error_depth == depth) { + } else if (ctx->error_depth == depth && !ctx->dump_chain) { (void) x509_verify_cert_error(ctx, cert, depth, ctx->error, 0); } -- cgit v1.2.3-55-g6feb