fix(#756): RFC 9001 §6 key update — initiator ACK gate, consecutive detection, old-key pktnum check

[cherylsy]

Summary

Fixes three RFC 9001 Section 6 key update compliance bugs identified in issue #756.

BUG1 — Initiator does not gate re-initiation on ACK (§6.1)

Before: After initiating a key update, xquic immediately confirmed and could re-initiate as soon as enc_pkt_cnt exceeded the threshold, regardless of whether the peer had ACKed any new-phase packet. Violates RFC 9001 §6.1: the initiator must not start a subsequent key update until the peer has ACKed a packet protected with the current key phase.

After: key_update_initiator flag set on initiation. xqc_key_update_acked() blocks re-initiation until peer ACKs a new-phase packet. ACK processing in xqc_send_ctl_on_ack_received() clears the flag on confirmation.

BUG2 — Consecutive key update detection is dead code (§6.2)

Before: The original detection was gated on pkt_num > first_recv_pktno, but first_recv_pktno is reset to XQC_MAX_UINT64_VALUE by confirm_key_update — comparison always false, detection never fires.

First attempt (commit efec39d, reverted in beddfec): Used key_update_initiator as a pre-decrypt gate on the initiator side. This caused false positives: after the initiator flips next_in_key_phase, any old-phase packet still in flight from the peer would trigger KEY_UPDATE_ERROR, breaking key update and key update 0RTT integration tests.

After (commit d581513): Detects from the responder side instead. When the responder accepts a peer-initiated key update, it sets peer_key_update_ack_pending = TRUE and records peer_key_update_pktno. This gate is cleared only after the responder sends an APP_DATA packet containing an ACK that covers peer_key_update_pktno. If a second key-phase change arrives (with a higher packet number) before that ACK is sent, it is treated as KEY_UPDATE_ERROR. Reordered old packets with lower packet numbers are correctly allowed through.

BUG3 — Old-key high packet number not detected (§6.4)

Before: TODO comment, no enforcement.

After: Post-decrypt detection during 3xPTO old-key retention window. Guard RX key derivation with !timer_set to preserve old keys for this check.

Refactor — key_update_confirmed -> key_phase_synced, xqc_key_update_acked()

• Rename key_update_confirmed to key_phase_synced — the old name implied ACK-based confirmation but actually tracked RX/TX key alignment (TLS-layer). key_phase_synced describes the actual semantics.
• Extract xqc_key_update_acked() — replaces raw largest_acked >= first_sent_pktno at trigger, ACK confirmation, and test sites.

Prerequisite — TRA_KEY_UPDATE_ERROR error code

RFC 9000 assigns 0x0E to KEY_UPDATE_ERROR. xquic used 0x0E for TRA_0RTT_TRANS_PARAMS_ERROR (library-internal). Moved TRA_0RTT_TRANS_PARAMS_ERROR to 0x54, added TRA_KEY_UPDATE_ERROR = 0x0E.

CI fix — FEC test race condition

Fix clear_log / killall ordering race in FEC stream tests (XOR, RSC, PM) and frame_type_bit tests. Previous server could write [error] to slog between log truncation and process termination. Reorder to killall + sleep + clear_log.

Before / After

#	RFC	Before	After
BUG1	§6.1	Re-initiates without ACK gate	Blocked by xqc_key_update_acked() + key_update_initiator
BUG2	§6.2	pkt_num > UINT64_MAX always false	Responder-side peer_key_update_ack_pending gate
BUG3	§6.4	TODO comment, no enforcement	Post-decrypt check + RX derivation guard

Files Changed

File	Change
include/xquic/xqc_errno.h	Add TRA_KEY_UPDATE_ERROR = 0x0E, move TRA_0RTT_TRANS_PARAMS_ERROR to 0x54
src/transport/xqc_conn.h	Add key_update_initiator, peer_key_update_ack_pending, peer_key_update_pktno fields; add xqc_key_update_acked(), xqc_key_update_peer_consecutive(), xqc_key_update_peer_ack_sent()
src/transport/xqc_conn.c	Initialize new fields; add xqc_conn_maybe_confirm_peer_key_update_ack_sent() hook on both send paths
src/tls/xqc_tls.h	Rename is_key_update_confirmed -> is_key_phase_synced
src/tls/xqc_tls.c	Rename field, why-comments on state transitions
src/transport/xqc_packet_parser.c	BUG2 responder-side consecutive detection + BUG3 post-decrypt detection + RX derivation guard (!timer_set) + xqc_key_update_acked() in trigger
src/transport/xqc_send_ctl.c	ACK-based key_update_initiator confirmation (BUG1)
scripts/case_test.sh	Update grep patterns for error code value change; fix FEC test race (XOR/RSC/PM + frame_type_bit)
tests/unittest/xqc_send_ctl_test.{c,h}	3 regression tests: BUG1 (3 cases), BUG2 (5 assertions), BUG3 (5 cases)
tests/unittest/main.c	Register new test suites

Test Plan

• run_tests — unit tests pass, 0 failures
• xqc_test_key_update_initiator_confirmation — 3 cases: ACK below threshold, ACK at threshold, idempotency
• xqc_test_consecutive_key_update_detection — 5 assertions: consecutive detected, reorder-low allowed, current-phase allowed, partial-ACK still gated, full-ACK clears gate
• xqc_test_old_key_high_pktnum_detection — 5 cases: all-conditions-met, low-pktnum, timer-not-set, same-phase, mid-update
• FEC stream tests (XOR/RSC/PM) — race condition fixed

🤖 Generated with [Claude Code]