Skip to content

Commit 330dd1b

Browse files
committed
[~] fix issue #581: reject coalesced packets with mismatched DCID per RFC 9000 12.2
The receive path in xqc_conn_process_packet walks every QUIC packet inside a single UDP datagram but never compares the Destination Connection ID of the second-and-later packets against the first one. RFC 9000 Section 12.2 requires receivers to ignore those mismatching follow-up packets while still processing the rest of the datagram; without that check a peer that holds more than one valid SCID for the connection can sneak frames in under a different CID once handshake is past, and during handshake the long header path skips the connection-level DCID set entirely for INIT and 0-RTT. Anchor the datagram on the DCID of its first successfully parsed packet and drop any subsequent packet whose pi_pkt.pkt_dcid does not match, emitting a WARN log plus a TRA_PACKET_DROPPED qlog event and bumping packet_dropped_count. The rest of the datagram keeps flowing. Coverage: tests/unittest/xqc_coalesced_test.c adds five CUnit cases that build coalesced Initials with controlled DCIDs and assert single-packet, match, mismatch, length-mismatch, and A+B+A patterns. Fixes: #581
1 parent 0e70499 commit 330dd1b

5 files changed

Lines changed: 550 additions & 2 deletions

File tree

src/transport/xqc_conn.c

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5012,7 +5012,7 @@ xqc_conn_log_recvd_packet(xqc_connection_t *c, xqc_packet_in_t *pi,
50125012

50135013
xqc_int_t
50145014
xqc_conn_process_packet(xqc_connection_t *c,
5015-
const unsigned char *packet_in_buf, size_t packet_in_size,
5015+
const unsigned char *packet_in_buf, size_t packet_in_size,
50165016
xqc_usec_t recv_time)
50175017
{
50185018
xqc_int_t ret = XQC_OK;
@@ -5022,6 +5022,17 @@ xqc_conn_process_packet(xqc_connection_t *c,
50225022
xqc_packet_in_t packet;
50235023
unsigned char decrypt_payload[XQC_MAX_PACKET_IN_LEN];
50245024

5025+
/*
5026+
* RFC 9000 Section 12.2: every coalesced packet that follows the first
5027+
* one in a UDP datagram is required to carry the same Destination
5028+
* Connection ID as the first packet; receivers SHOULD discard any
5029+
* mismatching subsequent packet but continue to process the rest.
5030+
* first_dcid only needs to be valid once first_dcid_seen flips to TRUE,
5031+
* so leaving the rest of the struct uninitialized is intentional.
5032+
*/
5033+
xqc_cid_t first_dcid;
5034+
xqc_bool_t first_dcid_seen = XQC_FALSE;
5035+
50255036
/* process all QUIC packets in UDP datagram */
50265037
while (pos < end) {
50275038
last_pos = pos;
@@ -5039,6 +5050,44 @@ xqc_conn_process_packet(xqc_connection_t *c,
50395050
xqc_conn_log_recvd_packet(c, packet_in, packet_in_size, ret, recv_time);
50405051

50415052
if (ret == XQC_OK) {
5053+
/*
5054+
* Anchor the datagram on the DCID of its first successfully
5055+
* parsed packet. Subsequent packets must match, otherwise the
5056+
* peer is either confused or attempting to smuggle traffic
5057+
* onto another connection's path through coalescing.
5058+
*/
5059+
if (first_dcid_seen == XQC_FALSE) {
5060+
xqc_cid_set(&first_dcid, packet_in->pi_pkt.pkt_dcid.cid_buf,
5061+
packet_in->pi_pkt.pkt_dcid.cid_len);
5062+
first_dcid_seen = XQC_TRUE;
5063+
5064+
} else if (xqc_cid_is_equal(&first_dcid,
5065+
&packet_in->pi_pkt.pkt_dcid) != XQC_OK)
5066+
{
5067+
/*
5068+
* Drop only the offending packet, not the whole datagram;
5069+
* xqc_dcid_str/xqc_scid_str use distinct engine-side
5070+
* scratch buffers, so it is safe to print both CIDs in one
5071+
* log statement.
5072+
*/
5073+
xqc_log(c->log, XQC_LOG_WARN,
5074+
"|coalesced pkt dcid mismatch|first_dcid:%s|pkt_dcid:%s|pkt_type:%s|pkt_num:%ui|",
5075+
xqc_dcid_str(c->engine, &first_dcid),
5076+
xqc_scid_str(c->engine, &packet_in->pi_pkt.pkt_dcid),
5077+
xqc_pkt_type_2_str(packet_in->pi_pkt.pkt_type),
5078+
packet_in->pi_pkt.pkt_num);
5079+
5080+
c->packet_dropped_count++;
5081+
xqc_log_event(c->log, TRA_PACKET_DROPPED,
5082+
"coalesced pkt dcid mismatch", XQC_OK,
5083+
xqc_pkt_type_2_str(packet_in->pi_pkt.pkt_type),
5084+
packet_in->pi_pkt.pkt_num);
5085+
5086+
pos = packet_in->last;
5087+
xqc_log_event(c->log, TRA_PACKET_RECEIVED, packet_in);
5088+
continue;
5089+
}
5090+
50425091
ret = xqc_conn_on_pkt_processed(c, packet_in, recv_time);
50435092

50445093
} else if (xqc_conn_tolerant_error(ret)) {

tests/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ if(HAVE_CUNIT)
114114
${UNIT_TEST_DIR}/xqc_h3_ext_test.c
115115
${UNIT_TEST_DIR}/xqc_ack_with_timestamp_test.c
116116
${UNIT_TEST_DIR}/xqc_send_ctl_test.c
117+
${UNIT_TEST_DIR}/xqc_coalesced_test.c
117118
)
118119

119120
if(XQC_ENABLE_FEC)

tests/unittest/main.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
#include "xqc_fec_test.h"
4343
#include "xqc_ack_with_timestamp_test.h"
4444
#include "xqc_send_ctl_test.h"
45+
#include "xqc_coalesced_test.h"
4546

4647
static int xqc_init_suite(void) { return 0; }
4748
static int xqc_clean_suite(void) { return 0; }
@@ -126,7 +127,12 @@ main()
126127
xqc_test_pto_uses_remote_max_ack_delay)
127128
|| !CU_add_test(pSuite, "xqc_test_pto_remote_default_when_unset",
128129
xqc_test_pto_remote_default_when_unset)
129-
/* ADD TESTS HERE */)
130+
|| !CU_add_test(pSuite, "xqc_test_coalesced_single_pkt", xqc_test_coalesced_single_pkt)
131+
|| !CU_add_test(pSuite, "xqc_test_coalesced_dcid_match", xqc_test_coalesced_dcid_match)
132+
|| !CU_add_test(pSuite, "xqc_test_coalesced_dcid_mismatch", xqc_test_coalesced_dcid_mismatch)
133+
|| !CU_add_test(pSuite, "xqc_test_coalesced_dcid_len_mismatch", xqc_test_coalesced_dcid_len_mismatch)
134+
|| !CU_add_test(pSuite, "xqc_test_coalesced_dcid_a_b_a", xqc_test_coalesced_dcid_a_b_a)
135+
/* ADD TESTS HERE */)
130136
{
131137
CU_cleanup_registry();
132138
return (int)CU_get_error();

0 commit comments

Comments
 (0)