From 661de442e4231a9b0411dc8562f9e465d1d7fabc Mon Sep 17 00:00:00 2001 From: Viktor Dukhovni Date: Mon, 30 Aug 2021 14:17:16 -0400 Subject: [PATCH] Prioritise DANE TLSA issuer certs over peer certs MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit When building the certificate chain, prioritise any Cert(0) Full(0) certificates from TLSA records over certificates received from the peer. This is important when the server sends a cross cert, but TLSA records include the underlying root CA cert. We want to construct a chain with the issuer from the TLSA record, which can then match the TLSA records (while the associated cross cert may not). Reviewed-by: Tomáš Mráz --- crypto/x509/x509_vfy.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c index 18c6172c980..0e5b18f67ef 100644 --- a/crypto/x509/x509_vfy.c +++ b/crypto/x509/x509_vfy.c @@ -3023,22 +3023,26 @@ static int build_chain(X509_STORE_CTX *ctx) may_trusted = 1; } - /* - * Shallow-copy the stack of untrusted certificates (with TLS, this is - * typically the content of the peer's certificate message) so can make - * multiple passes over it, while free to remove elements as we go. - */ - if ((sk_untrusted = sk_X509_dup(ctx->untrusted)) == NULL) + /* Initialize empty untrusted stack. */ + if ((sk_untrusted = sk_X509_new_null()) == NULL) goto memerr; /* - * If we got any "DANE-TA(2) Cert(0) Full(0)" trust anchors from DNS, add - * them to our working copy of the untrusted certificate stack. + * If we got any "Cert(0) Full(0)" trust anchors from DNS, *prepend* them + * to our working copy of the untrusted certificate stack. */ if (DANETLS_ENABLED(dane) && dane->certs != NULL && !X509_add_certs(sk_untrusted, dane->certs, X509_ADD_FLAG_DEFAULT)) goto memerr; + /* + * Shallow-copy the stack of untrusted certificates (with TLS, this is + * typically the content of the peer's certificate message) so we can make + * multiple passes over it, while free to remove elements as we go. + */ + if (!X509_add_certs(sk_untrusted, ctx->untrusted, X509_ADD_FLAG_DEFAULT)) + goto memerr; + /* * Still absurdly large, but arithmetically safe, a lower hard upper bound * might be reasonable. -- 2.47.3