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, <hash64> 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.

diff --git a/include/haproxy/quic_cid.h b/include/haproxy/quic_cid.h
index d95092282b07c5d39f0e98d810ea5170f78c57da..13a0669d1d4b7aba618a772f804eb157dd8b33bb 100644 (file)
--- 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 288f914673dd2e81473c247147a05bf45595f7dd..166d9074d06dfa574621868f4ccc2625b03d3e42 100644 (file)
--- a/src/quic_cid.c
+++ b/src/quic_cid.c
@@ -145,32 +145,53 @@ struct quic_connection_id *quic_cid_alloc(void)
 }
 
 /* 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.
+ * 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));
+       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;
        }
-       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;
-               }
+
+       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 <conn_id> and its associated stateless token. The CID
+ * value is calculated via quic_newcid_from_hash64() external callback with
+ * <hash64> 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;
@@ -186,8 +207,8 @@ int quic_cid_generate(struct quic_connection_id *conn_id, uint64_t hash64)
 
 /* Generate the value of <conn_id> and its associated stateless token. The CID
  * value is derived from client <orig> ODCID and <addr> 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 213ffd7e63f83f37a28d8fee17e4e68c9d9eaa6c..cf0d6f2ab5eb0c6dec57fb0df431d2ec2ea214bc 100644 (file)
--- 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 c1dd4d0faf4ab7e8d746b108c2514cc1032e645c..90cbe476b4acfdf4a4b7be9745184a5589168065 100644 (file)
--- 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);