BUG/MEDIUM: ssl-gencert: fix a NULL dereference with QUIC listeners

ssl_sock_generate_certificate_from_conn() retrieves the connection from
the SSL object through ssl_app_data_index. A QUIC SSL object only
carries the quic_conn, stored under ssl_qc_app_data_index, so the
connection is always NULL there and conn_get_dst() dereferences a NULL
pointer.

This function is called from ssl_sock_switchctx_cbk() when the
ClientHello has no SNI and 'generate-certificates' is set on the bind
line. Any client that omits the server_name extension therefore kills
the worker (SIGSEGV), with no skill required: connecting to an IP
address is enough, since RFC 6066 forbids IP literals in SNI.

The bug is not specific to the OpenSSL compatibility layer. It
reproduces with USE_QUIC=1 whether or not USE_QUIC_OPENSSL_COMPAT is
set: the callback is registered the same way (HAVE_SSL_CLIENT_HELLO_CB)
and the ex_data assignment in qc_ssl_sess_init() is identical. It was
reproduced on a quictls build (USE_QUIC=1 alone, no 'limited-quic'
needed) and on an OpenSSL build with USE_QUIC_OPENSSL_COMPAT=1, both
crashing in ssl_sock_generate_certificate_from_conn().

Use ssl_sock_get_conn(), which handles both the connection and the
quic_conn, and fall back on the quic_conn's local address, the QUIC
counterpart of conn->dst, when there is no connection. When no address
is available, return 0 so that the caller keeps its default certificate
selection instead of crashing.

This must be backported to all stable versions.

Reported-by: Martin Mirchev <mirchevmartin2203@gmail.com>

diff --git a/src/ssl_gencert.c b/src/ssl_gencert.c
index 4bff2d2d84348ce01c43a35b91b77f20736c50c0..39e6fc7f7c6c1586f5613cae843d80ad0c08aa69 100644 (file)
--- a/src/ssl_gencert.c
+++ b/src/ssl_gencert.c
@@ -424,13 +424,26 @@ error:
 int ssl_sock_generate_certificate_from_conn(struct bind_conf *bind_conf, SSL *ssl)
 {
        uint64_t key;
-       struct connection *conn = SSL_get_ex_data(ssl, ssl_app_data_index);
+       struct connection *conn = ssl_sock_get_conn(ssl, NULL);
+#ifdef USE_QUIC
+       struct quic_conn *qc = SSL_get_ex_data(ssl, ssl_qc_app_data_index);
+#endif
+       struct sockaddr_storage *dst = NULL;
+
+       if (conn && conn_get_dst(conn))
+               dst = conn->dst;
+#ifdef USE_QUIC
+       else if (!conn && qc)
+               dst = &qc->local_addr;
+#endif
+
+       if (!dst)
+               return 0;
+
+       key = ssl_sock_generated_cert_key(dst, get_addr_len(dst));
+       if (ssl_sock_assign_generated_cert(key, bind_conf, ssl))
+               return 1;
 
-       if (conn_get_dst(conn)) {
-               key = ssl_sock_generated_cert_key(conn->dst, get_addr_len(conn->dst));
-               if (ssl_sock_assign_generated_cert(key, bind_conf, ssl))
-                       return 1;
-       }
        return 0;
 }