diff --git a/quic/plans/2026-04-26-quic-stack-status.md b/quic/plans/2026-04-26-quic-stack-status.md index 07f878f819..d3fa3da4eb 100644 --- a/quic/plans/2026-04-26-quic-stack-status.md +++ b/quic/plans/2026-04-26-quic-stack-status.md @@ -1,16 +1,24 @@ -# QUIC + WebTransport stack — current state (2026-04-26) +# QUIC + WebTransport stack — current state -This document is the post-implementation snapshot of `:quic`. It supersedes -the original [pure-kotlin QUIC + WebTransport plan](../../docs/plans/2026-04-22-pure-kotlin-quic-webtransport-plan.md) -which was written before any code shipped and is now historical. +Living status doc for `:quic`. First written 2026-04-26 as the post-implementation +snapshot of the [pure-kotlin QUIC + WebTransport +plan](../../docs/plans/2026-04-22-pure-kotlin-quic-webtransport-plan.md); +last full rewrite **2026-05-09** after a four-RFC compliance audit (RFC 9000, +9001, 9002, 9221 + 9114 + 9204) found the doc had drifted ~2 weeks behind +shipping work. The original plan is now historical. ## TL;DR `:quic` is a self-contained Kotlin-Multiplatform module that speaks QUIC v1 -+ HTTP/3 + WebTransport against real-world servers (aioquic + picoquic -verified; nestsClient/MoQ on top). It uses Quartz crypto primitives only — -no BouncyCastle, no JNI. ~8.5k lines of production code, ~5k lines of -tests, 39 test files, five rounds of parallel audit + fix passes. ++ HTTP/3 + WebTransport against real-world servers (aioquic + picoquic + +quic-go interop verified, including 0-RTT). Pure Kotlin: Quartz crypto +primitives + JCA AEAD only — no BouncyCastle, no JNI. ~17k lines of +production code, 81 test files, ten-plus rounds of audit + fix passes +since 2026-04-26. + +The 2026-05-09 audit confirmed the core handshake / packet protection / +loss recovery / frame routing surface is RFC-grade. Real gaps are listed +in [§ RFC compliance gaps](#rfc-compliance-gaps-2026-05-09). ## What shipped vs the original plan @@ -20,18 +28,18 @@ tests, 39 test files, five rounds of parallel audit + fix passes. | B. TLS 1.3 | 3 wk | done | RFC 8446 client state machine, X25519 ECDHE, RFC 8448 §3 vectors pass bit-for-bit | | C. Initial + Handshake packets | 2 wk | done | RFC 9001 §A.2/§A.3 vectors pass; ChaCha20 per §A.5 | | D. 1-RTT + STREAM | 1 wk | done | Stream offset reassembly, FIN, fuzzed | -| E. ACK + flow control | 1 wk | done | MAX_DATA / MAX_STREAM_DATA / MAX_STREAMS routing + writer enforcement | +| E. ACK + flow control | 1 wk | done | MAX_DATA / MAX_STREAM_DATA / MAX_STREAMS routing + writer enforcement + per-stream receive enforcement | | F. Loss recovery + congestion control | 1 wk | done (no CC) | RFC 9002 §5/§6 RTT estimator + packet/time-threshold loss detection + PTO + per-frame retransmit shipped 2026-05-05; congestion control still TBD | | G. Datagram extension | ½ wk | done | RFC 9221 frames + bounded incoming queue | -| H. Connection lifecycle | 1 wk | done | CONNECTION_CLOSE, idle timeout, draining/closing, idempotent driver close | +| H. Connection lifecycle | 1 wk | done | CONNECTION_CLOSE, draining/closing, idempotent driver close (idle-timeout enforcement still missing — see gaps) | | I. HTTP/3 | 2 wk | done | Control stream + SETTINGS + GOAWAY (with id-regression check) + duplicate-id rejection | | J. QPACK | 2 wk | done | Static-table + Huffman + integer codec (RFC 7541 §B + RFC 9204 §B.1 vectors) | | K. Extended CONNECT + WT | 1 wk | done | Stream-type prefixes, HTTP Datagram + WT_CLOSE_SESSION capsule | -| L. Interop + hardening | 2 wk | done + much more | Live interop against aioquic Docker; **5 rounds of audit + fix** beyond the plan | +| L. Interop + hardening | 2 wk | done + much more | Live interop against aioquic + picoquic + quic-go; **10+ rounds of audit + fix**, multiplexing, 0-RTT, key update, ECN, path migration | -The original plan estimated 17–19 weeks. We ran the full sequence plus five -unscheduled audit rounds. Every audit found real bugs; the suite is what -caught them on regression. +The original plan estimated 17–19 weeks. We shipped the full sequence plus +a long tail of audit-driven hardening. Every audit found real bugs; the +test suite is what catches them on regression. ## What's actually in the module @@ -43,51 +51,61 @@ quic/ │ ├── Buffer.kt ← QuicReader / QuicWriter │ ├── Varint.kt ← RFC 9000 §16 │ ├── connection/ ← QuicConnection orchestrator + Driver - │ │ ├── QuicConnection.kt (≈ 600 lines, the hub) + │ │ ├── QuicConnection.kt (≈ 2800 lines, the hub) │ │ ├── QuicConnectionDriver.kt (read/send loops + close) │ │ ├── QuicConnectionParser.kt (feedDatagram + dispatchFrames) │ │ ├── QuicConnectionWriter.kt (drainOutbound + flow-control updates) + │ │ ├── PathValidator.kt ← RFC 9000 §5.1.2 + §8.2 + §9 │ │ ├── PacketProtection.kt + builder │ │ ├── PacketNumberSpace.kt │ │ ├── ConnectionId.kt + TransportParameters.kt - │ │ └── EncryptionLevel.kt + LevelState.kt - │ ├── crypto/ ← Aead, header protection, HKDF helpers, AesEcbHeaderProtection, - │ │ ChaCha20HeaderProtection, ChaCha20Poly1305Aead, InitialSecrets, + │ │ ├── EncryptionLevel.kt + LevelState.kt + │ │ └── recovery/ ← RecoveryToken, SentPacket, + │ │ AckedPackets, QuicLossDetection + │ ├── crypto/ ← Aead, header protection, HKDF helpers, + │ │ AesEcbHeaderProtection, ChaCha20HeaderProtection, + │ │ ChaCha20Poly1305Aead, InitialSecrets, │ │ PlatformAesOneBlock, PlatformChaCha20Block (expect), │ │ bestAes128GcmAead (expect) │ ├── frame/ ← Frame.kt sealed hierarchy + FrameFuzzerTest target - │ │ includes RESET_STREAM / STOP_SENDING / NEW_TOKEN + │ │ includes RESET_STREAM / STOP_SENDING / NEW_TOKEN / + │ │ PATH_CHALLENGE / PATH_RESPONSE / NEW_CONNECTION_ID / + │ │ RETIRE_CONNECTION_ID / ACK_ECN │ ├── http3/ ← Http3FrameReader + Http3Settings + frame types - │ ├── packet/ ← LongHeaderPacket, ShortHeaderPacket, RetryPacket, peekHeader + │ ├── packet/ ← LongHeaderPacket, ShortHeaderPacket (with + │ │ reserved-bit + key-phase peek), RetryPacket, peekHeader │ ├── qpack/ ← QpackDecoder, QpackEncoder, QpackHuffman, QpackInteger, │ │ QpackStaticTable - │ ├── connection/recovery/ ← RecoveryToken + SentPacket + QuicLossDetection - │ │ (RFC 9002 §5/§6 RTT estimator + loss detection + - │ │ PTO; per-frame retransmit dispatch) │ ├── recovery/ ← AckTracker (with ack-eliciting gating) - │ ├── stream/ ← QuicStream, ReceiveBuffer (with FIN-fully-read), SendBuffer, StreamId - │ ├── tls/ ← TlsClient state machine + ClientHello/ServerHello/EE/Cert/CV/Finished - │ │ codecs, TlsKeySchedule, TlsTranscriptHash (incremental), TlsConstants, + │ ├── stream/ ← QuicStream, ReceiveBuffer (with FIN-fully-read + + │ │ per-stream receive-limit), SendBuffer, StreamId + │ ├── tls/ ← TlsClient state machine + ClientHello (incl. PSK + + │ │ early_data resumption variant) / ServerHello / EE / + │ │ Cert / CV / Finished / NewSessionTicket codecs, + │ │ TlsKeySchedule (incl. 0-RTT + resumption_master_secret), + │ │ TlsTranscriptHash (incremental), TlsConstants, │ │ PermissiveCertificateValidator, TlsRunningSha256 (expect) │ ├── transport/ ← UdpSocket (expect) │ └── webtransport/ ← QuicWebTransportFactory + QuicWebTransportSessionState + │ WtPeerStreamDemux + WtCapsule + WtDatagram + ExtendedConnect └── jvmAndroid/kotlin/com/vitorpamplona/quic/ ├── crypto/JcaAesGcmAead.kt ← cached JCA Cipher per direction with IV-reuse fallback + ├── crypto/JcaChaCha20Poly1305Aead.kt ← JCA ChaCha20-Poly1305 (with Quartz fallback) ├── crypto/PlatformCrypto.kt ← actuals - ├── tls/JdkCertificateValidator.kt ← system-trust-store chain validation + RSA-PSS / ECDSA / Ed25519 + ├── tls/JdkCertificateValidator.kt ← system-trust-store chain validation + PSL filter + + │ RSA-PSS / ECDSA / Ed25519 ├── tls/TlsRunningSha256.kt ← MessageDigest.clone()-based incremental hash └── transport/UdpSocket.kt ← DatagramChannel + suspend wrapper ``` ## Crypto surface -Quartz primitives only: +Quartz primitives + JCA only: | Primitive | Source | |---|---| | AES-128-GCM | `JcaAesGcmAead` (jvmAndroid) — cached `Cipher` per direction; `Aes128Gcm` singleton (commonMain) for non-hot paths | -| ChaCha20-Poly1305 | Quartz `ChaCha20Poly1305` (commonMain pure-Kotlin) wrapped in `ChaCha20Poly1305Aead` | +| ChaCha20-Poly1305 | `JcaChaCha20Poly1305Aead` (jvmAndroid, JCA fast path) with Quartz pure-Kotlin fallback in `ChaCha20Poly1305Aead` | | HKDF-Extract / Expand-Label | Quartz `Hkdf` + `MacInstance`; thin RFC 8446 §7.1 helper in `crypto/HkdfHelpers.kt` | | SHA-256 (one-shot) | Quartz `sha256(...)` | | SHA-256 (incremental, for transcript) | `TlsRunningSha256` (expect/actual; jvmAndroid wraps `MessageDigest.clone()`) | @@ -101,32 +119,58 @@ X.509 chain validation, hostname verification, signature verification all delegate to JDK `TrustManagerFactory` / `Signature.getInstance(...)`. `CertificateValidator` is a non-null typed parameter — tests pass an explicit `PermissiveCertificateValidator`; production passes -`JdkCertificateValidator`. +`JdkCertificateValidator` (with PSL-subset wildcard rules). ## What we deliberately don't do - **QUIC server role.** Client-only. -- **0-RTT / session resumption.** No PSK extension offered; an arriving - ServerFinished without prior Certificate is hard-failed. -- **Connection migration / preferred address / multiple paths.** `NEW_CONNECTION_ID` - and `PATH_*` frames decode (so peers don't break us) but aren't acted on. - **Path MTU discovery.** Fixed 1200-byte ceiling per RFC 9000 §14. +- **Anti-amplification limits (server side).** We're a client; the server + applies its own. - **HTTP/3 server push.** -- **QPACK dynamic-table inserts on the encoder.** We send literal-only; - decoder accepts dynamic-table indexed lines. -- **ECN / anti-amplification limits.** We're a client. -- **TLS Key-Update / NewSessionTicket.** Detected and refused (KeyUpdate - fails the connection rather than silently desynchronising). -- **Congestion control (NewReno / CUBIC / BBR).** Loss detection is in - place (RFC 9002 §5/§6) but no rate-limiting feedback loop reacts to - losses; we send as fast as the application provides bytes. Independent - follow-up. +- **QPACK dynamic-table inserts on the encoder.** We send literal + + static-indexed only; decoder accepts dynamic-table indexed lines. +- **Congestion control (NewReno / CUBIC / BBR).** Loss detection is + in place (RFC 9002 §5/§6) but no rate-limiting feedback loop reacts + to losses; we send as fast as the application provides bytes. See + [`2026-05-05-congestion-control.md`](2026-05-05-congestion-control.md) + (parked indefinitely; acceptable for moq-lite audio). + +## What's now shipped that the original "deliberately don't do" list disclaimed + +The plan's pre-rewrite version listed the following as out-of-scope. They +shipped between 2026-04-26 and 2026-05-09 and now WORK: + +- **0-RTT / session resumption.** TLS 1.3 PSK + `early_data` extension on + both ends; client caches `resumption_master_secret`, parses + `NewSessionTicket`, and replays 0-RTT data with prior-connection ALPN + + transport params. Verified against picoquic and quic-go. +- **TLS Key-Update / NewSessionTicket.** RFC 9001 §6 1-RTT key update — + client-initiated AND peer-initiated. `KEY_PHASE`-bit peek before AEAD, + in-flight gate, PN-regression check on the rotation packet, proper + HKDF roll of `application_traffic_secret_N+1`. +- **NEW_CONNECTION_ID / RETIRE_CONNECTION_ID handling, including + `retire_prior_to`** per RFC 9000 §5.1.2 + §19.15 (`PathValidator.recordPeerNewConnectionId` + with full state machine: stale-offer retire, watermark monotonicity, + pool-full bound, duplicate-with-mismatch rejection). +- **Path validation (client- and server-initiated).** RFC 9000 §8.2 + + §9 — full PATH_CHALLENGE / PATH_RESPONSE round-trip, 3*PTO timeout + per §8.2.4, abrupt migration to a new DCID on success, forced + rotation when `retire_prior_to` exceeds the active CID. +- **ECN (1-RTT only).** ECT(0) on outbound, `ACK_ECN` (0x03) frames + with truthful CE / ECT(0) / ECT(1) counters. Initial / Handshake + ACKs intentionally skip ECN counts (interop conservatism). + +The remaining pre-rewrite "don't do" items (server role, PMTU, HTTP/3 +push, QPACK encoder dynamic table, CC) are still out-of-scope. ## Verified interop - **aioquic** (Python): `quic-interop-runner`-style Docker setup; full - handshake + Extended CONNECT + h3 datagram round-trip. -- **picoquic** (C): Docker image, lightweight HTTP/3 GET. + handshake + multiplexing + Extended CONNECT + h3 datagram round-trip. +- **picoquic** (C): Docker image, h3, 0-RTT, key-update, longRTT, ECN + test cases pass. +- **quic-go** (Go): 0-RTT pass. - **In-memory pipe** (`InMemoryQuicPipe`, modeled on Cloudflare quiche's `Pipe`) drives both sides of the handshake in one JVM for fast tests without sockets. @@ -137,13 +181,19 @@ gates on the audio-rooms completion plan ## Audit summary -| Round | Focus | Findings | Status | -|---|---|---|---| -| 1 | Initial review (pre-interop) | 6 critical correctness/security bugs | all fixed | -| 2 | TLS hardening + lifecycle | hangs + TLS edge cases | all fixed | -| 3 | Performance + concurrency | cipher caching, polling, transcript O(n²) | all fixed | -| 4 | Core + TLS + perf + coverage gaps (4 parallel agents) | ~30 items including 4 CRITICAL interop blockers | all fixed; comprehensive regression tests added | -| 5 | Regression check + concurrency-specific (2 parallel agents) | 1 CRITICAL ackEliciting regression I'd just introduced + WT scope leak + others | all fixed | +| Round | Date | Focus | Findings | Status | +|---|---|---|---|---| +| 1 | 2026-04-22 | Initial review (pre-interop) | 6 critical correctness/security bugs | all fixed | +| 2 | 2026-04-23 | TLS hardening + lifecycle | hangs + TLS edge cases | all fixed | +| 3 | 2026-04-24 | Performance + concurrency | cipher caching, polling, transcript O(n²) | all fixed | +| 4 | 2026-04-25 | Core + TLS + perf + coverage gaps (4 parallel agents) | ~30 items including 4 CRITICAL interop blockers | all fixed; comprehensive regression tests added | +| 5 | 2026-04-26 | Regression check + concurrency-specific (2 parallel agents) | 1 CRITICAL ackEliciting regression I'd just introduced + WT scope leak + others | all fixed | +| 6 | 2026-05-04 | Per-frame retransmit + SendBuffer rewrite | retain-until-ACK + control-frame retx + STREAM/CRYPTO recovery | all fixed; see [`2026-05-04-control-frame-retransmit.md`](2026-05-04-control-frame-retransmit.md) | +| 7 | 2026-05-05 | Loss-detection timer + PSK rejection signal | RFC 9002 §6.1.2 timer; PSK rejection recover-in-place | all fixed | +| 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; **all 🔴 High and all 🟡 Medium items resolved same-day across 7 commits** | 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 @@ -151,35 +201,48 @@ git log (commits whose subject starts with `fix(quic):` or `perf(quic):`). ## Test inventory -Roughly grouped: +81 `*Test.kt` files. Roughly grouped: - **RFC vectors:** RFC 8448 §3 (TLS handshake), RFC 9001 §A.1–A.5 (Initial encrypt/decrypt, Retry, ChaCha20, server-side HP), RFC 9204 §B.1 (QPACK), RFC 7541 (Huffman). - **End-to-end pipe tests:** `InMemoryQuicPipeTest`, `CoalescedPacketSkipTest`, `ReceiveLimitEnforcementTest`, `PeerStreamLimitTest`, `FrameRoutingTest`, - `AckElicitingFramesTest`. + `AckElicitingFramesTest`, `MultiplexingRoundTripTest`, + `MultiplexingThroughputTest`, `MultiplexingCoalescingTest`, + `MultiStreamFinDeliveryTest`, `MultiplexingAioquicTpsTest`. - **Adversarial:** `FrameFuzzerTest`, `HostilePacketInputTest`, - `TlsSecurityPropertiesTest`, `HelloRetryRequestTest`. + `TlsSecurityPropertiesTest`, `HelloRetryRequestTest`, + `CloseDatagramRfcComplianceTest`, `CloseUnderLoadTest`. - **Crypto:** `JcaAesGcmAeadTest`, `ChaCha20Poly1305AeadTest`, `TlsTranscriptHashTest`. - **WT / HTTP/3:** `CapsuleReaderTest`, `WtPeerStreamDemuxTest`, `WtFramingTest`. - **Recovery:** `AckTrackerCoalescedTest`, `AckTrackerGatingTest`, - `RecoveryTokenTest`, `SentPacketTest`, `ReceiverFlowControlTest` (9 - cases mirrored from neqo `fc.rs`), `QuicLossDetectionTest`, - `PtoTest`, `QuicConnectionRetransmitTest`, - `SendBufferRetainUntilAckTest` (14 cases for the rewrite), - `StreamRetransmitTest`, `CryptoRetransmitTest`, - `ResetStopSendingEmitTest` (7 cases). -- **Interop:** `InteropRunner` (jvmTest, drives a real socket against a - Dockerised aioquic; opt-in, not in CI). + `AckTrackerPurgeOnAckOfAckTest`, `RecoveryTokenTest`, `SentPacketTest`, + `ReceiverFlowControlTest` (9 cases mirrored from neqo `fc.rs`), + `QuicLossDetectionTest`, `PtoTest`, `PtoCryptoRetransmitTest`, + `QuicConnectionRetransmitTest`, `RetransmitIntegrationTest`, + `SendBufferRetainUntilAckTest` (14 cases), `StreamRetransmitTest`, + `CryptoRetransmitTest`, `ResetStopSendingEmitTest` (7 cases), + `OnTokensLostTest`, `MoqLiteLossHarnessTest`. +- **Path validation / migration:** `PathValidatorTest`, + `PathValidationTest`, `ClientPathMigrationTest`. +- **Key lifecycle:** `KeyDiscardTest`, `KeyUpdatePeerInitiatedTest`. +- **Concurrency / soak:** `BatchedOpenLockContractTest`, + `StreamRetirementSoakTest`, `QuicHeapSoakTest`, + `QuicConnectionDriverLifecycleTest`. +- **Misc:** `VersionNegotiationTest`, `RetryHandlingTest`, + `PacketNumberSpaceTest`, `ConnectionIdTest`, + `FlowControlSnapshotTest`, `PendingFlowControlEmitTest`, + `PeerStreamCreditExtensionTest`, `PeerUniStreamDrainTest`. +- **Interop (jvmTest, opt-in):** `InteropRunner` drives a real socket + against a Dockerised aioquic / picoquic / quic-go. Not in CI. ## Known limitations / deferred work -These are the items future audit rounds keep flagging that we've -consciously not tackled — all confined to the steady-state path that audio -rooms don't exercise heavily: +These are items future audit rounds keep flagging that we've consciously +not tackled: 1. ~~**No STREAM retransmit on loss** (audit-4 #10).~~ **Resolved 2026-05-05** — `SendBuffer` rewritten for retain-until-ACK with three-state range @@ -188,27 +251,89 @@ rooms don't exercise heavily: and RESET_STREAM / STOP_SENDING / NEW_CONNECTION_ID retransmit. See [`2026-05-04-control-frame-retransmit.md`](2026-05-04-control-frame-retransmit.md). 2. ~~**`SendBuffer` doesn't retain bytes until ACK.**~~ Resolved with #1. -3. ~~**No Initial / Handshake key discard.**~~ **Resolved 2026-05-05** — - `LevelState.discardKeys()` nulls the AEAD protection, replaces the - per-level CRYPTO buffers and ack-tracker with empty instances, and - clears the sent-packet map. Initial keys discarded by the writer - after the first Handshake packet is built (RFC 9001 §4.9.1); - Handshake keys discarded by the parser on receipt of HANDSHAKE_DONE - (RFC 9001 §4.9.2 + §4.1.2 client-side handshake confirmation). -4. **No path validation for `NEW_CONNECTION_ID`.** We don't migrate. -5. **Stateless reset detection.** Stateless-reset packets look like - corruption to us. +3. ~~**No Initial / Handshake key discard.**~~ **Resolved 2026-05-05.** +4. ~~**No path validation for `NEW_CONNECTION_ID`.**~~ **Resolved 2026-05-06–08** — + full client-initiated DCID rotation + server-initiated path + validation per RFC 9000 §8.2 + §9. See `PathValidator.kt` and the + three test files (`PathValidatorTest`, `PathValidationTest`, + `ClientPathMigrationTest`). +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** — `RecoveryToken.Ack` now carries `(level, largestAcked)`; - the writer captures these from the AckFrame at emit time, and - `QuicConnection.onTokensAcked` purges the matching per-level inbound - tracker on ACK-of-ACK. The wrong purge in the parser is gone. + 2026-05-05.** 7. **Driver direct unit tests** require turning `UdpSocket` from `expect class` into an interface so the test side can stub. The driver is covered indirectly by every pipe-based test plus the live interop runner. 8. **No congestion control.** Loss detection is wired but nothing - throttles send rate in response. Independent ~1-2 wk project. + throttles send rate in response. Independent ~1–2 wk project. + See [`2026-05-05-congestion-control.md`](2026-05-05-congestion-control.md). + +## RFC compliance gaps (2026-05-09) + +Catalogued from a four-agent compliance audit (RFC 9000 / 9001 / 9002 / +9221+9114+9204). Each item lists the spec section, the gap, and +file:line evidence. Severity: + +- 🔴 **High** — exploitable by a hostile peer (resource exhaustion, + silent desync, RTT poisoning). +- 🟡 **Medium** — interop or correctness risk; cooperative peers + unaffected. +- 🟦 **Low / cosmetic** — diagnostic loss, doesn't break the wire. + +### Receive-side validation gaps + +| RFC § | Sev | Gap | Evidence | +|---|---|---|---| +| RFC 9000 §17.2 + §17.3 | ~~🔴~~ | ~~**Inbound fixed-bit (0x40) not validated.**~~ **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.**~~ **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 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 + +| RFC § | Sev | Gap | Evidence | +|---|---|---|---| +| RFC 9000 §18.2 | ~~🟡~~ | ~~**`max_udp_payload_size < 1200` not rejected.**~~ **Resolved 2026-05-09** — `applyPeerTransportParameters` closes with TRANSPORT_PARAMETER_ERROR. Tests in `TransportParameterBoundsTest`. | +| RFC 9000 §18.2 | ~~🟡~~ | ~~**`ack_delay_exponent > 20` not rejected.**~~ **Resolved 2026-05-09** — same path. | +| RFC 9000 §18.2 | ~~🟡~~ | ~~**`active_connection_id_limit < 2` not rejected.**~~ **Resolved 2026-05-09** — same path. | +| RFC 9000 §13.2.5 | ~~🟦~~ | ~~**`ack_delay_exponent` decode uses our config, not peer's.**~~ **Resolved 2026-05-09** — parser now uses `peerTransportParameters?.ackDelayExponent` with the §18.2 default of 3 as fallback; defensive 0..20 coercion preserved. | + +### HTTP/3 + DATAGRAM gaps + +| RFC § | Sev | Gap | Evidence | +|---|---|---|---| +| 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 + 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 + +| RFC § | Sev | Gap | Evidence | +|---|---|---|---| +| RFC 9001 §4.8 | ~~🟦~~ | ~~**TLS alerts not mapped to CRYPTO_ERROR + alert_code.**~~ **Resolved 2026-05-09** — new `TlsAlertException(alertCode, ...)` carrier raised by the well-defined TLS error sites (HRR, TLS 1.3 version mismatch, cipher mismatch, ALPN mismatch, cert chain validation, CertificateVerify, Finished MAC, TLS KeyUpdate forbidden over QUIC). The QUIC parser's `feedDatagram` catches it and stamps `closeErrorCode = 0x100 + alertCode` per §4.8. Generic `QuicCodecException` falls back to `0x100 + 80` (internal_error). Tests in `ErrorCodeMappingTest`. | +| RFC 9000 §22 | ~~🟦~~ | ~~**Several error codes never emitted.**~~ **Partially resolved 2026-05-09:**
• **CRYPTO_BUFFER_EXCEEDED (0x0d)** — pre-insert per-level cap of 64 KiB on inbound CRYPTO data (covers worst-case RSA-4096 cert chains with headroom; bounds heap to 192 KiB across all 3 levels). Closes with the spec code on overshoot.
• **KEY_UPDATE_ERROR (0x0e)** — the original audit point referred to TLS KeyUpdate over QUIC, which is now mapped to CRYPTO_ERROR(unexpected_message=0x10a) via the `TlsAlertException` path above (RFC 9001 §6 mandates that specific code, NOT 0x0e). Constant exposed for future use; no QUIC-level rotation failure currently maps to it (PN-regression on a key-update packet is spec-allowed to handle silently per §6.1, which we do).
• **INVALID_TOKEN (0x0b)** — N/A for client role (only servers validate Retry tokens).
• Added `QuicTransportError` constants object covering the full §20.1 table; `markClosedExternally(reason, errorCode)` overload lets call sites pin the spec code on `closeErrorCode` for qlog observability. | + +### Gaps the 2026-05-09 audit got WRONG + +For audit traceability — three audit findings fingered code that's +actually compliant. Listed here so future audit rounds don't repeat +them: + +| Audit claim | Reality | +|---|---| +| "Inbound flow-control enforcement missing." | Per-stream IS enforced. `QuicConnectionParser.kt:655` raises a connection close when STREAM offset+len exceeds `stream.receiveLimit`. The actual gap is *connection-level* (above). | +| "ack_delay_exponent not applied on decode." | IS applied at `QuicConnectionParser.kt:540-553`, with explicit overflow protection per audit-4. The actual gap is using *our* exponent instead of the peer's. | +| "Short-header reserved bits not validated." | ARE validated at `ShortHeaderPacket.kt:182-187` — `QuicProtocolViolationException` raised on any of the 0x18 bits set. The `0x30` mask the audit cited was a mis-read of RFC 9000 §17.3 (reserved bits are 0x18). | ## Pointers @@ -216,5 +341,7 @@ rooms don't exercise heavily: - Audio-rooms NIP draft: `docs/plans/2026-04-22-nip-audio-rooms-draft.md` - Completion plan: `nestsClient/plans/2026-04-26-audio-rooms-completion.md` - Retransmit plan + implementation log: [`2026-05-04-control-frame-retransmit.md`](2026-05-04-control-frame-retransmit.md) +- Congestion control deferral rationale: [`2026-05-05-congestion-control.md`](2026-05-05-congestion-control.md) +- Lock-split design: [`2026-05-08-lock-split-design.md`](2026-05-08-lock-split-design.md) - Live interop runner: `quic/src/jvmTest/.../interop/InteropRunner.kt` -- Audit history: `git log --grep='audit' -- quic/` +- Audit history: `git log --grep='audit\|fix(quic)\|feat(quic)' -- quic/` diff --git a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/Buffer.kt b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/Buffer.kt index 1d94428fd6..e68ea11a15 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/Buffer.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/Buffer.kt @@ -305,3 +305,28 @@ class QuicCodecException( class QuicProtocolViolationException( message: String, ) : RuntimeException(message) + +/** + * RFC 9001 §4.8 TLS-alert-shaped exception. The TLS layer raises this + * when a handshake violation maps cleanly to one of the RFC 8446 §6 + * alert descriptions (decode_error=50, handshake_failure=40, + * no_application_protocol=120, etc.). The QUIC layer catches it and + * closes the connection with `errorCode = 0x100 + alertCode`, so qlog / + * tcpdump observers see the precise TLS reason instead of the generic + * `INTERNAL_ERROR` we used to emit. + * + * `alertCode` is the raw TLS alert description value (0..255) per RFC + * 8446 §B.2; the +0x100 offset is added at close time per RFC 9001 §4.8. + */ +class TlsAlertException( + val alertCode: Int, + message: String, + cause: Throwable? = null, +) : RuntimeException(message, cause) { + init { + require(alertCode in 0..255) { "TLS alert code must be a uint8: $alertCode" } + } + + /** RFC 9001 §4.8: QUIC error code = 0x100 + TLS alert description. */ + val quicErrorCode: Long get() = 0x100L + alertCode.toLong() +} diff --git a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/PathValidator.kt b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/PathValidator.kt index eb6534bbf9..57fdc88e79 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/PathValidator.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/PathValidator.kt @@ -280,6 +280,57 @@ class PathValidator( fun unusedSequences(): List = unusedCids.keys.toList() + /** + * Stateless-reset-token lookup for an unused-pool entry. Returns + * null if no entry exists for [sequenceNumber] (e.g. it was + * retired between [unusedSequences] returning and this call). + * Used by [QuicConnection.isStatelessReset] for token matching + * against arriving look-like-noise datagrams per RFC 9000 §10.3. + */ + 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 = 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 = ArrayList() + /** * Record a peer-issued `NEW_CONNECTION_ID` (RFC 9000 §19.15). * Returns the result code so the caller (parser) can decide @@ -366,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 } 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 6436a32a05..985e1a06fb 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnection.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnection.kt @@ -24,6 +24,7 @@ import com.vitorpamplona.quic.crypto.AesEcbHeaderProtection import com.vitorpamplona.quic.crypto.InitialSecrets import com.vitorpamplona.quic.crypto.PlatformAesOneBlock import com.vitorpamplona.quic.crypto.bestAes128GcmAead +import com.vitorpamplona.quic.frame.QuicTransportError import com.vitorpamplona.quic.observability.QlogObserver import com.vitorpamplona.quic.packet.QuicVersion import com.vitorpamplona.quic.stream.QuicStream @@ -296,9 +297,32 @@ class QuicConnection( */ @Volatile var peerTransportParameters: TransportParameters? = null - private set + internal set - enum class Status { HANDSHAKING, CONNECTED, CLOSING, CLOSED } + enum class Status { + HANDSHAKING, + CONNECTED, + + /** + * RFC 9000 §10.2.1 closing period. We've sent a + * CONNECTION_CLOSE (or are about to); the writer flushes the + * close datagram and then the connection holds until the + * peer's last responses drain out of flight. + */ + CLOSING, + + /** + * RFC 9000 §10.2.2 draining period. We received a + * CONNECTION_CLOSE from the peer; we MUST NOT send any further + * packets and SHOULD discard inbound packets without + * processing. The state MUST persist for at least 3 * PTO + * before transitioning to fully [CLOSED] so the peer's + * retransmits can converge. + */ + DRAINING, + + CLOSED, + } /** * Lock-split refactor (2026-05-08): @Volatile so concurrent loops can @@ -310,6 +334,39 @@ 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() + + /** + * RFC 9000 §10.2.2 draining-period deadline. Set by + * [enterDraining] to `now + 3 * PTO`; the driver's send loop + * folds it into the timeout and transitions to [Status.CLOSED] + * via [markClosedExternally] when it elapses. Null whenever the + * connection is not in [Status.DRAINING]. + */ + @Volatile + internal var drainingDeadlineMs: Long? = null + /** * Non-suspend monitor protecting the atomic transition of * [status] / [closeReason] / [closeErrorCode] from a non-CLOSED to a @@ -458,6 +515,64 @@ class QuicConnection( */ internal var advertisedMaxData: Long = config.initialMaxData + /** + * RFC 9000 §4.1 connection-level inbound flow-control accounting. + * Sum across all streams (live + retired) of the largest stream + * offset ever observed on an inbound STREAM / RESET_STREAM frame. + * The receiver MUST close the connection with FLOW_CONTROL_ERROR + * when this sum exceeds [advertisedMaxData]; that's enforced in + * the parser at frame-ingest time. + * + * Different from the writer's `totalRecvAdvanced` which uses + * `stream.receive.contiguousEnd()` (a slight under-count) for + * MAX_DATA threshold logic. Here we track the strict spec + * quantity — necessary to close before bookkeeping diverges + * if the peer sends data with gaps that pushes total received + * past the limit. + */ + @Volatile + internal var connectionInboundOffsetSum: Long = 0L + + /** + * RFC 9001 §6.6 / §B.1: per-key encryption count for the active + * 1-RTT send key. The writer increments this on each + * application-level packet it builds. When the count reaches the + * AEAD's `confidentialityLimit` we MUST initiate a key update + * before the next packet (else AEAD security degrades). The + * counter resets to 0 every time the send key phase rotates. + * + * Initial / Handshake levels share their keys briefly enough that + * the limit isn't reachable, so we only track 1-RTT. + */ + @Volatile + internal var aeadEncryptCount: Long = 0L + + /** + * RFC 9001 §6.6 / §B.1: count of failed AEAD verifications on + * the active 1-RTT receive key. Each `parseAndDecrypt` that + * returns null at APPLICATION level bumps this. When it reaches + * the AEAD's `integrityLimit` we MUST close the connection with + * AEAD_LIMIT_REACHED. + * + * Resets every time the receive key phase rotates. The integrity + * limit for AES-128-GCM (2^52) is unreachable in practice; for + * ChaCha20-Poly1305 (2^36) it's reachable only by a determined + * attacker spamming forged packets — close in either case is + * spec-mandated. + */ + @Volatile + internal var aeadDecryptFailureCount: Long = 0L + + /** + * Soft-trigger latch for [aeadEncryptCount]. Set true once we've + * called [initiateKeyUpdate] in response to crossing the soft + * threshold (half the confidentiality limit). Prevents repeatedly + * firing key-updates while the in-progress one is still pending. + * Cleared when the rotation actually completes (counter resets). + */ + @Volatile + internal var aeadKeyUpdateRequested: Boolean = false + /** * Cumulative count of peer-initiated unidirectional streams we've * accepted (incremented in [getOrCreatePeerStreamLocked]). Compared @@ -1109,6 +1224,42 @@ class QuicConnection( ) return } + // RFC 9000 §18.2 bounds checks. A peer that violates these is in + // protocol violation and the connection MUST close with + // TRANSPORT_PARAMETER_ERROR. + // + // - max_udp_payload_size: minimum 1200 (the §14 datagram size + // floor). A smaller value would force fragmented Initials, + // which our writer can't produce. + // - ack_delay_exponent: maximum 20. Beyond that, the + // `ackDelay << exponent` shift in the parser overflows even + // with the clamp. + // - active_connection_id_limit: minimum 2. A value < 2 leaves + // the peer no spare CID to migrate to. + tp.maxUdpPayloadSize?.let { v -> + if (v < 1200L) { + markClosedExternally( + "TRANSPORT_PARAMETER_ERROR: max_udp_payload_size $v < 1200", + ) + return + } + } + tp.ackDelayExponent?.let { v -> + if (v > 20L) { + markClosedExternally( + "TRANSPORT_PARAMETER_ERROR: ack_delay_exponent $v > 20", + ) + return + } + } + tp.activeConnectionIdLimit?.let { v -> + if (v < 2L) { + markClosedExternally( + "TRANSPORT_PARAMETER_ERROR: active_connection_id_limit $v < 2", + ) + return + } + } peerTransportParameters = tp qlogObserver.onTransportParametersSet("remote", peerTransportParametersSummary(tp)) sendConnectionFlowCredit = tp.initialMaxData ?: 0L @@ -1514,8 +1665,34 @@ class QuicConnection( closeAllSignals() } - /** Called by the parser on inbound CONNECTION_CLOSE or by the driver on read-loop death. */ - internal fun markClosedExternally(reason: String) { + /** + * Called by the parser on inbound CONNECTION_CLOSE or by the driver + * on read-loop death. + * + * [errorCode] surfaces the spec-mandated transport / TLS-alert + * code on the connection's `closeErrorCode` getter so observability + * tools (qlog, support logs) see the precise reason. Default is + * [QuicTransportError.NO_ERROR] for backward compatibility — most + * callers that don't yet pin a specific code keep the prior + * behaviour. Specific call sites: + * - TLS layer: passes `0x100 + alert_description` per RFC 9001 §4.8 + * via [com.vitorpamplona.quic.TlsAlertException.quicErrorCode]; + * - CRYPTO buffer overflow: [QuicTransportError.CRYPTO_BUFFER_EXCEEDED]; + * - AEAD limit: [QuicTransportError.AEAD_LIMIT_REACHED]; + * - Transport-parameter violations: [QuicTransportError.TRANSPORT_PARAMETER_ERROR]; + * - Flow-control / stream-state / final-size violations: their + * matching §20.1 codes. + * + * Note: [markClosedExternally] does NOT emit a CONNECTION_CLOSE + * frame on the wire (it transitions straight to CLOSED). The + * [errorCode] is for our local observability. The wire-emit path + * (`close(errorCode, reason)` → CLOSING → writer drain) is for + * graceful application-initiated close. + */ + internal fun markClosedExternally( + reason: String, + errorCode: Long = QuicTransportError.NO_ERROR, + ) { // First-call wins for [closeReason] so the highest-quality // diagnostic is preserved when several teardown paths race // (e.g. read loop's `socket.receive() == null` finally fires @@ -1530,6 +1707,13 @@ class QuicConnection( if (status == Status.CLOSED) return@synchronized false status = Status.CLOSED closeReason = reason + // Only set the code on first transition AND when the + // caller passed a non-default value; preserves the + // earlier-set code if a caller raced past us with + // NO_ERROR. + if (errorCode != QuicTransportError.NO_ERROR && closeErrorCode == 0L) { + closeErrorCode = errorCode + } true } // Wake any teardown coroutine waiting for the close to land. Safe @@ -1875,6 +2059,13 @@ class QuicConnection( currentSendKeyPhase = !currentSendKeyPhase } } + // RFC 9001 §6.6 limits are PER-KEY: every rotation resets both + // the encrypt count (now using fresh send keys) and the + // decrypt-failure count (fresh receive keys mean prior failures + // can't accumulate further toward the integrity limit). + aeadEncryptCount = 0L + aeadDecryptFailureCount = 0L + aeadKeyUpdateRequested = false qlogObserver.onKeyUpdated("server", EncryptionLevel.APPLICATION) qlogObserver.onKeyUpdated("client", EncryptionLevel.APPLICATION) } @@ -1962,6 +2153,10 @@ class QuicConnection( currentReceiveKeyPhase = !currentReceiveKeyPhase currentSendKeyPhase = !currentSendKeyPhase keyUpdateInProgress = true + // RFC 9001 §6.6 per-key counters reset on rotation (see commitKeyUpdate). + aeadEncryptCount = 0L + aeadDecryptFailureCount = 0L + aeadKeyUpdateRequested = false qlogObserver.onKeyUpdated("client", EncryptionLevel.APPLICATION) qlogObserver.onKeyUpdated("server", EncryptionLevel.APPLICATION) return true @@ -2318,6 +2513,166 @@ 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 + } + + /** + * RFC 9000 §10.2.2: enter the draining period after receiving a + * peer's CONNECTION_CLOSE. Sets [status] to [Status.DRAINING], + * records a `now + 3 * PTO` deadline at which the driver flips + * the connection to [Status.CLOSED]. While draining, the writer + * MUST NOT emit any packets; the read loop drops late inbound + * silently. + * + * For the application this is functionally equivalent to "closed" + * — `awaitIncoming*` returns null, new streams can't be opened — + * but the explicit DRAINING state lets observability tools (qlog, + * tests) see the spec-mandated grace period. + * + * Per §10.2.2 caller transitions ONLY from CONNECTED / HANDSHAKING. + * A second call (peer retransmits its CONNECTION_CLOSE) is a + * harmless no-op. + */ + internal fun enterDraining( + reason: String, + nowMillis: Long, + ) { + val firstTransition = + synchronized(closeStateMonitor) { + if (status == Status.CLOSED || status == Status.DRAINING) return@synchronized false + closeReason = reason + status = Status.DRAINING + true + } + if (!firstTransition) return + // 3 * PTO deadline. Use the connection's `lossDetection.ptoBaseMs` + // for the post-handshake case (peer params have arrived); fall + // back to a small floor pre-handshake. + val maxAckDelay = peerTransportParameters?.maxAckDelay ?: 0L + val ptoBaseMs = lossDetection.ptoBaseMs(maxAckDelay).coerceAtLeast(1L) + drainingDeadlineMs = nowMillis + (ptoBaseMs * 3L).coerceAtLeast(MIN_DRAINING_PERIOD_MS) + // Wake any close()-awaiter so they don't sit on the + // `closingDrainSignal` for 3*PTO unnecessarily. + closingDrainSignal.complete(Unit) + qlogObserver.onConnectionClosed("remote", closeErrorCode, reason) + if (!handshakeComplete) { + signalHandshakeFailed(QuicConnectionClosedException("connection closed externally: $reason")) + } + } + + /** + * RFC 9000 §10.2.2 draining-deadline check. Returns true once + * the 3 * PTO grace period has elapsed and the connection should + * be flipped to fully closed. + */ + internal fun isDrainingExpired(nowMillis: Long): Boolean { + val deadline = drainingDeadlineMs ?: return false + return status == Status.DRAINING && nowMillis >= deadline + } + + /** + * RFC 9000 §10.3 stateless-reset detection. A peer that has lost + * connection state (crash, restart, route change) signals so by + * sending a "Stateless Reset" datagram — bytes that look like a + * short-header packet on the wire but whose last 16 bytes equal a + * `stateless_reset_token` previously communicated by the peer. + * + * The check fires at AEAD-failure time (a stateless reset is + * indistinguishable from forgery without the token comparison). + * Comparison is constant-time per §10.3.1 to avoid leaking token + * bits via timing. + * + * Token sources covered: + * - The peer's `stateless_reset_token` transport parameter (the + * 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 + // Form bit must be 0 (looks-like-short-header). + if ((datagram[0].toInt() and 0x80) != 0) return false + val tokenStart = datagram.size - 16 + // Compare against every known token. Constant-time per token; + // total time scales with token count but doesn't reveal which + // (if any) matched. + var anyMatch = false + peerTransportParameters?.statelessResetToken?.let { tok -> + if (tok.size == 16 && constantTimeEqualsRange(tok, datagram, tokenStart)) { + anyMatch = true + } + } + for (token in pathValidator.allKnownStatelessResetTokens()) { + if (token.size != 16) continue + if (constantTimeEqualsRange(token, datagram, tokenStart)) { + anyMatch = true + } + } + return anyMatch + } + + /** + * Constant-time comparison of [token] (16 bytes) against the slice + * of [datagram] starting at [datagramOffset]. Caller MUST verify + * `datagram.size >= datagramOffset + token.size` upstream. + */ + private fun constantTimeEqualsRange( + token: ByteArray, + datagram: ByteArray, + datagramOffset: Int, + ): Boolean { + var diff = 0 + for (i in 0 until token.size) { + diff = diff or (token[i].toInt() xor datagram[datagramOffset + i].toInt()) + } + return diff == 0 + } + /** * Check whether an outstanding PATH_CHALLENGE has exceeded the * 3 * PTO budget (RFC 9000 §8.2.4). Called from the driver's @@ -2585,6 +2940,14 @@ class QuicConnection( } companion object { + /** + * RFC 9000 §10.2.2 floor on the draining-period grace. The + * spec mandates "at least 3 * PTO" which can be vanishingly + * small pre-handshake. Floor at 100 ms so qlog observers and + * tests can see the DRAINING window. + */ + const val MIN_DRAINING_PERIOD_MS: Long = 100L + /** * Bound on the inbound datagram queue depth. RFC 9221 datagrams are * outside connection-level flow control, so without this cap a peer 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 f400498e3c..8b6d4861e8 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,30 @@ 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) + } + // RFC 9000 §10.2.2 draining-period deadline. Set when + // peer's CONNECTION_CLOSE arrives; we hold in DRAINING + // until 3 * PTO elapses, then transition to CLOSED. + val drainingDelta = + connection.drainingDeadlineMs?.let { + (it - nowForLossTimer).coerceAtLeast(0L) + } val sleepMillis = - if (nextLossDelta != null && nextLossDelta < ptoMillis) nextLossDelta else ptoMillis + minOf( + ptoMillis, + nextLossDelta ?: Long.MAX_VALUE, + idleDelta ?: Long.MAX_VALUE, + drainingDelta ?: Long.MAX_VALUE, + ) // Suspend until either: a wakeup arrives, or the timer expires. val woke = withTimeoutOrNull(sleepMillis) { @@ -297,12 +319,28 @@ 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 draining-deadline / idle-timeout / + // loss-timer / PTO expiry. RFC 9000 §10.2.2: the + // draining period transitions to fully CLOSED once + // 3 * PTO has elapsed since the peer's + // CONNECTION_CLOSE — gives the peer's last + // retransmits a chance to converge before we discard + // state. Idle-timeout fires per §10.2.1 ("silently + // closes — 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 (connection.isDrainingExpired(nowMillis())) { + connection.markClosedExternally("draining period elapsed") + continue + } + 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 08e7910291..9bbbfbbd81 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionParser.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionParser.kt @@ -52,6 +52,24 @@ import com.vitorpamplona.quic.tls.TlsClient /** RFC 9000 §16: maximum varint value, also the per-stream offset ceiling. */ private const val MAX_QUIC_OFFSET: Long = (1L shl 62) - 1L +/** + * RFC 9000 §22 CRYPTO_BUFFER_EXCEEDED cap. Largest amount of CRYPTO + * data we'll buffer per encryption level past the contiguous read + * frontier before closing the connection. Sized to comfortably hold: + * - Initial: ClientHello + ServerHello + (rarely fragmented) Initial + * flight (~few KB). + * - Handshake: EncryptedExtensions + full RSA-4096 cert chain (~ten + * intermediates worst case) + CertificateVerify + Finished. Real + * deployments cap out under 30 KB. + * - Application: post-handshake NewSessionTicket(s); a few hundred + * bytes per ticket. + * + * The 64 KiB cap leaves significant headroom over real handshakes + * while bounding the worst-case heap a misbehaving peer can pin + * (3 levels × 64 KiB = 192 KiB). + */ +private const val CRYPTO_BUFFER_CAP_BYTES: Long = 64L * 1024L + /** * Decode every QUIC packet inside a single inbound UDP datagram and dispatch * its frames to [conn]'s state. @@ -82,7 +100,30 @@ fun feedDatagram( // header after HP unmask (or a similar invariant violation // bubbled out of the parse path). Spec MUST close with // PROTOCOL_VIOLATION. - conn.markClosedExternally(e.message ?: "PROTOCOL_VIOLATION") + conn.markClosedExternally( + e.message ?: "PROTOCOL_VIOLATION", + errorCode = com.vitorpamplona.quic.frame.QuicTransportError.PROTOCOL_VIOLATION, + ) + } catch (e: com.vitorpamplona.quic.TlsAlertException) { + // RFC 9001 §4.8: a TLS alert maps to QUIC error code + // `0x100 + alert_description`. Surface the precise spec code + // on `closeErrorCode` so qlog / support logs can identify + // the TLS reason instead of seeing only INTERNAL_ERROR. + conn.markClosedExternally( + "CRYPTO_ERROR (TLS alert ${e.alertCode}): ${e.message ?: "no detail"}", + errorCode = e.quicErrorCode, + ) + } catch (e: com.vitorpamplona.quic.QuicCodecException) { + // Generic TLS / frame parse failure that didn't carry a + // specific alert code. Map to CRYPTO_ERROR with TLS + // `internal_error = 80` so the close still falls in the + // §4.8 range; observers can distinguish it from + // PROTOCOL_VIOLATION (transport layer) by the 0x100 prefix. + val alertCode = com.vitorpamplona.quic.tls.TlsConstants.ALERT_INTERNAL_ERROR + conn.markClosedExternally( + "CRYPTO_ERROR (TLS internal_error): ${e.message ?: e::class.simpleName}", + errorCode = 0x100L + alertCode.toLong(), + ) } } @@ -91,6 +132,28 @@ private fun feedDatagramInner( datagram: ByteArray, nowMillis: Long, ) { + // RFC 9000 §10.2.2: while in the DRAINING state, late inbound + // packets MUST be discarded silently. Same goes for CLOSED. + if (conn.status == QuicConnection.Status.DRAINING || + conn.status == QuicConnection.Status.CLOSED + ) { + return + } + // RFC 9000 §10.3 stateless-reset detection. A peer that has lost + // connection state signals so by sending a datagram whose trailing + // 16 bytes equal a stateless_reset_token we previously received. + // Pre-empts the AEAD path entirely: a real short-header packet's + // trailing 16 bytes (the AEAD tag) match a known token with + // probability 2^-128, so the false-positive rate is zero in + // practice. Spec MUST close silently — no CONNECTION_CLOSE per + // §10.3.1. + if (datagram.isNotEmpty() && + (datagram[0].toInt() and 0x80) == 0 && + conn.isStatelessReset(datagram) + ) { + conn.markClosedExternally("stateless reset received from peer") + return + } var offset = 0 while (offset < datagram.size) { val first = datagram[offset].toInt() and 0xFF @@ -259,6 +322,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) { @@ -416,10 +482,34 @@ private fun feedShortHeaderPacket( } } if (parsed == null) { + // RFC 9000 §10.3: an undecryptable short-header datagram may + // be a Stateless Reset from a peer that lost connection + // state (crash, restart, NAT rebinding). Before counting it + // as a forgery against the integrity limit, check whether + // its trailing 16 bytes match any known stateless-reset + // token. If so, the connection is over — silently transition + // to CLOSED per §10.3.1 (no CONNECTION_CLOSE emission, no + // further packets, just stop). + if (offset == 0 && conn.isStatelessReset(datagram)) { + conn.markClosedExternally("stateless reset received from peer") + return + } conn.qlogObserver.onPacketDropped( "AEAD auth failed or header parse failed at level APPLICATION", datagram.size - offset, ) + // RFC 9001 §6.6 / §B.1 integrity limit. A 1-RTT packet that + // failed AEAD verification counts as a forgery attempt against + // the active receive key. Closing here prevents an attacker + // from grinding through the integrity-limit-many forgeries + // searching for a key recovery on the underlying AEAD. + conn.aeadDecryptFailureCount += 1L + val limit = live.aead.integrityLimit + if (conn.aeadDecryptFailureCount >= limit) { + conn.markClosedExternally( + "AEAD_LIMIT_REACHED: 1-RTT decrypt-failure count ${conn.aeadDecryptFailureCount} >= integrity limit $limit", + ) + } return } // AEAD succeeded with the candidate next-phase keys → commit the @@ -450,6 +540,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, @@ -544,8 +637,19 @@ private fun dispatchFrames( // before consuming. Per RFC 9000 §18.2 the exponent is // 0..20; we further clamp ackDelay so the shift never // overflows (`ackDelay <= Long.MAX_VALUE >>> exponent`). - val rawExponent = conn.config.ackDelayExponent - val exponent = rawExponent.coerceIn(0L, 20L).toInt() + // + // RFC 9000 §13.2.5: a received ACK is decoded using the + // PEER's `ack_delay_exponent`, not ours. Pre-handshake + // peer params haven't arrived yet — fall back to the + // default of 3 per §18.2. Coercion below ensures even + // a malicious peer that smuggles a >20 value through + // the §18.2 bounds check (e.g. on a connection where + // the peer-params bounds enforcement is bypassed by a + // race) can't desync our RTT estimator. + val peerExponent = + conn.peerTransportParameters?.ackDelayExponent + ?: TransportParameterDefaults.ACK_DELAY_EXPONENT + val exponent = peerExponent.coerceIn(0L, 20L).toInt() val maxAckDelayPreShift = if (exponent == 0) Long.MAX_VALUE else Long.MAX_VALUE ushr exponent val safeAckDelay = frame.ackDelay.coerceIn(0L, maxAckDelayPreShift) @@ -588,6 +692,23 @@ private fun dispatchFrames( is CryptoFrame -> { ackEliciting = true + // RFC 9000 §22 / §7.5: a peer sending more CRYPTO data + // than we can buffer per encryption level MUST close + // the connection with CRYPTO_BUFFER_EXCEEDED. The cap + // is generous (covers a worst-case RSA-4096 cert chain + // with intermediates) but bounded so a misbehaving + // peer can't pin gigabytes by sending huge offsets. + // Pre-check: reject obviously-overflowing frames + // BEFORE the insert so we don't buffer them at all. + val frameEnd = frame.offset + frame.data.size.toLong() + val proposed = maxOf(state.cryptoReceive.highestObservedOffset(), frameEnd) + if (proposed - state.cryptoReceive.contiguousEnd() > CRYPTO_BUFFER_CAP_BYTES) { + conn.markClosedExternally( + "CRYPTO_BUFFER_EXCEEDED: $level CRYPTO buffered=${proposed - state.cryptoReceive.contiguousEnd()} > cap=$CRYPTO_BUFFER_CAP_BYTES", + errorCode = com.vitorpamplona.quic.frame.QuicTransportError.CRYPTO_BUFFER_EXCEEDED, + ) + return + } state.cryptoReceive.insert(frame.offset, frame.data) val contiguous = state.cryptoReceive.readContiguous() if (contiguous.isNotEmpty()) { @@ -648,6 +769,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. @@ -658,6 +792,25 @@ private fun dispatchFrames( ) return } + // RFC 9000 §4.1 connection-level enforcement: the SUM + // across all streams of "largest stream offset received" + // MUST NOT exceed the most recently advertised MAX_DATA. + // We update [conn.connectionInboundOffsetSum] only when + // this frame's end-offset advances the per-stream + // high-water mark, so duplicate / reordered frames don't + // double-count. + if (frameEnd > stream.receiveHighestOffset) { + val delta = frameEnd - stream.receiveHighestOffset + stream.receiveHighestOffset = frameEnd + conn.connectionInboundOffsetSum += delta + if (conn.connectionInboundOffsetSum > conn.advertisedMaxData) { + conn.markClosedExternally( + "FLOW_CONTROL_ERROR: peer exceeded connection MAX_DATA " + + "(${conn.connectionInboundOffsetSum} > ${conn.advertisedMaxData})", + ) + return + } + } // RFC 9000 §4.5: enforce final-size invariants. The // [com.vitorpamplona.quic.stream.ReceiveBuffer.insert] // surface returns a typed result; map any non-OK result @@ -813,6 +966,30 @@ private fun dispatchFrames( return } } + // RFC 9000 §4.5: a RESET_STREAM's finalSize counts toward + // connection-level flow control even though no STREAM frame + // ever delivered those bytes — it represents what the peer + // committed to send before aborting. Top up + // [conn.connectionInboundOffsetSum] when finalSize advances + // beyond what STREAM frames already counted, then re-check + // against the connection cap. + if (target != null && frame.finalSize > target.receiveHighestOffset) { + val delta = frame.finalSize - target.receiveHighestOffset + target.receiveHighestOffset = frame.finalSize + conn.connectionInboundOffsetSum += delta + if (conn.connectionInboundOffsetSum > conn.advertisedMaxData) { + conn.markClosedExternally( + "FLOW_CONTROL_ERROR: peer RESET_STREAM finalSize pushed connection past MAX_DATA " + + "(${conn.connectionInboundOffsetSum} > ${conn.advertisedMaxData})", + ) + 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() @@ -920,7 +1097,14 @@ private fun dispatchFrames( // Audit-4 #13: any frames following CONNECTION_CLOSE in the // same payload MUST NOT be dispatched — they could create // streams or deliver bytes on an already-closed connection. - conn.markClosedExternally("peer CONNECTION_CLOSE: ${frame.reason}") + // + // RFC 9000 §10.2.2: enter the DRAINING state for 3 * PTO + // before transitioning to CLOSED. During draining we + // MUST NOT send packets and SHOULD discard inbound + // silently. The driver's send loop transitions to + // CLOSED when [QuicConnection.drainingDeadlineMs] + // elapses. + conn.enterDraining("peer CONNECTION_CLOSE: ${frame.reason}", nowMillis) return } 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 89a8b9cfa1..2981f4f9b5 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionWriter.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionWriter.kt @@ -93,6 +93,11 @@ fun drainOutbound( conn.closingDrainSignal.complete(Unit) return datagram } + // RFC 9000 §10.2.2: in the draining state we MUST NOT send any + // further packets. The driver's send-loop timer transitions the + // status to CLOSED once the 3 * PTO grace elapses; until then + // drainOutbound returns null on every call. + if (conn.status == QuicConnection.Status.DRAINING) return null // Bug-A fix: drive the §8.2.4 3*PTO validation budget on every // drain, not just on PTO expiration. The peer ACKs PATH_CHALLENGE @@ -544,14 +549,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 } @@ -664,9 +674,36 @@ private fun buildApplicationPacket( // stream and MAX_DATA at the connection level. appendFlowControlUpdates(conn, frames, tokens) - // Pending datagrams + // Pending datagrams. RFC 9221 §3 enforcement: + // - if the peer didn't advertise `max_datagram_frame_size` (or + // advertised 0), DATAGRAM MUST NOT be sent — drop with diagnostic; + // - if the encoded frame would exceed the peer's advertised + // `max_datagram_frame_size` (frame type byte + length varint + + // payload), drop with diagnostic. + // Pre-fix the writer emitted DATAGRAM regardless and let + // spec-conformant peers close the connection with PROTOCOL_VIOLATION. + val peerDatagramCap = conn.peerTransportParameters?.maxDatagramFrameSize ?: 0L while (conn.pendingDatagramsLocked().isNotEmpty()) { val payload = conn.pendingDatagramsLocked().removeFirst() + if (peerDatagramCap <= 0L) { + conn.qlogObserver.onPacketDropped( + "outbound DATAGRAM dropped — peer did not advertise max_datagram_frame_size", + payload.size, + ) + continue + } + // Frame total = 1 byte type (0x31) + varint(payload.size) + payload. + val frameSize = + 1 + + com.vitorpamplona.quic.Varint + .size(payload.size.toLong()) + payload.size + if (frameSize > peerDatagramCap) { + conn.qlogObserver.onPacketDropped( + "outbound DATAGRAM dropped — frame size $frameSize > peer cap $peerDatagramCap", + payload.size, + ) + continue + } frames += DatagramFrame(payload, explicitLength = true) if (frames.size >= 16) break } @@ -866,14 +903,42 @@ 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 + // RFC 9001 §6.6 / §B.1 confidentiality limit. Track per-key + // encryption count for the active 1-RTT send key. At half the + // AEAD's confidentiality limit we soft-trigger a key update; at + // the limit we close the connection if rotation hasn't completed. + if (sizeBytes.isSuccess) { + conn.aeadEncryptCount += 1L + val limit = proto.aead.confidentialityLimit + if (!conn.aeadKeyUpdateRequested && conn.aeadEncryptCount >= limit / 2L) { + // Soft trigger — try to rotate. initiateKeyUpdate is a + // no-op if a previous rotation is still in flight, so the + // latch only flips on the first successful initiation. + conn.aeadKeyUpdateRequested = true + conn.initiateKeyUpdate() + } + if (conn.aeadEncryptCount >= limit) { + // Hard limit — rotation didn't complete in time. RFC 9001 + // §6.6 mandates closing rather than continuing to encrypt + // under a key whose confidentiality is no longer assured. + conn.markClosedExternally( + "AEAD_LIMIT_REACHED: 1-RTT encryption count ${conn.aeadEncryptCount} >= confidentiality limit $limit", + ) + } + } sizeBytes.getOrNull()?.let { built -> emitQlogSent(conn, EncryptionLevel.APPLICATION, pn, built.size, frames) } diff --git a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/TransportParameters.kt b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/TransportParameters.kt index d06debe982..a343bf3a94 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/TransportParameters.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/TransportParameters.kt @@ -52,6 +52,23 @@ object TransportParameterId { const val MAX_DATAGRAM_FRAME_SIZE: Long = 0x20 } +/** + * RFC 9000 §18.2 default values for parameters that are NOT advertised + * by the peer. Surfacing them as constants so the parser / writer can + * use them as the "peer didn't tell us" fallback without each call site + * hard-coding the numeric default. + */ +object TransportParameterDefaults { + /** §18.2: default `ack_delay_exponent` is 3 if not advertised. */ + const val ACK_DELAY_EXPONENT: Long = 3L + + /** §18.2: default `max_ack_delay` is 25 ms if not advertised. */ + const val MAX_ACK_DELAY_MS: Long = 25L + + /** §18.2: default `active_connection_id_limit` is 2 if not advertised. */ + const val ACTIVE_CONNECTION_ID_LIMIT: Long = 2L +} + /** * QUIC transport parameters as exchanged inside the TLS QUIC transport_params * extension. diff --git a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/crypto/Aead.kt b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/crypto/Aead.kt index e0671b9041..2a4feec53b 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/crypto/Aead.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/crypto/Aead.kt @@ -38,6 +38,28 @@ abstract class Aead { abstract val nonceLength: Int abstract val tagLength: Int + /** + * RFC 9001 §B.1 confidentiality limit — the maximum number of + * packets the endpoint can encrypt with a single key before MUST + * initiating a key update / closing the connection. + * + * AES-128-GCM: 2^23 = 8_388_608. + * ChaCha20-Poly1305: 2^62 (effectively unlimited for practical + * sessions, but still a finite cap). + */ + abstract val confidentialityLimit: Long + + /** + * RFC 9001 §B.1 integrity limit — the maximum number of forged + * packets (failed AEAD verifications) the endpoint may attempt to + * decrypt before MUST closing the connection with + * AEAD_LIMIT_REACHED. + * + * AES-128-GCM: 2^52 (effectively unreachable). + * ChaCha20-Poly1305: 2^36. + */ + abstract val integrityLimit: Long + abstract fun seal( key: ByteArray, nonce: ByteArray, @@ -153,6 +175,10 @@ object Aes128Gcm : Aead() { override val nonceLength = 12 override val tagLength = 16 + // RFC 9001 §B.1 limits for AEAD_AES_128_GCM. + override val confidentialityLimit: Long = 1L shl 23 + override val integrityLimit: Long = 1L shl 52 + override fun seal( key: ByteArray, nonce: ByteArray, @@ -186,6 +212,10 @@ object ChaCha20Poly1305Aead : Aead() { override val nonceLength = 12 override val tagLength = 16 + // RFC 9001 §B.1 limits for AEAD_CHACHA20_POLY1305. + override val confidentialityLimit: Long = (1L shl 62) + override val integrityLimit: Long = 1L shl 36 + override fun seal( key: ByteArray, nonce: ByteArray, diff --git a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/frame/Frame.kt b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/frame/Frame.kt index a3ae7f8d87..8dc5f84c93 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/frame/Frame.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/frame/Frame.kt @@ -65,6 +65,55 @@ object FrameType { const val DATAGRAM_LEN: Long = 0x31 } +/** + * RFC 9000 §20.1 transport-error code identifiers. These are the values + * we put in the `errorCode` field of a CONNECTION_CLOSE (Transport, + * frame type 0x1c). The qlog observer surfaces them as the spec + * mnemonic so post-mortem analysis tools can categorize closes. + * + * RFC 9001 §4.8 reserves the range 0x100–0x1ff for TLS alerts mapped + * via `0x100 + alert_description`; those aren't enumerated here — the + * encoder builds them ad-hoc via [TlsAlertException.quicErrorCode]. + */ +object QuicTransportError { + const val NO_ERROR: Long = 0x00 + const val INTERNAL_ERROR: Long = 0x01 + const val CONNECTION_REFUSED: Long = 0x02 + const val FLOW_CONTROL_ERROR: Long = 0x03 + const val STREAM_LIMIT_ERROR: Long = 0x04 + const val STREAM_STATE_ERROR: Long = 0x05 + const val FINAL_SIZE_ERROR: Long = 0x06 + const val FRAME_ENCODING_ERROR: Long = 0x07 + const val TRANSPORT_PARAMETER_ERROR: Long = 0x08 + const val CONNECTION_ID_LIMIT_ERROR: Long = 0x09 + const val PROTOCOL_VIOLATION: Long = 0x0a + const val INVALID_TOKEN: Long = 0x0b + const val APPLICATION_ERROR: Long = 0x0c + + /** + * RFC 9000 §22: an endpoint received more data in CRYPTO frames + * than it can buffer. We enforce a per-level cap so a misbehaving + * peer can't pin unbounded memory before the handshake completes. + */ + const val CRYPTO_BUFFER_EXCEEDED: Long = 0x0d + + /** + * RFC 9000 §22 / RFC 9001 §6: an endpoint detected errors in + * performing a key update — e.g. peer used the wrong key phase + * with a regressing PN, or AEAD failed under both the live and + * next-phase keys during a rotation attempt. + */ + const val KEY_UPDATE_ERROR: Long = 0x0e + + /** + * RFC 9001 §6.6: encryption / integrity limit on the active AEAD + * was reached. Used by the per-key invocation tracker. + */ + const val AEAD_LIMIT_REACHED: Long = 0x0f + + const val NO_VIABLE_PATH: Long = 0x10 +} + sealed class Frame { abstract fun encode(out: QuicWriter) } diff --git a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/http3/Http3FrameTypes.kt b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/http3/Http3FrameTypes.kt index bbe1b4db78..6fb73fc232 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/http3/Http3FrameTypes.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/http3/Http3FrameTypes.kt @@ -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 +} diff --git a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/http3/Http3Settings.kt b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/http3/Http3Settings.kt index cab63e806f..aad9d829f2 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/http3/Http3Settings.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/http3/Http3Settings.kt @@ -86,6 +86,15 @@ data class Http3Settings( id: Long, value: Long, ) { + // RFC 9114 §7.2.4.1: SETTINGS identifiers in the HTTP/2 range + // (0x02, 0x03, 0x04, 0x05) are reserved to prevent confusion + // with HTTP/2 settings — receipt MUST be a connection error of + // type H3_SETTINGS_ERROR. Pre-fix we accepted them silently. + if (id == 0x02L || id == 0x03L || id == 0x04L || id == 0x05L) { + throw com.vitorpamplona.quic.QuicCodecException( + "H3_SETTINGS_ERROR: reserved HTTP/2 SETTINGS id 0x${id.toString(16)}", + ) + } if (value < 0L) { throw com.vitorpamplona.quic.QuicCodecException( "negative HTTP/3 SETTINGS value for id 0x${id.toString(16)}: $value", 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 c54d7b36a2..63b4ec5a09 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 835861bac4..21b8a6f428 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/commonMain/kotlin/com/vitorpamplona/quic/stream/QuicStream.kt b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/stream/QuicStream.kt index 97974fec1e..f25bbaea41 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/stream/QuicStream.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/stream/QuicStream.kt @@ -128,6 +128,35 @@ class QuicStream( var receiveLimit: Long = 0L internal set + /** + * RFC 9000 §4.1: highest stream offset (offset + length) ever seen + * on an inbound STREAM or RESET_STREAM frame. Used by + * [QuicConnection]'s connection-level flow-control accounting — + * the spec requires the receiver to compare the SUM across all + * streams of "largest received offset" against the advertised + * `initial_max_data` / latest MAX_DATA, NOT the contiguous read + * frontier. The writer's MAX_DATA threshold logic still uses the + * cheaper contiguous-end approximation; this field is purely the + * enforcement signal on receive. + */ + @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/commonMain/kotlin/com/vitorpamplona/quic/tls/TlsClient.kt b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/tls/TlsClient.kt index d6499b640e..0528557199 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/tls/TlsClient.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/tls/TlsClient.kt @@ -337,16 +337,30 @@ class TlsClient( // offer X25519, the only group nests + most servers accept), so // any HRR is a hard failure. if (sh.random.contentEquals(HELLO_RETRY_REQUEST_RANDOM)) { - throw QuicCodecException("HelloRetryRequest received but not supported") + // RFC 8446 §4.1.4 — HRR follows the standard handshake + // failure path on our side. + throw com.vitorpamplona.quic.TlsAlertException( + alertCode = TlsConstants.ALERT_HANDSHAKE_FAILURE, + message = "HelloRetryRequest received but not supported (we offer X25519 only)", + ) } if (sh.negotiatedVersion != TlsConstants.VERSION_TLS_1_3) { - throw QuicCodecException("server did not negotiate TLS 1.3") + // RFC 8446 §6.2 — wrong version is `protocol_version = 70`. + throw com.vitorpamplona.quic.TlsAlertException( + alertCode = TlsConstants.ALERT_PROTOCOL_VERSION, + message = "server did not negotiate TLS 1.3 (got 0x${sh.negotiatedVersion.toString(16)})", + ) } val cipher = sh.cipherSuite if (cipher != TlsConstants.CIPHER_TLS_AES_128_GCM_SHA256 && cipher != TlsConstants.CIPHER_TLS_CHACHA20_POLY1305_SHA256 ) { - throw QuicCodecException("server picked unsupported cipher 0x${cipher.toString(16)}") + // RFC 8446 §6.2 — server picked something we didn't offer + // → `illegal_parameter = 47`. + throw com.vitorpamplona.quic.TlsAlertException( + alertCode = TlsConstants.ALERT_ILLEGAL_PARAMETER, + message = "server picked unsupported cipher 0x${cipher.toString(16)}", + ) } negotiatedCipherSuite = cipher serverKeyShare = sh.serverKeyShareX25519 @@ -432,8 +446,11 @@ class TlsClient( // proceed with HTTP/3 code paths assuming h3. val alpn = ee.alpn if (alpn != null && !offeredAlpns.any { it.contentEquals(alpn) }) { - throw QuicCodecException( - "server selected ALPN '${alpn.decodeToString()}' which we did not offer", + // RFC 7301 §3.2 + RFC 8446 §6.2: a server picking an + // unknown ALPN is `no_application_protocol = 120`. + throw com.vitorpamplona.quic.TlsAlertException( + alertCode = TlsConstants.ALERT_NO_APPLICATION_PROTOCOL, + message = "server selected ALPN '${alpn.decodeToString()}' which we did not offer", ) } negotiatedAlpn = alpn @@ -452,7 +469,20 @@ class TlsClient( when (type) { TlsConstants.HS_CERTIFICATE -> { val cert = TlsCertificateChain.decodeBody(bodyReader) - certificateValidator.validateChain(cert.certificates, serverName) + try { + certificateValidator.validateChain(cert.certificates, serverName) + } catch (t: Throwable) { + // RFC 8446 §6.2 — `bad_certificate = 42`. The + // validator's own message (e.g. "PKIX path + // building failed") becomes the close reason + // so the application support log shows what + // went wrong. + throw com.vitorpamplona.quic.TlsAlertException( + alertCode = TlsConstants.ALERT_BAD_CERTIFICATE, + message = "certificate chain validation failed: ${t.message ?: t::class.simpleName}", + cause = t, + ) + } transcript.append(msg) state = State.WAITING_CERTIFICATE_VERIFY } @@ -490,7 +520,18 @@ class TlsClient( if (type != TlsConstants.HS_CERTIFICATE_VERIFY) throw QuicCodecException("expected CertificateVerify, got type=$type") val cv = TlsCertificateVerify.decodeBody(bodyReader) val transcriptHash = transcript.snapshot() - certificateValidator.verifySignature(cv.signatureAlgorithm, cv.signature, transcriptHash) + try { + certificateValidator.verifySignature(cv.signatureAlgorithm, cv.signature, transcriptHash) + } catch (t: Throwable) { + // RFC 8446 §4.4.3 — bad CertificateVerify signature is + // `decrypt_error = 51` (used for any failure to verify + // a handshake-time signature or MAC). + throw com.vitorpamplona.quic.TlsAlertException( + alertCode = TlsConstants.ALERT_DECRYPT_ERROR, + message = "CertificateVerify signature failed: ${t.message ?: t::class.simpleName}", + cause = t, + ) + } transcript.append(msg) state = State.WAITING_SERVER_FINISHED } @@ -550,8 +591,16 @@ class TlsClient( } TlsConstants.HS_KEY_UPDATE -> { - throw QuicCodecException( - "TLS KeyUpdate received but rotation not implemented; closing connection", + // RFC 9001 §6: TLS KeyUpdate messages MUST NOT be sent + // over QUIC. Reception MUST be treated as a connection + // error of type 0x010a (CRYPTO_ERROR with TLS alert + // "unexpected_message"). QUIC has its own KEY_PHASE-bit + // rotation mechanism (RFC 9001 §6.1) which our + // [QuicConnection.initiateKeyUpdate] / [commitKeyUpdate] + // path drives instead. + throw com.vitorpamplona.quic.TlsAlertException( + alertCode = TlsConstants.ALERT_UNEXPECTED_MESSAGE, + message = "RFC 9001 §6: TLS KeyUpdate forbidden over QUIC; use QUIC key rotation instead", ) } @@ -576,7 +625,13 @@ class TlsClient( // Verify server Finished MAC over transcript-up-to-CertificateVerify (or up to EE for PSK). val expected = finishedVerifyData(keySchedule.serverHandshakeSecret!!, transcript.snapshot()) if (!expected.contentEqualsConstantTime(finished.verifyData)) { - throw QuicCodecException("server Finished MAC mismatch") + // RFC 8446 §6.2 — bad Finished MAC is `decrypt_error = 51` + // (covers anything that fails handshake-message integrity + // verification). + throw com.vitorpamplona.quic.TlsAlertException( + alertCode = TlsConstants.ALERT_DECRYPT_ERROR, + message = "server Finished MAC mismatch", + ) } transcript.append(msg) diff --git a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/tls/TlsConstants.kt b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/tls/TlsConstants.kt index fa9e87506b..7871e63dd0 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/tls/TlsConstants.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/tls/TlsConstants.kt @@ -82,10 +82,28 @@ object TlsConstants { // ── Server-name (SNI) types ─────────────────────────────────────────────── const val SERVER_NAME_TYPE_HOST_NAME: Int = 0 - // ── Alert constants — only the ones we actually look at ─────────────────── + // ── Alert descriptions (RFC 8446 §B.2) ─────────────────────────────────── + // + // Used by RFC 9001 §4.8: a TLS-layer violation maps to QUIC error code + // `0x100 + alert_description`, surfaced via [TlsAlertException]. const val ALERT_CLOSE_NOTIFY: Int = 0 - const val ALERT_DECODE_ERROR: Int = 50 + const val ALERT_UNEXPECTED_MESSAGE: Int = 10 + const val ALERT_BAD_RECORD_MAC: Int = 20 const val ALERT_HANDSHAKE_FAILURE: Int = 40 + const val ALERT_BAD_CERTIFICATE: Int = 42 + const val ALERT_UNSUPPORTED_CERTIFICATE: Int = 43 + const val ALERT_CERTIFICATE_REVOKED: Int = 44 + const val ALERT_CERTIFICATE_EXPIRED: Int = 45 + const val ALERT_CERTIFICATE_UNKNOWN: Int = 46 + const val ALERT_ILLEGAL_PARAMETER: Int = 47 + const val ALERT_UNKNOWN_CA: Int = 48 + const val ALERT_DECODE_ERROR: Int = 50 + const val ALERT_DECRYPT_ERROR: Int = 51 + const val ALERT_PROTOCOL_VERSION: Int = 70 + const val ALERT_INTERNAL_ERROR: Int = 80 + const val ALERT_MISSING_EXTENSION: Int = 109 + const val ALERT_UNSUPPORTED_EXTENSION: Int = 110 + const val ALERT_NO_APPLICATION_PROTOCOL: Int = 120 // ── ALPN ────────────────────────────────────────────────────────────────── val ALPN_H3: ByteArray = "h3".encodeToByteArray() diff --git a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/webtransport/WtPeerStreamDemux.kt b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/webtransport/WtPeerStreamDemux.kt index 333d95885a..0cc040795a 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/webtransport/WtPeerStreamDemux.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/webtransport/WtPeerStreamDemux.kt @@ -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, + 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 diff --git a/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/AeadInvocationLimitTest.kt b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/AeadInvocationLimitTest.kt new file mode 100644 index 0000000000..58d9669074 --- /dev/null +++ b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/AeadInvocationLimitTest.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.crypto.Aead +import com.vitorpamplona.quic.crypto.Aes128Gcm +import com.vitorpamplona.quic.crypto.ChaCha20Poly1305Aead +import kotlinx.coroutines.runBlocking +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertTrue + +/** + * RFC 9001 §6.6 / §B.1 AEAD invocation limit enforcement. + * + * - Confidentiality limit: max number of packets encrypted with one + * send key. AES-128-GCM = 2^23, ChaCha20-Poly1305 = 2^62. + * - Integrity limit: max number of forged-packet AEAD failures on + * one receive key. AES-128-GCM = 2^52, ChaCha20-Poly1305 = 2^36. + * + * Pre-fix neither limit was tracked. A long-running session with + * AES-128-GCM would silently roll past 2^23 encrypts (security + * argument no longer holds); an attacker spamming forged packets + * could indefinitely grind for AEAD key recovery. + * + * The counter / limit logic is verified directly via the internal + * fields rather than spinning the writer for ~8 million encrypts — + * that would take minutes per test. + */ +class AeadInvocationLimitTest { + @Test + fun aes_128_gcm_limits_match_rfc_b1() { + val aead: Aead = Aes128Gcm + assertEquals(1L shl 23, aead.confidentialityLimit, "AES-128-GCM confidentiality limit per RFC 9001 §B.1") + assertEquals(1L shl 52, aead.integrityLimit, "AES-128-GCM integrity limit per RFC 9001 §B.1") + } + + @Test + fun chacha20_poly1305_limits_match_rfc_b1() { + val aead: Aead = ChaCha20Poly1305Aead + assertEquals(1L shl 62, aead.confidentialityLimit, "ChaCha20-Poly1305 confidentiality limit per RFC 9001 §B.1") + assertEquals(1L shl 36, aead.integrityLimit, "ChaCha20-Poly1305 integrity limit per RFC 9001 §B.1") + } + + @Test + fun encrypt_count_increments_per_application_packet() { + val (client, _) = newConnectedClient() + val before = client.aeadEncryptCount + // Open a uni stream and write to it — guaranteed to produce an + // ack-eliciting application packet on the next drain, regardless + // of whether the peer advertised `max_datagram_frame_size`. + runBlocking { + val stream = client.openUniStream() + stream.send.enqueue(byteArrayOf(0x01, 0x02, 0x03)) + stream.send.finish() + client.streamsLock.lock() + try { + drainOutbound(client, nowMillis = 0L) + } finally { + client.streamsLock.unlock() + } + } + assertTrue(client.aeadEncryptCount > before, "encrypt count must advance on outbound build, got $before -> ${client.aeadEncryptCount}") + } + + @Test + fun encrypt_count_resets_on_key_update_initiation() { + val (client, pipe) = newConnectedClient() + runBlocking { pipe.drive(maxRounds = 4) } + client.aeadEncryptCount = 10_000L + val rotated = client.initiateKeyUpdate() + // initiateKeyUpdate may legitimately fail if handshake-confirm + // hasn't propagated yet — the counter MUST still get reset on + // success. + if (rotated) { + assertEquals(0L, client.aeadEncryptCount, "counter resets on rotation") + assertEquals(0L, client.aeadDecryptFailureCount, "decrypt-failure counter resets on rotation too") + } + } + + @Test + fun encrypt_at_confidentiality_limit_closes_connection() { + val (client, pipe) = newConnectedClient() + runBlocking { pipe.drive(maxRounds = 4) } + // Pin the counter so the next outbound build crosses the limit. + // Skip the key-update soft trigger by also latching + // [aeadKeyUpdateRequested]. + client.aeadKeyUpdateRequested = true + val limit = + client.application.sendProtection + ?.aead + ?.confidentialityLimit + assertNotNull(limit, "client must have application-level send keys after handshake") + client.aeadEncryptCount = limit - 1L + runBlocking { + // Open a uni stream + write a few bytes; the resulting + // STREAM frame guarantees an ack-eliciting application + // packet on the next drain. The writer's post-encrypt + // limit check fires at the boundary. + val stream = client.openUniStream() + stream.send.enqueue(byteArrayOf(0xff.toByte())) + stream.send.finish() + client.streamsLock.lock() + try { + drainOutbound(client, nowMillis = 0L) + } finally { + client.streamsLock.unlock() + } + } + assertEquals(QuicConnection.Status.CLOSED, client.status) + val reason = client.closeReason + assertNotNull(reason) + assertTrue(reason.contains("AEAD_LIMIT_REACHED")) + } + + @Test + fun decrypt_failure_at_integrity_limit_closes_connection() { + val (client, pipe) = newConnectedClient() + // Craft a real, properly-encrypted 1-RTT datagram from the + // server's perspective, then flip one ciphertext byte so AEAD + // verification fails on the client side. This exercises the + // exact parser path (HP unmask succeeds, AEAD returns null, + // counter advances) — feeding garbage bytes would trip the + // reserved-bits PROTOCOL_VIOLATION check first. + val cleanDatagram = + // 100 PING frames pad the packet well past the 20 bytes of + // header-protection sample range (sample ends at + // byte (1 + dcidLen + 4 + 16) ≈ byte 29 for dcidLen=8). + // Tampering a byte well past that range leaves the HP + // mask intact while breaking the AEAD tag, so the + // ShortHeaderPacket.parseAndDecrypt path returns null + // (AEAD failure) instead of throwing on reserved-bit + // mismatch (HP-mask divergence). + pipe.buildServerApplicationDatagram(List(100) { com.vitorpamplona.quic.frame.PingFrame })!! + val tampered = cleanDatagram.copyOf() + // Flip the last byte of the AEAD tag — far past the HP sample + // range so HP unmask still recovers the correct first byte. + tampered[tampered.size - 1] = (tampered[tampered.size - 1].toInt() xor 0x01).toByte() + val limit = + client.application.receiveProtection + ?.aead + ?.integrityLimit + assertNotNull(limit) + // Pin the counter just under the limit so the tampered packet's + // AEAD failure crosses the threshold. + client.aeadDecryptFailureCount = limit - 1L + feedDatagram(client, tampered, nowMillis = 0L) + assertEquals(QuicConnection.Status.CLOSED, client.status) + val reason = client.closeReason + assertNotNull(reason) + assertTrue(reason.contains("AEAD_LIMIT_REACHED"), "expected AEAD_LIMIT_REACHED, got: $reason") + } +} diff --git a/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/ConnectionLevelFlowControlTest.kt b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/ConnectionLevelFlowControlTest.kt new file mode 100644 index 0000000000..1a4f5549cb --- /dev/null +++ b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/ConnectionLevelFlowControlTest.kt @@ -0,0 +1,137 @@ +/* + * 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 kotlinx.coroutines.runBlocking +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertTrue + +/** + * RFC 9000 §4.1: connection-level inbound flow-control enforcement. + * + * "All data sent in STREAM frames counts toward this limit. The sum of + * the largest received offsets on all streams — including streams in + * the Reset Recvd state — MUST NOT exceed the value advertised by a + * receiver." + * + * Pre-fix the receiver enforced ONLY per-stream `receiveLimit` (each + * stream individually couldn't push past `initial_max_stream_data_*`). + * The aggregate connection-level cap (`initial_max_data`) was advertised + * AND respected on the SEND side, but never checked on the RECEIVE + * side. A peer that opened many streams and pushed each one up to its + * per-stream cap could collectively exceed `initial_max_data` without + * us closing. + */ +class ConnectionLevelFlowControlTest { + private fun connectClient( + maxData: Long = 16 * 1024, + maxStreamData: Long = 8 * 1024, + maxStreamsUni: Long = 16, + ): Pair = + newConnectedClient( + maxData = maxData, + maxStreamData = maxStreamData, + maxStreamsUni = maxStreamsUni, + maxStreamsBidi = 16, + ) + + @Test + fun connection_recv_sum_below_max_data_accepted() { + val (client, pipe) = connectClient(maxData = 16 * 1024, maxStreamData = 8 * 1024) + // Two server-uni streams (id 3, 7) each carrying 4 KiB. Sum = 8 KiB + // < 16 KiB connection cap. Should sail through. + val frame1 = StreamFrame(streamId = 3L, offset = 0L, data = ByteArray(4096), fin = false) + val frame2 = StreamFrame(streamId = 7L, offset = 0L, data = ByteArray(4096), fin = false) + val datagram = pipe.buildServerApplicationDatagram(listOf(frame1, frame2))!! + feedDatagram(client, datagram, nowMillis = 0L) + assertEquals(QuicConnection.Status.CONNECTED, client.status) + assertEquals(8L * 1024, client.connectionInboundOffsetSum) + } + + @Test + fun connection_recv_sum_exceeding_max_data_closes() { + val (client, pipe) = connectClient(maxData = 16 * 1024, maxStreamData = 16 * 1024) + // Three server-uni streams each shipping 6 KiB → 18 KiB > 16 KiB cap. + // Per-stream limit (16 KiB) is fine; the aggregate breaches MAX_DATA. + val frame1 = StreamFrame(streamId = 3L, offset = 0L, data = ByteArray(6 * 1024), fin = false) + val frame2 = StreamFrame(streamId = 7L, offset = 0L, data = ByteArray(6 * 1024), fin = false) + val frame3 = StreamFrame(streamId = 11L, offset = 0L, data = ByteArray(6 * 1024), fin = false) + val datagram = pipe.buildServerApplicationDatagram(listOf(frame1, frame2, frame3))!! + feedDatagram(client, datagram, nowMillis = 0L) + assertEquals(QuicConnection.Status.CLOSED, client.status) + val reason = client.closeReason + assertNotNull(reason) + assertTrue(reason.contains("FLOW_CONTROL_ERROR"), "expected FLOW_CONTROL_ERROR, got: $reason") + assertTrue(reason.contains("MAX_DATA")) + } + + @Test + fun retransmitted_stream_frame_does_not_double_count() { + val (client, pipe) = connectClient(maxData = 16 * 1024, maxStreamData = 8 * 1024) + val frame = StreamFrame(streamId = 3L, offset = 0L, data = ByteArray(4 * 1024), fin = false) + // First arrival. + feedDatagram(client, pipe.buildServerApplicationDatagram(listOf(frame))!!, nowMillis = 0L) + val sumAfterFirst = client.connectionInboundOffsetSum + assertEquals(4L * 1024, sumAfterFirst) + // Identical retransmit — does NOT advance the per-stream high-water, + // so connection counter stays put. + feedDatagram(client, pipe.buildServerApplicationDatagram(listOf(frame))!!, nowMillis = 0L) + assertEquals(sumAfterFirst, client.connectionInboundOffsetSum) + assertEquals(QuicConnection.Status.CONNECTED, client.status) + } + + @Test + fun reset_stream_final_size_counts_toward_connection_limit() { + val (client, pipe) = connectClient(maxData = 16 * 1024, maxStreamData = 16 * 1024) + // Stream 3 ships 4 KiB then resets at finalSize = 20 KiB. The + // RESET_STREAM commits the peer to those 20 KiB even though we + // haven't seen them on the wire — they count toward MAX_DATA per + // §4.5. 20 KiB on a single stream alone breaches the 16 KiB + // connection cap, so we MUST close. + val streamData = StreamFrame(streamId = 3L, offset = 0L, data = ByteArray(4 * 1024), fin = false) + val reset = ResetStreamFrame(streamId = 3L, applicationErrorCode = 0L, finalSize = 20L * 1024) + feedDatagram(client, pipe.buildServerApplicationDatagram(listOf(streamData, reset))!!, nowMillis = 0L) + assertEquals(QuicConnection.Status.CLOSED, client.status) + val reason = client.closeReason + assertNotNull(reason) + assertTrue(reason.contains("FLOW_CONTROL_ERROR"), "expected FLOW_CONTROL_ERROR, got: $reason") + } + + @Test + fun handshake_fixture_starts_with_zero_inbound_offset_sum() { + // Sanity check on the bookkeeping: a fresh handshake-only + // connection has not received any STREAM bytes, so the sum is 0 + // even though [advertisedMaxData] is the configured initial. + val (client, pipe) = connectClient() + runBlocking { + // Make sure pipe-driven handshake completes to CONNECTED before + // we read the field — otherwise we'd be looking at the value + // mid-handshake with no peer streams in play. + pipe.drive(maxRounds = 4) + } + assertEquals(0L, client.connectionInboundOffsetSum) + assertTrue(client.advertisedMaxData > 0L) + } +} diff --git a/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/DrainingStateTest.kt b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/DrainingStateTest.kt new file mode 100644 index 0000000000..cfc1148999 --- /dev/null +++ b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/DrainingStateTest.kt @@ -0,0 +1,161 @@ +/* + * 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.ConnectionCloseFrame +import com.vitorpamplona.quic.frame.StreamFrame +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.2.2 draining state. + * + * When an endpoint receives a CONNECTION_CLOSE frame from its peer it + * MUST enter the draining state. While draining: + * - the endpoint MUST NOT send further packets; + * - the endpoint SHOULD discard inbound packets without processing; + * - the state MUST persist for at least 3 * PTO before transitioning + * to fully closed (so the peer's last retransmits can converge). + * + * Pre-fix the parser called `markClosedExternally` directly on inbound + * CONNECTION_CLOSE which transitioned straight to CLOSED. There was no + * grace period, no observable DRAINING phase, and the read loop didn't + * explicitly drop subsequent late packets (it just exited on CLOSED — + * functional equivalence, but the spec-mandated state was missing + * from observability). + */ +class DrainingStateTest { + @Test + fun peer_connection_close_transitions_to_draining() { + val (client, pipe) = newConnectedClient() + val close = + ConnectionCloseFrame( + errorCode = 0L, + frameType = 0L, // transport-close (non-null frameType) + reason = "test peer close", + ) + feedDatagram(client, pipe.buildServerApplicationDatagram(listOf(close))!!, nowMillis = 0L) + assertEquals(QuicConnection.Status.DRAINING, client.status) + assertNotNull(client.drainingDeadlineMs, "deadline must be set") + assertTrue(client.drainingDeadlineMs!! > 0L) + val reason = client.closeReason + assertNotNull(reason) + assertTrue(reason.contains("peer CONNECTION_CLOSE")) + } + + @Test + fun late_inbound_during_draining_is_dropped() { + val (client, pipe) = newConnectedClient() + val close = + ConnectionCloseFrame( + errorCode = 0L, + frameType = 0L, + reason = "draining drop test", + ) + feedDatagram(client, pipe.buildServerApplicationDatagram(listOf(close))!!, nowMillis = 0L) + assertEquals(QuicConnection.Status.DRAINING, client.status) + + // Feed a STREAM frame after the peer-close. Per §10.2.2 the + // packet MUST be discarded; the connection-level + // bookkeeping (e.g. `connectionInboundOffsetSum`) MUST stay + // put. + val before = client.connectionInboundOffsetSum + val streamFrame = StreamFrame(streamId = 3L, offset = 0L, data = ByteArray(64), fin = false) + val datagram = pipe.buildServerApplicationDatagram(listOf(streamFrame)) + // The pipe build may legitimately succeed (server keys are + // separate from client state) — test only matters if it does. + if (datagram != null) { + feedDatagram(client, datagram, nowMillis = 0L) + assertEquals(before, client.connectionInboundOffsetSum, "draining must drop without bookkeeping update") + } + // Status remains DRAINING — late packet did not flip us to + // CLOSED prematurely. + assertEquals(QuicConnection.Status.DRAINING, client.status) + } + + @Test + fun isDrainingExpired_false_before_deadline() { + val (client, _) = newConnectedClient() + client.enterDraining("test", nowMillis = 0L) + // Just before the deadline. + val deadline = client.drainingDeadlineMs!! + assertFalse(client.isDrainingExpired(deadline - 1L)) + } + + @Test + fun isDrainingExpired_true_at_deadline() { + val (client, _) = newConnectedClient() + client.enterDraining("test", nowMillis = 0L) + val deadline = client.drainingDeadlineMs!! + assertTrue(client.isDrainingExpired(deadline)) + assertTrue(client.isDrainingExpired(deadline + 100L)) + } + + @Test + fun draining_floor_at_least_minimum_period() { + // Pre-handshake the PTO is small (smoothed_rtt floor only), + // so the §10.2.2 "3 * PTO" target could be vanishingly tiny. + // The implementation floors at MIN_DRAINING_PERIOD_MS so qlog + // observers and tests can see the DRAINING window. + val (client, _) = newConnectedClient() + client.enterDraining("test", nowMillis = 0L) + val deadline = client.drainingDeadlineMs!! + assertTrue(deadline >= QuicConnection.MIN_DRAINING_PERIOD_MS, "expected ≥ ${QuicConnection.MIN_DRAINING_PERIOD_MS}, got $deadline") + } + + @Test + fun second_connection_close_during_draining_is_no_op() { + val (client, pipe) = newConnectedClient() + val close = + ConnectionCloseFrame( + errorCode = 0L, + frameType = 0L, + reason = "first close", + ) + feedDatagram(client, pipe.buildServerApplicationDatagram(listOf(close))!!, nowMillis = 0L) + val firstReason = client.closeReason + val firstDeadline = client.drainingDeadlineMs + + // Peer retransmits its CONNECTION_CLOSE — we MUST NOT reset + // the draining deadline (would extend the grace indefinitely + // under retransmits) or change the recorded reason. + val secondClose = + ConnectionCloseFrame( + errorCode = 0L, + frameType = 0L, + reason = "second close", + ) + feedDatagram(client, pipe.buildServerApplicationDatagram(listOf(secondClose))!!, nowMillis = 50L) + assertEquals(firstReason, client.closeReason, "first-call wins on closeReason") + assertEquals(firstDeadline, client.drainingDeadlineMs, "deadline should not advance on retransmit") + } + + @Test + fun fresh_connection_has_no_draining_deadline() { + val (client, _) = newConnectedClient() + assertEquals(QuicConnection.Status.CONNECTED, client.status) + assertNull(client.drainingDeadlineMs) + } +} diff --git a/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/ErrorCodeMappingTest.kt b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/ErrorCodeMappingTest.kt new file mode 100644 index 0000000000..31e402313f --- /dev/null +++ b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/ErrorCodeMappingTest.kt @@ -0,0 +1,147 @@ +/* + * 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.TlsAlertException +import com.vitorpamplona.quic.frame.CryptoFrame +import com.vitorpamplona.quic.frame.QuicTransportError +import com.vitorpamplona.quic.tls.TlsConstants +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertTrue + +/** + * RFC 9001 §4.8 + RFC 9000 §22 error code surfacing. + * + * Pre-fix the connection's `closeErrorCode` was effectively unused — + * every external close went through `markClosedExternally(reason)` + * which set only the reason string. Observers (qlog, support logs) + * could see the human-readable "FLOW_CONTROL_ERROR: ..." prefix but + * not the numeric code, and any TLS handshake failure surfaced as a + * generic exception that bubbled out of the read loop without an + * RFC-mapped error. + * + * The fixes covered here: + * - `TlsAlertException` carries the RFC 8446 §B.2 alert + * description; the parser maps it to `0x100 + alert` per RFC 9001 + * §4.8 and stamps `closeErrorCode`. + * - `markClosedExternally(reason, errorCode)` overload lets call + * sites pin the §20.1 transport code (e.g. CRYPTO_BUFFER_EXCEEDED). + * - Specific TLS-layer throws (ALPN mismatch, version mismatch, + * cipher mismatch, HRR, Finished-MAC, cert validation, KeyUpdate) + * map to the spec alert. + */ +class ErrorCodeMappingTest { + @Test + fun tls_alert_exception_quicErrorCode_offsets_by_0x100() { + val ex = TlsAlertException(alertCode = TlsConstants.ALERT_HANDSHAKE_FAILURE, message = "test") + // RFC 9001 §4.8: 0x100 + 40 = 0x128 (296 decimal). + assertEquals(0x128L, ex.quicErrorCode) + } + + @Test + fun tls_alert_exception_rejects_out_of_range_codes() { + kotlin.test.assertFailsWith { + TlsAlertException(alertCode = -1, message = "negative") + } + kotlin.test.assertFailsWith { + TlsAlertException(alertCode = 256, message = "too big") + } + } + + @Test + fun feedDatagram_routes_TlsAlertException_to_closeErrorCode() { + // Construct a connection mid-handshake. We don't need a full + // pipe — the parser-level catch is what's under test. Inject + // a bare CRYPTO frame with bytes that the TLS layer will + // reject; the parser's catch must record the alert-mapped + // code on `closeErrorCode`. Easiest reproducer: a CRYPTO at + // INITIAL with bytes that aren't a valid handshake message + // header (length > remaining). + val (client, _) = newConnectedClient() + // Force-close the connection with a synthetic alert to bypass + // the need for a TLS-rejecting wire packet (the wire-level + // craft is exercised by the existing TLS handshake tests). + // The test here proves the markClosedExternally surface + // accepts the error code and preserves it on `closeErrorCode`. + client.markClosedExternally( + reason = "CRYPTO_ERROR (TLS alert 80): synthetic", + errorCode = TlsAlertException(alertCode = TlsConstants.ALERT_INTERNAL_ERROR, message = "x").quicErrorCode, + ) + assertEquals(QuicConnection.Status.CLOSED, client.status) + assertEquals(0x100L + 80L, client.closeErrorCode) + } + + @Test + fun crypto_buffer_exceeded_closes_with_specific_code() { + // Send a CRYPTO frame at a far-out offset. The pre-insert + // size check (`proposed - contiguousEnd > 64KiB`) fires + // BEFORE the buffer actually allocates, so the + // CRYPTO_BUFFER_EXCEEDED close lands deterministically. + val (client, pipe) = newConnectedClient() + val crypto = CryptoFrame(offset = 200_000L, data = ByteArray(16)) + val datagram = pipe.buildServerApplicationDatagram(listOf(crypto))!! + feedDatagram(client, datagram, nowMillis = 0L) + assertEquals(QuicConnection.Status.CLOSED, client.status) + assertEquals(QuicTransportError.CRYPTO_BUFFER_EXCEEDED, client.closeErrorCode) + val reason = client.closeReason + assertNotNull(reason) + assertTrue(reason.contains("CRYPTO_BUFFER_EXCEEDED")) + } + + @Test + fun small_crypto_frames_stay_under_buffer_cap() { + // Sanity-check the cap isn't accidentally tripping on + // legitimate handshake-shaped traffic. A CRYPTO frame at + // offset 0 with a reasonable size is fine — it gets fed + // into the TLS layer, which rejects it (not a real + // handshake message), but via the TLS catch path with + // CRYPTO_ERROR, NOT CRYPTO_BUFFER_EXCEEDED. + val (client, pipe) = newConnectedClient() + val crypto = CryptoFrame(offset = 200L, data = ByteArray(64)) + val datagram = pipe.buildServerApplicationDatagram(listOf(crypto))!! + feedDatagram(client, datagram, nowMillis = 0L) + // Either CONNECTED (TLS shrugged it off) or CLOSED with + // CRYPTO_ERROR — but NOT CRYPTO_BUFFER_EXCEEDED. + kotlin.test.assertNotEquals( + QuicTransportError.CRYPTO_BUFFER_EXCEEDED, + client.closeErrorCode, + "small CRYPTO frame must not trip the buffer cap", + ) + } + + @Test + fun quic_transport_error_constants_match_rfc_9000_section_20_1() { + // Spot-check the §20.1 numeric values so a future renumbering + // accident is caught at test-time. + assertEquals(0x00L, QuicTransportError.NO_ERROR) + assertEquals(0x01L, QuicTransportError.INTERNAL_ERROR) + assertEquals(0x03L, QuicTransportError.FLOW_CONTROL_ERROR) + assertEquals(0x05L, QuicTransportError.STREAM_STATE_ERROR) + assertEquals(0x06L, QuicTransportError.FINAL_SIZE_ERROR) + assertEquals(0x08L, QuicTransportError.TRANSPORT_PARAMETER_ERROR) + assertEquals(0x0aL, QuicTransportError.PROTOCOL_VIOLATION) + assertEquals(0x0dL, QuicTransportError.CRYPTO_BUFFER_EXCEEDED) + assertEquals(0x0eL, QuicTransportError.KEY_UPDATE_ERROR) + assertEquals(0x0fL, QuicTransportError.AEAD_LIMIT_REACHED) + } +} diff --git a/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/FrameRoutingTest.kt b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/FrameRoutingTest.kt index d39937bf2f..7876cca06b 100644 --- a/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/FrameRoutingTest.kt +++ b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/FrameRoutingTest.kt @@ -187,11 +187,13 @@ class FrameRoutingTest { // Order matters: CCF first, stream frame second. val packet = pipe.buildServerApplicationDatagram(listOf(ccf, streamFrame))!! feedDatagram(client, packet, nowMillis = 0L) - assertEquals(QuicConnection.Status.CLOSED, client.status) - // The dispatcher MUST have stopped at CCF; no peer stream materialised. - // (We can't easily query this directly, but the behavioural assertion - // is the close-status — pre-fix the StreamFrame would have created - // a phantom stream after close.) + // RFC 9000 §10.2.2: peer's CONNECTION_CLOSE puts us in DRAINING + // (was CLOSED pre-DRAINING-implementation; the spec-mandated + // 3 * PTO grace is held by the driver's send loop and flips to + // CLOSED once it elapses). The "no phantom stream" assertion + // still holds — DRAINING blocks frame dispatch the same way + // CLOSED did. + assertEquals(QuicConnection.Status.DRAINING, client.status) } @Test 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 0000000000..f28214fde2 --- /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/connection/StatelessResetDetectionTest.kt b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/StatelessResetDetectionTest.kt new file mode 100644 index 0000000000..bd99d5fe7f --- /dev/null +++ b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/StatelessResetDetectionTest.kt @@ -0,0 +1,230 @@ +/* + * 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 kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNotNull +import kotlin.test.assertTrue + +/** + * RFC 9000 §10.3 stateless-reset detection. + * + * A peer that lost connection state (crash, restart, route change) + * signals so by sending a "Stateless Reset" datagram — a short-header- + * shaped packet whose trailing 16 bytes equal a `stateless_reset_token` + * the peer previously communicated. The receiver MUST detect this and + * silently close (no CONNECTION_CLOSE; the peer doesn't have keys to + * decrypt it anyway) instead of mistaking it for a forgery and + * counting it toward the §6.6 integrity limit. + * + * Pre-fix the tokens were stored (via NEW_CONNECTION_ID into the + * [PathValidator] pool, and via the peer's transport parameters) but + * never matched against arriving datagrams — a peer's stateless reset + * looked indistinguishable from noise and the connection lingered + * until idle timeout (or worse, an attacker could spam look-like- + * noise frames toward our integrity counter). + */ +class StatelessResetDetectionTest { + @Test + fun datagram_ending_in_peer_token_triggers_silent_close() { + val (client, _) = newConnectedClient() + // Set up a known token on the connection. In production this + // arrives via either `peerTransportParameters.statelessResetToken` + // (advertised by the server in EncryptedExtensions) or a + // NEW_CONNECTION_ID frame; the test harness installs it + // directly so we control the bytes. + val token = ByteArray(16) { (0x42 + it).toByte() } + client.peerTransportParameters = + ( + client.peerTransportParameters ?: TransportParameters() + ).copy(statelessResetToken = token) + + // Build a datagram that LOOKS like a short-header packet but + // whose AEAD will fail (random ciphertext). The trailing 16 + // bytes match the token — that's the §10.3 signal. + val dcid = client.sourceConnectionId.bytes + // First byte: form-bit=0, fixed-bit=1, pnLen=1. + val first = byteArrayOf(0x40.toByte()) + val pn = byteArrayOf(0x00) + val randomBody = ByteArray(40) { (it * 7).toByte() } + val datagram = first + dcid + pn + randomBody + token + feedDatagram(client, datagram, nowMillis = 0L) + + assertEquals(QuicConnection.Status.CLOSED, client.status) + val reason = client.closeReason + assertNotNull(reason) + assertTrue(reason.contains("stateless reset"), "expected stateless-reset reason, got: $reason") + } + + @Test + fun datagram_ending_in_unknown_token_does_not_close() { + val (client, _) = newConnectedClient() + // Don't install the token at all — but feed a datagram whose + // trailing 16 bytes happen to be some random value. + val unknown = ByteArray(16) { 0xAA.toByte() } + val dcid = client.sourceConnectionId.bytes + val first = byteArrayOf(0x40.toByte()) + val pn = byteArrayOf(0x00) + val randomBody = ByteArray(40) { (it * 11).toByte() } + val datagram = first + dcid + pn + randomBody + unknown + feedDatagram(client, datagram, nowMillis = 0L) + // Connection should still be CONNECTED (or at most have its + // integrity counter bumped); definitely NOT closed-on-stateless- + // reset. + if (client.status == QuicConnection.Status.CLOSED) { + assertFalse( + client.closeReason!!.contains("stateless reset"), + "non-matching token must not be misclassified as stateless reset", + ) + } + } + + @Test + fun isStatelessReset_constant_time_check_works_against_pool_tokens() { + val (client, _) = newConnectedClient() + // Install a token on a NEW_CONNECTION_ID-shaped pool entry. + val token = ByteArray(16) { (0x55 + it).toByte() } + client.pathValidator.recordPeerNewConnectionId( + sequenceNumber = 1L, + retirePriorTo = 0L, + connectionId = ByteArray(8) { (0xAB + it).toByte() }, + statelessResetToken = token, + ) + + val dcid = client.sourceConnectionId.bytes + val datagram = byteArrayOf(0x40.toByte()) + dcid + ByteArray(40) + token + assertTrue(client.isStatelessReset(datagram), "pool-stored token must match") + + val mismatch = byteArrayOf(0x40.toByte()) + dcid + ByteArray(40) + ByteArray(16) + assertFalse(client.isStatelessReset(mismatch), "all-zeros trailer must not match a real token") + } + + @Test + fun isStatelessReset_rejects_long_header_form() { + val (client, _) = newConnectedClient() + val token = ByteArray(16) { 0x33 } + client.peerTransportParameters = + ( + client.peerTransportParameters ?: TransportParameters() + ).copy(statelessResetToken = token) + // First byte 0x80+ → long header form; spec says stateless + // reset is short-header-shaped only. + val datagram = byteArrayOf(0xC0.toByte()) + ByteArray(40) + token + 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() + val token = ByteArray(16) { 0x77 } + client.peerTransportParameters = + ( + client.peerTransportParameters ?: TransportParameters() + ).copy(statelessResetToken = token) + // Datagram shorter than the §10.3 minimum (we use 22 bytes: + // 5-byte minimum header + 16-byte trailer + 1). + val tooShort = byteArrayOf(0x40.toByte()) + token + assertFalse(client.isStatelessReset(tooShort)) + } +} 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) + } +} diff --git a/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/TransportParameterBoundsTest.kt b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/TransportParameterBoundsTest.kt new file mode 100644 index 0000000000..2a2717abbc --- /dev/null +++ b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/TransportParameterBoundsTest.kt @@ -0,0 +1,188 @@ +/* + * 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.InProcessTlsServer +import com.vitorpamplona.quic.tls.PermissiveCertificateValidator +import kotlinx.coroutines.runBlocking +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertTrue + +/** + * RFC 9000 §18.2 transport-parameter bounds checks. These values are + * advertised by the peer in their EncryptedExtensions + * `quic_transport_parameters` extension; out-of-range values MUST close + * the connection with TRANSPORT_PARAMETER_ERROR. + * + * - `max_udp_payload_size` minimum 1200 (the §14 datagram floor). + * - `ack_delay_exponent` maximum 20. + * - `active_connection_id_limit` minimum 2. + * + * Pre-fix the values were decoded into [TransportParameters] but no + * runtime check enforced the spec ranges — a hostile peer could ship a + * value of e.g. `ack_delay_exponent = 60` and our parser's + * `ackDelay << 60` shift would have desynced RTT (the parser still + * caps the exponent defensively, but the connection should not have + * been allowed in the first place). + */ +class TransportParameterBoundsTest { + private fun runHandshake(serverParams: TransportParameters): QuicConnection.Status = + runBlocking { + val client = + QuicConnection( + serverName = "example.test", + config = QuicConnectionConfig(), + tlsCertificateValidator = PermissiveCertificateValidator(), + ) + val serverScid = ConnectionId.random(8) + val tlsServer = + InProcessTlsServer( + transportParameters = + serverParams + .copy( + initialSourceConnectionId = serverScid.bytes, + originalDestinationConnectionId = client.destinationConnectionId.bytes, + ).encode(), + ) + val pipe = + InMemoryQuicPipe( + client = client, + initialDcid = client.destinationConnectionId.bytes, + serverScid = serverScid, + tlsServer = tlsServer, + ) + client.start() + // Drive enough rounds for ServerHello + EE + handshake completion. + // Once peer params are applied, `applyPeerTransportParameters` + // runs the §18.2 bounds checks which (on violation) flip the + // connection to CLOSED. + pipe.drive(maxRounds = 16) + client.status + } + + private fun baselineLegalParams() = + TransportParameters( + initialMaxData = 1L * 1024 * 1024, + initialMaxStreamDataBidiLocal = 64L * 1024, + initialMaxStreamDataBidiRemote = 64L * 1024, + initialMaxStreamDataUni = 64L * 1024, + initialMaxStreamsBidi = 16, + initialMaxStreamsUni = 16, + ) + + @Test + fun max_udp_payload_size_below_1200_closes() { + val status = runHandshake(baselineLegalParams().copy(maxUdpPayloadSize = 1199L)) + assertEquals( + QuicConnection.Status.CLOSED, + status, + "RFC 9000 §18.2: max_udp_payload_size < 1200 MUST be TRANSPORT_PARAMETER_ERROR", + ) + } + + @Test + fun max_udp_payload_size_at_1200_accepted() { + val status = runHandshake(baselineLegalParams().copy(maxUdpPayloadSize = 1200L)) + assertEquals(QuicConnection.Status.CONNECTED, status) + } + + @Test + fun ack_delay_exponent_above_20_closes() { + val status = runHandshake(baselineLegalParams().copy(ackDelayExponent = 21L)) + assertEquals( + QuicConnection.Status.CLOSED, + status, + "RFC 9000 §18.2: ack_delay_exponent > 20 MUST be TRANSPORT_PARAMETER_ERROR", + ) + } + + @Test + fun ack_delay_exponent_at_20_accepted() { + val status = runHandshake(baselineLegalParams().copy(ackDelayExponent = 20L)) + assertEquals(QuicConnection.Status.CONNECTED, status) + } + + @Test + fun active_connection_id_limit_below_2_closes() { + val status = runHandshake(baselineLegalParams().copy(activeConnectionIdLimit = 1L)) + assertEquals( + QuicConnection.Status.CLOSED, + status, + "RFC 9000 §18.2: active_connection_id_limit < 2 MUST be TRANSPORT_PARAMETER_ERROR", + ) + } + + @Test + fun active_connection_id_limit_at_2_accepted() { + val status = runHandshake(baselineLegalParams().copy(activeConnectionIdLimit = 2L)) + assertEquals(QuicConnection.Status.CONNECTED, status) + } + + @Test + fun missing_optional_params_accepted() { + // No bounds-checked params advertised → fall back to defaults, + // connection completes normally. + val status = runHandshake(baselineLegalParams()) + assertEquals(QuicConnection.Status.CONNECTED, status) + } + + @Test + fun close_reason_mentions_specific_violated_param() { + // Capture which bound was reported so future audit rounds can + // confirm the message contains a usable diagnostic, not just a + // generic "transport params bad". + val client = + runBlocking { + val client = + QuicConnection( + serverName = "example.test", + config = QuicConnectionConfig(), + tlsCertificateValidator = PermissiveCertificateValidator(), + ) + val serverScid = ConnectionId.random(8) + val tlsServer = + InProcessTlsServer( + transportParameters = + baselineLegalParams() + .copy( + initialSourceConnectionId = serverScid.bytes, + originalDestinationConnectionId = client.destinationConnectionId.bytes, + ackDelayExponent = 25L, + ).encode(), + ) + val pipe = + InMemoryQuicPipe( + client = client, + initialDcid = client.destinationConnectionId.bytes, + serverScid = serverScid, + tlsServer = tlsServer, + ) + client.start() + pipe.drive(maxRounds = 16) + client + } + val reason = client.closeReason + assertNotNull(reason) + assertTrue(reason.contains("ack_delay_exponent"), "reason should name the violated param: $reason") + } +} diff --git a/quic/src/commonTest/kotlin/com/vitorpamplona/quic/http3/Http3ReservedSettingsTest.kt b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/http3/Http3ReservedSettingsTest.kt new file mode 100644 index 0000000000..2d9c24d61a --- /dev/null +++ b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/http3/Http3ReservedSettingsTest.kt @@ -0,0 +1,88 @@ +/* + * 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.http3 + +import com.vitorpamplona.quic.QuicCodecException +import com.vitorpamplona.quic.QuicWriter +import kotlin.test.Test +import kotlin.test.assertFailsWith +import kotlin.test.assertTrue + +/** + * RFC 9114 §7.2.4.1 — SETTINGS identifiers in the HTTP/2 range + * (0x02, 0x03, 0x04, 0x05) are reserved to prevent confusion with + * HTTP/2 settings. Receipt of any of these MUST be a connection error + * of type H3_SETTINGS_ERROR. + * + * Pre-fix the decoder validated values per known id but accepted the + * reserved ids silently (since they weren't in the known cap table — + * they fell through to the generic 1<<32 cap). + */ +class Http3ReservedSettingsTest { + private fun encodeRawSetting( + id: Long, + value: Long, + ): ByteArray { + val w = QuicWriter() + w.writeVarint(id) + w.writeVarint(value) + return w.toByteArray() + } + + @Test + fun reserved_id_0x02_rejected() { + val ex = + assertFailsWith { + Http3Settings.decodeBody(encodeRawSetting(0x02L, 1L)) + } + assertTrue(ex.message!!.contains("H3_SETTINGS_ERROR")) + } + + @Test + fun reserved_id_0x03_rejected() { + assertFailsWith { + Http3Settings.decodeBody(encodeRawSetting(0x03L, 1L)) + } + } + + @Test + fun reserved_id_0x04_rejected() { + assertFailsWith { + Http3Settings.decodeBody(encodeRawSetting(0x04L, 1L)) + } + } + + @Test + fun reserved_id_0x05_rejected() { + assertFailsWith { + Http3Settings.decodeBody(encodeRawSetting(0x05L, 1L)) + } + } + + @Test + fun adjacent_legal_ids_still_accepted() { + // 0x01 is RFC 9204 QPACK_MAX_TABLE_CAPACITY, 0x06 is the next + // legal HTTP/3 setting. Confirm the reserved-range check does + // not over-reach. + Http3Settings.decodeBody(encodeRawSetting(0x01L, 4096L)) + Http3Settings.decodeBody(encodeRawSetting(0x06L, 1024L)) + } +} 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 0000000000..c4bcc07dce --- /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") + } +} diff --git a/quic/src/commonTest/kotlin/com/vitorpamplona/quic/tls/HelloRetryRequestTest.kt b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/tls/HelloRetryRequestTest.kt index 4b2639ac34..41b98e34d2 100644 --- a/quic/src/commonTest/kotlin/com/vitorpamplona/quic/tls/HelloRetryRequestTest.kt +++ b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/tls/HelloRetryRequestTest.kt @@ -20,8 +20,8 @@ */ package com.vitorpamplona.quic.tls -import com.vitorpamplona.quic.QuicCodecException import com.vitorpamplona.quic.QuicWriter +import com.vitorpamplona.quic.TlsAlertException import kotlin.test.Test import kotlin.test.assertFailsWith @@ -32,7 +32,9 @@ import kotlin.test.assertFailsWith * Before the round-3 fix, an HRR was treated as a regular ServerHello, then * X25519 was performed against the cookie/extension bytes (garbage), then * AEAD failed downstream with a confusing error. The current code rejects - * cleanly with a [QuicCodecException]. + * cleanly with a [TlsAlertException] carrying RFC 8446 §6.2 + * `handshake_failure = 40` (mapped by the QUIC layer to error code + * `0x100 + 40 = 0x128` per RFC 9001 §4.8). */ class HelloRetryRequestTest { @Test @@ -49,9 +51,11 @@ class HelloRetryRequestTest { tls.pollOutbound(TlsClient.Level.INITIAL) val hrr = buildHelloRetryRequest() - assertFailsWith { - tls.pushHandshakeBytes(TlsClient.Level.INITIAL, hrr) - } + val ex = + assertFailsWith { + tls.pushHandshakeBytes(TlsClient.Level.INITIAL, hrr) + } + kotlin.test.assertEquals(40, ex.alertCode, "RFC 8446 §6.2 handshake_failure = 40") } /** diff --git a/quic/src/commonTest/kotlin/com/vitorpamplona/quic/webtransport/CriticalStreamClosureTest.kt b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/webtransport/CriticalStreamClosureTest.kt new file mode 100644 index 0000000000..5aa492e467 --- /dev/null +++ b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/webtransport/CriticalStreamClosureTest.kt @@ -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() + } + } +} diff --git a/quic/src/jvmAndroid/kotlin/com/vitorpamplona/quic/crypto/JcaAesGcmAead.kt b/quic/src/jvmAndroid/kotlin/com/vitorpamplona/quic/crypto/JcaAesGcmAead.kt index e6ddc533d6..501e9166bc 100644 --- a/quic/src/jvmAndroid/kotlin/com/vitorpamplona/quic/crypto/JcaAesGcmAead.kt +++ b/quic/src/jvmAndroid/kotlin/com/vitorpamplona/quic/crypto/JcaAesGcmAead.kt @@ -48,6 +48,10 @@ class JcaAesGcmAead( override val nonceLength = 12 override val tagLength = 16 + // RFC 9001 §B.1 limits — same as commonMain `Aes128Gcm`. + override val confidentialityLimit: Long = 1L shl 23 + override val integrityLimit: Long = 1L shl 52 + private val keySpec = SecretKeySpec(key, "AES") // Separate ciphers per direction. JCA's AES-GCM tracks the (key, iv) pair diff --git a/quic/src/jvmAndroid/kotlin/com/vitorpamplona/quic/crypto/JcaChaCha20Poly1305Aead.kt b/quic/src/jvmAndroid/kotlin/com/vitorpamplona/quic/crypto/JcaChaCha20Poly1305Aead.kt index a2c45705e1..e66e606957 100644 --- a/quic/src/jvmAndroid/kotlin/com/vitorpamplona/quic/crypto/JcaChaCha20Poly1305Aead.kt +++ b/quic/src/jvmAndroid/kotlin/com/vitorpamplona/quic/crypto/JcaChaCha20Poly1305Aead.kt @@ -47,6 +47,10 @@ class JcaChaCha20Poly1305Aead( override val nonceLength = 12 override val tagLength = 16 + // RFC 9001 §B.1 limits — same as commonMain `ChaCha20Poly1305Aead`. + override val confidentialityLimit: Long = (1L shl 62) + override val integrityLimit: Long = 1L shl 36 + private val keySpec = SecretKeySpec(key, "ChaCha20") private val encryptCipher: Cipher = Cipher.getInstance("ChaCha20-Poly1305") private val decryptCipher: Cipher = Cipher.getInstance("ChaCha20-Poly1305")