diff --git a/include/xquic/xqc_errno.h b/include/xquic/xqc_errno.h index 862fc9cc5..d1bc106e6 100644 --- a/include/xquic/xqc_errno.h +++ b/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; diff --git a/src/transport/xqc_conn.c b/src/transport/xqc_conn.c index 3765e961c..e4c94721e 100644 --- a/src/transport/xqc_conn.c +++ b/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); diff --git a/src/transport/xqc_packet.c b/src/transport/xqc_packet.c index 40cac595e..52b13f678 100644 --- a/src/transport/xqc_packet.c +++ b/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) { diff --git a/src/transport/xqc_packet.h b/src/transport/xqc_packet.h index 70036a326..a87e20288 100644 --- a/src/transport/xqc_packet.h +++ b/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_ */ diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 8b530b488..dae468d4a 100644 --- a/tests/CMakeLists.txt +++ b/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) diff --git a/tests/unittest/main.c b/tests/unittest/main.c index 9e4441993..a8c002c78 100644 --- a/tests/unittest/main.c +++ b/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(); diff --git a/tests/unittest/xqc_coalesced_test.c b/tests/unittest/xqc_coalesced_test.c new file mode 100644 index 000000000..634998004 --- /dev/null +++ b/tests/unittest/xqc_coalesced_test.c @@ -0,0 +1,626 @@ +/** + * @copyright Copyright (c) 2026, Alibaba Group Holding Limited + * + * Reverse-validation of the RFC 9000 Section 12.2 fix: lighting up the + * coalesced-DCID-mismatch branch in xqc_conn_process_packet, while keeping + * the happy path quiet. + * + * The trick is to encrypt each Initial under the *same* client-side Initial + * keys (which were derived from the original DCID) and only flip bytes in + * the wire-level DCID field. The AEAD AAD is the on-wire header, so as long + * as both endpoints see the same bytes the tag still validates - the only + * thing that diverges is what xqc_packet_parse_long_header records into + * pi_pkt.pkt_dcid, which is exactly what the new validation inspects. + */ + +#include +#include +#include + +#include "xqc_coalesced_test.h" + +#include "xquic/xquic.h" +#include "xquic/xquic_typedef.h" +#include "src/transport/xqc_conn.h" +#include "src/transport/xqc_engine.h" +#include "src/transport/xqc_packet.h" +#include "src/transport/xqc_packet_out.h" +#include "src/transport/xqc_packet_parser.h" +#include "src/transport/xqc_frame_parser.h" +#include "src/transport/xqc_cid.h" + + +extern xqc_usec_t xqc_now(void); + + +/* Each Initial must be at least 1200 bytes on the server's view; pad to + * 1300 to leave headroom for variable-length header fields. */ +#define COALESCED_TEST_PKT_TARGET 1300 + + +typedef struct coalesced_ctx_s { + xqc_engine_t *engine; + xqc_connection_t *c; + xqc_cid_t cid; + unsigned char buf[2048]; + size_t buf_len; +} coalesced_ctx_t; + + +static ssize_t +coalesced_write_socket(const unsigned char *buf, size_t size, + const struct sockaddr *peer_addr, socklen_t peer_addrlen, + void *conn_user_data) +{ + coalesced_ctx_t *ctx = (coalesced_ctx_t *)conn_user_data; + if (size > sizeof(ctx->buf)) { + size = sizeof(ctx->buf); + } + memcpy(ctx->buf, buf, size); + ctx->buf_len = size; + return size; +} + +static int +coalesced_conn_create_notify(xqc_connection_t *conn, const xqc_cid_t *cid, + void *user_data, void *conn_proto_data) +{ + coalesced_ctx_t *ctx = (coalesced_ctx_t *)user_data; + ctx->c = conn; + memcpy(&ctx->cid, cid, sizeof(xqc_cid_t)); + xqc_conn_set_alp_user_data(conn, ctx); + return 0; +} + +static void +coalesced_set_event_timer(xqc_msec_t wake_after, void *engine_user_data) +{ + return; +} + +static xqc_engine_t * +coalesced_create_engine(coalesced_ctx_t *ctx, xqc_engine_type_t etype) +{ + xqc_engine_ssl_config_t ssl_config; + memset(&ssl_config, 0, sizeof(ssl_config)); + ssl_config.private_key_file = "./server.key"; + ssl_config.cert_file = "./server.crt"; + ssl_config.ciphers = XQC_TLS_CIPHERS; + ssl_config.groups = XQC_TLS_GROUPS; + + xqc_engine_callback_t engine_cb = { + .set_event_timer = coalesced_set_event_timer, + }; + + xqc_transport_callbacks_t tcbs = { + .write_socket = coalesced_write_socket, + }; + + xqc_app_proto_callbacks_t alp_cbs = { + .conn_cbs.conn_create_notify = coalesced_conn_create_notify, + }; + + xqc_engine_t *engine = xqc_engine_create(etype, NULL, &ssl_config, + &engine_cb, &tcbs, ctx); + if (engine == NULL) { + return NULL; + } + xqc_engine_register_alpn(engine, "transport", 9, &alp_cbs, NULL); + return engine; +} + + +/* + * Build a single Initial packet with a caller-supplied DCID, padding-only + * payload, and a chosen packet number. Returns 0 on failure, otherwise the + * size written into out_buf (which must be at least 2048 bytes). + * + * The encryption is performed against cli_conn so the keys come from the + * client's Initial secret; the DCID we put in the header is purely a wire + * value that ends up in the AEAD AAD on both sides. + */ +static size_t +build_initial_pkt(xqc_connection_t *cli_conn, + const unsigned char *dcid_buf, uint8_t dcid_len, + xqc_packet_number_t pkt_num, + uint8_t *out_buf, size_t out_buf_cap) +{ + xqc_packet_out_t *po = xqc_packet_out_create(2048); + if (po == NULL) { + return 0; + } + + po->po_pkt.pkt_type = XQC_PTYPE_INIT; + po->po_pkt.pkt_pns = XQC_PNS_INIT; + po->po_pkt.pkt_num = pkt_num; + + /* SCID = client's user_scid; DCID supplied by caller. */ + memcpy(po->po_pkt.pkt_scid.cid_buf, + cli_conn->scid_set.user_scid.cid_buf, + cli_conn->scid_set.user_scid.cid_len); + po->po_pkt.pkt_scid.cid_len = cli_conn->scid_set.user_scid.cid_len; + + memcpy(po->po_pkt.pkt_dcid.cid_buf, dcid_buf, dcid_len); + po->po_pkt.pkt_dcid.cid_len = dcid_len; + + ssize_t hdr_len = xqc_gen_long_packet_header( + po, + po->po_pkt.pkt_dcid.cid_buf, po->po_pkt.pkt_dcid.cid_len, + po->po_pkt.pkt_scid.cid_buf, po->po_pkt.pkt_scid.cid_len, + NULL, 0, XQC_VERSION_V1, XQC_PKTNO_BITS); + if (hdr_len <= 0) { + xqc_packet_out_destroy(po); + return 0; + } + po->po_used_size += hdr_len; + + /* Pad with zero bytes so the wire-format Initial passes the server-side + * 1200-byte minimum check (xqc_packet_parse_initial). All zero bytes + * become a single PADDING frame on the receive path. */ + size_t aead_tag = 16; /* AES-128-GCM, matches BoringSSL initial AEAD */ + size_t target_payload = + (COALESCED_TEST_PKT_TARGET > (size_t)hdr_len + aead_tag) + ? COALESCED_TEST_PKT_TARGET - (size_t)hdr_len - aead_tag + : 64; + + if (po->po_used_size + target_payload > po->po_buf_cap) { + xqc_packet_out_destroy(po); + return 0; + } + memset(po->po_buf + po->po_used_size, 0x00, target_payload); + po->po_used_size += target_payload; + + size_t enc_len = 0; + xqc_int_t ret = xqc_packet_encrypt_buf(cli_conn, po, out_buf, + out_buf_cap, &enc_len); + xqc_packet_out_destroy(po); + if (ret != XQC_OK) { + return 0; + } + return enc_len; +} + + +/* + * Variant of build_initial_pkt that injects a CONNECTION_CLOSE frame + * (0x1c, transport-level, err_code=0x0a PROTOCOL_VIOLATION) at the head + * of the payload before padding. The frame is observable through two + * server-side side effects produced by xqc_process_conn_close_frame: + * + * - conn_close_recv_time flips from 0 to a non-zero timestamp + * - conn_state advances to XQC_CONN_STATE_DRAINING + * + * Neither of those is touched by xqc_conn_on_pkt_processed, so they + * only appear if the frame really reached xqc_process_frames. This + * makes the helper a smoking-gun probe for "were the offending packet's + * frames actually applied to connection state?". + */ +static size_t +build_initial_pkt_with_conn_close(xqc_connection_t *cli_conn, + const unsigned char *dcid_buf, + uint8_t dcid_len, + xqc_packet_number_t pkt_num, + uint8_t *out_buf, size_t out_buf_cap) +{ + xqc_packet_out_t *po = xqc_packet_out_create(2048); + if (po == NULL) { + return 0; + } + + po->po_pkt.pkt_type = XQC_PTYPE_INIT; + po->po_pkt.pkt_pns = XQC_PNS_INIT; + po->po_pkt.pkt_num = pkt_num; + + memcpy(po->po_pkt.pkt_scid.cid_buf, + cli_conn->scid_set.user_scid.cid_buf, + cli_conn->scid_set.user_scid.cid_len); + po->po_pkt.pkt_scid.cid_len = cli_conn->scid_set.user_scid.cid_len; + + memcpy(po->po_pkt.pkt_dcid.cid_buf, dcid_buf, dcid_len); + po->po_pkt.pkt_dcid.cid_len = dcid_len; + + ssize_t hdr_len = xqc_gen_long_packet_header( + po, + po->po_pkt.pkt_dcid.cid_buf, po->po_pkt.pkt_dcid.cid_len, + po->po_pkt.pkt_scid.cid_buf, po->po_pkt.pkt_scid.cid_len, + NULL, 0, XQC_VERSION_V1, XQC_PKTNO_BITS); + if (hdr_len <= 0) { + xqc_packet_out_destroy(po); + return 0; + } + po->po_used_size += hdr_len; + + /* CONNECTION_CLOSE (transport, PROTOCOL_VIOLATION, no triggering frame). */ + ssize_t cc_len = xqc_gen_conn_close_frame(po, 0x0a, 0, 0); + if (cc_len <= 0) { + xqc_packet_out_destroy(po); + return 0; + } + po->po_used_size += cc_len; + + /* Pad to roughly the same nominal size as build_initial_pkt. */ + size_t aead_tag = 16; + size_t fixed = (size_t)hdr_len + (size_t)cc_len + aead_tag; + size_t target_payload = + (COALESCED_TEST_PKT_TARGET > fixed) ? COALESCED_TEST_PKT_TARGET - fixed : 64; + + if (po->po_used_size + target_payload > po->po_buf_cap) { + xqc_packet_out_destroy(po); + return 0; + } + memset(po->po_buf + po->po_used_size, 0x00, target_payload); + po->po_used_size += target_payload; + + size_t enc_len = 0; + xqc_int_t ret = xqc_packet_encrypt_buf(cli_conn, po, out_buf, + out_buf_cap, &enc_len); + xqc_packet_out_destroy(po); + if (ret != XQC_OK) { + return 0; + } + return enc_len; +} + + +/* + * Boilerplate: stand up a client+server engine pair, drive a real Initial + * from client to server so the server-side connection exists. + */ +static int +coalesced_test_setup(coalesced_ctx_t *cli, coalesced_ctx_t *svr) +{ + cli->engine = coalesced_create_engine(cli, XQC_ENGINE_CLIENT); + svr->engine = coalesced_create_engine(svr, XQC_ENGINE_SERVER); + if (cli->engine == NULL || svr->engine == NULL) { + return -1; + } + + xqc_conn_settings_t conn_settings; + memset(&conn_settings, 0, sizeof(conn_settings)); + conn_settings.proto_version = XQC_VERSION_V1; + xqc_conn_ssl_config_t conn_ssl_config; + memset(&conn_ssl_config, 0, sizeof(conn_ssl_config)); + + const xqc_cid_t *cid = xqc_connect(cli->engine, &conn_settings, + NULL, 0, "", 0, &conn_ssl_config, + NULL, 0, "transport", cli); + if (cid == NULL || cli->c == NULL) { + return -1; + } + + struct sockaddr_in6 peer_addr; + socklen_t peer_addrlen = sizeof(peer_addr); + struct sockaddr_in6 local_addr; + socklen_t local_addrlen = sizeof(local_addr); + + xqc_engine_packet_process(svr->engine, cli->buf, cli->buf_len, + (struct sockaddr *)&local_addr, local_addrlen, + (struct sockaddr *)&peer_addr, peer_addrlen, + xqc_now(), svr); + + return svr->c == NULL ? -1 : 0; +} + + +static void +coalesced_test_teardown(coalesced_ctx_t *cli, coalesced_ctx_t *svr) +{ + if (cli->engine != NULL) { + if (cli->c != NULL) { + xqc_conn_close(cli->engine, &cli->cid); + } + xqc_engine_destroy(cli->engine); + } + if (svr->engine != NULL) { + if (svr->c != NULL) { + xqc_conn_close(svr->engine, &svr->cid); + } + xqc_engine_destroy(svr->engine); + } +} + + +/* T1 - regression: a single Initial in a datagram never trips the new + * mismatch branch (drop counter must remain at its baseline). */ +void +xqc_test_coalesced_single_pkt(void) +{ + coalesced_ctx_t cli = {0}; + coalesced_ctx_t svr = {0}; + + if (coalesced_test_setup(&cli, &svr) != 0) { + CU_FAIL("coalesced_test_setup failed"); + coalesced_test_teardown(&cli, &svr); + return; + } + + uint32_t baseline = svr.c->packet_dropped_count; + + uint8_t enc[2048]; + size_t enc_len = build_initial_pkt( + cli.c, + cli.c->dcid_set.current_dcid.cid_buf, + cli.c->dcid_set.current_dcid.cid_len, + 1, enc, sizeof(enc)); + CU_ASSERT(enc_len > 0); + + (void)xqc_conn_process_packet(svr.c, enc, enc_len, xqc_now()); + + /* Single packet must never increment the dropped count via the new + * coalesced-mismatch branch. */ + CU_ASSERT_EQUAL(svr.c->packet_dropped_count, baseline); + + coalesced_test_teardown(&cli, &svr); +} + + +/* T2 - core reverse-validation: a coalesced Initial(A) + Initial(B) datagram + * with A != B must drop exactly one packet (the offender) and keep + * processing the rest of the datagram. */ +void +xqc_test_coalesced_dcid_mismatch(void) +{ + coalesced_ctx_t cli = {0}; + coalesced_ctx_t svr = {0}; + + if (coalesced_test_setup(&cli, &svr) != 0) { + CU_FAIL("coalesced_test_setup failed"); + coalesced_test_teardown(&cli, &svr); + return; + } + + uint32_t baseline_dropped = svr.c->packet_dropped_count; + uint32_t baseline_rcvd = svr.c->rcv_pkt_stats.conn_rcvd_pkts; + + /* DCID A: client's view of the current DCID. */ + uint8_t dcid_a[XQC_MAX_CID_LEN]; + uint8_t dcid_a_len = cli.c->dcid_set.current_dcid.cid_len; + memcpy(dcid_a, cli.c->dcid_set.current_dcid.cid_buf, dcid_a_len); + + /* DCID B: same length, first byte flipped. */ + uint8_t dcid_b[XQC_MAX_CID_LEN]; + uint8_t dcid_b_len = dcid_a_len; + memcpy(dcid_b, dcid_a, dcid_a_len); + dcid_b[0] ^= 0xFF; + + uint8_t enc1[2048]; + size_t enc1_len = build_initial_pkt(cli.c, dcid_a, dcid_a_len, 1, + enc1, sizeof(enc1)); + CU_ASSERT(enc1_len > 0); + + uint8_t enc2[2048]; + size_t enc2_len = build_initial_pkt(cli.c, dcid_b, dcid_b_len, 2, + enc2, sizeof(enc2)); + CU_ASSERT(enc2_len > 0); + + uint8_t combined[8192]; + CU_ASSERT(enc1_len + enc2_len <= sizeof(combined)); + memcpy(combined, enc1, enc1_len); + memcpy(combined + enc1_len, enc2, enc2_len); + + xqc_int_t ret = xqc_conn_process_packet(svr.c, combined, + enc1_len + enc2_len, xqc_now()); + /* The function must return cleanly (non-fatal): the second packet was + * dropped but the first was processed. */ + CU_ASSERT_EQUAL(ret, XQC_OK); + + /* Exactly one drop attributable to the new branch. */ + CU_ASSERT_EQUAL(svr.c->packet_dropped_count, baseline_dropped + 1); + + /* The first packet must have been logged as received. */ + CU_ASSERT(svr.c->rcv_pkt_stats.conn_rcvd_pkts > baseline_rcvd); + + coalesced_test_teardown(&cli, &svr); +} + + +/* T3 - boundary: when both coalesced Initials share the same DCID, the new + * branch must NOT trip. */ +void +xqc_test_coalesced_dcid_match(void) +{ + coalesced_ctx_t cli = {0}; + coalesced_ctx_t svr = {0}; + + if (coalesced_test_setup(&cli, &svr) != 0) { + CU_FAIL("coalesced_test_setup failed"); + coalesced_test_teardown(&cli, &svr); + return; + } + + uint32_t baseline_dropped = svr.c->packet_dropped_count; + + uint8_t dcid_a[XQC_MAX_CID_LEN]; + uint8_t dcid_a_len = cli.c->dcid_set.current_dcid.cid_len; + memcpy(dcid_a, cli.c->dcid_set.current_dcid.cid_buf, dcid_a_len); + + uint8_t enc1[2048]; + size_t enc1_len = build_initial_pkt(cli.c, dcid_a, dcid_a_len, 1, + enc1, sizeof(enc1)); + CU_ASSERT(enc1_len > 0); + + uint8_t enc2[2048]; + size_t enc2_len = build_initial_pkt(cli.c, dcid_a, dcid_a_len, 2, + enc2, sizeof(enc2)); + CU_ASSERT(enc2_len > 0); + + uint8_t combined[8192]; + CU_ASSERT(enc1_len + enc2_len <= sizeof(combined)); + memcpy(combined, enc1, enc1_len); + memcpy(combined + enc1_len, enc2, enc2_len); + + (void)xqc_conn_process_packet(svr.c, combined, + enc1_len + enc2_len, xqc_now()); + + /* DCIDs match; the new branch must stay silent. */ + CU_ASSERT_EQUAL(svr.c->packet_dropped_count, baseline_dropped); + + coalesced_test_teardown(&cli, &svr); +} + + +/* T4 - boundary: same byte prefix but different length must still mismatch + * (xqc_cid_is_equal first compares cid_len). */ +void +xqc_test_coalesced_dcid_len_mismatch(void) +{ + coalesced_ctx_t cli = {0}; + coalesced_ctx_t svr = {0}; + + if (coalesced_test_setup(&cli, &svr) != 0) { + CU_FAIL("coalesced_test_setup failed"); + coalesced_test_teardown(&cli, &svr); + return; + } + + uint32_t baseline_dropped = svr.c->packet_dropped_count; + + uint8_t dcid_a[XQC_MAX_CID_LEN]; + uint8_t dcid_a_len = cli.c->dcid_set.current_dcid.cid_len; + memcpy(dcid_a, cli.c->dcid_set.current_dcid.cid_buf, dcid_a_len); + + /* DCID B: prefix of A, shorter length (dcid_a_len > 4 by default). */ + uint8_t dcid_b[XQC_MAX_CID_LEN]; + uint8_t dcid_b_len = (dcid_a_len > 4) ? (uint8_t)(dcid_a_len - 4) : (uint8_t)4; + memcpy(dcid_b, dcid_a, dcid_b_len); + + uint8_t enc1[2048]; + size_t enc1_len = build_initial_pkt(cli.c, dcid_a, dcid_a_len, 1, + enc1, sizeof(enc1)); + CU_ASSERT(enc1_len > 0); + + uint8_t enc2[2048]; + size_t enc2_len = build_initial_pkt(cli.c, dcid_b, dcid_b_len, 2, + enc2, sizeof(enc2)); + CU_ASSERT(enc2_len > 0); + + uint8_t combined[8192]; + CU_ASSERT(enc1_len + enc2_len <= sizeof(combined)); + memcpy(combined, enc1, enc1_len); + memcpy(combined + enc1_len, enc2, enc2_len); + + (void)xqc_conn_process_packet(svr.c, combined, + enc1_len + enc2_len, xqc_now()); + + CU_ASSERT_EQUAL(svr.c->packet_dropped_count, baseline_dropped + 1); + + coalesced_test_teardown(&cli, &svr); +} + + +/* T5 - extension: A + B + A. The middle packet must be dropped, the third + * (which matches first_dcid) must be accepted again - so drop count is + * exactly 1 (no double-counting, no spurious accept-then-mismatch). */ +void +xqc_test_coalesced_dcid_a_b_a(void) +{ + coalesced_ctx_t cli = {0}; + coalesced_ctx_t svr = {0}; + + if (coalesced_test_setup(&cli, &svr) != 0) { + CU_FAIL("coalesced_test_setup failed"); + coalesced_test_teardown(&cli, &svr); + return; + } + + uint32_t baseline_dropped = svr.c->packet_dropped_count; + + uint8_t dcid_a[XQC_MAX_CID_LEN]; + uint8_t dcid_a_len = cli.c->dcid_set.current_dcid.cid_len; + memcpy(dcid_a, cli.c->dcid_set.current_dcid.cid_buf, dcid_a_len); + + uint8_t dcid_b[XQC_MAX_CID_LEN]; + uint8_t dcid_b_len = dcid_a_len; + memcpy(dcid_b, dcid_a, dcid_a_len); + dcid_b[0] ^= 0xFF; + + uint8_t enc1[2048], enc2[2048], enc3[2048]; + size_t l1 = build_initial_pkt(cli.c, dcid_a, dcid_a_len, 1, enc1, sizeof(enc1)); + size_t l2 = build_initial_pkt(cli.c, dcid_b, dcid_b_len, 2, enc2, sizeof(enc2)); + size_t l3 = build_initial_pkt(cli.c, dcid_a, dcid_a_len, 3, enc3, sizeof(enc3)); + CU_ASSERT(l1 > 0 && l2 > 0 && l3 > 0); + + uint8_t combined[8192]; + CU_ASSERT(l1 + l2 + l3 <= sizeof(combined)); + size_t off = 0; + memcpy(combined + off, enc1, l1); off += l1; + memcpy(combined + off, enc2, l2); off += l2; + memcpy(combined + off, enc3, l3); off += l3; + + (void)xqc_conn_process_packet(svr.c, combined, off, xqc_now()); + + /* Only the middle packet should have been dropped. */ + CU_ASSERT_EQUAL(svr.c->packet_dropped_count, baseline_dropped + 1); + + coalesced_test_teardown(&cli, &svr); +} + + +/* + * Smoking-gun test for the §12.2 semantics that drop counters alone + * cannot prove: when the offending packet carries an observable frame + * (CONNECTION_CLOSE), the receiver state must be untouched. If the + * mismatch check fires only after xqc_packet_decrypt_single (which + * internally calls xqc_process_frames), the CLOSE frame still lands + * and drives the connection to DRAINING - which is exactly the bug + * this case is here to catch. + */ +void +xqc_test_coalesced_mismatch_frames_not_applied(void) +{ + coalesced_ctx_t cli = {0}; + coalesced_ctx_t svr = {0}; + + if (coalesced_test_setup(&cli, &svr) != 0) { + CU_FAIL("coalesced_test_setup failed"); + coalesced_test_teardown(&cli, &svr); + return; + } + + uint32_t baseline_dropped = svr.c->packet_dropped_count; + xqc_conn_state_t baseline_state = svr.c->conn_state; + CU_ASSERT_EQUAL(svr.c->conn_close_recv_time, 0); + CU_ASSERT(baseline_state < XQC_CONN_STATE_DRAINING); + + uint8_t dcid_a[XQC_MAX_CID_LEN]; + uint8_t dcid_a_len = cli.c->dcid_set.current_dcid.cid_len; + memcpy(dcid_a, cli.c->dcid_set.current_dcid.cid_buf, dcid_a_len); + + uint8_t dcid_b[XQC_MAX_CID_LEN]; + uint8_t dcid_b_len = dcid_a_len; + memcpy(dcid_b, dcid_a, dcid_a_len); + dcid_b[0] ^= 0xFF; + + /* First packet: legitimate anchor, padding-only. */ + uint8_t enc1[2048]; + size_t enc1_len = build_initial_pkt(cli.c, dcid_a, dcid_a_len, 1, + enc1, sizeof(enc1)); + CU_ASSERT(enc1_len > 0); + + /* Second packet: mismatching DCID, CONNECTION_CLOSE payload. */ + uint8_t enc2[2048]; + size_t enc2_len = build_initial_pkt_with_conn_close( + cli.c, dcid_b, dcid_b_len, 2, enc2, sizeof(enc2)); + CU_ASSERT(enc2_len > 0); + + uint8_t combined[8192]; + CU_ASSERT(enc1_len + enc2_len <= sizeof(combined)); + memcpy(combined, enc1, enc1_len); + memcpy(combined + enc1_len, enc2, enc2_len); + + (void)xqc_conn_process_packet(svr.c, combined, + enc1_len + enc2_len, xqc_now()); + + /* + * Core invariants of RFC 9000 §12.2: the offending packet's frames + * MUST NOT take effect on the connection. + */ + CU_ASSERT_EQUAL(svr.c->conn_close_recv_time, 0); + CU_ASSERT(svr.c->conn_state < XQC_CONN_STATE_DRAINING); + + /* And the drop counter still has to move forward, otherwise we would + * silently accept the rogue packet. */ + CU_ASSERT_EQUAL(svr.c->packet_dropped_count, baseline_dropped + 1); + + coalesced_test_teardown(&cli, &svr); +} diff --git a/tests/unittest/xqc_coalesced_test.h b/tests/unittest/xqc_coalesced_test.h new file mode 100644 index 000000000..009ff314f --- /dev/null +++ b/tests/unittest/xqc_coalesced_test.h @@ -0,0 +1,19 @@ +/** + * @copyright Copyright (c) 2026, Alibaba Group Holding Limited + * + * Tests for RFC 9000 Section 12.2: every coalesced QUIC packet that follows + * the first one in a UDP datagram MUST share the same Destination Connection + * ID. Receivers SHOULD discard offenders but continue processing the rest. + */ + +#ifndef _XQC_COALESCED_TEST_H_INCLUDED_ +#define _XQC_COALESCED_TEST_H_INCLUDED_ + +void xqc_test_coalesced_single_pkt(void); +void xqc_test_coalesced_dcid_match(void); +void xqc_test_coalesced_dcid_mismatch(void); +void xqc_test_coalesced_dcid_len_mismatch(void); +void xqc_test_coalesced_dcid_a_b_a(void); +void xqc_test_coalesced_mismatch_frames_not_applied(void); + +#endif /* _XQC_COALESCED_TEST_H_INCLUDED_ */