From: Yegor Yefremov <[email protected]>
GnuTLS takes a "success" answer from a gnutls_certificate_retrieve_function2
or ...3 at face value: it dereferences the returned private key without
checking it. An application callback that returns zero while leaving the key
NULL therefore crashes the process inside gnutls_handshake(), on a stack in
which MHD does not appear -- the callback was installed directly into the
credentials, so nothing of ours sits between it and the crash.
Install a wrapper around the application callback instead, and reject an
answer that is not internally consistent: a non-empty certificate list with a
NULL list pointer or without a private key, and for the ...3 variant a
non-empty OCSP list with a NULL list pointer. An answer with no certificate
at all is passed through untouched, as that is the documented way for the
callback to say it has nothing for this connection. The out-parameters are
cleared before the application is called, so a callback that reports success
without setting them is caught as well rather than validated against
whatever GnuTLS left there.
The cost of a rejected answer is one failed handshake for that connection.
The callback is at fault either way, but the failure is worth catching here:
it happens inside a library the application cannot see into, and only for
those connections that actually reach the callback, which can be a small and
hard to reproduce subset of the traffic.
Found by fuzz_tls with the SNI_NO_KEY callback behaviour.
Assisted-by: Claude:claude-opus-5
---
src/microhttpd/daemon.c | 212 +++++++++++++++++++++++++++++++++++++++-
1 file changed, 210 insertions(+), 2 deletions(-)
diff --git a/src/microhttpd/daemon.c b/src/microhttpd/daemon.c
index b42d4ee1..788eac8e 100644
--- a/src/microhttpd/daemon.c
+++ b/src/microhttpd/daemon.c
@@ -484,6 +484,214 @@ MHD_ip_limit_del (struct MHD_Daemon *daemon,
#ifdef HTTPS_SUPPORT
+#if GNUTLS_VERSION_MAJOR >= 3
+/**
+ * Check the answer of an application certificate retrieve callback for
+ * internal consistency.
+ *
+ * GnuTLS takes a "success" answer at face value: it dereferences the
+ * private key without checking it, so a callback that returns zero while
+ * leaving the key NULL crashes the process inside gnutls_handshake().
+ * Such a callback is at fault, but it is worth catching here anyway: the
+ * crash happens inside a library the application cannot see into, and
+ * only for those connections that actually reach the callback, which can
+ * be a small and hard to reproduce subset of the traffic. Refusing the
+ * answer instead costs one failed handshake.
+ *
+ * An answer with no certificate at all is left alone: that is the
+ * documented way for the callback to say "I have nothing for this
+ * connection", and GnuTLS handles it.
+ *
+ * @param daemon the daemon to log the problem to
+ * @param pcert_length the number of certificates reported by the callback
+ * @param pcert the certificate list reported by the callback
+ * @param pkey the private key reported by the callback
+ * @return true if the answer may be handed to GnuTLS,
+ * false if it must be rejected
+ */
+static bool
+cert_retrieve_answer_is_sane (struct MHD_Daemon *daemon,
+ unsigned int pcert_length,
+ const gnutls_pcert_st *pcert,
+ gnutls_privkey_t pkey)
+{
+ if (0 == pcert_length)
+ return true; /* No certificate offered, nothing to be inconsistent */
+ if (NULL == pcert)
+ {
+#ifdef HAVE_MESSAGES
+ MHD_DLOG (daemon,
+ _ ("The application certificate callback reported success " \
+ "with a non-empty certificate list, but the list pointer " \
+ "is NULL. Failing the TLS handshake.\n"));
+#else /* ! HAVE_MESSAGES */
+ (void) daemon; /* Mute compiler warning */
+#endif /* ! HAVE_MESSAGES */
+ return false;
+ }
+ if (NULL == pkey)
+ {
+#ifdef HAVE_MESSAGES
+ MHD_DLOG (daemon,
+ _ ("The application certificate callback reported success " \
+ "with a certificate, but without a private key. " \
+ "Failing the TLS handshake.\n"));
+#else /* ! HAVE_MESSAGES */
+ (void) daemon; /* Mute compiler warning */
+#endif /* ! HAVE_MESSAGES */
+ return false;
+ }
+ return true;
+}
+
+
+/**
+ * Wrapper around the application callback set by
+ * #MHD_OPTION_HTTPS_CERT_CALLBACK, see cert_retrieve_answer_is_sane().
+ *
+ * @param session the session to get the certificate for
+ * @param req_ca_dn the distinguished names of the acceptable CAs
+ * @param nreqs the number of entries in @a req_ca_dn
+ * @param pk_algos the public key algorithms the client supports
+ * @param pk_algos_length the number of entries in @a pk_algos
+ * @param[out] pcert the certificate list to use
+ * @param[out] pcert_length the number of entries in @a pcert
+ * @param[out] pkey the private key matching @a pcert
+ * @return 0 on success, negative value on error
+ */
+static int
+cert_retrieve_wrapper (gnutls_session_t session,
+ const gnutls_datum_t *req_ca_dn,
+ int nreqs,
+ const gnutls_pk_algorithm_t *pk_algos,
+ int pk_algos_length,
+ gnutls_pcert_st **pcert,
+ unsigned int *pcert_length,
+ gnutls_privkey_t *pkey)
+{
+ struct MHD_Connection *connection;
+ struct MHD_Daemon *daemon;
+ int ret;
+
+ connection = gnutls_session_get_ptr (session);
+ if (NULL == connection)
+ return -1;
+ daemon = connection->daemon;
+ mhd_assert (NULL != daemon->cert_callback);
+ if (NULL == daemon->cert_callback)
+ return -1; /* Cannot happen: the wrapper is installed only
+ when the application set the callback */
+
+ /* GnuTLS promises nothing about the initial content of these, so clear
+ them: a callback that reports success without setting them must be
+ caught below rather than read as garbage. */
+ *pcert = NULL;
+ *pcert_length = 0;
+ *pkey = NULL;
+
+ ret = daemon->cert_callback (session,
+ req_ca_dn,
+ nreqs,
+ pk_algos,
+ pk_algos_length,
+ pcert,
+ pcert_length,
+ pkey);
+ if (0 != ret)
+ return ret;
+ if (! cert_retrieve_answer_is_sane (daemon,
+ *pcert_length,
+ *pcert,
+ *pkey))
+ return -1;
+ return 0;
+}
+
+
+#endif /* GNUTLS_VERSION_MAJOR >= 3 */
+#if GNUTLS_VERSION_NUMBER >= 0x030603
+/**
+ * Wrapper around the application callback set by
+ * #MHD_OPTION_HTTPS_CERT_CALLBACK2, see cert_retrieve_answer_is_sane().
+ *
+ * Anything the callback allocated is left alone when the answer is
+ * rejected, even if it asked for #GNUTLS_CERT_RETR_DEINIT_ALL: the answer
+ * is by definition not in the shape the deinitialisation expects, so
+ * leaking it is the safer of the two outcomes.
+ *
+ * @param session the session to get the certificate for
+ * @param info the information about the request from GnuTLS
+ * @param[out] certs the certificate list to use
+ * @param[out] certs_length the number of entries in @a certs
+ * @param[out] ocsp the OCSP responses to staple
+ * @param[out] ocsp_length the number of entries in @a ocsp
+ * @param[out] pkey the private key matching @a certs
+ * @param[out] flags the #gnutls_certificate_flags to apply
+ * @return 0 on success, negative value on error
+ */
+static int
+cert_retrieve2_wrapper (gnutls_session_t session,
+ const struct gnutls_cert_retr_st *info,
+ gnutls_pcert_st **certs,
+ unsigned int *certs_length,
+ gnutls_ocsp_data_st **ocsp,
+ unsigned int *ocsp_length,
+ gnutls_privkey_t *pkey,
+ unsigned int *flags)
+{
+ struct MHD_Connection *connection;
+ struct MHD_Daemon *daemon;
+ int ret;
+
+ connection = gnutls_session_get_ptr (session);
+ if (NULL == connection)
+ return -1;
+ daemon = connection->daemon;
+ mhd_assert (NULL != daemon->cert_callback2);
+ if (NULL == daemon->cert_callback2)
+ return -1; /* Cannot happen: the wrapper is installed only
+ when the application set the callback */
+
+ /* See the same assignments in cert_retrieve_wrapper() */
+ *certs = NULL;
+ *certs_length = 0;
+ *ocsp = NULL;
+ *ocsp_length = 0;
+ *pkey = NULL;
+ *flags = 0;
+
+ ret = daemon->cert_callback2 (session,
+ info,
+ certs,
+ certs_length,
+ ocsp,
+ ocsp_length,
+ pkey,
+ flags);
+ if (0 != ret)
+ return ret;
+ if (! cert_retrieve_answer_is_sane (daemon,
+ *certs_length,
+ *certs,
+ *pkey))
+ return -1;
+ if ((0 != *ocsp_length) && (NULL == *ocsp))
+ {
+#ifdef HAVE_MESSAGES
+ MHD_DLOG (daemon,
+ _ ("The application certificate callback reported success " \
+ "with a non-empty OCSP response list, but the list " \
+ "pointer is NULL. Failing the TLS handshake.\n"));
+#endif /* HAVE_MESSAGES */
+ return -1;
+ }
+ return 0;
+}
+
+
+#endif /* GNUTLS_VERSION_NUMBER >= 0x030603 */
+
+
/**
* Read and setup our certificate and key.
*
@@ -501,14 +709,14 @@ MHD_init_daemon_certificate (struct MHD_Daemon *daemon)
if (NULL != daemon->cert_callback)
{
gnutls_certificate_set_retrieve_function2 (daemon->x509_cred,
- daemon->cert_callback);
+ &cert_retrieve_wrapper);
}
#endif
#if GNUTLS_VERSION_NUMBER >= 0x030603
else if (NULL != daemon->cert_callback2)
{
gnutls_certificate_set_retrieve_function3 (daemon->x509_cred,
- daemon->cert_callback2);
+ &cert_retrieve2_wrapper);
}
#endif
--
2.34.1
