[+] #756 remove §6.2 pre-decrypt consecutive key update detection

The pre-decrypt detection (key_phase != next_in_key_phase &&
key_update_initiator) causes false positives on the initiator side:
after flipping next_in_key_phase, any old-phase packet still in flight
from peer will trigger KEY_UPDATE_ERROR.  This breaks "key update" and
"key update 0RTT" integration tests.

RFC 9001 §6.2 consecutive detection is MAY (not MUST) and is a
responder-side concern.  Pre-decrypt checks cannot distinguish
legitimate old packets from actual violations—only post-decrypt §6.4
detection (pkt_num > first_recv_pktno during retention window) can
reliably do so, and that check remains.

Co-Authored-By: Claude (claude-opus-4-6) <noreply@anthropic.com>

Author: cherylsy

src/transport/xqc_conn.h

@@ -293,7 +293,7 @@ typedef struct {
     xqc_usec_t              initiate_time_guard;  /* time limit for initiating next key update */
 
     /* §6.1: TRUE if we initiated this key update.  Blocks next initiation
-     * until peer ACKs new-phase packet.  Also gates §6.2 consecutive detect. */
+     * until peer ACKs new-phase packet (xqc_key_update_acked). */
     xqc_bool_t              key_update_initiator;
 
 } xqc_key_update_ctx_t;

src/transport/xqc_packet_parser.c

@@ -766,26 +766,6 @@ xqc_packet_decrypt(xqc_connection_t *conn, xqc_packet_in_t *packet_in)
     /* check key phase, determine weather to update read keys */
     xqc_uint_t key_phase = XQC_PACKET_SHORT_HEADER_KEY_PHASE(header);
 
-    /*
-     * §6.2: if we initiated a key update (key_update_initiator) and the peer
-     * changes key_phase again before ACKing ours, the peer skipped a phase.
-     * Cannot gate on pkt_num > first_recv_pktno: confirm resets it to MAX,
-     * making that comparison always false.
-     */
-    if (packet_in->pi_pkt.pkt_type == XQC_PTYPE_SHORT_HEADER && level == XQC_ENC_LEV_1RTT
-        && key_phase != conn->key_update_ctx.next_in_key_phase
-        && conn->key_update_ctx.key_update_initiator)
-    {
-        xqc_log(conn->log, XQC_LOG_ERROR,
-                "|consecutive key update from peer|pkt_num:%ui|key_phase:%ui|"
-                "expected:%ui|first_recv_pktno:%ui|",
-                packet_in->pi_pkt.pkt_num, key_phase,
-                conn->key_update_ctx.next_in_key_phase,
-                conn->key_update_ctx.first_recv_pktno);
-        XQC_CONN_ERR(conn, TRA_KEY_UPDATE_ERROR);
-        return -XQC_EPROTO;
-    }
-
     /*
      * RX key derivation for a peer-initiated key update.
      * pkt_num > first_recv_pktno: skip reordered late arrivals.

tests/unittest/main.c

@@ -108,9 +108,6 @@ main()
         /* issue #823: key update initiator confirmation (RFC 9001 §6.1) */
         || !CU_add_test(pSuite, "xqc_test_key_update_initiator_confirmation",
                         xqc_test_key_update_initiator_confirmation)
-        /* issue #756 BUG2: consecutive key update detection (RFC 9001 §6.2) */
-        || !CU_add_test(pSuite, "xqc_test_consecutive_key_update_detection",
-                        xqc_test_consecutive_key_update_detection)
         /* issue #756 BUG3: old-key high pkt_num detection (RFC 9001 §6.4) */
         || !CU_add_test(pSuite, "xqc_test_old_key_high_pktnum_detection",
                         xqc_test_old_key_high_pktnum_detection)

tests/unittest/xqc_send_ctl_test.c

@@ -675,100 +675,6 @@ xqc_test_key_update_initiator_confirmation(void)
 }
 
 
-/*
- * Issue #756 BUG2 regression test (RFC 9001 §6.2).
- *
- * When a key update is in progress (key_update_initiator == TRUE),
- * a second key-phase change from the peer (different from next_in_key_phase)
- * must be detected as a consecutive key update violation and treated as
- * KEY_UPDATE_ERROR.
- *
- * The detection condition must NOT include pkt_num > first_recv_pktno,
- * because xqc_conn_confirm_key_update() resets first_recv_pktno to
- * XQC_MAX_UINT64_VALUE, which would make the comparison always false
- * and the detection dead code.
- */
-void
-xqc_test_consecutive_key_update_detection(void)
-{
-    xqc_connection_t *conn = test_engine_connect();
-    CU_ASSERT_FATAL(conn != NULL);
-
-    /*
-     * Setup: simulate a key update in progress.
-     * next_in_key_phase = 0 (expecting key_phase 0 for current epoch)
-     * key_update_initiator = TRUE (we initiated, awaiting ACK confirmation)
-     * first_recv_pktno = XQC_MAX_UINT64_VALUE (reset by confirm_key_update)
-     */
-    conn->key_update_ctx.next_in_key_phase = 0;
-    conn->key_update_ctx.first_recv_pktno = XQC_MAX_UINT64_VALUE;
-    conn->key_update_ctx.key_update_initiator = XQC_TRUE;
-
-    /*
-     * Case 1: Packet with same key_phase as expected.
-     * This is a normal packet, NOT a consecutive key update.
-     * key_phase == next_in_key_phase → condition does not fire.
-     */
-    xqc_uint_t key_phase = 0; /* matches next_in_key_phase */
-    xqc_bool_t detected = XQC_FALSE;
-
-    if (key_phase != conn->key_update_ctx.next_in_key_phase
-        && conn->key_update_ctx.key_update_initiator)
-    {
-        detected = XQC_TRUE;
-    }
-    CU_ASSERT_EQUAL(detected, XQC_FALSE);
-
-    /*
-     * Case 2: Packet with different key_phase,
-     * but key_update_initiator is FALSE → no detection (normal RX key update).
-     */
-    conn->key_update_ctx.key_update_initiator = XQC_FALSE;
-    key_phase = 1;
-    detected = XQC_FALSE;
-
-    if (key_phase != conn->key_update_ctx.next_in_key_phase
-        && conn->key_update_ctx.key_update_initiator)
-    {
-        detected = XQC_TRUE;
-    }
-    CU_ASSERT_EQUAL(detected, XQC_FALSE);
-
-    /*
-     * Case 3: All conditions met → consecutive key update DETECTED.
-     * key_phase != next_in_key_phase, initiator = TRUE.
-     */
-    conn->key_update_ctx.key_update_initiator = XQC_TRUE;
-    key_phase = 1;
-    detected = XQC_FALSE;
-
-    if (key_phase != conn->key_update_ctx.next_in_key_phase
-        && conn->key_update_ctx.key_update_initiator)
-    {
-        detected = XQC_TRUE;
-    }
-    CU_ASSERT_EQUAL(detected, XQC_TRUE);
-
-    /*
-     * Case 4: first_recv_pktno = XQC_MAX_UINT64_VALUE (post-confirm reset).
-     * This is the typical scenario after initiator calls confirm_key_update().
-     * The detection must still fire — this was the original bug where
-     * pkt_num > XQC_MAX_UINT64_VALUE was always false.
-     */
-    conn->key_update_ctx.first_recv_pktno = XQC_MAX_UINT64_VALUE;
-    conn->key_update_ctx.key_update_initiator = XQC_TRUE;
-    key_phase = 1;
-    detected = XQC_FALSE;
-
-    if (key_phase != conn->key_update_ctx.next_in_key_phase
-        && conn->key_update_ctx.key_update_initiator)
-    {
-        detected = XQC_TRUE;
-    }
-    CU_ASSERT_EQUAL(detected, XQC_TRUE);
-
-    xqc_engine_destroy(conn->engine);
-}
 
 
 /*

tests/unittest/xqc_send_ctl_test.h

@@ -50,13 +50,6 @@ void xqc_test_send_ctl_persistent_congestion_no_rtt_sample_early_return(void);
  */
 void xqc_test_key_update_initiator_confirmation(void);
 
-/*
- * Regression test for issue #756 BUG2 (RFC 9001 §6.2):
- * Consecutive key update from peer must be detected and treated as
- * KEY_UPDATE_ERROR when key_update_initiator is TRUE.
- */
-void xqc_test_consecutive_key_update_detection(void);
-
 /*
  * Regression test for issue #756 BUG3 (RFC 9001 §6.4):
  * Old-key packet with pkt_num higher than any new-key packet must be