From 8720130cc7052ac1c5931fff8f4b49a38eab9668 Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Mon, 10 Nov 2025 14:38:45 +0100 Subject: [PATCH] MINOR: quic: do not use quic_newcid_from_hash64 on BE side quic_newcid_from_hash64 is an external callback. If defined, it serves as a CID method generation, as an alternative to the default random implementation. This mechanism was not correctly implemented on the backend side. Indeed, quic_conn member is only setted for frontend connections. The simplest solution would be to properly define it also for backend ones. However, quic_newcid_from_hash64 derivation is really only useful for the frontend side for now. Thus, this patch disables using it on the backend side in favor of the default random generator. To implement this, quic_cid_generate() is splitted in two functions, for both methods of CIDs generation. This is the responsibility of the caller to select the proper method. On backend side, only random implementation is now used. --- include/haproxy/quic_cid.h | 4 +-- src/quic_cid.c | 55 ++++++++++++++++++++++++++------------ src/quic_conn.c | 8 ++++-- src/quic_rx.c | 8 ++++-- 4 files changed, 52 insertions(+), 23 deletions(-) diff --git a/include/haproxy/quic_cid.h b/include/haproxy/quic_cid.h index d95092282..13a0669d1 100644 --- a/include/haproxy/quic_cid.h +++ b/include/haproxy/quic_cid.h @@ -19,8 +19,8 @@ extern struct quic_cid_tree *quic_cid_trees; struct quic_connection_id *quic_cid_alloc(void); -int quic_cid_generate(struct quic_connection_id *conn_id, uint64_t hash); - +int quic_cid_generate_random(struct quic_connection_id *conn_id); +int quic_cid_generate_from_hash(struct quic_connection_id *conn_id, uint64_t hash64); int quic_cid_derive_from_odcid(struct quic_connection_id *conn_id, const struct quic_cid *orig, const struct sockaddr_storage *addr); diff --git a/src/quic_cid.c b/src/quic_cid.c index 288f91467..166d9074d 100644 --- a/src/quic_cid.c +++ b/src/quic_cid.c @@ -145,30 +145,22 @@ struct quic_connection_id *quic_cid_alloc(void) } /* Generate the value of 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 key. + * value is calculated from a random generator. * * Returns 0 on success else non-zero. */ -int quic_cid_generate(struct quic_connection_id *conn_id, uint64_t hash64) +int quic_cid_generate_random(struct quic_connection_id *conn_id) { /* TODO use a better trace scope */ 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); - 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); - 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); - goto err; - } + 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); + goto err; } if (quic_stateless_reset_token_init(conn_id) != 1) { @@ -184,10 +176,39 @@ int quic_cid_generate(struct quic_connection_id *conn_id, uint64_t hash64) return 1; } +/* Generate the value of and its associated stateless token. The CID + * value is calculated via quic_newcid_from_hash64() external callback with + * as input. + * + * Returns 0 on success else non-zero. + */ +int quic_cid_generate_from_hash(struct quic_connection_id *conn_id, uint64_t hash64) +{ + /* TODO use a better trace scope */ + TRACE_ENTER(QUIC_EV_CONN_TXPKT); + + conn_id->cid.len = QUIC_HAP_CID_LEN; + 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)); + + if (quic_stateless_reset_token_init(conn_id) != 1) { + TRACE_ERROR("quic_stateless_reset_token_init() failed", QUIC_EV_CONN_TXPKT); + goto err; + } + + TRACE_LEAVE(QUIC_EV_CONN_TXPKT); + return 0; + + err: + TRACE_LEAVE(QUIC_EV_CONN_TXPKT); + return 1; +} + /* Generate the value of and its associated stateless token. The CID * value is derived from client ODCID and address. This is an - * alternative method to quic_cid_generate() which is used for the first CID of - * a server connection in response to a client INITIAL packet. + * alternative method to other CID generation methods which is used for the + * first CID of a server connection in response to a client INITIAL packet. * * The benefit of this CID value is to skip storage of ODCIDs in the global * CIDs tree. This is an optimization to reduce contention on the CIDs tree diff --git a/src/quic_conn.c b/src/quic_conn.c index 213ffd7e6..cf0d6f2ab 100644 --- a/src/quic_conn.c +++ b/src/quic_conn.c @@ -542,6 +542,7 @@ int quic_build_post_handshake_frames(struct quic_conn *qc, max = QUIC_MIN(qc->tx.params.active_connection_id_limit - 1, (uint64_t)3); while (max--) { struct quic_connection_id *conn_id; + int ret_cid; frm = qc_frm_alloc(QUIC_FT_NEW_CONNECTION_ID); if (!frm) { @@ -556,7 +557,10 @@ int quic_build_post_handshake_frames(struct quic_conn *qc, goto err; } - if (quic_cid_generate(conn_id, qc->hash64)) { + ret_cid = !qc_is_back(qc) && quic_newcid_from_hash64 ? + quic_cid_generate_from_hash(conn_id, qc->hash64) : + quic_cid_generate_random(conn_id); + if (ret_cid) { 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); @@ -1265,7 +1269,7 @@ struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4, goto err; } - if (quic_cid_generate(conn_cid, qc->hash64)) { + if (quic_cid_generate_random(conn_cid)) { TRACE_ERROR("error on CID generation", QUIC_EV_CONN_INIT, qc); pool_free(pool_head_quic_connection_id, conn_cid); goto err; diff --git a/src/quic_rx.c b/src/quic_rx.c index c1dd4d0fa..90cbe476b 100644 --- a/src/quic_rx.c +++ b/src/quic_rx.c @@ -840,7 +840,7 @@ static int qc_parse_pkt_frms(struct quic_conn *qc, struct quic_rx_packet *pkt, { struct quic_frame *frm = NULL; const unsigned char *pos, *end; - int fast_retrans = 0; + int fast_retrans = 0, ret; TRACE_ENTER(QUIC_EV_CONN_PRSHPKT, qc); /* Skip the AAD */ @@ -1061,7 +1061,11 @@ 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, qc->hash64)) { + ret = !qc_is_back(qc) && quic_newcid_from_hash64 ? + quic_cid_generate_from_hash(conn_id, qc->hash64) : + quic_cid_generate_random(conn_id); + + if (ret) { 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);