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.
This commit is contained in:
Amaury Denoyelle 2023-11-06 17:47:17 +01:00
parent a7ba679fe7
commit 6f9b65f952
3 changed files with 11 additions and 16 deletions

View File

@ -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 */

View File

@ -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:

View File

@ -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;
}