From: Amaury Denoyelle <adenoyelle@haproxy.com>
Date: Mon, 6 Nov 2023 16:47:17 +0000 (+0100)
Subject: BUG/MEDIUM: quic: fix sslconns on quic_conn alloc failure
X-Git-Tag: v2.9-dev10~145
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=6f9b65f952c7435d6bdb61ba547c0b56a32e2c0a;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: quic: fix sslconns on quic_conn alloc failure

QUIC connections are accounted inside global sslconns. As with QUIC
actconn, it suffered from a similar issue if an intermediary allocation
failed inside qc_new_conn().

Fix this similarly by moving increment operation inside qc_new_conn().
Increment and error path are now centralized and much easier to
validate.

The consequences are similar to the actconn fix : on memory allocation
global sslconns may wrap, this time blocking any future QUIC or SSL
connections on the process.

This must be backported up to 2.6.
---

diff --git a/include/haproxy/quic_ssl.h b/include/haproxy/quic_ssl.h
index f2518ba8a0..f31cafc3d8 100644
--- a/include/haproxy/quic_ssl.h
+++ b/include/haproxy/quic_ssl.h
@@ -43,8 +43,6 @@ static inline void qc_free_ssl_sock_ctx(struct ssl_sock_ctx **ctx)
 	SSL_free((*ctx)->ssl);
 	pool_free(pool_head_quic_ssl_sock_ctx, *ctx);
 	*ctx = NULL;
-
-	_HA_ATOMIC_DEC(&global.sslconns);
 }
 
 #endif /* _HAPROXY_QUIC_SSL_H */
diff --git a/src/quic_conn.c b/src/quic_conn.c
index 7f300c9d03..f25bf4c9ff 100644
--- a/src/quic_conn.c
+++ b/src/quic_conn.c
@@ -1162,7 +1162,7 @@ struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4,
 	struct quic_conn *qc = NULL;
 	struct listener *l = NULL;
 	struct quic_cc_algo *cc_algo = NULL;
-	unsigned int next_actconn = 0;
+	unsigned int next_actconn = 0, next_sslconn = 0;
 
 	TRACE_ENTER(QUIC_EV_CONN_INIT);
 
@@ -1173,6 +1173,12 @@ struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4,
 		goto err;
 	}
 
+	next_sslconn = increment_sslconn();
+	if (!next_sslconn) {
+		TRACE_STATE("sslconn reached", QUIC_EV_CONN_INIT);
+		goto err;
+	}
+
 	qc = pool_alloc(pool_head_quic_conn);
 	if (!qc) {
 		TRACE_ERROR("Could not allocate a new connection", QUIC_EV_CONN_INIT);
@@ -1182,7 +1188,7 @@ struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4,
 	/* Now that quic_conn instance is allocated, quic_conn_release() will
 	 * ensure global accounting is decremented.
 	 */
-	next_actconn = 0;
+	next_sslconn = next_actconn = 0;
 
 	/* Initialize in priority qc members required for a safe dealloc. */
 	qc->nictx = NULL;
@@ -1382,6 +1388,8 @@ struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4,
 	 */
 	if (next_actconn)
 		_HA_ATOMIC_DEC(&actconn);
+	if (next_sslconn)
+		_HA_ATOMIC_DEC(&global.sslconns);
 
 	TRACE_LEAVE(QUIC_EV_CONN_INIT);
 	return NULL;
@@ -1519,6 +1527,7 @@ void quic_conn_release(struct quic_conn *qc)
 	 * time with limited ressources.
 	 */
 	_HA_ATOMIC_DEC(&actconn);
+	_HA_ATOMIC_DEC(&global.sslconns);
 
 	TRACE_PROTO("QUIC conn. freed", QUIC_EV_CONN_FREED, qc);
  leave:
diff --git a/src/quic_rx.c b/src/quic_rx.c
index da98403a44..c921c97513 100644
--- a/src/quic_rx.c
+++ b/src/quic_rx.c
@@ -1902,7 +1902,6 @@ static struct quic_conn *quic_rx_pkt_retrieve_conn(struct quic_rx_packet *pkt,
 	struct quic_conn *qc = NULL;
 	struct proxy *prx;
 	struct quic_counters *prx_counters;
-	unsigned int next_sslconn = 0;
 
 	TRACE_ENTER(QUIC_EV_CONN_LPKT);
 
@@ -1960,13 +1959,6 @@ static struct quic_conn *quic_rx_pkt_retrieve_conn(struct quic_rx_packet *pkt,
 			pkt->saddr = dgram->saddr;
 			ipv4 = dgram->saddr.ss_family == AF_INET;
 
-			next_sslconn = increment_sslconn();
-			if (!next_sslconn) {
-				TRACE_STATE("drop packet on sslconn reached",
-				            QUIC_EV_CONN_LPKT, NULL, NULL, NULL, pkt->version);
-				goto err;
-			}
-
 			/* Generate the first connection CID. This is derived from the client
 			 * ODCID and address. This allows to retrieve the connection from the
 			 * ODCID without storing it in the CID tree. This is an interesting
@@ -1985,7 +1977,6 @@ static struct quic_conn *quic_rx_pkt_retrieve_conn(struct quic_rx_packet *pkt,
 				goto err;
 			}
 
-			next_sslconn = 0;
 			/* Compute and store into the quic_conn the hash used to compute extra CIDs */
 			if (quic_hash64_from_cid)
 				qc->hash64 = quic_hash64_from_cid(conn_id->cid.data, conn_id->cid.len,
@@ -2036,9 +2027,6 @@ static struct quic_conn *quic_rx_pkt_retrieve_conn(struct quic_rx_packet *pkt,
 	else
 		HA_ATOMIC_INC(&prx_counters->dropped_pkt);
 
-	if (next_sslconn)
-		_HA_ATOMIC_DEC(&global.sslconns);
-
 	TRACE_LEAVE(QUIC_EV_CONN_LPKT);
 	return NULL;
 }