Skip to content

Commit

Permalink
BUG/MEDIUM: quic: fix sslconns on quic_conn alloc failure
Browse files Browse the repository at this point in the history
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.

(cherry picked from commit 6f9b65f)
[cf: quic_rx_pkt_retrieve_conn() is in quic_conn.c and quic_rx does not
     exsit in 2.8]
Signed-off-by: Christopher Faulet <[email protected]>
(cherry picked from commit 62b950b)
Signed-off-by: Amaury Denoyelle <[email protected]>
  • Loading branch information
a-denoyelle committed Nov 15, 2023
1 parent 4e4f654 commit a85e5f9
Showing 1 changed file with 11 additions and 16 deletions.
27 changes: 11 additions & 16 deletions src/quic_conn.c
Original file line number Diff line number Diff line change
Expand Up @@ -5590,7 +5590,7 @@ static struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4,
struct listener *l = NULL;
struct quic_cc_algo *cc_algo = NULL;
struct quic_tls_ctx *ictx;
unsigned int next_actconn = 0;
unsigned int next_actconn = 0, next_sslconn = 0;
TRACE_ENTER(QUIC_EV_CONN_INIT);

next_actconn = increment_actconn();
Expand All @@ -5599,6 +5599,12 @@ static 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;
}

/* TODO replace pool_zalloc by pool_alloc(). This requires special care
* to properly initialized internal quic_conn members to safely use
* quic_conn_release() on alloc failure.
Expand All @@ -5612,7 +5618,7 @@ static 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. */

Expand Down Expand Up @@ -5783,6 +5789,8 @@ static 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;
Expand Down Expand Up @@ -5882,7 +5890,6 @@ void quic_conn_release(struct quic_conn *qc)
SSL_free(conn_ctx->ssl);
pool_free(pool_head_quic_conn_ctx, conn_ctx);
}
_HA_ATOMIC_DEC(&global.sslconns);

quic_tls_ku_free(qc);
for (i = 0; i < QUIC_TLS_ENC_LEVEL_MAX; i++) {
Expand Down Expand Up @@ -5913,6 +5920,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);

Expand Down Expand Up @@ -6952,7 +6960,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);

Expand Down Expand Up @@ -7010,13 +7017,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
Expand All @@ -7035,8 +7035,6 @@ static struct quic_conn *quic_rx_pkt_retrieve_conn(struct quic_rx_packet *pkt,
goto err;
}

next_sslconn = 0;

tree = &quic_cid_trees[quic_cid_tree_idx(&conn_id->cid)];
HA_RWLOCK_WRLOCK(QC_CID_LOCK, &tree->lock);
node = ebmb_insert(&tree->root, &conn_id->node, conn_id->cid.len);
Expand Down Expand Up @@ -7082,9 +7080,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;
}
Expand Down

0 comments on commit a85e5f9

Please sign in to comment.