BUG/MEDIUM: ssl: fix msg callbacks on QUIC connections

With QUIC backend implementation, SSL code has been adjusted in several
place when accessing connection instance. Indeed, with QUIC usage, SSL
context is tied up to quic_conn, and code may be executed prior/after
connection instantiation. For example, on frontend side, connection is
only created after QUIC handshake completion.

The following patch tried to fix unsafe accesses to connection. In
particular, msg callbacks are not called anymore if connection is NULL.

  fab7da0fd0a67a9e7286839efca95ecdbf3a059e
  BUG/MEDIUM: quic-be/ssl_sock: TLS callback called without connection

However, most msg callbacks do not need to use the connection instance.
The only occurence where it is accessed is for heartbeat message
parsing, which is the only case of crash solved. The above fix is too
restrictive as it completely prevents execution of these callbacks when
connection is unset. This breaks several features with QUIC, such as SSL
key logging or samples based on ClientHello capture.

The current patch reverts the above one. Thus, this restores invokation
of msg callbacks for QUIC during the whole low-level connection
lifetime. This requires a small adjustment in heartbeat parsing callback
to prevent access on a NULL connection.

The issue on ClientHello capture was mentionned in github issue #2495.

This must be backported up to 3.3.

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 27218e4a86544e3a76a1680ccb4eed8cef812310..413f07a5ec3092b200bfe44a25b2eb5271405d0f 100644 (file)
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -1806,11 +1806,15 @@ static void ssl_sock_parse_heartbeat(struct connection *conn, int write_p, int v
        /* test heartbeat received (write_p is set to 0
           for a received record) */
        if ((content_type == TLS1_RT_HEARTBEAT) && (write_p == 0)) {
-               struct ssl_sock_ctx *ctx = __conn_get_ssl_sock_ctx(conn);
+               struct ssl_sock_ctx *ctx = NULL;
                const unsigned char *p = buf;
                unsigned int payload;
 
-               ctx->xprt_st |= SSL_SOCK_RECV_HEARTBEAT;
+               /* <conn> may be NULL in QUIC context */
+               if (conn) {
+                       ctx = __conn_get_ssl_sock_ctx(conn);
+                       ctx->xprt_st |= SSL_SOCK_RECV_HEARTBEAT;
+               }
 
                /* Check if this is a CVE-2014-0160 exploitation attempt. */
                if (*p != TLS1_HB_REQUEST)
@@ -2161,12 +2165,6 @@ static __maybe_unused void ssl_sock_msgcbk(int write_p, int version, int content
        struct connection *conn = ssl_sock_get_conn(ssl, NULL);
        struct ssl_sock_msg_callback *cbk;
 
-       /* The connection be NULL only for QUIC which does not free its SSL object
-        * as this done for TCP.
-        */
-       if (!conn)
-               return;
-
        /* Try to call all callback functions that were registered by using
         * ssl_sock_register_msg_callback().
         */