From 58b6b580199463c8dbd27db738dcde6772020e71 Mon Sep 17 00:00:00 2001 From: Frederic Lecaille Date: Mon, 2 Sep 2024 09:36:19 +0200 Subject: [PATCH] BUG/MINOR: quic: Crash from trace dumping SSL eary data status (AWS-LC) This bug follows this patch: MINOR: quic: Add trace for QUIC_EV_CONN_IO_CB event. where a new third variable was added to be dumped from QUIC_EV_CONN_IO_CB trace event. The quic_trace() code did not reveal there was already another variable passed as third argument but not dumped. This leaded to crash when dereferencing a point to an int in place of a point to an SSL object. This issue was reproduced only by handshakecorruption aws-lc interop test with s2n-quic as client. Note that this patch must be backported with this one: BUG/MEDIUM: quic: always validate sender address on 0-RTT which depends on the commit mentionned above. (cherry picked from commit db13df3d6e64e9993b90f048734538491082cbed) Signed-off-by: Frederic Lecaille --- src/quic_ssl.c | 6 +++--- src/quic_trace.c | 5 ++++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/quic_ssl.c b/src/quic_ssl.c index af25206e5..f8cc059da 100644 --- a/src/quic_ssl.c +++ b/src/quic_ssl.c @@ -353,7 +353,7 @@ static int ha_quic_add_handshake_data(SSL *ssl, enum ssl_encryption_level_t leve TRACE_ENTER(QUIC_EV_CONN_ADDDATA, qc); - TRACE_PROTO("ha_quic_add_handshake_data() called", QUIC_EV_CONN_IO_CB, qc, NULL, ssl); + TRACE_PROTO("ha_quic_add_handshake_data() called", QUIC_EV_CONN_IO_CB, qc, NULL, NULL, ssl); #ifdef HAVE_SSL_0RTT_QUIC /* Detect asap if some 0-RTT data were accepted for this connection. @@ -547,10 +547,10 @@ static int qc_ssl_provide_quic_data(struct ncbuf *ncbuf, state = qc->state; if (state < QUIC_HS_ST_COMPLETE) { ssl_err = SSL_do_handshake(ctx->ssl); - TRACE_PROTO("SSL_do_handshake() called", QUIC_EV_CONN_IO_CB, qc, NULL, ctx->ssl); + TRACE_PROTO("SSL_do_handshake() called", QUIC_EV_CONN_IO_CB, qc, NULL, NULL, ctx->ssl); if (qc->flags & QUIC_FL_CONN_TO_KILL) { - TRACE_DEVEL("connection to be killed", QUIC_EV_CONN_IO_CB, qc, &state, ctx->ssl); + TRACE_DEVEL("connection to be killed", QUIC_EV_CONN_IO_CB, qc, &state, NULL, ctx->ssl); goto leave; } diff --git a/src/quic_trace.c b/src/quic_trace.c index fdea49580..d0eb3b1e5 100644 --- a/src/quic_trace.c +++ b/src/quic_trace.c @@ -259,10 +259,13 @@ static void quic_trace(enum trace_level level, uint64_t mask, const struct trace if (mask & QUIC_EV_CONN_IO_CB) { const enum quic_handshake_state *state = a2; - const SSL *ssl = a3; + const int *ssl_err = a3; + const SSL *ssl = a4; if (state) chunk_appendf(&trace_buf, " state=%s", quic_hdshk_state_str(*state)); + if (ssl_err) + chunk_appendf(&trace_buf, " ssl_err=%d", *ssl_err); if (ssl) chunk_appendf(&trace_buf, " early_data_status=%s", quic_ssl_early_data_status_str(ssl));