MINOR: quic: compare coalesced packets by DCID

If an UDP datagram contains multiple QUIC packets, they must all use the
same DCID. The datagram context is used partly for this.

To ensure this, a comparison was made on the dcid_node of DCID tree. As
this is a comparison based on pointer address, it can be faulty when
nodes are removed/readded on the same pointer address.

Replace this comparison by a proper comparison on the DCID data itself.
To this end, the dgram_ctx structure contains now a quic_cid member.

diff --git a/include/haproxy/xprt_quic-t.h b/include/haproxy/xprt_quic-t.h
index 7383a4f3978e10dcdaa227e957fc3a202e071e23..a572aac9b68ddca17697ff5f6072000fb9face1c 100644 (file)
--- a/include/haproxy/xprt_quic-t.h
+++ b/include/haproxy/xprt_quic-t.h
@@ -452,7 +452,7 @@ struct quic_rx_packet {
  */
 struct quic_dgram_ctx {
        struct quic_conn *qc;
-       struct ebmb_node *dcid_node;
+       struct quic_cid dcid;
        void *owner;
 };
 

diff --git a/src/xprt_quic.c b/src/xprt_quic.c
index e872a2fd5ccb8f495f9dba0001aed3366d205a27..ecc2a2f7d94413693848f37acde4ae2858838492 100644 (file)
--- a/src/xprt_quic.c
+++ b/src/xprt_quic.c
@@ -3756,11 +3756,7 @@ static ssize_t qc_srv_pkt_rcv(unsigned char **buf, const unsigned char *end,
                qc = ebmb_entry(node, struct quic_conn, scid_node);
                *buf += QUIC_HAP_CID_LEN;
        }
-       /* Store the DCID used for this packet to check the packet which
-        * come in this UDP datagram match with it.
-        */
-       if (!dgram_ctx->dcid_node)
-               dgram_ctx->dcid_node = node;
+
        /* Only packets packets with long headers and not RETRY or VERSION as type
         * have a length field.
         */
@@ -3779,11 +3775,21 @@ static ssize_t qc_srv_pkt_rcv(unsigned char **buf, const unsigned char *end,
 
        /* Increase the total length of this packet by the header length. */
        pkt->len += *buf - beg;
-       /* Do not check the DCID node before the length. */
-       if (dgram_ctx->dcid_node != node) {
+
+       /* When multiple QUIC packets are coalesced on the same UDP datagram,
+        * they must have the same DCID.
+        *
+        * This check must be done after the final update to pkt.len to
+        * properly drop the packet on failure.
+        */
+       if (!dgram_ctx->dcid.len) {
+               memcpy(dgram_ctx->dcid.data, pkt->dcid.data, pkt->dcid.len);
+       }
+       else if (memcmp(dgram_ctx->dcid.data, pkt->dcid.data, pkt->dcid.len)) {
                TRACE_PROTO("Packet dropped", QUIC_EV_CONN_SPKT, qc->conn);
                goto err;
        }
+       dgram_ctx->qc = qc;
 
        HA_RWLOCK_WRLOCK(QUIC_LOCK, &qc->rx.buf_rwlock);
        b_cspace = b_contig_space(&qc->rx.buf);
@@ -4125,29 +4131,36 @@ static ssize_t qc_lstnr_pkt_rcv(unsigned char **buf, const unsigned char *end,
                qc = cid->qc;
                HA_RWLOCK_RDUNLOCK(QUIC_LOCK, &l->rx.cids_lock);
 
+               memcpy(pkt->dcid.data, *buf, QUIC_HAP_CID_LEN);
+               pkt->dcid.len = QUIC_HAP_CID_LEN;
+               *buf += QUIC_HAP_CID_LEN;
+
                if (HA_ATOMIC_LOAD(&qc->conn))
                        conn_ctx = HA_ATOMIC_LOAD(&qc->conn->xprt_ctx);
-               *buf += QUIC_HAP_CID_LEN;
+
                pkt->qc = qc;
                /* A short packet is the last one of an UDP datagram. */
                pkt->len = end - *buf;
        }
 
-       /* Store the DCID used for this packet to check the packet which
-        * come in this UDP datagram match with it.
-        */
-       if (!dgram_ctx->dcid_node) {
-               dgram_ctx->dcid_node = node;
-               dgram_ctx->qc = qc;
-       }
-
        /* Increase the total length of this packet by the header length. */
        pkt->raw_len = pkt->len += *buf - beg;
-       /* Do not check the DCID node before the length. */
-       if (dgram_ctx->dcid_node != node) {
+
+       /* When multiple QUIC packets are coalesced on the same UDP datagram,
+        * they must have the same DCID.
+        *
+        * This check must be done after the final update to pkt.len to
+        * properly drop the packet on failure.
+        */
+       if (!dgram_ctx->dcid.len) {
+               memcpy(dgram_ctx->dcid.data, pkt->dcid.data, pkt->dcid.len);
+       }
+       else if (memcmp(dgram_ctx->dcid.data, pkt->dcid.data, pkt->dcid.len)) {
                TRACE_PROTO("Packet dropped", QUIC_EV_CONN_LPKT, qc->conn);
                goto err;
        }
+       dgram_ctx->qc = qc;
+
 
        if (HA_ATOMIC_LOAD(&qc->err_code)) {
                TRACE_PROTO("Connection error", QUIC_EV_CONN_LPKT, qc->conn);
@@ -5179,7 +5192,8 @@ static ssize_t quic_dgram_read(struct buffer *buf, size_t len, void *owner,
        unsigned char *pos;
        const unsigned char *end;
        struct quic_dgram_ctx dgram_ctx = {
-               .dcid_node = NULL,
+               .qc = NULL,
+               .dcid.len = 0,
                .owner = owner,
        };