mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2025-11-14 15:31:00 +01:00
Code Issues Projects Releases Wiki Activity

MEDIUM/OPTIM: quic: alloc quic_conn after CID collision check

Browse Source 
On Initial packet parsing, a new quic_conn instance is allocated via
qc_new_conn(). Then a CID is allocated with its value derivated from
client ODCID. On CID tree insert, a collision can occur if another
thread was already parsing an Initial packet from the same client. In
this case, the connection is released and the packet will be requeued to
the other thread.

Originally, CID collision check was performed prior to quic_conn
allocation. This was changed by the commit below, as this could cause
issue on quic_conn alloc failure.

  commit 4ae29be18c5b212dd2a1a8e9fa0ee2fcb9dbb4b3
  BUG/MINOR: quic: Possible endless loop in quic_lstnr_dghdlr()

However, this procedure is less optimal. Indeed, qc_new_conn() performs
many steps, thus it could be better to skip it on Initial CID collision,
which can happen frequently. This patch restores the older order of
operations, with CID collision check prior to quic_conn allocation.

To ensure this does not cause again the same bug, the CID is removed in
case of quic_conn alloc failure. This should prevent any loop as it
ensures that a CID found in the global tree does not point to a NULL
quic_conn, unless if CID is attach to a foreign thread. When this thread
will parse a re-enqueued packet, either the quic_conn is already
allocated or the CID has been removed, triggering a fresh CID and
quic_conn allocation procedure.
This commit is contained in:
Amaury Denoyelle 2025-11-10 11:02:45 +01:00
parent a9d11ab7f3
commit 5a8728d03a
4 changed files with 56 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
include/haproxy/quic_cid.h
View File

@ -17,15 +17,16 @@
extern struct quic_cid_tree *quic_cid_trees;
struct quic_connection_id *quic_cid_alloc(struct quic_conn *qc);
struct quic_connection_id *quic_cid_alloc(void);
int quic_cid_generate(struct quic_connection_id *conn_id);
int quic_cid_generate(struct quic_connection_id *conn_id, uint64_t hash);
int quic_cid_derive_from_odcid(struct quic_connection_id *conn_id,
const struct quic_cid *orig,
const struct sockaddr_storage *addr);
void quic_cid_register_seq_num(struct quic_connection_id *conn_id);
void quic_cid_register_seq_num(struct quic_connection_id *conn_id,
struct quic_conn *qc);
int quic_cid_insert(struct quic_connection_id *conn_id, int *new_tid);
int quic_cmp_cid_conn(const unsigned char *cid, size_t cid_len,

50
src/quic_cid.c
View File

@ -114,74 +114,73 @@ static struct quic_cid quic_derive_cid(const struct quic_cid *orig,
return cid;
}
/* Allocate a quic_connection_id object and associate it with <qc> connection.
* The CID object is not yet inserted in any tree storage.
/* Allocate a quic_connection_id object and associate it to the current thread.
* The CID object is not yet associated to a connection or inserted in any tree
* storage.
*
* Returns the CID or NULL on allocation failure.
*/
struct quic_connection_id *quic_cid_alloc(struct quic_conn *qc)
struct quic_connection_id *quic_cid_alloc(void)
{
struct quic_connection_id *conn_id;
/* TODO use a better trace scope */
TRACE_ENTER(QUIC_EV_CONN_TXPKT, qc);
TRACE_ENTER(QUIC_EV_CONN_TXPKT);
conn_id = pool_alloc(pool_head_quic_connection_id);
if (!conn_id) {
TRACE_ERROR("cid allocation failed", QUIC_EV_CONN_TXPKT, qc);
TRACE_ERROR("cid allocation failed", QUIC_EV_CONN_TXPKT);
goto err;
}
conn_id->qc = qc;
HA_ATOMIC_STORE(&conn_id->tid, tid);
conn_id->qc = NULL;
TRACE_LEAVE(QUIC_EV_CONN_TXPKT, qc);
TRACE_LEAVE(QUIC_EV_CONN_TXPKT);
return conn_id;
err:
TRACE_LEAVE(QUIC_EV_CONN_TXPKT, qc);
TRACE_LEAVE(QUIC_EV_CONN_TXPKT);
return NULL;
}
/* Generate the value of <conn_id> and its associated stateless token. The CID
* value is calculated from a random generator or via quic_newcid_from_hash64()
* external callback if defined with hash64 key from connection.
* external callback if defined with <hash64> key.
*
* Returns 0 on success else non-zero.
*/
int quic_cid_generate(struct quic_connection_id *conn_id)
int quic_cid_generate(struct quic_connection_id *conn_id, uint64_t hash64)
{
const struct quic_conn *qc = conn_id->qc;
/* TODO use a better trace scope */
TRACE_ENTER(QUIC_EV_CONN_TXPKT, qc);
TRACE_ENTER(QUIC_EV_CONN_TXPKT);
conn_id->cid.len = QUIC_HAP_CID_LEN;
if (quic_newcid_from_hash64) {
TRACE_DEVEL("calculate CID value from conn hash", QUIC_EV_CONN_TXPKT, qc);
quic_newcid_from_hash64(conn_id->cid.data, conn_id->cid.len, qc->hash64,
TRACE_DEVEL("calculate CID value from conn hash", QUIC_EV_CONN_TXPKT);
quic_newcid_from_hash64(conn_id->cid.data, conn_id->cid.len, hash64,
global.cluster_secret, sizeof(global.cluster_secret));
}
else {
TRACE_DEVEL("generate CID value from random generator", QUIC_EV_CONN_TXPKT, qc);
TRACE_DEVEL("generate CID value from random generator", QUIC_EV_CONN_TXPKT);
if (RAND_bytes(conn_id->cid.data, conn_id->cid.len) != 1) {
/* TODO: RAND_bytes() should be replaced */
TRACE_ERROR("RAND_bytes() failed", QUIC_EV_CONN_TXPKT, qc);
TRACE_ERROR("RAND_bytes() failed", QUIC_EV_CONN_TXPKT);
goto err;
}
}
if (quic_stateless_reset_token_init(conn_id) != 1) {
TRACE_ERROR("quic_stateless_reset_token_init() failed", QUIC_EV_CONN_TXPKT, qc);
TRACE_ERROR("quic_stateless_reset_token_init() failed", QUIC_EV_CONN_TXPKT);
goto err;
}
TRACE_LEAVE(QUIC_EV_CONN_TXPKT, qc);
TRACE_LEAVE(QUIC_EV_CONN_TXPKT);
return 0;
err:
TRACE_LEAVE(QUIC_EV_CONN_TXPKT, qc);
TRACE_LEAVE(QUIC_EV_CONN_TXPKT);
return 1;
}
@ -227,13 +226,13 @@ int quic_cid_derive_from_odcid(struct quic_connection_id *conn_id,
* sequence number available. The CID should already be stored in the global
* tree to ensure there is no value collision.
*/
void quic_cid_register_seq_num(struct quic_connection_id *conn_id)
void quic_cid_register_seq_num(struct quic_connection_id *conn_id,
struct quic_conn *qc)
{
struct quic_conn *qc = conn_id->qc;
/* TODO use a better trace scope */
TRACE_ENTER(QUIC_EV_CONN_TXPKT, qc);
conn_id->qc = qc;
conn_id->seq_num.key = qc->next_cid_seq_num;
conn_id->retire_prior_to = 0;
/* insert the allocated CID in the quic_conn tree */
@ -413,6 +412,11 @@ struct quic_conn *retrieve_qc_conn_from_cid(struct quic_rx_packet *pkt,
*new_tid = conn_id_tid;
goto end;
}
/* Ensures that conn is always set if CID is found on its thread.
* Necessary to prevent loop of re-enqueued Initial packets.
*/
BUG_ON(!conn_id->qc);
qc = conn_id->qc;
TRACE_DEVEL("found connection", QUIC_EV_CONN_RXPKT, qc);

12
src/quic_conn.c
View File

@ -525,14 +525,14 @@ int quic_build_post_handshake_frames(struct quic_conn *qc)
goto err;
}
conn_id = quic_cid_alloc(qc);
conn_id = quic_cid_alloc();
if (!conn_id) {
qc_frm_free(qc, &frm);
TRACE_ERROR("CID allocation error", QUIC_EV_CONN_IO_CB, qc);
goto err;
}
if (quic_cid_generate(conn_id)) {
if (quic_cid_generate(conn_id, qc->hash64)) {
qc_frm_free(qc, &frm);
pool_free(pool_head_quic_connection_id, conn_id);
TRACE_ERROR("error on CID generation", QUIC_EV_CONN_IO_CB, qc);
@ -546,7 +546,7 @@ int quic_build_post_handshake_frames(struct quic_conn *qc)
continue;
}
quic_cid_register_seq_num(conn_id);
quic_cid_register_seq_num(conn_id, qc);
quic_connection_id_to_frm_cpy(frm, conn_id);
LIST_APPEND(&frm_list, &frm->list);
}
@ -1235,13 +1235,13 @@ struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4,
memcpy(&qc->odcid, qc->dcid.data, sizeof(qc->dcid.data));
qc->odcid.len = qc->dcid.len;
conn_cid = quic_cid_alloc(qc);
conn_cid = quic_cid_alloc();
if (!conn_cid) {
TRACE_ERROR("error on CID allocation", QUIC_EV_CONN_INIT, qc);
goto err;
}
if (quic_cid_generate(conn_cid)) {
if (quic_cid_generate(conn_cid, qc->hash64)) {
TRACE_ERROR("error on CID generation", QUIC_EV_CONN_INIT, qc);
pool_free(pool_head_quic_connection_id, conn_cid);
goto err;
@ -1252,7 +1252,7 @@ struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4,
goto err;
}
quic_cid_register_seq_num(conn_cid);
quic_cid_register_seq_num(conn_cid, qc);
dcid = &qc->dcid;
conn_id = conn_cid;
}

41
src/quic_rx.c
View File

@ -1018,7 +1018,7 @@ static int qc_parse_pkt_frms(struct quic_conn *qc, struct quic_rx_packet *pkt,
pool_free(pool_head_quic_connection_id, conn_id);
TRACE_PROTO("CID retired", QUIC_EV_CONN_PSTRM, qc);
conn_id = quic_cid_alloc(qc);
conn_id = quic_cid_alloc();
if (!conn_id) {
TRACE_ERROR("CID allocation error", QUIC_EV_CONN_IO_CB, qc);
quic_set_connection_close(qc, quic_err_transport(QC_ERR_INTERNAL_ERROR));
@ -1027,7 +1027,7 @@ static int qc_parse_pkt_frms(struct quic_conn *qc, struct quic_rx_packet *pkt,
}
while (retry_rand_cid--) {
if (quic_cid_generate(conn_id)) {
if (quic_cid_generate(conn_id, qc->hash64)) {
TRACE_ERROR("error on CID generation", QUIC_EV_CONN_PSTRM, qc);
quic_set_connection_close(qc, quic_err_transport(QC_ERR_INTERNAL_ERROR));
pool_free(pool_head_quic_connection_id, conn_id);
@ -1051,7 +1051,7 @@ static int qc_parse_pkt_frms(struct quic_conn *qc, struct quic_rx_packet *pkt,
goto err;
}
quic_cid_register_seq_num(conn_id);
quic_cid_register_seq_num(conn_id, qc);
qc_build_new_connection_id_frm(qc, conn_id);
break;
@ -1762,7 +1762,7 @@ 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;
conn_id = quic_cid_alloc(NULL);
conn_id = quic_cid_alloc();
if (!conn_id) {
TRACE_ERROR("error on first CID allocation",
QUIC_EV_CONN_LPKT, NULL, NULL, NULL, pkt->version);
@ -1776,21 +1776,6 @@ static struct quic_conn *quic_rx_pkt_retrieve_conn(struct quic_rx_packet *pkt,
goto err;
}
qc = qc_new_conn(pkt->version, ipv4, &pkt->dcid, &pkt->scid, &token_odcid,
conn_id, &dgram->daddr, &pkt->saddr,
!!pkt->token_len, l);
if (qc == NULL) {
pool_free(pool_head_quic_connection_id, conn_id);
goto err;
}
conn_id->qc = qc;
/* 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,
global.cluster_secret, sizeof(global.cluster_secret));
}
if (quic_cid_insert(conn_id, new_tid)) {
/* Collision during CID insertion. This may
* happen when handling two Initial packets
@ -1800,11 +1785,23 @@ static struct quic_conn *quic_rx_pkt_retrieve_conn(struct quic_rx_packet *pkt,
*/
TRACE_STATE("duplicate CID on Initial", QUIC_EV_CONN_LPKT);
pool_free(pool_head_quic_connection_id, conn_id);
quic_conn_release(qc);
qc = NULL;
}
else {
quic_cid_register_seq_num(conn_id);
qc = qc_new_conn(pkt->version, ipv4, &pkt->dcid, &pkt->scid, &token_odcid,
conn_id, &dgram->daddr, &pkt->saddr,
!!pkt->token_len, l);
if (qc == NULL) {
quic_cid_delete(conn_id); /* Removes CID from global tree as it points to a NULL qc. */
pool_free(pool_head_quic_connection_id, conn_id);
goto err;
}
/* Compute the hash used to derive extra CIDs. */
if (quic_hash64_from_cid) {
qc->hash64 = quic_hash64_from_cid(conn_id->cid.data, conn_id->cid.len,
global.cluster_secret, sizeof(global.cluster_secret));
}
quic_cid_register_seq_num(conn_id, qc);
if (dgram->flags & QUIC_DGRAM_FL_REJECT)
quic_set_connection_close(qc, quic_err_transport(QC_ERR_CONNECTION_REFUSED));