BUG/MEDIUM: quic-be: too early SSL_SESSION initialization

When an SNI is set on a QUIC server line, ssl_sock_set_servername() is called
from connect_server() (backend.c). This leads some BUG_ON() to be triggered
because the CO_FL_WAIT_L6_CONN | CO_FL_SSL_WAIT_HS were not set. This must
be done into the ->init() xprt callback. This patch move the flags settings
from ->start() to ->init() callback.

Indeed, connect_server() calls these functions in this order:

   ->init(),
   ssl_sock_set_servername() # => crash if CO_FL_WAIT_L6_CONN | CO_FL_SSL_WAIT_HS not set
   ->start()

Furthermore ssl_sock_set_servername() has a side effect to reset the SSL_SESSION
object (attached to SSL object) calling SSL_set_session(), leading to crashes as follows:

 [Thread debugging using libthread_db enabled]
 Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
 Core was generated by `./haproxy -f quic_srv.cfg'.
 Program terminated with signal SIGSEGV, Segmentation fault.
 #0  tls_process_server_hello (s=0x560c259733b0, pkt=0x7fffac239f20)
     at ssl/statem/statem_clnt.c:1624
 1624            if (s->session->session_id_length > 0) {
 [Current thread is 1 (Thread 0x7fc364e53dc0 (LWP 35514))]
 (gdb) bt
 #0  tls_process_server_hello (s=0x560c259733b0, pkt=0x7fffac239f20)
     at ssl/statem/statem_clnt.c:1624
 #1  0x00007fc36540fba4 in ossl_statem_client_process_message (s=0x560c259733b0,
     pkt=0x7fffac239f20) at ssl/statem/statem_clnt.c:1042
 #2  0x00007fc36540d028 in read_state_machine (s=0x560c259733b0) at ssl/statem/statem.c:646
 #3  0x00007fc36540ca70 in state_machine (s=0x560c259733b0, server=0)
     at ssl/statem/statem.c:439
 #4  0x00007fc36540c576 in ossl_statem_connect (s=0x560c259733b0) at ssl/statem/statem.c:250
 #5  0x00007fc3653f1698 in SSL_do_handshake (s=0x560c259733b0) at ssl/ssl_lib.c:3835
 #6  0x0000560c22620327 in qc_ssl_do_hanshake (qc=qc@entry=0x560c25961f60,
     ctx=ctx@entry=0x560c25963020) at src/quic_ssl.c:863
 #7  0x0000560c226210be in qc_ssl_provide_quic_data (len=90, data=<optimized out>,
     ctx=0x560c25963020, level=ssl_encryption_initial, ncbuf=0x560c2588bb18)
     at src/quic_ssl.c:1071
 #8  qc_ssl_provide_all_quic_data (qc=qc@entry=0x560c25961f60, ctx=0x560c25963020)
     at src/quic_ssl.c:1123
 #9  0x0000560c2260ca5f in quic_conn_io_cb (t=0x560c25962f80, context=0x560c25961f60,
     state=<optimized out>) at src/quic_conn.c:791
 #10 0x0000560c228255ed in run_tasks_from_lists (budgets=<optimized out>) at src/task.c:648
 #11 0x0000560c22825f7a in process_runnable_tasks () at src/task.c:889
 #12 0x0000560c22793dc7 in run_poll_loop () at src/haproxy.c:2836
 #13 0x0000560c22794481 in run_thread_poll_loop (data=<optimized out>) at src/haproxy.c:3056
 #14 0x0000560c2259082d in main (argc=<optimized out>, argv=<optimized out>)
     at src/haproxy.c:3667

<s> is the SSL object, and <s->session> is the SSL_SESSION object.

For the client, this is the first call do SSL_do_handshake() which initializes this
SSL_SESSION object from ->init() xpt callback. Then it is reset by
ssl_sock_set_servername(), then tls_process_server_hello() TLS stack is called with
NULL value for s->session when receiving the ServerHello TLS message.

To fix this, simply move the first call to SSL_do_handshake to ->start xprt call
back (qc_xprt_start()).

No need to backport.

diff --git a/src/quic_ssl.c b/src/quic_ssl.c
index 5cdb9dfc1b37baa87d98e220fc6e8f496cc65f57..a809884cefed689965feda6784c03695125b446f 100644 (file)
--- a/src/quic_ssl.c
+++ b/src/quic_ssl.c
@@ -1281,7 +1281,6 @@ int qc_alloc_ssl_sock_ctx(struct quic_conn *qc, struct connection *conn)
                SSL_set_accept_state(ctx->ssl);
        }
        else {
-               int ssl_err;
                struct server *srv = __objt_server(ctx->conn->target);
 
                if (qc_ssl_sess_init(qc, srv->ssl_ctx.ctx, &ctx->ssl, conn, 0) == -1)
@@ -1291,24 +1290,6 @@ int qc_alloc_ssl_sock_ctx(struct quic_conn *qc, struct connection *conn)
                        goto err;
 
                SSL_set_connect_state(ctx->ssl);
-               ssl_err = SSL_do_handshake(ctx->ssl);
-               TRACE_PROTO("SSL_do_handshake() called", QUIC_EV_CONN_NEW, qc, &ssl_err);
-               if (ssl_err != 1) {
-                       ssl_err = SSL_get_error(ctx->ssl, ssl_err);
-                       if (ssl_err == SSL_ERROR_WANT_READ || ssl_err == SSL_ERROR_WANT_WRITE) {
-                               TRACE_PROTO("SSL handshake in progress", QUIC_EV_CONN_NEW, qc, &ssl_err);
-                       }
-                       else {
-                               TRACE_ERROR("SSL handshake error", QUIC_EV_CONN_NEW, qc, &ssl_err);
-                               HA_ATOMIC_INC(&qc->prx_counters->hdshk_fail);
-                               qc_ssl_dump_errors(ctx->conn);
-                               ERR_clear_error();
-                               goto err;
-                       }
-               }
-
-               /* Wakeup the handshake I/O handler tasklet asap to send data */
-               tasklet_wakeup(qc->wait_event.tasklet);
        }
 
        ctx->xprt = xprt_get(XPRT_QUIC);

diff --git a/src/xprt_quic.c b/src/xprt_quic.c
index 47a8843a3c9e554eaacf44cea9c552bab0829c58..dbcbb4caf997d2b8bb60dfd3609480277c1d5de8 100644 (file)
--- a/src/xprt_quic.c
+++ b/src/xprt_quic.c
@@ -14,6 +14,7 @@
 #include <haproxy/buf.h>
 #include <haproxy/connection.h>
 #include <haproxy/quic_conn.h>
+#include <haproxy/quic_ssl.h>
 #include <haproxy/ssl_sock.h>
 #include <haproxy/quic_trace.h>
 #include <haproxy/trace.h>
@@ -124,6 +125,8 @@ static int qc_conn_init(struct connection *conn, void **xprt_ctx)
                struct server *srv = objt_server(conn->target);
                qc = qc_new_conn(quic_version_1, ipv4, NULL, NULL, NULL,
                                 NULL, NULL, &srv->addr, 0, srv, conn);
+               if (qc)
+                       conn->flags |= CO_FL_SSL_WAIT_HS | CO_FL_WAIT_L6_CONN;
        }
 
        if (!qc)
@@ -160,14 +163,18 @@ static int qc_xprt_start(struct connection *conn, void *ctx)
                qc->mux_state = QC_MUX_READY;
        }
        else {
-               conn->flags |= CO_FL_SSL_WAIT_HS | CO_FL_WAIT_L6_CONN;
+               /* This has as side effet to create a SSL_SESSION object attached to
+                * the SSL object.
+                */
+               if (!qc_ssl_do_hanshake(qc, ctx))
+                       goto out;
        }
 
        /* Schedule quic-conn to ensure post handshake frames are emitted. This
         * is not done for 0-RTT as xprt->start happens before handshake
         * completion.
         */
-       if (qc->flags & QUIC_FL_CONN_NEED_POST_HANDSHAKE_FRMS)
+       if (qc_is_back(qc) || (qc->flags & QUIC_FL_CONN_NEED_POST_HANDSHAKE_FRMS))
                tasklet_wakeup(qc->wait_event.tasklet);
 
        ret = 1;