From 6cc1bf64331ac1d4da03d28d683c411a26ea2daf Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 9 May 2026 13:44:35 +0000 Subject: [PATCH] =?UTF-8?q?fix(quic):=20RFC=209001=20=C2=A74.8=20TLS=20ale?= =?UTF-8?q?rt=20mapping=20+=20RFC=209000=20=C2=A722=20CRYPTO=5FBUFFER=5FEX?= =?UTF-8?q?CEEDED?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The remaining ๐ŸŸฆ cosmetic items from the 2026-05-09 audit. Pre-fix the connection's `closeErrorCode` was effectively unused โ€” TLS handshake failures bubbled up as generic `QuicCodecException` and observers had to grep the reason string for the spec category. Now closeErrorCode carries the RFC 9000 ยง20.1 transport code or the RFC 9001 ยง4.8 TLS-alert-mapped CRYPTO_ERROR. Implementation: * `TlsAlertException(alertCode, message)` carrier raised by the TLS layer for the well-defined cases: - HelloRetryRequest received (handshake_failure = 40) - TLS 1.3 not negotiated (protocol_version = 70) - Unsupported cipher (illegal_parameter = 47) - ALPN mismatch (no_application_protocol = 120) - Cert chain validation failure (bad_certificate = 42) - CertificateVerify signature failure (decrypt_error = 51) - Server Finished MAC mismatch (decrypt_error = 51) - TLS KeyUpdate over QUIC (unexpected_message = 10) โ€” RFC 9001 ยง6 mandates this specific code; fixes the misleading prior message that said "rotation not implemented" (we DO implement QUIC's own KEY_PHASE-bit rotation). The QUIC parser maps `0x100 + alertCode` per RFC 9001 ยง4.8 and stamps `closeErrorCode`. * `QuicTransportError` constants object covering the RFC 9000 ยง20.1 table (NO_ERROR through NO_VIABLE_PATH including the previously- flagged CRYPTO_BUFFER_EXCEEDED 0x0d, KEY_UPDATE_ERROR 0x0e, AEAD_LIMIT_REACHED 0x0f). * `markClosedExternally(reason, errorCode = NO_ERROR)` overload โ€” backward compatible; lets call sites pin the spec code on `closeErrorCode` for qlog observability without changing the wire-emit semantics (still no CONNECTION_CLOSE frame, same as before โ€” that's a separate, larger refactor). * RFC 9000 ยง22 CRYPTO_BUFFER_EXCEEDED enforcement โ€” pre-insert per-level cap of 64 KiB on inbound CRYPTO data. Generous enough for an RSA-4096 cert chain with intermediates; bounds the worst-case heap a misbehaving peer can pin to 192 KiB across all 3 encryption levels. Fires before the receive buffer actually allocates. * INVALID_TOKEN โ€” N/A for client role (only servers validate Retry tokens). KEY_UPDATE_ERROR โ€” exposed as a constant; no current failure path maps to it (PN regression on a key-update packet is spec-allowed to handle silently per RFC 9001 ยง6.1, which we do). Tests: `ErrorCodeMappingTest` (6 cases) covers TlsAlertException construction + offset, alert code bounds, markClosedExternally preservation, CRYPTO_BUFFER_EXCEEDED close, sanity-check that small CRYPTO frames don't trip the cap, and ยง20.1 numeric values. `HelloRetryRequestTest` updated to expect TlsAlertException(40) instead of generic QuicCodecException. Plan updated to mark the ยง4.8 + ยง22 (CRYPTO_BUFFER_EXCEEDED) items resolved; KEY_UPDATE_ERROR documented as TLS-side covered + QUIC-side spec-allowed-silent; INVALID_TOKEN documented as N/A for client. https://claude.ai/code/session_01XGmmaVuy2wsSZQx2AqN2ik --- quic/plans/2026-04-26-quic-stack-status.md | 4 +- .../kotlin/com/vitorpamplona/quic/Buffer.kt | 25 +++ .../quic/connection/QuicConnection.kt | 38 ++++- .../quic/connection/QuicConnectionParser.kt | 60 ++++++- .../com/vitorpamplona/quic/frame/Frame.kt | 49 ++++++ .../com/vitorpamplona/quic/tls/TlsClient.kt | 75 +++++++-- .../vitorpamplona/quic/tls/TlsConstants.kt | 22 ++- .../quic/connection/ErrorCodeMappingTest.kt | 147 ++++++++++++++++++ .../quic/tls/HelloRetryRequestTest.kt | 14 +- 9 files changed, 412 insertions(+), 22 deletions(-) create mode 100644 quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/ErrorCodeMappingTest.kt diff --git a/quic/plans/2026-04-26-quic-stack-status.md b/quic/plans/2026-04-26-quic-stack-status.md index b3140f41fc..d3fa3da4eb 100644 --- a/quic/plans/2026-04-26-quic-stack-status.md +++ b/quic/plans/2026-04-26-quic-stack-status.md @@ -320,8 +320,8 @@ file:line evidence. Severity: | RFC ยง | Sev | Gap | Evidence | |---|---|---|---| -| RFC 9001 ยง4.8 | ๐ŸŸฆ | **TLS alerts not mapped to CRYPTO_ERROR + alert_code.** Connection still closes (a generic `QuicCodecException` propagates) but the alert code is lost โ€” debuggability hit, not a wire-spec violation. | `TlsClient.kt:311-317` (catches as `Throwable`) | -| RFC 9000 ยง22 | ๐ŸŸฆ | **Several error codes never emitted.** INVALID_TOKEN (no Retry token validation), CRYPTO_BUFFER_EXCEEDED (no buffer limit), KEY_UPDATE_ERROR (uses generic exception path on KeyUpdate failures, despite key update being implemented). | various | +| 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 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/QuicConnection.kt b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnection.kt index debd32617c..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 @@ -1664,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 @@ -1680,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 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 48899bf86b..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(), + ) } } @@ -651,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()) { 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/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/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/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") } /**