BUG/MEDIUM: quic: listener connection stuck during handshakes (OpenSSL 3.5)

This issue was reported in GH #3071 by @famfo where a wireshark capture
reveals that some handshake could not complete after having received
two Initial packets. This could happen when the packets were parsed
in two times, calling qc_ssl_provide_all_quic_data() two times.

This is due to crypto data stream counter which was incremented two times
from qc_ssl_provide_all_quic_data() (see cstream->rx.offset += data
statement around line 1223 in quic_ssl.c). One time by the callback
which "receives" the crypto data, and on time by qc_ssl_provide_all_quic_data().

Then when parsing the second crypto data frame, the parser detected
that the crypto were already provided.

To fix this, one could comment the code which increment the crypto data
stream counter by <data>. That said, when using the OpenSSL 3.5 QUIC API
one should not modified the crypto data stream outside of the OpenSSL 3.5
QUIC API.

So, this patch stop calling qc_ssl_provide_all_quic_data() and
qc_ssl_provide_quic_data() and only calls qc_ssl_do_hanshake() after
having received some crypto data. In addition to this, as these functions
are no more called when building haproxy against OpenSSL 3.5, this patch
disable their compilations (with #ifndef HAVE_OPENSSL_QUIC).

This patch depends on this previous one:

     MINOR: quic: implement qc_ssl_do_hanshake()

Thank you to @famto for this report.

Must be backported to 3.2.

diff --git a/include/haproxy/quic_ssl.h b/include/haproxy/quic_ssl.h
index d19020213e6e1fa8a666f221cfec763b41ca6137..d8f25fb65e9cb1f9d5301398a7d8d4d99853ed4f 100644 (file)
--- a/include/haproxy/quic_ssl.h
+++ b/include/haproxy/quic_ssl.h
@@ -37,6 +37,7 @@ int ssl_quic_initial_ctx(struct bind_conf *bind_conf);
 SSL_CTX *ssl_quic_srv_new_ssl_ctx(void);
 int qc_alloc_ssl_sock_ctx(struct quic_conn *qc, struct connection *conn);
 int qc_ssl_provide_all_quic_data(struct quic_conn *qc, struct ssl_sock_ctx *ctx);
+int qc_ssl_do_hanshake(struct quic_conn *qc, struct ssl_sock_ctx *ctx);
 
 static inline void qc_free_ssl_sock_ctx(struct ssl_sock_ctx **ctx)
 {

diff --git a/src/quic_conn.c b/src/quic_conn.c
index 0e377bc5c81edde88184714fd134c887bdfff74c..2358073a7bd7c04f2a5f2ace870ba6c59f93260f 100644 (file)
--- a/src/quic_conn.c
+++ b/src/quic_conn.c
@@ -785,7 +785,11 @@ struct task *quic_conn_io_cb(struct task *t, void *context, unsigned int state)
 
        /* TASK_HEAVY is set when received CRYPTO data have to be handled. */
        if (HA_ATOMIC_LOAD(&tl->state) & TASK_HEAVY) {
+#ifdef HAVE_OPENSSL_QUIC
+               qc_ssl_do_hanshake(qc, qc->xprt_ctx);
+#else
                qc_ssl_provide_all_quic_data(qc, qc->xprt_ctx);
+#endif
                HA_ATOMIC_AND(&tl->state, ~TASK_HEAVY);
        }
 

diff --git a/src/quic_ssl.c b/src/quic_ssl.c
index e603b6dad30a5461b2004dc570ba187f4c266515..b34cbf2161fd11720add3c4327949d98857d434e 100644 (file)
--- a/src/quic_ssl.c
+++ b/src/quic_ssl.c
@@ -850,7 +850,7 @@ static forceinline void qc_ssl_dump_errors(struct connection *conn)
  * connection for servers or start the mux for clients.
  * Return 1 if succeeded, 0 if not.
  */
-static int qc_ssl_do_hanshake(struct quic_conn *qc, struct ssl_sock_ctx *ctx)
+int qc_ssl_do_hanshake(struct quic_conn *qc, struct ssl_sock_ctx *ctx)
 {
        int ret, ssl_err, state;
 
@@ -1041,6 +1041,7 @@ static int qc_ssl_do_hanshake(struct quic_conn *qc, struct ssl_sock_ctx *ctx)
        goto leave;
 }
 
+#ifndef HAVE_OPENSSL_QUIC
 /* Provide CRYPTO data to the TLS stack found at <data> with <len> as length
  * from <qel> encryption level with <ctx> as QUIC connection context.
  * Remaining parameter are there for debugging purposes.
@@ -1061,13 +1062,11 @@ static int qc_ssl_provide_quic_data(struct ncbuf *ncbuf,
 
        TRACE_ENTER(QUIC_EV_CONN_SSLDATA, qc);
 
-#ifndef HAVE_OPENSSL_QUIC
        if (SSL_provide_quic_data(ctx->ssl, level, data, len) != 1) {
                TRACE_ERROR("SSL_provide_quic_data() error",
                            QUIC_EV_CONN_SSLDATA, qc, NULL, NULL, ctx->ssl);
                goto leave;
        }
-#endif
 
        if (!qc_ssl_do_hanshake(qc, ctx))
                goto leave;
@@ -1141,6 +1140,7 @@ int qc_ssl_provide_all_quic_data(struct quic_conn *qc, struct ssl_sock_ctx *ctx)
        TRACE_LEAVE(QUIC_EV_CONN_PHPKTS, qc);
        return ret;
 }
+#endif
 
 /* Simple helper to set the specific OpenSSL/quictls QUIC API callbacks */
 static int quic_ssl_set_tls_cbs(SSL *ssl)