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 7383a4f..a572aac 100644
--- a/include/haproxy/xprt_quic-t.h
+++ b/include/haproxy/xprt_quic-t.h
@@ -452,7 +452,7 @@
*/
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 e872a2f..ecc2a2f 100644
--- a/src/xprt_quic.c
+++ b/src/xprt_quic.c
@@ -3756,11 +3756,7 @@
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 @@
/* 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 @@
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 @@
unsigned char *pos;
const unsigned char *end;
struct quic_dgram_ctx dgram_ctx = {
- .dcid_node = NULL,
+ .qc = NULL,
+ .dcid.len = 0,
.owner = owner,
};