[WIP]fix issue #581: reject coalesced packets with mismatched DCID per RFC 9000 12.2

include/xquic/xqc_errno.h

@@ -155,6 +155,8 @@ typedef enum {
 
     XQC_EACK_EXT_ABN_VAL                = 690,      /**< ACK Extension - abnormal value */
 
+    XQC_ECOALESCED_DCID_MISMATCH        = 691,      /**< RFC 9000 §12.2: coalesced packet DCID differs from first packet in datagram, drop just this packet */
+
     XQC_E_MAX,
 } xqc_transport_error_t;
 

src/transport/xqc_conn.c

@@ -5012,7 +5012,7 @@ xqc_conn_log_recvd_packet(xqc_connection_t *c, xqc_packet_in_t *pi,
 
 xqc_int_t
 xqc_conn_process_packet(xqc_connection_t *c,
-    const unsigned char *packet_in_buf, size_t packet_in_size, 
+    const unsigned char *packet_in_buf, size_t packet_in_size,
     xqc_usec_t recv_time)
 {
     xqc_int_t ret = XQC_OK;
@@ -5022,6 +5022,23 @@ xqc_conn_process_packet(xqc_connection_t *c,
     xqc_packet_in_t packet;
     unsigned char decrypt_payload[XQC_MAX_PACKET_IN_LEN];
 
+    /*
+     * RFC 9000 Section 12.2: every coalesced packet that follows the first
+     * one in a UDP datagram is required to carry the same Destination
+     * Connection ID as the first packet; receivers MUST discard any
+     * mismatching subsequent packet (without delivering its frames to the
+     * connection state machine) but continue to process the rest of the
+     * datagram. The anchor is set inside xqc_packet_process_single_anchored
+     * after parsing succeeds and before xqc_packet_decrypt_single performs
+     * AEAD and frame application, so mismatched packets cost neither cycles
+     * nor side effects.
+     *
+     * first_dcid only needs to be valid once first_dcid_seen flips to TRUE,
+     * so leaving the rest of the struct uninitialized is intentional.
+     */
+    xqc_cid_t first_dcid;
+    xqc_bool_t first_dcid_seen = XQC_FALSE;
+
     /* process all QUIC packets in UDP datagram */
     while (pos < end) {
         last_pos = pos;
@@ -5034,13 +5051,26 @@ xqc_conn_process_packet(xqc_connection_t *c,
         packet_in->pi_path_id = XQC_UNKNOWN_PATH_ID;
 
         /* packet_in->pos will update inside */
-        ret = xqc_packet_process_single(c, packet_in);
+        ret = xqc_packet_process_single_anchored(c, packet_in,
+                                                 &first_dcid, &first_dcid_seen);
 
         xqc_conn_log_recvd_packet(c, packet_in, packet_in_size, ret, recv_time);
 
         if (ret == XQC_OK) {
             ret = xqc_conn_on_pkt_processed(c, packet_in, recv_time);
 
+        } else if (ret == -XQC_ECOALESCED_DCID_MISMATCH) {
+            /*
+             * §12.2 drop: skip on_pkt_processed for this packet (no PN
+             * space update, no ACK arming, no idle timer refresh) but
+             * keep walking the datagram. packet_in->last already points
+             * to the end of this coalesced packet because parsing
+             * succeeded before the DCID check fired.
+             */
+            pos = packet_in->last;
+            ret = XQC_OK;
+            continue;
+
         } else if (xqc_conn_tolerant_error(ret)) {
             /* ignore the remain bytes */
             xqc_log(c->log, XQC_LOG_INFO, "|ignore err|%d|", ret);

src/transport/xqc_packet.c

@@ -12,6 +12,7 @@
 #include "src/transport/xqc_send_ctl.h"
 #include "src/transport/xqc_recv_record.h"
 #include "src/transport/xqc_packet_parser.h"
+#include "src/transport/xqc_cid.h"
 #include "src/transport/xqc_utils.h"
 #include "src/transport/xqc_engine.h"
 #include "src/tls/xqc_tls.h"
@@ -247,6 +248,34 @@ xqc_packet_decrypt_single(xqc_connection_t *c, xqc_packet_in_t *packet_in)
 xqc_int_t
 xqc_packet_process_single(xqc_connection_t *c,
     xqc_packet_in_t *packet_in)
+{
+    return xqc_packet_process_single_anchored(c, packet_in, NULL, NULL);
+}
+
+/*
+ * RFC 9000 Section 12.2: every coalesced packet that follows the first
+ * one in a UDP datagram is required to carry the same Destination
+ * Connection ID as the first packet; receivers MUST discard any
+ * mismatching subsequent packet without processing its payload, while
+ * continuing to process the rest of the datagram.
+ *
+ * The check has to happen after xqc_packet_parse_single (so that
+ * pi_pkt.pkt_dcid and packet_in->last are populated for the current
+ * packet) but BEFORE xqc_packet_decrypt_single, because the "decrypt"
+ * step internally calls xqc_process_frames and applies STREAM/ACK/CRYPTO
+ * frames to the connection state. Once that returns, dropping the packet
+ * at the caller level cannot un-apply those frames - it would only skip
+ * xqc_conn_on_pkt_processed bookkeeping, which is not what §12.2 requires.
+ *
+ * Only packets that carry a packet number participate in anchoring and
+ * mismatch detection. VERSION_NEGOTIATION and RETRY are stand-alone by
+ * spec and must not be coalesced, so they are skipped here to avoid
+ * polluting the anchor.
+ */
+xqc_int_t
+xqc_packet_process_single_anchored(xqc_connection_t *c,
+    xqc_packet_in_t *packet_in,
+    xqc_cid_t *first_dcid, xqc_bool_t *first_dcid_seen)
 {
     xqc_int_t ret = XQC_ERROR;
 
@@ -261,6 +290,37 @@ xqc_packet_process_single(xqc_connection_t *c,
         return XQC_OK;
     }
 
+    /*
+     * Anchor on the DCID of the first parsed packet that needs decryption,
+     * and reject every subsequent packet whose DCID does not match. Doing
+     * this before xqc_packet_decrypt_single guarantees that the offending
+     * packet's frames are never delivered to the connection state machine
+     * and that no AEAD work is spent on a packet we are required to drop.
+     */
+    if (first_dcid != NULL && first_dcid_seen != NULL) {
+        if (*first_dcid_seen == XQC_FALSE) {
+            xqc_cid_set(first_dcid, packet_in->pi_pkt.pkt_dcid.cid_buf,
+                        packet_in->pi_pkt.pkt_dcid.cid_len);
+            *first_dcid_seen = XQC_TRUE;
+
+        } else if (xqc_cid_is_equal(first_dcid,
+                                    &packet_in->pi_pkt.pkt_dcid) != XQC_OK)
+        {
+            xqc_log(c->log, XQC_LOG_WARN,
+                    "|coalesced pkt dcid mismatch|first_dcid:%s|pkt_dcid:%s|pkt_type:%s|",
+                    xqc_dcid_str(c->engine, first_dcid),
+                    xqc_scid_str(c->engine, &packet_in->pi_pkt.pkt_dcid),
+                    xqc_pkt_type_2_str(packet_in->pi_pkt.pkt_type));
+
+            xqc_log_event(c->log, TRA_PACKET_DROPPED,
+                          "coalesced pkt dcid mismatch", XQC_OK,
+                          xqc_pkt_type_2_str(packet_in->pi_pkt.pkt_type),
+                          (uint64_t)0);
+            c->packet_dropped_count++;
+            return -XQC_ECOALESCED_DCID_MISMATCH;
+        }
+    }
+
     /* decrypt packet */
     ret = xqc_packet_decrypt_single(c, packet_in);
     if (ret != XQC_OK) {

src/transport/xqc_packet.h

@@ -100,5 +100,22 @@ xqc_pkt_type_t xqc_state_to_pkt_type(xqc_connection_t *conn);
  */
 xqc_int_t xqc_packet_process_single(xqc_connection_t *c, xqc_packet_in_t *packet_in);
 
+/**
+ * Process a single QUIC packet, enforcing the RFC 9000 §12.2 coalesced
+ * DCID rule before decryption. first_dcid / first_dcid_seen are owned by
+ * the datagram-level caller and persist across packets in the same UDP
+ * datagram. Passing NULL for either pointer disables the check (used by
+ * the legacy xqc_packet_process_single wrapper).
+ *
+ * Returns -XQC_ECOALESCED_DCID_MISMATCH when a subsequent coalesced
+ * packet's DCID does not match the anchor; the caller must skip the
+ * offending packet without invoking xqc_conn_on_pkt_processed but still
+ * advance pos past packet_in->last to continue with the next coalesced
+ * packet in the same UDP datagram.
+ */
+xqc_int_t xqc_packet_process_single_anchored(xqc_connection_t *c,
+    xqc_packet_in_t *packet_in,
+    xqc_cid_t *first_dcid, xqc_bool_t *first_dcid_seen);
+
 
 #endif /* _XQC_PACKET_H_INCLUDED_ */

tests/CMakeLists.txt

@@ -114,6 +114,7 @@ if(HAVE_CUNIT)
         ${UNIT_TEST_DIR}/xqc_h3_ext_test.c
         ${UNIT_TEST_DIR}/xqc_ack_with_timestamp_test.c
         ${UNIT_TEST_DIR}/xqc_send_ctl_test.c
+        ${UNIT_TEST_DIR}/xqc_coalesced_test.c
     )
 
     if(XQC_ENABLE_FEC)

tests/unittest/main.c

@@ -42,6 +42,7 @@
 #include "xqc_fec_test.h"
 #include "xqc_ack_with_timestamp_test.h"
 #include "xqc_send_ctl_test.h"
+#include "xqc_coalesced_test.h"
 
 static int xqc_init_suite(void) { return 0; }
 static int xqc_clean_suite(void) { return 0; }
@@ -126,7 +127,13 @@ main()
                         xqc_test_pto_uses_remote_max_ack_delay)
         || !CU_add_test(pSuite, "xqc_test_pto_remote_default_when_unset",
                         xqc_test_pto_remote_default_when_unset)
-        /* ADD TESTS HERE */) 
+        || !CU_add_test(pSuite, "xqc_test_coalesced_single_pkt", xqc_test_coalesced_single_pkt)
+        || !CU_add_test(pSuite, "xqc_test_coalesced_dcid_match", xqc_test_coalesced_dcid_match)
+        || !CU_add_test(pSuite, "xqc_test_coalesced_dcid_mismatch", xqc_test_coalesced_dcid_mismatch)
+        || !CU_add_test(pSuite, "xqc_test_coalesced_dcid_len_mismatch", xqc_test_coalesced_dcid_len_mismatch)
+        || !CU_add_test(pSuite, "xqc_test_coalesced_dcid_a_b_a", xqc_test_coalesced_dcid_a_b_a)
+        || !CU_add_test(pSuite, "xqc_test_coalesced_mismatch_frames_not_applied", xqc_test_coalesced_mismatch_frames_not_applied)
+        /* ADD TESTS HERE */)
     {
         CU_cleanup_registry();
         return (int)CU_get_error();