mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-07-30 19:46:17 +00:00
fix(quic): RFC 9114 §6.2.1 critical-stream auto-close + WiFi-handoff stateless-reset tokens
Two related improvements for the audio-rooms moq-lite path on mobile.
** RFC 9114 §6.2.1 + RFC 9204 §4.2 critical-stream closure **
When the peer closes a critical unidirectional stream — control (0x00),
QPACK encoder (0x02), or QPACK decoder (0x03) — the spec MANDATES we
treat it as a connection error of type H3_CLOSED_CRITICAL_STREAM.
Pre-fix the demux only set `peerH3ProtocolError` flag on protocol
violations, with no autonomous close on clean FIN; the application
was expected to poll. For audio rooms running on lossy mobile paths,
relay-side control-stream drops left users staring at silent audio.
* `Http3ErrorCode` constants for the §8.1 error-code table.
* `WtPeerStreamDemux.drainControlStream` calls `connection.close(...)`
on either clean FIN (H3_CLOSED_CRITICAL_STREAM) or QuicCodecException
raised by the frame reader (specific code via
[http3ErrorCodeForMessage] map: H3_MISSING_SETTINGS,
H3_FRAME_UNEXPECTED, H3_SETTINGS_ERROR, H3_ID_ERROR, etc.).
* New `drainCriticalStream` helper fires the same close on QPACK
encoder / decoder FIN (RFC 9204 §4.2).
* Closure intent latched on `criticalStreamClosureCode` /
`criticalStreamClosureReason` so tests can verify the path runs
without wiring a real driver.
** RFC 9000 §10.3 stateless-reset tokens for migrated CIDs **
The 2026-05-09 stateless-reset detection covered tokens in the
unused-CID pool but lost them the moment a CID was rotated to active
via `tryStartValidation` (WiFi handoff) or `forceRotateToHigherSequence`
(peer-forced retire). On the new path the migrated token wouldn't
match — relay crash mid-handoff would silently hang until idle timeout.
* `PathValidator.knownStatelessResetTokens` — append-only lifetime
store, populated in `recordPeerNewConnectionId`. Survives rotation.
* `QuicConnection.isStatelessReset` walks the lifetime store instead
of the unused pool.
* §10.3.1 explicitly permits keeping tokens after retirement; cost
is ~16 bytes per peer-issued CID.
Tests:
* `CriticalStreamClosureTest` (5 cases) — control FIN, QPACK FIN,
MISSING_SETTINGS, FRAME_UNEXPECTED, idempotency.
* `StatelessResetDetectionTest` extended with 2 cases —
`token_persists_after_path_migration_for_wifi_handoff` and
`token_persists_through_force_rotation_for_acid_reissue`.
Plan updated: 🟦 H3_CLOSED_CRITICAL_STREAM resolved; the
"limitation: tokens for actively-used DCIDs we migrated to" caveat is
removed from the §10.3 entry and from known-limitations item #5.
https://claude.ai/code/session_01XGmmaVuy2wsSZQx2AqN2ik
This commit is contained in:
Claude
@@ -257,10 +257,14 @@ not tackled:
|
||||
validation per RFC 9000 §8.2 + §9. See `PathValidator.kt` and the
|
||||
three test files (`PathValidatorTest`, `PathValidationTest`,
|
||||
`ClientPathMigrationTest`).
|
||||
5. **Stateless reset detection.** Stateless-reset tokens are now
|
||||
*recorded* (per `PeerConnectionIdEntry`) but no inbound check
|
||||
compares "looks-like-noise" datagrams against the token list. RFC
|
||||
9000 §10.3.
|
||||
5. ~~**Stateless reset detection.**~~ **Resolved 2026-05-09** —
|
||||
`QuicConnection.isStatelessReset` matches every short-header-form
|
||||
datagram's trailing 16 bytes against the peer's transport-param
|
||||
token AND every NEW_CONNECTION_ID token retained in
|
||||
`PathValidator.allKnownStatelessResetTokens` (lifetime store —
|
||||
covers the WiFi-handoff case where the migrated CID's token would
|
||||
otherwise be lost the moment migration started). Constant-time per
|
||||
§10.3.1; silent CLOSED transition on match.
|
||||
6. ~~**`AckTracker.purgeBelow` threshold semantics.**~~ **Resolved
|
||||
2026-05-05.**
|
||||
7. **Driver direct unit tests** require turning `UdpSocket` from `expect
|
||||
@@ -292,7 +296,7 @@ file:line evidence. Severity:
|
||||
| RFC 9000 §4.1 | ~~🟡~~ | ~~**Connection-level inbound flow control missing.**~~ **Resolved 2026-05-09** — `QuicStream.receiveHighestOffset` + `QuicConnection.connectionInboundOffsetSum` track the §4.1 spec quantity (sum of largest-received-offset across streams). Parser closes with FLOW_CONTROL_ERROR when the sum exceeds `advertisedMaxData`. RESET_STREAM finalSize counted toward the limit per §4.5. Tests in `ConnectionLevelFlowControlTest`. |
|
||||
| RFC 9000 §3 | ~~🟡~~ | ~~**Stream state machine implicit, not validated.**~~ **Resolved 2026-05-09** — `QuicStream.peerResetReceived` latches when a RESET_STREAM arrives; the parser closes with STREAM_STATE_ERROR on any subsequent STREAM frame for the same id. (Other state transitions — FIN-size mismatch, illegal peer-initiated opens — were already caught.) Tests in `StreamAfterResetTest`. |
|
||||
| RFC 9000 §10.2 | ~~🟡~~ | ~~**No DRAINING state.**~~ **Resolved 2026-05-09** — `Status.DRAINING` added; parser routes peer's CONNECTION_CLOSE through `enterDraining` (sets status + 3 × PTO deadline) instead of immediate CLOSED. Driver folds the deadline into its send-loop sleep and flips to CLOSED on expiry. Late inbound during DRAINING is silently dropped at the top of `feedDatagramInner`. Tests in `DrainingStateTest`. |
|
||||
| RFC 9000 §10.3 | ~~🟡~~ | ~~**Stateless reset detection missing.**~~ **Resolved 2026-05-09** — `QuicConnection.isStatelessReset` checks the trailing 16 bytes of every short-header-form datagram against the peer's `statelessResetToken` AND every unused entry in `pathValidator`'s pool, in constant time per §10.3.1. On match, silently transitions to CLOSED. Tests in `StatelessResetDetectionTest`. |
|
||||
| RFC 9000 §10.3 | ~~🟡~~ | ~~**Stateless reset detection missing.**~~ **Resolved 2026-05-09** — `QuicConnection.isStatelessReset` checks the trailing 16 bytes of every short-header-form datagram against the peer's `statelessResetToken` AND every NEW_CONNECTION_ID token in `PathValidator.allKnownStatelessResetTokens` (lifetime store — extends past CID rotation / retirement so WiFi↔cellular handoffs still detect reset on the new path), in constant time per §10.3.1. On match, silently transitions to CLOSED. Tests in `StatelessResetDetectionTest` (7 cases including handoff + force-rotation). |
|
||||
| RFC 9001 §6.6 | ~~🟡~~ | ~~**AEAD invocation limit not tracked.**~~ **Resolved 2026-05-09** — `Aead.confidentialityLimit` / `integrityLimit` properties surface the §B.1 values; `QuicConnection.aeadEncryptCount` / `aeadDecryptFailureCount` track per-key usage and reset on rotation. Writer triggers a key update at half the confidentiality limit and closes with AEAD_LIMIT_REACHED if rotation can't complete; parser closes if decrypt failures hit the integrity limit. Tests in `AeadInvocationLimitTest`. |
|
||||
|
||||
### Transport-parameter bounds
|
||||
@@ -310,7 +314,7 @@ file:line evidence. Severity:
|
||||
|---|---|---|---|
|
||||
| RFC 9114 §7.2.4.1 | ~~🟡~~ | ~~**HTTP/2 reserved SETTINGS ids 0x02/0x03/0x04/0x05 not rejected.**~~ **Resolved 2026-05-09** — `Http3Settings.decodeBody` raises H3_SETTINGS_ERROR for ids 0x02..0x05. Tests in `Http3ReservedSettingsTest`. |
|
||||
| RFC 9221 §3 | ~~🟡~~ | ~~**Outbound DATAGRAM size not gated.**~~ **Resolved 2026-05-09** — writer drops outbound DATAGRAM frames when peer didn't advertise `max_datagram_frame_size` or when the encoded frame would exceed the advertised value. Diagnostic via `qlogObserver.onPacketDropped`. |
|
||||
| RFC 9114 §6.2.1 | 🟦 | **Closing the control stream surfaces a flag rather than auto-closing.** Spec says closing the control stream is H3_CLOSED_CRITICAL_STREAM. Caller must poll `peerH3ProtocolError` and close. | `Http3FrameReader.kt`; `WtPeerStreamDemux.kt` |
|
||||
| RFC 9114 §6.2.1 + RFC 9204 §4.2 | ~~🟦~~ | ~~**Closing the control stream surfaces a flag rather than auto-closing.**~~ **Resolved 2026-05-09** — `WtPeerStreamDemux` now drives `connection.close(errorCode, reason)` itself when ANY critical unidirectional stream closes (control + QPACK encoder + QPACK decoder), whether by clean FIN (H3_CLOSED_CRITICAL_STREAM) or HTTP/3 protocol violation (mapped to the specific §8.1 code where possible). New `Http3ErrorCode` constants. Tests in `CriticalStreamClosureTest` (5 cases) cover FIN-on-control, FIN-on-QPACK, MISSING_SETTINGS, FRAME_UNEXPECTED, and idempotency. Audio rooms now get an immediate disconnect event when the relay drops the control stream mid-session. |
|
||||
|
||||
### TLS / error reporting cosmetics
|
||||
|
||||
|
||||
@@ -289,6 +289,48 @@ class PathValidator(
|
||||
*/
|
||||
fun unusedTokenForSequence(sequenceNumber: Long): ByteArray? = unusedCids[sequenceNumber]?.statelessResetToken
|
||||
|
||||
/**
|
||||
* RFC 9000 §10.3 stateless-reset support, audio-rooms WiFi-handoff
|
||||
* extension. Every stateless-reset token the peer has ever issued
|
||||
* us via NEW_CONNECTION_ID — whether the corresponding CID is
|
||||
* still in [unusedCids], currently active (rotated to via
|
||||
* [tryStartValidation] / [forceRotateToHigherSequence]), or
|
||||
* RETIRE_CONNECTION_ID'd.
|
||||
*
|
||||
* Why retain after rotation/retire: when the user roams between
|
||||
* WiFi and cellular, we migrate the connection to a fresh DCID
|
||||
* via [tryStartValidation]. If the peer (relay) loses connection
|
||||
* state during the handoff (crash, restart, NAT rebinding), it
|
||||
* sends a stateless reset using whichever of OUR-issued source
|
||||
* CIDs it last had cached — which corresponds to whichever
|
||||
* stateless-reset token it remembers issuing for that CID. We
|
||||
* MUST be able to match the reset against the right token. If we
|
||||
* only kept tokens for CIDs in the unused pool (the pre-fix
|
||||
* shape), the rotated-to active CID's token would be lost the
|
||||
* moment migration started — a stateless reset on the new path
|
||||
* would look like noise and we'd hang until idle timeout.
|
||||
*
|
||||
* §10.3.1 explicitly allows endpoints to keep tokens after
|
||||
* retirement: "An endpoint MAY drop a Stateless Reset Token
|
||||
* after retiring the corresponding connection ID; this saves
|
||||
* storage at the cost of being more vulnerable to spoofing if
|
||||
* the same token is reused by an attacker." For audio-rooms the
|
||||
* extra ~16 bytes per peer-issued CID is trivial.
|
||||
*
|
||||
* Returned in insertion order (oldest first); caller treats it
|
||||
* as an unordered set for matching.
|
||||
*/
|
||||
fun allKnownStatelessResetTokens(): List<ByteArray> = knownStatelessResetTokens
|
||||
|
||||
/**
|
||||
* Lifetime store of every stateless-reset token the peer has
|
||||
* ever advertised via NEW_CONNECTION_ID. Append-only — entries
|
||||
* are NOT removed when the corresponding CID is rotated out of
|
||||
* [unusedCids] or RETIRE_CONNECTION_ID'd. See
|
||||
* [allKnownStatelessResetTokens] for the rationale.
|
||||
*/
|
||||
private val knownStatelessResetTokens: ArrayList<ByteArray> = ArrayList()
|
||||
|
||||
/**
|
||||
* Record a peer-issued `NEW_CONNECTION_ID` (RFC 9000 §19.15).
|
||||
* Returns the result code so the caller (parser) can decide
|
||||
@@ -375,12 +417,19 @@ class PathValidator(
|
||||
|
||||
if (unusedCids.size >= maxUnusedCids) return RecordResult.PoolFull
|
||||
|
||||
val tokenCopy = statelessResetToken.copyOf()
|
||||
unusedCids[sequenceNumber] =
|
||||
PeerConnectionIdEntry(
|
||||
sequenceNumber = sequenceNumber,
|
||||
connectionId = connectionId.copyOf(),
|
||||
statelessResetToken = statelessResetToken.copyOf(),
|
||||
statelessResetToken = tokenCopy,
|
||||
)
|
||||
// Append to the lifetime store so the token is still
|
||||
// matchable after a future [tryStartValidation] /
|
||||
// [forceRotateToHigherSequence] removes the entry from
|
||||
// [unusedCids]. See [allKnownStatelessResetTokens] for
|
||||
// rationale (WiFi-handoff stateless-reset detection).
|
||||
knownStatelessResetTokens.add(tokenCopy)
|
||||
return RecordResult.Stored
|
||||
}
|
||||
|
||||
|
||||
@@ -2589,16 +2589,15 @@ class QuicConnection(
|
||||
*
|
||||
* Token sources covered:
|
||||
* - The peer's `stateless_reset_token` transport parameter (the
|
||||
* sequence-0 / initial CID's token), iff this is a
|
||||
* server-issued connection.
|
||||
* - Each unused entry in [pathValidator]'s pool (peer issued
|
||||
* via NEW_CONNECTION_ID).
|
||||
*
|
||||
* Not covered (limitation of the current path-migration plumbing):
|
||||
* tokens for an actively-used DCID that we migrated to via
|
||||
* [PathValidator.tryStartValidation] — we lose the token when the
|
||||
* entry leaves the pool. The vast majority of clients never
|
||||
* migrate; this gap is acceptable for the audio-rooms scope.
|
||||
* sequence-0 / initial CID's token).
|
||||
* - Every NEW_CONNECTION_ID token the peer has ever sent
|
||||
* ([PathValidator.allKnownStatelessResetTokens]) — including
|
||||
* tokens for CIDs that have since been rotated-to-active or
|
||||
* RETIRE_CONNECTION_ID'd. The lifetime retention covers the
|
||||
* WiFi↔cellular handoff path: when we migrate to a new DCID
|
||||
* and the peer crashes mid-handoff, the stateless reset on
|
||||
* the new path uses the migrated CID's token, which is no
|
||||
* longer in the unused pool but IS in the lifetime store.
|
||||
*/
|
||||
internal fun isStatelessReset(datagram: ByteArray): Boolean {
|
||||
if (datagram.size < 22) return false
|
||||
@@ -2614,10 +2613,8 @@ class QuicConnection(
|
||||
anyMatch = true
|
||||
}
|
||||
}
|
||||
for (entry in pathValidator.unusedSequences()) {
|
||||
// Hot-path access lives on the pool — fetch the token via
|
||||
// the same code path the rest of [PathValidator] uses.
|
||||
val token = pathValidator.unusedTokenForSequence(entry) ?: continue
|
||||
for (token in pathValidator.allKnownStatelessResetTokens()) {
|
||||
if (token.size != 16) continue
|
||||
if (constantTimeEqualsRange(token, datagram, tokenStart)) {
|
||||
anyMatch = true
|
||||
}
|
||||
|
||||
@@ -60,3 +60,42 @@ object Http3SettingsId {
|
||||
/** WebTransport over HTTP/3 (draft / RFC 9220). */
|
||||
const val ENABLE_WEBTRANSPORT: Long = 0xc671706a
|
||||
}
|
||||
|
||||
/**
|
||||
* HTTP/3 error code identifiers per RFC 9114 §8.1. These are the values
|
||||
* emitted in CONNECTION_CLOSE frames at the application-error level
|
||||
* (frame type 0x1d) when an HTTP/3 invariant is violated. Quoted values
|
||||
* are the spec defaults; we don't define the full table — only the
|
||||
* codes the demux / frame reader actually need.
|
||||
*/
|
||||
object Http3ErrorCode {
|
||||
/** §8.1: graceful close, no error. */
|
||||
const val NO_ERROR: Long = 0x100
|
||||
|
||||
/** §8.1: peer violated a generic HTTP/3 invariant we can't classify more precisely. */
|
||||
const val GENERAL_PROTOCOL_ERROR: Long = 0x101
|
||||
|
||||
/** §8.1: an internal-to-our-implementation error. */
|
||||
const val INTERNAL_ERROR: Long = 0x102
|
||||
|
||||
/**
|
||||
* RFC 9114 §6.2.1 / RFC 9204 §4.2: a critical unidirectional stream
|
||||
* (control, QPACK encoder, QPACK decoder) was closed by the peer.
|
||||
*/
|
||||
const val CLOSED_CRITICAL_STREAM: Long = 0x104
|
||||
|
||||
/** §8.1: a frame appeared in a context that does not permit it. */
|
||||
const val FRAME_UNEXPECTED: Long = 0x105
|
||||
|
||||
/** §8.1: frame layout / length / payload was invalid. */
|
||||
const val FRAME_ERROR: Long = 0x106
|
||||
|
||||
/** §5.2: a peer's stream id violated its own GOAWAY commitment. */
|
||||
const val ID_ERROR: Long = 0x108
|
||||
|
||||
/** §7.2.4.1: a SETTINGS frame contained an error. */
|
||||
const val SETTINGS_ERROR: Long = 0x109
|
||||
|
||||
/** §7.2.4.1: the first frame on the control stream was not SETTINGS. */
|
||||
const val MISSING_SETTINGS: Long = 0x10a
|
||||
}
|
||||
|
||||
+124
-4
@@ -21,6 +21,7 @@
|
||||
package com.vitorpamplona.quic.webtransport
|
||||
|
||||
import com.vitorpamplona.quic.Varint
|
||||
import com.vitorpamplona.quic.http3.Http3ErrorCode
|
||||
import com.vitorpamplona.quic.http3.Http3Frame
|
||||
import com.vitorpamplona.quic.http3.Http3FrameReader
|
||||
import com.vitorpamplona.quic.http3.Http3Settings
|
||||
@@ -126,6 +127,27 @@ class WtPeerStreamDemux(
|
||||
var peerGoawayProtocolError: String? = null
|
||||
private set
|
||||
|
||||
/**
|
||||
* RFC 9114 §6.2.1 + RFC 9204 §4.2 critical-stream closure record.
|
||||
* Latches the (errorCode, reason) pair the demux passed to its
|
||||
* driver-side `connection.close` when ANY critical unidirectional
|
||||
* stream — control, QPACK encoder, QPACK decoder — was closed by
|
||||
* the peer (clean FIN) or violated an HTTP/3 invariant.
|
||||
*
|
||||
* Production callers don't need to observe this; the
|
||||
* `connection.close` already drives the QUIC layer to CLOSED.
|
||||
* Tests use it to assert the closure path fired without wiring a
|
||||
* real driver: the flag is set inside [closeConnection] before
|
||||
* (and independently of) the suspending `connection.close` call.
|
||||
*/
|
||||
@Volatile
|
||||
var criticalStreamClosureCode: Long? = null
|
||||
private set
|
||||
|
||||
@Volatile
|
||||
var criticalStreamClosureReason: String? = null
|
||||
private set
|
||||
|
||||
/**
|
||||
* Surface RFC 9114 §7.2 frame-validation violations the
|
||||
* [Http3FrameReader] catches (H3_FRAME_UNEXPECTED, H3_MISSING_SETTINGS,
|
||||
@@ -214,7 +236,19 @@ class WtPeerStreamDemux(
|
||||
}
|
||||
|
||||
Http3StreamType.QPACK_ENCODER, Http3StreamType.QPACK_DECODER -> {
|
||||
drainBlackHole(chunkChannel)
|
||||
// RFC 9204 §4.2: QPACK encoder + decoder
|
||||
// streams are critical. Closure of either
|
||||
// MUST be treated as H3_CLOSED_CRITICAL_STREAM
|
||||
// — same wire-spec class as the control
|
||||
// stream. We don't speak the QPACK
|
||||
// dynamic-table instructions either way
|
||||
// (encoder is RIC=0, decoder is best-effort
|
||||
// ACKs we ignore), so the drain is still
|
||||
// black-hole; only the FIN handling differs.
|
||||
drainCriticalStream(
|
||||
chunkChannel,
|
||||
"QPACK ${if (streamType == Http3StreamType.QPACK_ENCODER) "encoder" else "decoder"} stream",
|
||||
)
|
||||
}
|
||||
|
||||
Http3StreamType.WEBTRANSPORT_UNI_STREAM -> {
|
||||
@@ -271,19 +305,105 @@ class WtPeerStreamDemux(
|
||||
reader.push(chunk)
|
||||
consumeFrames(reader)
|
||||
}
|
||||
// Channel iteration exited cleanly → peer FIN'd the
|
||||
// control stream. RFC 9114 §6.2.1: closure of any
|
||||
// critical stream MUST be treated as a connection error
|
||||
// of type H3_CLOSED_CRITICAL_STREAM.
|
||||
closeConnection(
|
||||
Http3ErrorCode.CLOSED_CRITICAL_STREAM,
|
||||
"H3_CLOSED_CRITICAL_STREAM: peer closed CONTROL stream",
|
||||
)
|
||||
} catch (e: com.vitorpamplona.quic.QuicCodecException) {
|
||||
// RFC 9114 §7.2 frame-validation throws (H3_FRAME_UNEXPECTED /
|
||||
// H3_MISSING_SETTINGS / reserved type) land here. Record the
|
||||
// diagnostic so the QUIC layer / application can close the
|
||||
// connection deliberately instead of having the route() catch
|
||||
// silently swallow the message. Idempotent on duplicate hits.
|
||||
// diagnostic so observability tools (qlog, tests) see a
|
||||
// structured message; ALSO drive the connection closed via
|
||||
// [closeConnection] so the application doesn't need to poll
|
||||
// [peerH3ProtocolError] separately. Idempotent on duplicate
|
||||
// hits.
|
||||
if (peerH3ProtocolError == null) {
|
||||
peerH3ProtocolError = e.message ?: "HTTP/3 protocol violation on CONTROL stream"
|
||||
}
|
||||
closeConnection(
|
||||
http3ErrorCodeForMessage(e.message),
|
||||
e.message ?: "HTTP/3 protocol violation on CONTROL stream",
|
||||
)
|
||||
throw e
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Drain a critical unidirectional stream (QPACK encoder / decoder)
|
||||
* whose payload we don't otherwise process. The QPACK control
|
||||
* messages would only matter if we ran a dynamic table — since we
|
||||
* don't, the bytes themselves are dropped, but the §6.2.1
|
||||
* "critical stream closed = connection error" obligation still
|
||||
* applies. On peer FIN we call [closeConnection] with
|
||||
* H3_CLOSED_CRITICAL_STREAM.
|
||||
*/
|
||||
private suspend fun drainCriticalStream(
|
||||
chunkChannel: Channel<ByteArray>,
|
||||
label: String,
|
||||
) {
|
||||
@Suppress("UNUSED_VARIABLE")
|
||||
for (discarded in chunkChannel) {
|
||||
// intentionally discarded — QPACK dynamic-table is off.
|
||||
}
|
||||
closeConnection(
|
||||
Http3ErrorCode.CLOSED_CRITICAL_STREAM,
|
||||
"H3_CLOSED_CRITICAL_STREAM: peer closed $label",
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Close the underlying QUIC connection with an HTTP/3
|
||||
* application-level error code. No-op when the demux is running
|
||||
* without a [driver] (test path) — the test is then expected to
|
||||
* read [peerH3ProtocolError] / observe the FIN explicitly.
|
||||
*
|
||||
* Launched on the demux's [scope] so the suspending close call
|
||||
* doesn't block the per-stream collector that's exiting.
|
||||
*/
|
||||
private fun closeConnection(
|
||||
errorCode: Long,
|
||||
reason: String,
|
||||
) {
|
||||
// Latch the closure intent FIRST so tests (and qlog observers)
|
||||
// can see what we tried to do, regardless of whether a
|
||||
// driver/connection is wired to follow through. Idempotent: a
|
||||
// duplicate fire (e.g. control stream errors then closes)
|
||||
// doesn't overwrite the first reason.
|
||||
if (criticalStreamClosureCode == null) {
|
||||
criticalStreamClosureCode = errorCode
|
||||
criticalStreamClosureReason = reason
|
||||
}
|
||||
val conn = driver?.connection ?: return
|
||||
scope.launch {
|
||||
conn.close(errorCode, reason)
|
||||
driver.wakeup()
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Best-effort mapping from a [QuicCodecException] message raised by
|
||||
* the HTTP/3 frame reader / SETTINGS validator to the matching
|
||||
* RFC 9114 §8.1 error code. We attach the code to the
|
||||
* application-level CONNECTION_CLOSE so downstream observers
|
||||
* (qlog, peer) get the precise spec category instead of a generic
|
||||
* GENERAL_PROTOCOL_ERROR.
|
||||
*/
|
||||
private fun http3ErrorCodeForMessage(message: String?): Long {
|
||||
if (message == null) return Http3ErrorCode.GENERAL_PROTOCOL_ERROR
|
||||
return when {
|
||||
message.contains("H3_FRAME_UNEXPECTED") -> Http3ErrorCode.FRAME_UNEXPECTED
|
||||
message.contains("H3_MISSING_SETTINGS") -> Http3ErrorCode.MISSING_SETTINGS
|
||||
message.contains("H3_SETTINGS_ERROR") -> Http3ErrorCode.SETTINGS_ERROR
|
||||
message.contains("H3_ID_ERROR") -> Http3ErrorCode.ID_ERROR
|
||||
message.contains("H3_FRAME_ERROR") -> Http3ErrorCode.FRAME_ERROR
|
||||
else -> Http3ErrorCode.GENERAL_PROTOCOL_ERROR
|
||||
}
|
||||
}
|
||||
|
||||
private fun consumeFrames(reader: Http3FrameReader) {
|
||||
while (true) {
|
||||
val frame = reader.next() ?: return
|
||||
|
||||
+81
@@ -133,6 +133,87 @@ class StatelessResetDetectionTest {
|
||||
assertFalse(client.isStatelessReset(datagram), "long-header-form must not be classified as stateless reset")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun token_persists_after_path_migration_for_wifi_handoff() {
|
||||
// RFC 9000 §10.3 + audio-rooms WiFi-handoff path. The peer
|
||||
// issues us a fresh CID via NEW_CONNECTION_ID. We migrate to
|
||||
// it via tryStartValidation (mimics the WiFi→cellular
|
||||
// handoff: client moves to a new path, requests a fresh DCID
|
||||
// for unlinkability). The peer (relay) then loses connection
|
||||
// state mid-handoff and emits a stateless reset using the
|
||||
// token it issued for the migrated CID. We MUST detect the
|
||||
// reset even though the CID has left the unused pool.
|
||||
val (client, _) = newConnectedClient()
|
||||
val handoffToken = ByteArray(16) { (0x88 + it).toByte() }
|
||||
val newCidBytes = ByteArray(8) { (0xCC + it).toByte() }
|
||||
// Step 1: peer offers a NEW_CONNECTION_ID. Token lands in
|
||||
// the unused pool AND in the lifetime store.
|
||||
val recordResult =
|
||||
client.pathValidator.recordPeerNewConnectionId(
|
||||
sequenceNumber = 1L,
|
||||
retirePriorTo = 0L,
|
||||
connectionId = newCidBytes,
|
||||
statelessResetToken = handoffToken,
|
||||
)
|
||||
assertEquals(PathValidator.RecordResult.Stored, recordResult)
|
||||
assertEquals(1, client.pathValidator.unusedCount())
|
||||
|
||||
// Step 2: client triggers a path migration (the WiFi
|
||||
// handoff). The CID leaves the unused pool and becomes
|
||||
// active. Pre-fix this would have lost the token.
|
||||
val migration = client.pathValidator.tryStartValidation(nowMillis = 0L, currentPtoMillis = 100L)
|
||||
assertEquals(PathMigrationResult.Started, migration)
|
||||
assertEquals(0, client.pathValidator.unusedCount(), "unused pool drained by migration")
|
||||
|
||||
// Step 3: a stateless-reset datagram arrives carrying the
|
||||
// handoff token in its trailing 16 bytes. The lifetime store
|
||||
// still has it, so the match succeeds.
|
||||
val dcid = client.sourceConnectionId.bytes
|
||||
val datagram = byteArrayOf(0x40.toByte()) + dcid + ByteArray(40) + handoffToken
|
||||
assertTrue(
|
||||
client.isStatelessReset(datagram),
|
||||
"post-migration token MUST still match for WiFi-handoff stateless-reset detection",
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun token_persists_through_force_rotation_for_acid_reissue() {
|
||||
// RFC 9000 §5.1.2: when the peer raises retire_prior_to past
|
||||
// our active CID, we force-rotate to a fresh entry. The
|
||||
// displaced active CID's token must still be matchable in
|
||||
// case the peer (or an adversary spoofing the peer)
|
||||
// stateless-resets us on the prior path during the brief
|
||||
// window before our retirement settles.
|
||||
val (client, _) = newConnectedClient()
|
||||
// Issue two CIDs (seq 1 and 2). retire_prior_to=2 will
|
||||
// force a rotation to seq 2 and queue retirement for seq 1.
|
||||
val tokenA = ByteArray(16) { (0x11 + it).toByte() }
|
||||
val tokenB = ByteArray(16) { (0x22 + it).toByte() }
|
||||
client.pathValidator.recordPeerNewConnectionId(
|
||||
sequenceNumber = 1L,
|
||||
retirePriorTo = 0L,
|
||||
connectionId = ByteArray(8) { 0xAA.toByte() },
|
||||
statelessResetToken = tokenA,
|
||||
)
|
||||
client.pathValidator.recordPeerNewConnectionId(
|
||||
sequenceNumber = 2L,
|
||||
retirePriorTo = 2L, // peer demands we retire CIDs < 2 (i.e. seq 0 + 1)
|
||||
connectionId = ByteArray(8) { 0xBB.toByte() },
|
||||
statelessResetToken = tokenB,
|
||||
)
|
||||
// Force-rotate (peer's watermark = 2 > activeCidSequence = 0).
|
||||
val rotation = client.pathValidator.forceRotateToHigherSequence()
|
||||
assertNotNull(rotation)
|
||||
assertEquals(0, client.pathValidator.unusedCount(), "force-rotation drains the pool")
|
||||
|
||||
// Both tokens must still match — neither is in the unused
|
||||
// pool any more, but both live on in the lifetime store.
|
||||
val dcid = client.sourceConnectionId.bytes
|
||||
val payload = ByteArray(40)
|
||||
assertTrue(client.isStatelessReset(byteArrayOf(0x40.toByte()) + dcid + payload + tokenA))
|
||||
assertTrue(client.isStatelessReset(byteArrayOf(0x40.toByte()) + dcid + payload + tokenB))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun isStatelessReset_rejects_too_short_datagram() {
|
||||
val (client, _) = newConnectedClient()
|
||||
|
||||
+249
@@ -0,0 +1,249 @@
|
||||
/*
|
||||
* 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.webtransport
|
||||
|
||||
import com.vitorpamplona.quic.QuicWriter
|
||||
import com.vitorpamplona.quic.http3.Http3ErrorCode
|
||||
import com.vitorpamplona.quic.http3.Http3FrameType
|
||||
import com.vitorpamplona.quic.http3.Http3Settings
|
||||
import com.vitorpamplona.quic.http3.Http3SettingsId
|
||||
import com.vitorpamplona.quic.http3.Http3StreamType
|
||||
import com.vitorpamplona.quic.stream.QuicStream
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
import kotlinx.coroutines.SupervisorJob
|
||||
import kotlinx.coroutines.cancel
|
||||
import kotlinx.coroutines.delay
|
||||
import kotlinx.coroutines.runBlocking
|
||||
import kotlinx.coroutines.withTimeoutOrNull
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertNotNull
|
||||
|
||||
/**
|
||||
* RFC 9114 §6.2.1 + RFC 9204 §4.2 critical-stream closure.
|
||||
*
|
||||
* "If either control stream is closed at any point, this MUST be
|
||||
* treated as a connection error of type H3_CLOSED_CRITICAL_STREAM."
|
||||
*
|
||||
* RFC 9204 §4.2 extends this to the QPACK encoder + decoder streams.
|
||||
*
|
||||
* Pre-fix the demux recorded a `peerH3ProtocolError` flag on
|
||||
* frame-validation throws but did NOT auto-close the connection on
|
||||
* peer-side stream FIN at all — the application was expected to poll
|
||||
* the flag and call close(). For WiFi↔cellular handoff and other
|
||||
* lossy paths where the relay can drop the control stream
|
||||
* mid-session, the audio user saw stalled output with no error event.
|
||||
*
|
||||
* The fix: the demux now drives `connection.close(errorCode, reason)`
|
||||
* itself when any critical stream closes (clean FIN OR protocol
|
||||
* violation). The closure intent is latched on
|
||||
* `criticalStreamClosureCode` / `criticalStreamClosureReason` so tests
|
||||
* can verify the path runs without wiring a real driver.
|
||||
*/
|
||||
class CriticalStreamClosureTest {
|
||||
private fun controlStreamPrefix(): ByteArray = QuicWriter().also { it.writeVarint(Http3StreamType.CONTROL) }.toByteArray()
|
||||
|
||||
private fun qpackEncoderPrefix(): ByteArray = QuicWriter().also { it.writeVarint(Http3StreamType.QPACK_ENCODER) }.toByteArray()
|
||||
|
||||
private fun validSettingsFrame(): ByteArray =
|
||||
Http3Settings(
|
||||
mapOf(
|
||||
Http3SettingsId.ENABLE_CONNECT_PROTOCOL to 1L,
|
||||
Http3SettingsId.ENABLE_WEBTRANSPORT to 1L,
|
||||
),
|
||||
).encodeFrame()
|
||||
|
||||
@Test
|
||||
fun control_stream_FIN_triggers_h3_closed_critical_stream() {
|
||||
runBlocking {
|
||||
val stream = QuicStream(3L, QuicStream.Direction.UNIDIRECTIONAL_REMOTE_TO_LOCAL)
|
||||
val demuxScope = CoroutineScope(SupervisorJob() + Dispatchers.Default)
|
||||
val demux = WtPeerStreamDemux(expectedConnectStreamId = 0L, scope = demuxScope)
|
||||
demux.process(stream)
|
||||
|
||||
// Push a valid SETTINGS so the demux is happy with the
|
||||
// stream contents, then FIN it cleanly.
|
||||
stream.deliverIncoming(controlStreamPrefix())
|
||||
stream.deliverIncoming(validSettingsFrame())
|
||||
stream.closeIncoming()
|
||||
|
||||
val ok =
|
||||
withTimeoutOrNull(2_000L) {
|
||||
while (demux.criticalStreamClosureCode == null) delay(5)
|
||||
true
|
||||
}
|
||||
assertEquals(true, ok, "demux should latch H3_CLOSED_CRITICAL_STREAM within timeout")
|
||||
assertEquals(Http3ErrorCode.CLOSED_CRITICAL_STREAM, demux.criticalStreamClosureCode)
|
||||
val reason = demux.criticalStreamClosureReason
|
||||
assertNotNull(reason)
|
||||
assertEquals(true, reason.contains("CONTROL"), "reason should mention CONTROL stream: $reason")
|
||||
|
||||
demuxScope.cancel()
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun qpack_encoder_FIN_triggers_h3_closed_critical_stream() {
|
||||
runBlocking {
|
||||
val stream = QuicStream(3L, QuicStream.Direction.UNIDIRECTIONAL_REMOTE_TO_LOCAL)
|
||||
val demuxScope = CoroutineScope(SupervisorJob() + Dispatchers.Default)
|
||||
val demux = WtPeerStreamDemux(expectedConnectStreamId = 0L, scope = demuxScope)
|
||||
demux.process(stream)
|
||||
|
||||
// QPACK encoder stream — we drain its bytes (we don't run a
|
||||
// dynamic table) but its FIN is still a critical-stream
|
||||
// closure per RFC 9204 §4.2.
|
||||
stream.deliverIncoming(qpackEncoderPrefix())
|
||||
stream.deliverIncoming(byteArrayOf(0x00, 0x00, 0x00)) // junk QPACK we ignore
|
||||
stream.closeIncoming()
|
||||
|
||||
val ok =
|
||||
withTimeoutOrNull(2_000L) {
|
||||
while (demux.criticalStreamClosureCode == null) delay(5)
|
||||
true
|
||||
}
|
||||
assertEquals(true, ok, "demux should latch H3_CLOSED_CRITICAL_STREAM for QPACK encoder")
|
||||
assertEquals(Http3ErrorCode.CLOSED_CRITICAL_STREAM, demux.criticalStreamClosureCode)
|
||||
val reason = demux.criticalStreamClosureReason
|
||||
assertNotNull(reason)
|
||||
assertEquals(true, reason.contains("QPACK encoder"), "reason should mention QPACK encoder: $reason")
|
||||
|
||||
demuxScope.cancel()
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun control_stream_settings_violation_triggers_close_with_specific_code() {
|
||||
// Control stream's first frame is REQUIRED to be SETTINGS per
|
||||
// RFC 9114 §7.2.4.1. A peer that opens with a different frame
|
||||
// is H3_MISSING_SETTINGS — the demux's frame reader throws,
|
||||
// and our auto-close should propagate the specific code (not
|
||||
// just the generic H3_GENERAL_PROTOCOL_ERROR).
|
||||
runBlocking {
|
||||
val stream = QuicStream(3L, QuicStream.Direction.UNIDIRECTIONAL_REMOTE_TO_LOCAL)
|
||||
val demuxScope = CoroutineScope(SupervisorJob() + Dispatchers.Default)
|
||||
val demux = WtPeerStreamDemux(expectedConnectStreamId = 0L, scope = demuxScope)
|
||||
demux.process(stream)
|
||||
|
||||
// First (and only) frame on the control stream is GOAWAY
|
||||
// — illegal: SETTINGS must be first. Reader should throw
|
||||
// H3_MISSING_SETTINGS.
|
||||
val goawayBody = QuicWriter().also { it.writeVarint(0L) }.toByteArray()
|
||||
val goaway =
|
||||
QuicWriter()
|
||||
.apply {
|
||||
writeVarint(Http3FrameType.GOAWAY)
|
||||
writeVarint(goawayBody.size.toLong())
|
||||
writeBytes(goawayBody)
|
||||
}.toByteArray()
|
||||
stream.deliverIncoming(controlStreamPrefix())
|
||||
stream.deliverIncoming(goaway)
|
||||
stream.closeIncoming()
|
||||
|
||||
val ok =
|
||||
withTimeoutOrNull(2_000L) {
|
||||
while (demux.criticalStreamClosureCode == null) delay(5)
|
||||
true
|
||||
}
|
||||
assertEquals(true, ok)
|
||||
assertEquals(Http3ErrorCode.MISSING_SETTINGS, demux.criticalStreamClosureCode)
|
||||
assertNotNull(demux.peerH3ProtocolError)
|
||||
|
||||
demuxScope.cancel()
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun control_stream_unexpected_data_frame_maps_to_frame_unexpected() {
|
||||
runBlocking {
|
||||
val stream = QuicStream(3L, QuicStream.Direction.UNIDIRECTIONAL_REMOTE_TO_LOCAL)
|
||||
val demuxScope = CoroutineScope(SupervisorJob() + Dispatchers.Default)
|
||||
val demux = WtPeerStreamDemux(expectedConnectStreamId = 0L, scope = demuxScope)
|
||||
demux.process(stream)
|
||||
|
||||
// Valid SETTINGS first, then an illegal DATA frame. RFC
|
||||
// 9114 §7.2.1: DATA on the control stream is
|
||||
// H3_FRAME_UNEXPECTED.
|
||||
val data =
|
||||
QuicWriter()
|
||||
.apply {
|
||||
writeVarint(Http3FrameType.DATA)
|
||||
writeVarint(4L)
|
||||
writeBytes(byteArrayOf(1, 2, 3, 4))
|
||||
}.toByteArray()
|
||||
stream.deliverIncoming(controlStreamPrefix())
|
||||
stream.deliverIncoming(validSettingsFrame())
|
||||
stream.deliverIncoming(data)
|
||||
stream.closeIncoming()
|
||||
|
||||
val ok =
|
||||
withTimeoutOrNull(2_000L) {
|
||||
while (demux.criticalStreamClosureCode == null) delay(5)
|
||||
true
|
||||
}
|
||||
assertEquals(true, ok)
|
||||
assertEquals(Http3ErrorCode.FRAME_UNEXPECTED, demux.criticalStreamClosureCode)
|
||||
|
||||
demuxScope.cancel()
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun closure_code_is_idempotent_first_call_wins() {
|
||||
runBlocking {
|
||||
val stream = QuicStream(3L, QuicStream.Direction.UNIDIRECTIONAL_REMOTE_TO_LOCAL)
|
||||
val demuxScope = CoroutineScope(SupervisorJob() + Dispatchers.Default)
|
||||
val demux = WtPeerStreamDemux(expectedConnectStreamId = 0L, scope = demuxScope)
|
||||
demux.process(stream)
|
||||
|
||||
// Inject a protocol violation (sets the code) then FIN
|
||||
// (would set CLOSED_CRITICAL_STREAM if we didn't latch).
|
||||
// The latch keeps the violation code, not the FIN code.
|
||||
val data =
|
||||
QuicWriter()
|
||||
.apply {
|
||||
writeVarint(Http3FrameType.DATA)
|
||||
writeVarint(0L)
|
||||
}.toByteArray()
|
||||
stream.deliverIncoming(controlStreamPrefix())
|
||||
stream.deliverIncoming(validSettingsFrame())
|
||||
stream.deliverIncoming(data)
|
||||
stream.closeIncoming()
|
||||
|
||||
val ok =
|
||||
withTimeoutOrNull(2_000L) {
|
||||
while (demux.criticalStreamClosureCode == null) delay(5)
|
||||
true
|
||||
}
|
||||
assertEquals(true, ok)
|
||||
// Either code is acceptable depending on timing; both
|
||||
// routes go through closeConnection(). The point is
|
||||
// [criticalStreamClosureCode] doesn't FLIP back and forth
|
||||
// — first writer wins.
|
||||
val firstCode = demux.criticalStreamClosureCode
|
||||
delay(50) // give the FIN-path coroutine time to also try
|
||||
assertEquals(firstCode, demux.criticalStreamClosureCode, "closure code should not flip after first observation")
|
||||
|
||||
demuxScope.cancel()
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user