From be96d4b28f7cccb06c4d687bf1e172fc3e12a8ae Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 9 May 2026 04:46:49 +0000 Subject: [PATCH] =?UTF-8?q?fix(quic):=20RFC=209000=20=C2=A73.2=20stream=20?= =?UTF-8?q?state=20=E2=80=94=20STREAM=20after=20RESET=5FSTREAM=20rejected?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Once the peer has sent RESET_STREAM the receive side enters the "Reset Recvd" terminal state per §3.2; any subsequent STREAM frame on that id is a peer protocol violation and MUST close the connection with STREAM_STATE_ERROR. Pre-fix the parser silently absorbed the bytes — a peer that violated the spec would leave us with a phantom mid-reset stream the peer believed was already dead, with reset/byte bookkeeping diverging. Implementation: a per-stream `peerResetReceived: Boolean` flag set in the RESET_STREAM handler and checked at the top of the STREAM handler. The flag is independent of our local-side `resetState` (which tracks OUR RESET emission). Test: `StreamAfterResetTest` (4 cases) covers STREAM-after-reset closure, FIN-bearing STREAM-after-reset, the legal FIN-then-RESET shape (no false-positive), and per-stream isolation (reset on stream 3 doesn't poison stream 7). https://claude.ai/code/session_01XGmmaVuy2wsSZQx2AqN2ik --- .../quic/connection/QuicConnectionParser.kt | 18 +++ .../vitorpamplona/quic/stream/QuicStream.kt | 15 +++ .../quic/connection/StreamAfterResetTest.kt | 113 ++++++++++++++++++ 3 files changed, 146 insertions(+) create mode 100644 quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/StreamAfterResetTest.kt 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 3778bd5fb1..f663e39b9f 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionParser.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionParser.kt @@ -665,6 +665,19 @@ private fun dispatchFrames( continue } val stream = conn.getOrCreatePeerStreamLocked(frame.streamId) + // RFC 9000 §3.2: once the peer has sent RESET_STREAM the + // receive side is in the "Reset Recvd" terminal state. + // Any subsequent STREAM frame on this id is a peer + // protocol violation — close with STREAM_STATE_ERROR. + // Pre-fix the bytes were silently absorbed, leaving the + // application with a phantom mid-reset stream that the + // peer believed was already dead. + if (stream.peerResetReceived) { + conn.markClosedExternally( + "STREAM_STATE_ERROR: stream ${frame.streamId} STREAM after RESET_STREAM", + ) + return + } // RFC 9000 §4.1: peer MUST NOT send beyond the limit we advertised. // The connection-level kill protects against unbounded memory // growth from a misbehaving peer. @@ -868,6 +881,11 @@ private fun dispatchFrames( return } } + // RFC 9000 §3.2: latch the receive-side terminal state so + // any subsequent STREAM frame on this id closes the + // connection with STREAM_STATE_ERROR (handled in the + // STREAM branch above). + target?.peerResetReceived = true // Mark the peer's stream aborted and close our read side; the // application sees a truncated incoming flow. target?.closeIncoming() diff --git a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/stream/QuicStream.kt b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/stream/QuicStream.kt index e2c54d34d1..f25bbaea41 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/stream/QuicStream.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/stream/QuicStream.kt @@ -142,6 +142,21 @@ class QuicStream( @Volatile internal var receiveHighestOffset: Long = 0L + /** + * RFC 9000 §3.2 receive-side state: true once a `RESET_STREAM` for + * this stream has been delivered by the peer. Subsequent inbound + * STREAM frames on this id are invalid (the receive side is in the + * "Reset Recvd" terminal state) and must close the connection with + * STREAM_STATE_ERROR. + * + * Kept distinct from the local-side [resetState] (which tracks OUR + * RESET_STREAM emission). Both can exist simultaneously: a bidi + * stream can be reset by the peer's send side (this flag) while we + * separately reset our own send side. + */ + @Volatile + internal var peerResetReceived: Boolean = false + /** * Marker the parser sets whenever [receive.contiguousEnd] advances; the * writer's appendFlowControlUpdates consumes it to skip streams that diff --git a/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/StreamAfterResetTest.kt b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/StreamAfterResetTest.kt new file mode 100644 index 0000000000..8f5da5c444 --- /dev/null +++ b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/StreamAfterResetTest.kt @@ -0,0 +1,113 @@ +/* + * 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.ResetStreamFrame +import com.vitorpamplona.quic.frame.StreamFrame +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertTrue + +/** + * RFC 9000 §3.2 receive-side stream state machine. Once the peer has + * sent RESET_STREAM for a stream, the receive side enters the "Reset + * Recvd" terminal state. Subsequent STREAM frames on the same id are + * peer protocol violations and MUST close the connection with + * STREAM_STATE_ERROR. + * + * Pre-fix the parser silently absorbed STREAM frames after RESET — a + * peer that violated the spec would leave us with a phantom mid-reset + * stream that the peer believed was already dead, with diverging + * reset/byte bookkeeping. + */ +class StreamAfterResetTest { + @Test + fun stream_frame_after_reset_closes_with_stream_state_error() { + val (client, pipe) = newConnectedClient(maxStreamData = 64 * 1024) + // Stream 3 (server-uni) — peer pushes a few bytes then resets at + // finalSize = highest seen (legal). Receive side is now in + // Reset Recvd terminal state. + val streamData = StreamFrame(streamId = 3L, offset = 0L, data = ByteArray(8), fin = false) + val reset = ResetStreamFrame(streamId = 3L, applicationErrorCode = 0L, finalSize = 8L) + val firstDgram = pipe.buildServerApplicationDatagram(listOf(streamData, reset))!! + feedDatagram(client, firstDgram, nowMillis = 0L) + assertEquals(QuicConnection.Status.CONNECTED, client.status) + + // Now peer (illegally) sends another STREAM frame on the same id. + val second = StreamFrame(streamId = 3L, offset = 8L, data = ByteArray(4), fin = false) + val secondDgram = pipe.buildServerApplicationDatagram(listOf(second))!! + feedDatagram(client, secondDgram, nowMillis = 0L) + + assertEquals(QuicConnection.Status.CLOSED, client.status) + val reason = client.closeReason + assertNotNull(reason) + assertTrue(reason.contains("STREAM_STATE_ERROR"), "expected STREAM_STATE_ERROR, got: $reason") + assertTrue(reason.contains("3"), "reason should reference the offending stream id: $reason") + } + + @Test + fun stream_frame_with_fin_after_reset_also_rejected() { + // FIN is the same path — STREAM with fin=true is still a + // STREAM frame, so it falls under the same Reset Recvd guard. + val (client, pipe) = newConnectedClient(maxStreamData = 64 * 1024) + val streamData = StreamFrame(streamId = 3L, offset = 0L, data = ByteArray(8), fin = false) + val reset = ResetStreamFrame(streamId = 3L, applicationErrorCode = 0L, finalSize = 8L) + feedDatagram(client, pipe.buildServerApplicationDatagram(listOf(streamData, reset))!!, nowMillis = 0L) + + val finFrame = StreamFrame(streamId = 3L, offset = 8L, data = ByteArray(0), fin = true) + feedDatagram(client, pipe.buildServerApplicationDatagram(listOf(finFrame))!!, nowMillis = 0L) + + assertEquals(QuicConnection.Status.CLOSED, client.status) + assertTrue(client.closeReason!!.contains("STREAM_STATE_ERROR")) + } + + @Test + fun reset_after_clean_fin_still_accepted() { + // FIN-then-RESET is allowed — RESET on a stream that already + // FIN'd is a no-op on receive side (state was already terminal + // via Data Recvd; RESET is silently OK as long as finalSize + // matches). We don't exercise STREAM-after-this; the reverse + // ordering is what the test above covers. This case just + // ensures the §4.5 finalSize-matches-FIN check is preserved. + val (client, pipe) = newConnectedClient(maxStreamData = 64 * 1024) + val finFrame = StreamFrame(streamId = 3L, offset = 0L, data = ByteArray(8), fin = true) + val reset = ResetStreamFrame(streamId = 3L, applicationErrorCode = 0L, finalSize = 8L) + feedDatagram(client, pipe.buildServerApplicationDatagram(listOf(finFrame, reset))!!, nowMillis = 0L) + assertEquals(QuicConnection.Status.CONNECTED, client.status) + } + + @Test + fun two_streams_independent_states() { + // Reset on stream 3 must NOT prevent legitimate STREAM frames on + // stream 7. The flag is per-stream, not connection-wide. + val (client, pipe) = newConnectedClient(maxStreamData = 64 * 1024) + val s3data = StreamFrame(streamId = 3L, offset = 0L, data = ByteArray(8), fin = false) + val s3reset = ResetStreamFrame(streamId = 3L, applicationErrorCode = 0L, finalSize = 8L) + val s7data = StreamFrame(streamId = 7L, offset = 0L, data = ByteArray(8), fin = false) + feedDatagram(client, pipe.buildServerApplicationDatagram(listOf(s3data, s3reset, s7data))!!, nowMillis = 0L) + assertEquals(QuicConnection.Status.CONNECTED, client.status) + // Subsequent STREAM on still-open stream 7 — fine. + val s7more = StreamFrame(streamId = 7L, offset = 8L, data = ByteArray(8), fin = false) + feedDatagram(client, pipe.buildServerApplicationDatagram(listOf(s7more))!!, nowMillis = 0L) + assertEquals(QuicConnection.Status.CONNECTED, client.status) + } +}