diff --git a/quic/plans/2026-04-26-quic-stack-status.md b/quic/plans/2026-04-26-quic-stack-status.md index 4008faa4b..83e6afc9c 100644 --- a/quic/plans/2026-04-26-quic-stack-status.md +++ b/quic/plans/2026-04-26-quic-stack-status.md @@ -193,7 +193,7 @@ gates on the audio-rooms completion plan | 8 | 2026-05-06 | Path validation pass 1 + 2 | DCID rotation, retire-prior-to monotonicity, abrupt migration | all fixed | | 9 | 2026-05-07 | DoS hardening + reserved-bit checks + TLS bounds + SendBuffer shrink | bound peer-controlled buffers and channels | all fixed | | 10 | 2026-05-08 | Lock-split design + close-under-load + key-update PN gate | streamsLock + lifecycleLock split | all fixed; see [`2026-05-08-lock-split-design.md`](2026-05-08-lock-split-design.md) | -| 11 | 2026-05-09 | Four-RFC compliance audit (4 parallel agents: 9000, 9001, 9002, 9221+9114+9204) | ~15 items, mostly receive-side validation gaps | catalogued in [§ RFC compliance gaps](#rfc-compliance-gaps-2026-05-09) below | +| 11 | 2026-05-09 | Four-RFC compliance audit (4 parallel agents: 9000, 9001, 9002, 9221+9114+9204) | ~15 items, mostly receive-side validation gaps | catalogued in [§ RFC compliance gaps](#rfc-compliance-gaps-2026-05-09) below; both 🔴 High items (fixed-bit, idle timeout) fixed same-day | Every fix carries an inline `audit-N #M` reference comment so the regression test → fix → comment chain is auditable. The whole audit corpus is in the @@ -287,8 +287,8 @@ file:line evidence. Severity: | RFC § | Sev | Gap | Evidence | |---|---|---|---| -| RFC 9000 §17.3 | 🔴 | **Inbound short-header fixed-bit (0x40) not validated.** RFC says fixed-bit MUST be 1 on receive. Form-bit (0x80) and reserved-bit (0x18) ARE checked, but 0x40 is never asserted post-HP-unmask. | `ShortHeaderPacket.kt:163-187` (form-bit checked, fixed-bit not) | -| RFC 9000 §10.1 | 🔴 | **`max_idle_timeout` not enforced.** Decoded into config and advertised, but no idle deadline tracking, no auto-close on silence. Connection lives forever if no I/O happens. | `QuicConnection.kt:1040,1054,1069`; `QuicConnectionDriver.kt` (no idle timer) | +| RFC 9000 §17.2 + §17.3 | ~~🔴~~ | ~~**Inbound fixed-bit (0x40) not validated** on long- or short-header receive.~~ **Resolved 2026-05-09** — both `LongHeaderPacket.parseAndDecrypt`, `ShortHeaderPacket.parseAndDecrypt`, and `ShortHeaderPacket.peekKeyPhase` now reject fixed-bit=0 packets per RFC §17.2 / §17.3 (silent discard). Tests in `FixedBitValidationTest`. | +| RFC 9000 §10.1 | ~~🔴~~ | ~~**`max_idle_timeout` not enforced.**~~ **Resolved 2026-05-09** — `QuicConnection.lastActivityMs` updated on inbound packet receipt and outbound ack-eliciting send; `effectiveIdleTimeoutMs()` computes min(local, peer) with 3*PTO floor per §10.1; driver send-loop folds the deadline into its `withTimeoutOrNull` and silently closes via `markClosedExternally` per §10.2.1 on expiry. Tests in `IdleTimeoutTest` (8 cases). | | RFC 9000 §4.1 | 🟡 | **Connection-level inbound flow control missing.** Per-stream `receiveLimit` IS enforced (`QuicConnectionParser.kt:655`) — peer overshoot closes the connection. But the aggregate connection-level cap (matching what we advertised in `initial_max_data`) is not tracked on the receive side. | `QuicConnectionParser.kt:735-743` (only updates send-side credit on MAX_DATA) | | RFC 9000 §3 | 🟡 | **Stream state machine implicit, not validated.** The 10-state machine (Ready / Send / Data Sent / Data Recvd / Reset Sent / Reset Recvd × directions) is encoded as flags + buffer content. STREAM frames arriving after a RESET_STREAM are silently absorbed instead of raising STREAM_STATE_ERROR. (FIN-size mismatch *is* caught — `QuicConnectionParser.kt:672-685`.) | `QuicStream.kt:35-51` (no explicit state field) | | RFC 9000 §10.2 | 🟡 | **No DRAINING state.** Closing transitions go CONNECTED → CLOSING → CLOSED without the 3 × PTO hold the spec mandates. Late inbound packets after our own CONNECTION_CLOSE may not get a re-emitted close. | `QuicConnection.kt:301,1441-1531` (Status enum has no DRAINING) | 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 6436a32a0..647f9442e 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnection.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnection.kt @@ -296,7 +296,7 @@ class QuicConnection( */ @Volatile var peerTransportParameters: TransportParameters? = null - private set + internal set enum class Status { HANDSHAKING, CONNECTED, CLOSING, CLOSED } @@ -310,6 +310,29 @@ class QuicConnection( var status: Status = Status.HANDSHAKING internal set + /** + * RFC 9000 §10.1 idle-timeout tracking. The driver fires + * [markClosedExternally] (silent close, no CONNECTION_CLOSE per + * §10.2.1) when the time since this timestamp exceeds + * [effectiveIdleTimeoutMs]. + * + * Updated on: + * - successful inbound packet decrypt (parser) + * - outbound ack-eliciting packet emission (writer) + * + * Initialized to the connection's start time so the timer can fire + * even if no inbound packet ever arrives (e.g. handshake never + * completes on a black-holed path). + * + * @Volatile because the driver reads it from the send loop while the + * parser / writer mutate it from feed/drain coroutines under + * `streamsLock`. A torn long read would mis-arm the deadline by at + * most one packet's worth of time, but volatile is cheaper than the + * alternative (locking around every observation). + */ + @Volatile + internal var lastActivityMs: Long = nowMillis() + /** * Non-suspend monitor protecting the atomic transition of * [status] / [closeReason] / [closeErrorCode] from a non-CLOSED to a @@ -2318,6 +2341,48 @@ class QuicConnection( currentPtoMillis: Long = lossDetection.ptoBaseMs(peerTransportParameters?.maxAckDelay ?: 0L), ): PathMigrationResult = streamsLock.withLock { triggerPathMigrationLocked(nowMillis, currentPtoMillis) } + /** + * RFC 9000 §10.1: effective idle-timeout in milliseconds, or null + * if neither endpoint advertised a non-zero value (timeout is + * disabled). The effective timeout is the minimum of the + * locally-configured value and the peer's advertised value, after + * dropping any side that advertised 0 (per §18.2 a value of 0 + * means "no timeout"). + * + * §10.1 also requires we increase the timeout to at least 3 * PTO + * to avoid timing out before loss detection has had a chance to + * recover the path. + */ + internal fun effectiveIdleTimeoutMs(): Long? { + val local = config.maxIdleTimeoutMillis.takeIf { it > 0L } + val peer = peerTransportParameters?.maxIdleTimeoutMillis?.takeIf { it > 0L } + val agreed = + when { + local != null && peer != null -> minOf(local, peer) + local != null -> local + peer != null -> peer + else -> return null + } + // §10.1: floor at 3 * PTO so loss recovery has time to fire. + // Pre-handshake we don't have peer max_ack_delay yet, fall back + // to 0 (matches the writer's pre-handshake gating). + val maxAckDelayMs = peerTransportParameters?.maxAckDelay ?: 0L + val ptoBaseMs = lossDetection.ptoBaseMs(maxAckDelayMs).coerceAtLeast(1L) + val floor = ptoBaseMs * 3L + return agreed.coerceAtLeast(floor) + } + + /** + * RFC 9000 §10.1.1 idle-timer tick. Called by the driver after a + * [withTimeoutOrNull] expiry to check whether the silence has + * exceeded [effectiveIdleTimeoutMs]. Returns true on timeout + * (caller should silently close per §10.2.1). + */ + internal fun isIdleTimedOut(nowMillis: Long): Boolean { + val timeoutMs = effectiveIdleTimeoutMs() ?: return false + return nowMillis - lastActivityMs >= timeoutMs + } + /** * Check whether an outstanding PATH_CHALLENGE has exceeded the * 3 * PTO budget (RFC 9000 §8.2.4). Called from the driver's diff --git a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionDriver.kt b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionDriver.kt index f400498e3..642a86ddc 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionDriver.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionDriver.kt @@ -288,8 +288,22 @@ class QuicConnectionDriver( connection.handshake.nextLossTimeMs, connection.application.nextLossTimeMs, ).minOrNull()?.let { (it - nowForLossTimer).coerceAtLeast(0L) } + // RFC 9000 §10.1 idle-timeout deadline. Null when neither + // endpoint advertised a non-zero `max_idle_timeout`. We + // fold the remaining time into the same `withTimeoutOrNull` + // sleep so an idle connection wakes exactly at expiry + // instead of polling. + val idleTimeoutMs = connection.effectiveIdleTimeoutMs() + val idleDelta = + idleTimeoutMs?.let { + (connection.lastActivityMs + it - nowForLossTimer).coerceAtLeast(0L) + } val sleepMillis = - if (nextLossDelta != null && nextLossDelta < ptoMillis) nextLossDelta else ptoMillis + minOf( + ptoMillis, + nextLossDelta ?: Long.MAX_VALUE, + idleDelta ?: Long.MAX_VALUE, + ) // Suspend until either: a wakeup arrives, or the timer expires. val woke = withTimeoutOrNull(sleepMillis) { @@ -297,12 +311,20 @@ class QuicConnectionDriver( Unit } if (woke == null) { - // Distinguish loss-timer expiry from PTO expiry. A - // loss-timer wake just runs `detectAndRemoveLost` - // across the levels — newly time-threshold-lost - // packets get their tokens re-queued for retransmit on - // the next drain. A PTO wake additionally bumps the - // consecutive-PTO counter and arms the probe budget. + // Distinguish idle-timeout / loss-timer / PTO expiry. An + // idle-timeout wake silently closes the connection per + // RFC 9000 §10.2.1 ("the connection enters the closing + // state silently — discarding the connection state + // without sending a CONNECTION_CLOSE frame"). A + // loss-timer wake just runs `detectAndRemoveLost`. A + // PTO wake additionally bumps the consecutive-PTO + // counter and arms the probe budget. + if (idleTimeoutMs != null && connection.isIdleTimedOut(nowMillis())) { + connection.markClosedExternally( + "idle timeout ($idleTimeoutMs ms with no activity)", + ) + continue + } val pickedLossTimer = nextLossDelta != null && nextLossDelta < ptoMillis if (pickedLossTimer) { handleLossTimerFired(connection) diff --git a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionParser.kt b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionParser.kt index 08e791029..1d0042158 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionParser.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionParser.kt @@ -259,6 +259,9 @@ private fun feedLongHeaderPacket( } state.pnSpace.observeInbound(parsed.packet.packetNumber, nowMillis) + // RFC 9000 §10.1.1: any successfully processed inbound packet + // resets the idle timer. + conn.lastActivityMs = nowMillis // The server's source CID becomes our destination CID for subsequent packets. if (level == EncryptionLevel.INITIAL) { @@ -450,6 +453,9 @@ private fun feedShortHeaderPacket( conn.keyUpdateInProgress = false } state.pnSpace.observeInbound(parsed.packet.packetNumber, nowMillis) + // RFC 9000 §10.1.1: any successfully processed inbound packet + // resets the idle timer. + conn.lastActivityMs = nowMillis if (conn.qlogObserver !== com.vitorpamplona.quic.observability.QlogObserver.NoOp) { conn.qlogObserver.onPacketReceived( level = EncryptionLevel.APPLICATION, 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 89a8b9cfa..137255ac2 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionWriter.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionWriter.kt @@ -544,14 +544,19 @@ private fun buildLongHeaderFromFrames( // map insert below overwrites the prior entry, so the recorded // SentPacket reflects the final padded packet's size — correct // for retransmit purposes. + val ackEliciting = isAckEliciting(frames) state.sentPackets[pn] = SentPacket( packetNumber = pn, sentAtMillis = nowMillis, - ackEliciting = isAckEliciting(frames), + ackEliciting = ackEliciting, sizeBytes = packet.size, tokens = tokens, ) + // RFC 9000 §10.1.1: an ack-eliciting outbound packet resets the + // idle timer. (Non-ack-eliciting packets — pure ACK or PADDING-only + // — do NOT reset.) + if (ackEliciting) conn.lastActivityMs = nowMillis emitQlogSent(conn, level, pn, packet.size, frames) return packet } @@ -866,14 +871,19 @@ private fun buildApplicationPacket( ) } } + val ackEliciting = isAckEliciting(frames) state.sentPackets[pn] = SentPacket( packetNumber = pn, sentAtMillis = nowMillis, - ackEliciting = isAckEliciting(frames), + ackEliciting = ackEliciting, sizeBytes = sizeBytes.getOrNull()?.size ?: 0, tokens = tokens.toList(), ) + // RFC 9000 §10.1.1: an ack-eliciting outbound packet resets the + // idle timer. (Non-ack-eliciting packets — pure ACK or PADDING-only + // — do NOT reset.) + if (ackEliciting) conn.lastActivityMs = nowMillis sizeBytes.getOrNull()?.let { built -> emitQlogSent(conn, EncryptionLevel.APPLICATION, pn, built.size, frames) } diff --git a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/packet/LongHeaderPacket.kt b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/packet/LongHeaderPacket.kt index c54d7b36a..63b4ec5a0 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/packet/LongHeaderPacket.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/packet/LongHeaderPacket.kt @@ -164,6 +164,13 @@ object LongHeaderPacket { val r = QuicReader(bytes, offset) val first = r.readByte() if ((first and 0x80) == 0) return null // not a long header — silently drop + // RFC 9000 §17.2: the fixed-bit (0x40) MUST be 1 in a v1 long-header + // packet. Packets with 0 here are not valid v1 packets and MUST be + // discarded. Version Negotiation (§17.2.1) is the one exception + // (its fixed-bit is unconstrained), but VN is detected upstream in + // [feedDatagram] before we get here, so any path that reaches + // parseAndDecrypt is required to have fixed-bit=1. + if ((first and 0x40) == 0) return null val typeBits = (first ushr 4) and 0x03 val type = LongHeaderType.fromTypeBits(typeBits) val version = r.readUint32().toInt() diff --git a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/packet/ShortHeaderPacket.kt b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/packet/ShortHeaderPacket.kt index 835861bac..21b8a6f42 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/packet/ShortHeaderPacket.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/packet/ShortHeaderPacket.kt @@ -130,6 +130,11 @@ object ShortHeaderPacket { if (offset >= bytes.size) return null val first = bytes[offset].toInt() and 0xFF if ((first and 0x80) != 0) return null + // RFC 9000 §17.3: short-header fixed-bit (0x40) MUST be 1. Packets + // with 0 here are not valid v1 packets and MUST be discarded. The + // bit is not header-protected (HP only XORs the low 5 bits), so we + // check it on the raw byte before paying for HP unmask + AEAD. + if ((first and 0x40) == 0) return null val pnOffset = offset + 1 + dcidLen val sampleStart = pnOffset + 4 if (sampleStart + 16 > bytes.size) return null @@ -162,6 +167,9 @@ object ShortHeaderPacket { if (offset >= bytes.size) return null val first = bytes[offset].toInt() and 0xFF if ((first and 0x80) != 0) return null + // RFC 9000 §17.3: short-header fixed-bit (0x40) MUST be 1. Packets + // with 0 here are not valid v1 packets and MUST be discarded. + if ((first and 0x40) == 0) return null val pnOffset = offset + 1 + dcidLen val sampleStart = pnOffset + 4 if (sampleStart + 16 > bytes.size) return null diff --git a/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/IdleTimeoutTest.kt b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/IdleTimeoutTest.kt new file mode 100644 index 000000000..f28214fde --- /dev/null +++ b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/IdleTimeoutTest.kt @@ -0,0 +1,172 @@ +/* + * 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.tls.PermissiveCertificateValidator +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertTrue + +/** + * RFC 9000 §10.1 idle-timeout enforcement. + * + * - §10.1: effective timeout = min of local + peer advertisements + * (skipping any side that advertised 0). Floored at 3 * PTO so loss + * recovery has time to fire before the connection times out. + * - §10.1.1: idle timer resets on (a) any successfully processed + * inbound packet, (b) outbound ack-eliciting packet emission. + * - §10.2.1: on expiry the connection enters CLOSED silently — no + * CONNECTION_CLOSE frame. + * + * Pre-fix `max_idle_timeout` was decoded into config + advertised in + * the ClientHello transport-params extension, but never enforced — a + * black-holed connection lived forever. + */ +class IdleTimeoutTest { + private fun newConnection( + nowMillis: () -> Long, + localIdleMs: Long, + ): QuicConnection = + QuicConnection( + serverName = "example.test", + config = + QuicConnectionConfig( + maxIdleTimeoutMillis = localIdleMs, + ), + tlsCertificateValidator = PermissiveCertificateValidator(), + nowMillis = nowMillis, + ) + + @Test + fun effective_timeout_uses_min_of_local_and_peer() { + val conn = newConnection(nowMillis = { 0L }, localIdleMs = 30_000L) + // No peer params yet → only local advertised. + assertEquals(30_000L, conn.effectiveIdleTimeoutMs()) + + // Peer advertises smaller — min wins. + conn.peerTransportParameters = TransportParameters(maxIdleTimeoutMillis = 10_000L) + assertEquals(10_000L, conn.effectiveIdleTimeoutMs()) + + // Peer advertises larger — local still smaller, local wins. + conn.peerTransportParameters = TransportParameters(maxIdleTimeoutMillis = 60_000L) + assertEquals(30_000L, conn.effectiveIdleTimeoutMs()) + } + + @Test + fun effective_timeout_skips_zero_advertisements() { + // Local 0, peer 30s → peer wins (RFC 9000 §18.2: 0 means disabled). + val conn = newConnection(nowMillis = { 0L }, localIdleMs = 0L) + conn.peerTransportParameters = TransportParameters(maxIdleTimeoutMillis = 30_000L) + assertEquals(30_000L, conn.effectiveIdleTimeoutMs()) + + // Both 0 / unset → disabled (null). + val conn2 = newConnection(nowMillis = { 0L }, localIdleMs = 0L) + assertNull(conn2.effectiveIdleTimeoutMs()) + + val conn3 = newConnection(nowMillis = { 0L }, localIdleMs = 0L) + conn3.peerTransportParameters = TransportParameters(maxIdleTimeoutMillis = 0L) + assertNull(conn3.effectiveIdleTimeoutMs()) + } + + @Test + fun effective_timeout_floored_at_three_PTO() { + // §10.1: floor = 3 * PTO. Pre-handshake PTO base ≈ + // smoothed_rtt(100ms) + max(4*rttvar(50ms), 1ms) + max_ack_delay(0) + // = 300 ms; 3 * 300 = 900 ms. + // A configured 200 ms idle timeout MUST be floored, not honored + // verbatim, otherwise a one-RTT loss recovery period would + // outlast the connection. + val conn = newConnection(nowMillis = { 0L }, localIdleMs = 200L) + val effective = conn.effectiveIdleTimeoutMs() + assertNotNull(effective) + assertTrue(effective >= 900L, "expected floor of 900 ms, got $effective") + } + + @Test + fun isIdleTimedOut_false_at_construction() { + val now = mutableLongOf(0L) + val conn = newConnection(nowMillis = now::get, localIdleMs = 10_000L) + assertFalse(conn.isIdleTimedOut(now.get())) + } + + @Test + fun isIdleTimedOut_false_before_deadline() { + val now = mutableLongOf(0L) + val conn = newConnection(nowMillis = now::get, localIdleMs = 10_000L) + // Just under the deadline. + now.set(9_999L) + assertFalse(conn.isIdleTimedOut(now.get())) + } + + @Test + fun isIdleTimedOut_true_at_deadline() { + val now = mutableLongOf(0L) + val conn = newConnection(nowMillis = now::get, localIdleMs = 10_000L) + now.set(10_000L) + assertTrue(conn.isIdleTimedOut(now.get())) + } + + @Test + fun lastActivityMs_bump_postpones_deadline() { + val now = mutableLongOf(0L) + val conn = newConnection(nowMillis = now::get, localIdleMs = 10_000L) + // Halfway through the idle window an inbound packet arrives. + now.set(5_000L) + conn.lastActivityMs = now.get() + // Deadline shifts to 15_000 — still inside at 14_000. + now.set(14_000L) + assertFalse(conn.isIdleTimedOut(now.get())) + // Past the new deadline. + now.set(15_000L) + assertTrue(conn.isIdleTimedOut(now.get())) + } + + @Test + fun isIdleTimedOut_never_when_disabled() { + val now = mutableLongOf(0L) + val conn = newConnection(nowMillis = now::get, localIdleMs = 0L) + // Even at simulated wallclock infinity, no timeout is enforced. + now.set(Long.MAX_VALUE / 2) + assertFalse(conn.isIdleTimedOut(now.get())) + } +} + +/** + * Minimal mutable Long holder for clock injection. Avoids depending on + * kotlinx.atomicfu in test sources where atomicity isn't needed (the + * clock is bumped from the test thread, read from the same thread). + */ +private class MutableLong( + initial: Long, +) { + private var value = initial + + fun get(): Long = value + + fun set(v: Long) { + value = v + } +} + +private fun mutableLongOf(initial: Long): MutableLong = MutableLong(initial) diff --git a/quic/src/commonTest/kotlin/com/vitorpamplona/quic/packet/FixedBitValidationTest.kt b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/packet/FixedBitValidationTest.kt new file mode 100644 index 000000000..c4bcc07dc --- /dev/null +++ b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/packet/FixedBitValidationTest.kt @@ -0,0 +1,192 @@ +/* + * 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.packet + +import com.vitorpamplona.quic.connection.ConnectionId +import com.vitorpamplona.quic.crypto.Aes128Gcm +import com.vitorpamplona.quic.crypto.AesEcbHeaderProtection +import com.vitorpamplona.quic.crypto.InitialSecrets +import com.vitorpamplona.quic.crypto.PlatformAesOneBlock +import kotlin.test.Test +import kotlin.test.assertNotNull +import kotlin.test.assertNull + +/** + * RFC 9000 §17.2 (long-header) and §17.3 (short-header): the fixed-bit + * (0x40) of the first byte MUST be 1 in every v1 packet. Packets with + * fixed-bit=0 MUST be discarded. + * + * Pre-fix the parsers checked only the form-bit (0x80) and accepted the + * packet regardless of fixed-bit. A peer (or off-path attacker) sending a + * packet with fixed-bit=0 would have its bytes routed through AEAD — + * cheap denial-of-service amplifier and a wire-spec violation we accepted + * silently. + * + * The fixed-bit is NOT header-protected (HP only XORs the low five bits + * of the first byte), so we can validate it on the raw wire byte BEFORE + * paying for HP unmask + AEAD. Both `parseAndDecrypt` and the + * `peekKeyPhase` shortcut do the check. + */ +class FixedBitValidationTest { + private val dcid = ConnectionId("8394c8f03e515708".hexToByteArray()) + private val scid = ConnectionId("01020304".hexToByteArray()) + private val proto = InitialSecrets.derive(dcid.bytes) + private val hp = AesEcbHeaderProtection(PlatformAesOneBlock) + + @Test + fun long_header_fixed_bit_zero_is_discarded() { + val plain = + LongHeaderPlaintextPacket( + type = LongHeaderType.INITIAL, + dcid = dcid, + scid = scid, + packetNumber = 0L, + payload = "01".hexToByteArray(), + ) + val wire = + LongHeaderPacket.build( + plain = plain, + aead = Aes128Gcm, + key = proto.clientKey, + iv = proto.clientIv, + hp = hp, + hpKey = proto.clientHp, + largestAckedInSpace = -1L, + ) + + // Sanity check: the unmodified wire round-trips. + val good = + LongHeaderPacket.parseAndDecrypt( + bytes = wire, + offset = 0, + aead = Aes128Gcm, + key = proto.clientKey, + iv = proto.clientIv, + hp = hp, + hpKey = proto.clientHp, + largestReceivedInSpace = -1L, + ) + assertNotNull(good) + + // Flip fixed-bit (0x40) on the wire's first byte. The bit is NOT + // header-protected, so flipping it does not affect HP unmask of + // the type / pnLen bits — the packet is structurally valid in + // every other way; it just isn't a v1 packet anymore. + val tampered = wire.copyOf() + tampered[0] = (tampered[0].toInt() xor 0x40).toByte() + val parsed = + LongHeaderPacket.parseAndDecrypt( + bytes = tampered, + offset = 0, + aead = Aes128Gcm, + key = proto.clientKey, + iv = proto.clientIv, + hp = hp, + hpKey = proto.clientHp, + largestReceivedInSpace = -1L, + ) + assertNull(parsed, "RFC 9000 §17.2: long-header fixed-bit=0 MUST be discarded") + } + + @Test + fun short_header_fixed_bit_zero_is_discarded_in_parseAndDecrypt() { + val plain = + ShortHeaderPlaintextPacket( + dcid = dcid, + packetNumber = 0L, + payload = byteArrayOf(0x01), + ) + val wire = + ShortHeaderPacket.build( + plain = plain, + aead = Aes128Gcm, + key = proto.clientKey, + iv = proto.clientIv, + hp = hp, + hpKey = proto.clientHp, + largestAckedInSpace = -1L, + ) + + val good = + ShortHeaderPacket.parseAndDecrypt( + bytes = wire, + offset = 0, + dcidLen = dcid.length, + aead = Aes128Gcm, + key = proto.clientKey, + iv = proto.clientIv, + hp = hp, + hpKey = proto.clientHp, + largestReceivedInSpace = -1L, + ) + assertNotNull(good) + + val tampered = wire.copyOf() + tampered[0] = (tampered[0].toInt() xor 0x40).toByte() + val parsed = + ShortHeaderPacket.parseAndDecrypt( + bytes = tampered, + offset = 0, + dcidLen = dcid.length, + aead = Aes128Gcm, + key = proto.clientKey, + iv = proto.clientIv, + hp = hp, + hpKey = proto.clientHp, + largestReceivedInSpace = -1L, + ) + assertNull(parsed, "RFC 9000 §17.3: short-header fixed-bit=0 MUST be discarded") + } + + @Test + fun short_header_fixed_bit_zero_is_discarded_in_peekKeyPhase() { + val plain = + ShortHeaderPlaintextPacket( + dcid = dcid, + packetNumber = 0L, + payload = byteArrayOf(0x01), + ) + val wire = + ShortHeaderPacket.build( + plain = plain, + aead = Aes128Gcm, + key = proto.clientKey, + iv = proto.clientIv, + hp = hp, + hpKey = proto.clientHp, + largestAckedInSpace = -1L, + ) + // peekKeyPhase is the upstream gate the parser uses to pick keys + // before AEAD; if it accepts a fixed-bit=0 packet we waste an + // AEAD attempt before parseAndDecrypt drops it. + val tampered = wire.copyOf() + tampered[0] = (tampered[0].toInt() xor 0x40).toByte() + val peek = + ShortHeaderPacket.peekKeyPhase( + bytes = tampered, + offset = 0, + dcidLen = dcid.length, + hp = hp, + hpKey = proto.clientHp, + ) + assertNull(peek, "RFC 9000 §17.3: peekKeyPhase MUST reject fixed-bit=0") + } +}