On Sat, Nov 25, 2023 at 01:19:54PM -0500, Viktor Dukhovni wrote:
> > Now the question is: is this a bug in my application or in 3.2.0?
> > Maybe someone who knows/understands the code can take a look?
> > BTW: All the other added tlsa_free() calls are seemingly before
> > a return statement.
> >
>
> Thanks for the report!
>
> This is a freshly minted bug (just prior to the release) introducing a
> double-free (rather than the intent of avoiding a memory leak):
Proposed fix:
commit 2518d166d4671478072428aacbfcdbf32a876cd2
Author: Viktor Dukhovni <openssl-users@xxxxxxxxxxxx>
Date: Sat Nov 25 13:26:20 2023 -0500
Fix freshly introduced double-free.
We don't need the decoded X.509 Full(0) certificate for the EE usages 1 and 3,
because the leaf certificate is always part of the presented chain, so the
certificate is only validated as well-formed, and then discarded, but the
TLSA record is of course still used after the validation step.
Reported by Claus Assmann.
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -340,8 +340,19 @@ static int dane_tlsa_add(SSL_DANE *dane,
}
if ((DANETLS_USAGE_BIT(usage) & DANETLS_TA_MASK) == 0) {
+ /*
+ * The Full(0) certificate decodes to a seemingly valid X.509
+ * object with a plausible key, so the TLSA record is well
+ * formed. However, we don't actually need the certifiate for
+ * usages PKIX-EE(1) or DANE-EE(3), because at least the EE
+ * certificate is always presented by the peer. We discard the
+ * certificate, and just use the TLSA data as an opaque blob
+ * for matching the raw presented DER octets.
+ *
+ * DO NOT FREE `t` here, it will be added to the TLSA record
+ * list below!
+ */
X509_free(cert);
- tlsa_free(t);
break;
}
--
Viktor.
