From 0920aba2b85f8948138072a3c81deaccfc42a80a Mon Sep 17 00:00:00 2001 From: Vitor Pamplona Date: Sat, 9 May 2026 10:59:58 -0400 Subject: [PATCH] fix(quic): issue spare source CIDs to peer post-handshake-confirmed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RFC 9000 §5.1.1 + §19.15: a client SHOULD advertise spare source CIDs to the peer via NEW_CONNECTION_ID frames so the peer has DCIDs available for path migration / NAT rebind. Strict server stacks (quic-go, picoquic, msquic, mvfst) refuse to validate a new client path until they have a spare DCID — server log shows "skipping validation of new path … since no connection ID is available". Quinn migrates on src-port-change alone, which is why rebind-port / rebind-addr passed against quinn pre-fix and failed against the others. Adds: - IssuedSourceConnectionIdEntry (data class) — sequence + CID + stateless-reset token tuple. - QuicConnection.issuedSourceConnectionIds (LinkedHashMap) — pool of currently-active issued SCIDs, seeded with seq=0 (the initial CID, no token because the stateless_reset_token TP is server-only per RFC 9000 §18.2). - QuicConnection.issueOwnConnectionIdsLocked(count) — drives the initial issuance once handshake is confirmed; appends recovery tokens to pendingOwnNewConnectionIdEmits for the writer to emit. - QuicConnection.issueOwnReplacementSourceCidLocked() — top-up after each peer RETIRE_CONNECTION_ID so the pool stays at full capacity for repeated migrations. - QuicConnectionWriter — once handshakeConfirmed flips, calls issueOwnConnectionIdsLocked with `peer.activeConnectionIdLimit - 1` (cap 7); drains pendingOwnNewConnectionIdEmits as fresh NEW_CONNECTION_ID frames; the existing pendingNewConnectionId map continues to handle loss-recovery retransmits. - applyPeerRetireConnectionIdLocked — three cases: in-pool seq is freed and replaced; below-next-seq missing seq is benign retransmit; above-next-seq is PROTOCOL_VIOLATION (peer retiring something we never advertised). Verified against the live interop runner: - quic-go rebind-port: 2/2 (was 0/N — Task 2). Now passes in ~64s. - quic-go rebind-addr: 2/2 (was 0/N). Now passes in ~125-138s. - picoquic rebind-port: 2/2. - picoquic rebind-addr: 1/2 (flaky — separate investigation; one run the connection migrates correctly, the other times out at 110s; not blocking the rebind-port / quic-go win). - picoquic connectionmigration: 0/2 still failing (the runner's "active migration" testcase exercises a more sophisticated migration shape — TBD; tracked separately). Tests: IssuedSourceConnectionIdTest (5 cases pinning the post-handshake emission, peer-RETIRE replacement, retransmit tolerance, and protocol-violation closure for above-next-seq retires). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../quic/connection/QuicConnection.kt | 235 ++++++++++++++++-- .../quic/connection/QuicConnectionWriter.kt | 44 +++- .../IssuedSourceConnectionIdTest.kt | 227 +++++++++++++++++ 3 files changed, 477 insertions(+), 29 deletions(-) create mode 100644 quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/IssuedSourceConnectionIdTest.kt diff --git a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnection.kt b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnection.kt index c10a662e6..93b3f9f68 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnection.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnection.kt @@ -673,6 +673,72 @@ class QuicConnection( internal val pendingNewConnectionId: MutableMap = HashMap() + /** + * Active source CIDs we've issued to the peer (RFC 9000 §5.1.1). + * Keyed by sequence number. The initial CID is implicitly sequence + * 0 ([sourceConnectionId]) and is seeded into this pool at + * connection start. Additional sequences come from + * [issueOwnConnectionIdsLocked] which the writer drives once the + * handshake is confirmed. + * + * Why we need this: strict servers (quic-go, picoquic, msquic, + * mvfst) gate server-initiated path validation on having a SPARE + * destination CID for the new client path (RFC 9000 §9.5: "An + * endpoint MUST NOT reuse a connection ID for sending packets to + * more than one source address"). Without spares from us, server + * logs `"skipping validation of new path … since no connection + * ID is available"` and the rebind-port / rebind-addr / + * connectionmigration interop tests time out. Quinn is the + * exception — it migrates on src-port-change alone, which is why + * those cells passed against quinn pre-fix. + * + * Lifetime: entries are added by [issueOwnConnectionIdsLocked] + * and removed by [applyPeerRetireConnectionIdLocked]. We only + * cap at peer's `active_connection_id_limit`; the peer telling + * us to retire one creates room for a fresh issuance. + */ + internal val issuedSourceConnectionIds: LinkedHashMap = + LinkedHashMap().apply { + // Sequence 0 is implicit per §5.1.1 and has no + // stateless-reset token — we never advertised one in our + // transport parameters (the `stateless_reset_token` TP is + // server-only per §18.2). The empty-bytes sentinel here + // is correct: this entry exists for sequence-counter + // continuity and peer-RETIRE tracking, never for a + // RecoveryToken / NEW_CONNECTION_ID emission (it'd be a + // §19.15 protocol violation to send sequence 0 in a + // NEW_CONNECTION_ID frame). + put(0L, IssuedSourceConnectionIdEntry(0L, sourceConnectionId.bytes, ByteArray(0))) + } + + /** + * Counter for the next sequence number we'll issue via + * NEW_CONNECTION_ID. Starts at 1 because sequence 0 is reserved + * for the initial source CID. + */ + internal var nextOwnSourceCidSeq: Long = 1L + + /** + * NEW_CONNECTION_ID frames staged for FIRST emission (i.e. fresh + * issuance, distinct from [pendingNewConnectionId] which is the + * loss-recovery retransmit queue). The writer drains this on each + * application-level build; a sent token is recorded as + * [com.vitorpamplona.quic.connection.recovery.RecoveryToken.NewConnectionId] + * for the loss dispatcher. + */ + internal val pendingOwnNewConnectionIdEmits: + ArrayDeque = + ArrayDeque() + + /** + * Whether [issueOwnConnectionIdsLocked] has run. Single-shot + * because we issue the full peer-active-connection-id-limit pool + * at handshake-confirmation time; further issuance only happens + * in response to peer's RETIRE_CONNECTION_ID (which calls + * [issueOwnReplacementSourceCidLocked]). + */ + internal var ownSourceCidsIssued: Boolean = false + /** * RFC 9000 §8.2.2: queue of inbound PATH_CHALLENGE payloads we * still owe a PATH_RESPONSE for. Each entry is the EXACT 8 bytes @@ -2493,39 +2559,125 @@ class QuicConnection( * RFC 9000 §19.16: peer asked us to retire one of our own * source CIDs. * - * Two cases: - * - Sequence > 0: we never issued anything beyond seq 0, so - * this references a CID we never advertised. PROTOCOL_VIOLATION - * per §19.16. - * - Sequence == 0: peer is asking us to retire the SCID we - * picked at connection start. Per RFC 9000 §5.1.1 we'd be - * expected to have already issued a replacement via - * NEW_CONNECTION_ID and switch the peer to it. We don't - * issue our own NEW_CONNECTION_ID frames today, so we have - * no replacement to give the peer. Closing the connection - * surfaces this as a known limitation rather than continuing - * against a peer that thinks we agreed to stop using our - * only CID. + * Three cases: + * - Sequence is in [issuedSourceConnectionIds] — peer is + * retiring an SCID we issued via NEW_CONNECTION_ID. Drop the + * entry and issue a replacement (so the pool stays topped up + * for future migrations). Common during NAT rebind: peer + * burns through our spares as it migrates. + * - Sequence < [nextOwnSourceCidSeq] but missing from the pool + * — already-retired SCID. Silently drop per §19.16 ("a peer + * that retransmits RETIRE_CONNECTION_ID after observing the + * corresponding ACK ... it's harmless"). + * - Sequence ≥ [nextOwnSourceCidSeq] — peer is retiring a CID + * we never advertised. PROTOCOL_VIOLATION per §19.16. Same + * diagnostic for sequence 0 if it's still active and we have + * no spares to give the peer (which is impossible once + * [issueOwnConnectionIdsLocked] has run, but defensively + * surfaces the only failure mode left). * * Caller must hold [streamsLock]. */ internal fun applyPeerRetireConnectionIdLocked(sequenceNumber: Long) { - if (sequenceNumber > 0L) { + if (sequenceNumber >= nextOwnSourceCidSeq) { markClosedExternally( - "PROTOCOL_VIOLATION: RETIRE_CONNECTION_ID seq=$sequenceNumber > any we issued (0)", + "PROTOCOL_VIOLATION: RETIRE_CONNECTION_ID seq=$sequenceNumber ≥ next-issued seq=$nextOwnSourceCidSeq", ) return } - // Sequence 0: peer wants us off our initial SCID, but we never - // advertised additional SCIDs (we don't issue NEW_CONNECTION_ID - // frames yet). RFC 9000 §19.16 lets us close — and we MUST, - // because there's no replacement SCID to switch to. Reclassify - // as PROTOCOL_VIOLATION (peer-side fault) rather than - // INTERNAL_ERROR (our-side fault) — the original diagnostic - // misdescribed which side made the mistake. - markClosedExternally( - "PROTOCOL_VIOLATION: peer asked to retire our initial SCID (seq=0) but we never " + - "advertised additional SCIDs to switch to", + val removed = issuedSourceConnectionIds.remove(sequenceNumber) + if (removed == null) { + // Already-retired entry; benign §19.16 retransmit. + return + } + if (sequenceNumber == 0L && issuedSourceConnectionIds.isEmpty()) { + // Peer retired our LAST source CID. Without a spare we + // can't continue speaking on the wire — close per §19.16. + // Should be unreachable once [issueOwnConnectionIdsLocked] + // has run (the loop seeds the pool with handshake-confirmed + // spares); kept as a defence-in-depth so a misbehaving + // peer that retires faster than we issue can't drive us + // into a silent wedge. + markClosedExternally( + "PROTOCOL_VIOLATION: peer retired our initial SCID (seq=0) and no spare " + + "is left in the issued pool", + ) + return + } + // Top up the pool so a subsequent migration still has spares. + // RFC 9000 §5.1.2 — endpoints SHOULD replace retired CIDs. + issueOwnReplacementSourceCidLocked() + } + + /** + * RFC 9000 §5.1.1 + §19.15: issue spare source CIDs to the peer + * via NEW_CONNECTION_ID frames so it has DCIDs available to send + * to us on alternative paths (NAT rebind, connection migration). + * Strict servers (quic-go, picoquic, msquic, mvfst) refuse to + * validate a new path until they have a spare DCID from us — the + * tell is the server log line `"skipping validation of new path + * … since no connection ID is available"`. + * + * Issues `count` fresh entries (`nextOwnSourceCidSeq` .. + * `nextOwnSourceCidSeq + count - 1`), each with a random 8-byte + * CID and a random 16-byte stateless-reset token. The entries + * land in [issuedSourceConnectionIds] (peer-RETIRE tracking) and + * a [com.vitorpamplona.quic.connection.recovery.RecoveryToken.NewConnectionId] + * goes onto [pendingOwnNewConnectionIdEmits] so the writer + * emits the frame on the next application-level build. + * + * Idempotent via [ownSourceCidsIssued] — caller doesn't need to + * gate. Caller must hold [streamsLock]. + */ + internal fun issueOwnConnectionIdsLocked(count: Int) { + if (ownSourceCidsIssued || count <= 0) return + repeat(count) { + val seq = nextOwnSourceCidSeq + nextOwnSourceCidSeq += 1 + val cidBytes = + com.vitorpamplona.quartz.utils.RandomInstance + .bytes(sourceConnectionId.length) + val token = + com.vitorpamplona.quartz.utils.RandomInstance + .bytes(16) + issuedSourceConnectionIds[seq] = IssuedSourceConnectionIdEntry(seq, cidBytes, token) + pendingOwnNewConnectionIdEmits.addLast( + com.vitorpamplona.quic.connection.recovery.RecoveryToken.NewConnectionId( + sequenceNumber = seq, + retirePriorTo = 0L, + connectionId = cidBytes, + statelessResetToken = token, + ), + ) + } + ownSourceCidsIssued = true + } + + /** + * Issue exactly one replacement source CID after the peer has + * retired one of ours (RFC 9000 §5.1.2: the peer is allowed to + * cycle through our spares freely). Keeps the pool topped up so + * a subsequent rebind still has a fresh DCID available. Caller + * must hold [streamsLock]; should only fire from + * [applyPeerRetireConnectionIdLocked]. + */ + internal fun issueOwnReplacementSourceCidLocked() { + val seq = nextOwnSourceCidSeq + nextOwnSourceCidSeq += 1 + val cidBytes = + com.vitorpamplona.quartz.utils.RandomInstance + .bytes(sourceConnectionId.length) + val token = + com.vitorpamplona.quartz.utils.RandomInstance + .bytes(16) + issuedSourceConnectionIds[seq] = IssuedSourceConnectionIdEntry(seq, cidBytes, token) + pendingOwnNewConnectionIdEmits.addLast( + com.vitorpamplona.quic.connection.recovery.RecoveryToken.NewConnectionId( + sequenceNumber = seq, + retirePriorTo = 0L, + connectionId = cidBytes, + statelessResetToken = token, + ), ) } @@ -3114,6 +3266,39 @@ private fun defaultMonotonicNowMillis(): () -> Long { return { anchor.elapsedNow().inWholeMilliseconds } } +/** + * One source CID we've issued to the peer (RFC 9000 §5.1.1 + + * §19.15). Sequence 0 is the initial CID, implicitly issued via the + * client's first Initial. Sequences ≥ 1 are issued explicitly via + * NEW_CONNECTION_ID frames after the handshake is confirmed. + * + * `statelessResetToken` is the 16-byte token the peer would put in + * a stateless reset packet addressed to this CID. Sequence 0's + * entry has an empty token because clients can't advertise a + * stateless-reset token via transport parameters (the + * `stateless_reset_token` TP is server-only per RFC 9000 §18.2). + */ +internal data class IssuedSourceConnectionIdEntry( + val sequenceNumber: Long, + val connectionId: ByteArray, + val statelessResetToken: ByteArray, +) { + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (other !is IssuedSourceConnectionIdEntry) return false + return sequenceNumber == other.sequenceNumber && + connectionId.contentEquals(other.connectionId) && + statelessResetToken.contentEquals(other.statelessResetToken) + } + + override fun hashCode(): Int { + var h = sequenceNumber.hashCode() + h = 31 * h + connectionId.contentHashCode() + h = 31 * h + statelessResetToken.contentHashCode() + return h + } +} + /** Connection was closed (locally or by peer) before reaching CONNECTED. */ class QuicConnectionClosedException( message: String, diff --git a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionWriter.kt b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionWriter.kt index 2981f4f9b..ab1a4d32c 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionWriter.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionWriter.kt @@ -110,6 +110,26 @@ fun drainOutbound( // time comparison) when the validator is in [PathValidationState.Idle]. conn.checkPathValidationTimeoutLocked(nowMillis) + // RFC 9000 §5.1.1 + §19.15: once the handshake is confirmed, + // issue spare source CIDs to the peer so it has DCIDs available + // for path migration / NAT rebind. Strict servers (quic-go, + // picoquic, msquic, mvfst) refuse to validate a new path until + // they have a spare DCID from us; quinn migrates on src-port- + // change alone, which is why rebind-* tests passed against + // quinn pre-fix and failed against the others. Issue + // peer.active_connection_id_limit - 1 spares (we already implicitly + // issued seq=0 via the handshake). The default + // active_connection_id_limit per RFC 9000 §18.2 is 2; modern + // servers advertise 4–8. + if (!conn.ownSourceCidsIssued && conn.handshakeConfirmed) { + val peerLimit = conn.peerTransportParameters?.activeConnectionIdLimit ?: 2L + // Cap at a sensible upper bound to avoid issuing huge pools + // for misbehaving servers; 4 matches our own + // [QuicConnectionConfig.activeConnectionIdLimit] default. + val spareCount = (peerLimit - 1L).coerceIn(0L, 7L).toInt() + if (spareCount > 0) conn.issueOwnConnectionIdsLocked(spareCount) + } + // Drain destructive frame sources into local lists, ONCE. val initialContents = collectHandshakeLevelFrames(conn, EncryptionLevel.INITIAL, nowMillis) val handshakeContents = collectHandshakeLevelFrames(conn, EncryptionLevel.HANDSHAKE, nowMillis) @@ -1086,10 +1106,26 @@ private fun appendFlowControlUpdates( tokens += RecoveryToken.RetireConnectionId(seq) } - // NEW_CONNECTION_ID retransmits. No application path emits these - // initially today (connection-ID rotation isn't wired); the map - // is populated only by the loss dispatcher, so this branch only - // fires for genuine retransmits. + // NEW_CONNECTION_ID — fresh issuances queued by + // [QuicConnection.issueOwnConnectionIdsLocked] post-handshake. + // Each emission also records a recovery token so the loss + // dispatcher can re-queue if the carrying packet is declared lost. + while (conn.pendingOwnNewConnectionIdEmits.isNotEmpty()) { + val token = conn.pendingOwnNewConnectionIdEmits.removeFirst() + frames += + NewConnectionIdFrame( + sequenceNumber = token.sequenceNumber, + retirePriorTo = token.retirePriorTo, + connectionId = token.connectionId, + statelessResetToken = token.statelessResetToken, + ) + tokens += token + } + + // NEW_CONNECTION_ID retransmits — the loss dispatcher populates + // [pendingNewConnectionId] when a previously-emitted frame's + // carrier packet was declared lost. Same wire shape as a fresh + // issuance; we just preserve the original token. if (conn.pendingNewConnectionId.isNotEmpty()) { val pendingNewCidEntries = conn.pendingNewConnectionId.entries.toList() for ((_, token) in pendingNewCidEntries) { diff --git a/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/IssuedSourceConnectionIdTest.kt b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/IssuedSourceConnectionIdTest.kt new file mode 100644 index 000000000..121158330 --- /dev/null +++ b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/IssuedSourceConnectionIdTest.kt @@ -0,0 +1,227 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.quic.connection + +import com.vitorpamplona.quic.frame.NewConnectionIdFrame +import com.vitorpamplona.quic.frame.RetireConnectionIdFrame +import kotlinx.coroutines.runBlocking +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * Pins the client-side issuance of source CIDs (RFC 9000 §5.1.1 + + * §19.15). Once the handshake is confirmed, the client MUST be + * willing to advertise spare source CIDs to the peer so it has + * DCIDs available to send on alternative paths after a NAT rebind + * or migration. Strict server stacks (quic-go, picoquic, msquic, + * mvfst) refuse to validate a new path until they have a spare + * DCID — the giveaway is the server log line + * `"skipping validation of new path … since no connection ID is + * available"`. Pre-fix the rebind-port / rebind-addr / + * connectionmigration interop testcases against those servers all + * timed out at the runner's 60 s budget; quinn was the only server + * that passed because it migrates on src-port-change alone without + * waiting for a fresh DCID. + * + * The implementation lives in + * [QuicConnection.issueOwnConnectionIdsLocked] (called from the + * writer once `handshakeConfirmed` flips) + + * [QuicConnection.applyPeerRetireConnectionIdLocked] (which now + * tops the pool back up via + * [QuicConnection.issueOwnReplacementSourceCidLocked] instead of + * closing). + */ +class IssuedSourceConnectionIdTest { + @Test + fun postHandshakeDrainEmitsNewConnectionIdFramesPerActiveConnectionIdLimit() = + runBlocking { + val (client, pipe) = newConnectedClient() + // newConnectedClient now delivers HANDSHAKE_DONE so + // [handshakeConfirmed] is true; the next outbound drain + // gates spare-CID issuance off that flag. + assertTrue(client.handshakeConfirmed, "fixture must deliver HANDSHAKE_DONE") + assertEquals( + false, + client.ownSourceCidsIssued, + "issuance happens at writer-drain time, not at handshake-confirm time", + ) + + // Run the writer once. Should pull `peer.activeConnectionIdLimit + // - 1` fresh entries off the pool and emit them as + // NEW_CONNECTION_ID frames. + val out = drainOutbound(client, nowMillis = 0L) + assertTrue(out != null && out.isNotEmpty(), "writer must drain") + assertTrue(client.ownSourceCidsIssued, "writer must mark issuance done after first post-confirm drain") + + val frames = pipe.decryptClientApplicationFrames(out)!! + val ncids = frames.filterIsInstance() + + // The fixture's TLS server advertises + // `activeConnectionIdLimit = 4` by default (transport + // params symmetry — see [ConnectedClientFixture]). We + // already implicitly issued seq=0, so we expect 3 fresh + // sequences (1, 2, 3). + val expectedCount = (client.peerTransportParameters?.activeConnectionIdLimit ?: 2L) - 1L + assertEquals(expectedCount.toInt(), ncids.size, "wrong NEW_CONNECTION_ID count — got ${ncids.size}") + + // Sequence numbers are contiguous starting at 1. + val seqs = ncids.map { it.sequenceNumber }.sorted() + assertEquals((1L..expectedCount).toList(), seqs) + // Each frame is a fresh-issuance — retirePriorTo=0. + assertTrue(ncids.all { it.retirePriorTo == 0L }, "fresh issuances don't request retirement") + // Each carries an 8-byte CID and 16-byte stateless reset token. + assertTrue(ncids.all { it.connectionId.size == client.sourceConnectionId.length }, "CID length mismatch") + assertTrue(ncids.all { it.statelessResetToken.size == 16 }, "stateless reset token must be 16 bytes") + + // The pool tracks every issued sequence (including seq=0). + assertEquals( + (0L..expectedCount).toSet(), + client.issuedSourceConnectionIds.keys, + "issuedSourceConnectionIds must contain every emitted sequence (plus seq=0)", + ) + + // A second drain doesn't re-emit (the queue was drained, the + // ownSourceCidsIssued latch is set). + val followup = drainOutbound(client, nowMillis = 1L) + if (followup != null && followup.isNotEmpty()) { + val followupFrames = pipe.decryptClientApplicationFrames(followup) ?: emptyList() + assertTrue( + followupFrames.none { it is NewConnectionIdFrame }, + "second drain must NOT re-issue — got ${followupFrames.map { it::class.simpleName }}", + ) + } + } + + @Test + fun preHandshakeDrainDoesNotIssueOwnConnectionIds() = + runBlocking { + // Pre-confirmation the writer must skip issuance — the + // peer's active_connection_id_limit hasn't been parsed + // yet, and emitting NEW_CONNECTION_ID at handshake level + // is meaningless (frame is application-only per §19.15). + val (client, _) = newConnectedClient(deliverHandshakeDone = false) + assertEquals(false, client.handshakeConfirmed) + drainOutbound(client, nowMillis = 0L) + assertEquals( + false, + client.ownSourceCidsIssued, + "writer must not issue spare CIDs before HANDSHAKE_DONE arrives", + ) + assertEquals(setOf(0L), client.issuedSourceConnectionIds.keys) + } + + @Test + fun peerRetireOfIssuedSequenceFreesEntryAndIssuesReplacement() = + runBlocking { + // RFC 9000 §5.1.2 endorsement: top up the pool when the + // peer retires one of our spares. Without this, after + // each NAT rebind the strict server consumes one spare + // and we'd run out — leaving the next rebind unable to + // validate. + val (client, pipe) = newConnectedClient() + drainOutbound(client, nowMillis = 0L) // emit initial NEW_CONNECTION_IDs + val initialPool = client.issuedSourceConnectionIds.keys.toSet() + val initialNext = client.nextOwnSourceCidSeq + + // Peer retires sequence 1 (one of the post-handshake spares). + val packet = pipe.buildServerApplicationDatagram(listOf(RetireConnectionIdFrame(sequenceNumber = 1L)))!! + feedDatagram(client, packet, nowMillis = 1L) + + assertTrue( + !client.issuedSourceConnectionIds.containsKey(1L), + "retired seq=1 must be removed from issuedSourceConnectionIds — got ${client.issuedSourceConnectionIds.keys}", + ) + assertEquals( + initialNext + 1L, + client.nextOwnSourceCidSeq, + "must allocate a fresh sequence for the replacement", + ) + assertTrue( + client.issuedSourceConnectionIds.containsKey(initialNext), + "replacement seq=$initialNext must be in the pool", + ) + // Connection still CONNECTED. + assertEquals(QuicConnection.Status.CONNECTED, client.status) + + // The replacement is queued for the next outbound. + val out2 = drainOutbound(client, nowMillis = 2L) + val frames = pipe.decryptClientApplicationFrames(out2 ?: ByteArray(0)) ?: emptyList() + val replacement = frames.filterIsInstance().firstOrNull() + assertTrue(replacement != null, "replacement must emit on next drain — got ${frames.map { it::class.simpleName }}") + assertEquals(initialNext, replacement.sequenceNumber) + // Sanity: original pool minus seq=1 plus seq=initialNext is the new pool. + assertEquals( + initialPool - 1L + initialNext, + client.issuedSourceConnectionIds.keys, + ) + } + + @Test + fun peerRetireOfUnissuedSequenceIsProtocolViolation() = + runBlocking { + // §19.16 — the peer can't retire something we never + // advertised. Pre-fix any seq > 0 closed the connection + // because we never issued anything; now seq must be ≥ + // [nextOwnSourceCidSeq] to violate (anything below either + // exists in the pool or has been retired already, both + // of which are benign). + val (client, pipe) = newConnectedClient() + drainOutbound(client, nowMillis = 0L) // issuance runs + val far = client.nextOwnSourceCidSeq + 100L + val packet = pipe.buildServerApplicationDatagram(listOf(RetireConnectionIdFrame(sequenceNumber = far)))!! + feedDatagram(client, packet, nowMillis = 1L) + assertTrue( + client.status == QuicConnection.Status.CLOSING || client.status == QuicConnection.Status.CLOSED, + "got ${client.status}", + ) + } + + @Test + fun peerRetireRetransmitForAlreadyRetiredSequenceIsBenign() = + runBlocking { + // §19.16 nuance: a peer that doesn't see our ACK may + // retransmit RETIRE_CONNECTION_ID for an already-removed + // entry. Closing on that would be hostile — the second + // arrival is just retransmit noise. + val (client, pipe) = newConnectedClient() + drainOutbound(client, nowMillis = 0L) + // First retire of seq=1. + feedDatagram( + client, + pipe.buildServerApplicationDatagram(listOf(RetireConnectionIdFrame(sequenceNumber = 1L)))!!, + nowMillis = 1L, + ) + assertEquals(QuicConnection.Status.CONNECTED, client.status) + drainOutbound(client, nowMillis = 2L) // emit replacement + // Duplicate retire of seq=1 (already removed). + feedDatagram( + client, + pipe.buildServerApplicationDatagram(listOf(RetireConnectionIdFrame(sequenceNumber = 1L)))!!, + nowMillis = 3L, + ) + assertEquals( + QuicConnection.Status.CONNECTED, + client.status, + "duplicate retire must not close — peer is just retransmitting", + ) + } +}