From: Frederic Lecaille <flecaille@haproxy.com>
Date: Mon, 2 Sep 2024 07:36:19 +0000 (+0200)
Subject: BUG/MINOR: quic: Crash from trace dumping SSL eary data status (AWS-LC)
X-Git-Tag: v3.1-dev7~18
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=db13df3d6e64e9993b90f048734538491082cbed;p=thirdparty%2Fhaproxy.git

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.
---

diff --git a/src/quic_ssl.c b/src/quic_ssl.c
index 9dba4212fa..03631c0acd 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.
@@ -550,10 +550,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 26ebb6eeff..5ddef1d271 100644
--- a/src/quic_trace.c
+++ b/src/quic_trace.c
@@ -261,10 +261,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));