fix(quic): RFC 9000 §3.2 stream state — STREAM after RESET_STREAM rejected
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
This commit is contained in:
@@ -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()
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user