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 647f9442e..dee3f687d 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnection.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnection.kt @@ -1132,6 +1132,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 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 1d0042158..946f716d8 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionParser.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionParser.kt @@ -550,8 +550,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) 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 137255ac2..c7e76c3e9 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionWriter.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionWriter.kt @@ -669,9 +669,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 } 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 d06debe98..a343bf3a9 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/http3/Http3Settings.kt b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/http3/Http3Settings.kt index cab63e806..aad9d829f 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/commonTest/kotlin/com/vitorpamplona/quic/connection/TransportParameterBoundsTest.kt b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/connection/TransportParameterBoundsTest.kt new file mode 100644 index 000000000..2a2717abb --- /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 000000000..2d9c24d61 --- /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)) + } +}