From edd6eb5c10e260e79689806e726f98d3bb481c73 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 5 May 2026 13:37:41 +0000 Subject: [PATCH 1/2] =?UTF-8?q?fix(quic):=20pad=20short=20plaintext=20payl?= =?UTF-8?q?oads=20for=20HP=20sample=20(RFC=209001=20=C2=A75.4.2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ShortHeaderPacket.build / LongHeaderPacket.build crashed with `IllegalArgumentException: packet too short for HP sample` whenever the plaintext payload was small enough that pnLen + payload < 4 — most visibly on the 1-RTT path when buildApplicationPacket emitted a single 1-byte PING (PTO probe with no ACKs queued and no streams to drain) and the packet number still fit in 1 byte. The crash tore down the writer loop, which surfaced upstream as moq-lite "subscribe stream FIN before reply" because in-flight bidi streams got FIN'd by the peer. RFC 9001 §5.4.2 mandates the sender pad the plaintext so the encrypted output (plaintext + 16-byte AEAD tag) has at least 20 bytes after the packet-number offset for the 16-byte HP sample. The fix pads the plaintext with trailing 0x00 bytes — those decode as PADDING frames per RFC 9000 §19.1 and decodeFrames already absorbs them. For long-header packets the padded size feeds back into the Length varint. --- .../quic/packet/LongHeaderPacket.kt | 13 +- .../quic/packet/ShortHeaderPacket.kt | 25 +- .../ShortPayloadHeaderProtectionTest.kt | 214 ++++++++++++++++++ 3 files changed, 247 insertions(+), 5 deletions(-) create mode 100644 quic/src/commonTest/kotlin/com/vitorpamplona/quic/packet/ShortPayloadHeaderProtectionTest.kt 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 69319382a..4554cd1f3 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/packet/LongHeaderPacket.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/packet/LongHeaderPacket.kt @@ -94,8 +94,13 @@ object LongHeaderPacket { w.writeVarint(plain.token.size.toLong()) w.writeBytes(plain.token) } - // Length covers PN bytes + payload + AEAD tag. - val lengthValue = pnLen + plain.payload.size + aead.tagLength + // RFC 9001 §5.4.2: pad the plaintext so that pnLen + payload >= 4 — the + // 16-byte AEAD tag then provides the rest of the 20 bytes the HP sample + // window needs after pnOffset. Trailing 0x00 bytes decode as PADDING + // frames (RFC 9000 §19.1). Length covers PN bytes + (padded) payload + + // AEAD tag, so the padded size must feed into lengthValue. + val paddedPlaintext = padForHeaderProtectionSample(plain.payload, pnLen) + val lengthValue = pnLen + paddedPlaintext.size + aead.tagLength w.writeVarint(lengthValue.toLong()) val pnOffset = w.size // Encode the packet number big-endian, low bytes @@ -104,9 +109,9 @@ object LongHeaderPacket { } val headerBytes = w.toByteArray() - // Encrypt payload + // Encrypt padded payload val nonce = aeadNonce(iv, plain.packetNumber) - val ciphertext = aead.seal(key, nonce, headerBytes, plain.payload) + val ciphertext = aead.seal(key, nonce, headerBytes, paddedPlaintext) // Concatenate header + ciphertext val packet = ByteArray(headerBytes.size + ciphertext.size) 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 930022248..579b7bba0 100644 --- a/quic/src/commonMain/kotlin/com/vitorpamplona/quic/packet/ShortHeaderPacket.kt +++ b/quic/src/commonMain/kotlin/com/vitorpamplona/quic/packet/ShortHeaderPacket.kt @@ -69,8 +69,14 @@ object ShortHeaderPacket { } val headerBytes = w.toByteArray() + // RFC 9001 §5.4.2: pad the plaintext so that pnLen + payload >= 4. The + // 16-byte AEAD tag then guarantees the encrypted output has the 20 + // bytes following pnOffset that header-protection sampling needs. + // Trailing 0x00 bytes decode as PADDING frames (RFC 9000 §19.1). + val paddedPlaintext = padForHeaderProtectionSample(plain.payload, pnLen) + val nonce = aeadNonce(iv, plain.packetNumber) - val ciphertext = aead.seal(key, nonce, headerBytes, plain.payload) + val ciphertext = aead.seal(key, nonce, headerBytes, paddedPlaintext) val packet = ByteArray(headerBytes.size + ciphertext.size) headerBytes.copyInto(packet, 0) @@ -143,3 +149,20 @@ object ShortHeaderPacket { val consumed: Int, ) } + +/** + * Return [payload] padded with trailing zero bytes so that + * `pnLen + paddedSize >= 4`. After AEAD seal adds the 16-byte tag, the + * resulting ciphertext satisfies the RFC 9001 §5.4.2 requirement that 20 + * bytes follow the start of the packet number for the HP sample. + */ +internal fun padForHeaderProtectionSample( + payload: ByteArray, + pnLen: Int, +): ByteArray { + val minSize = (4 - pnLen).coerceAtLeast(0) + if (payload.size >= minSize) return payload + val padded = ByteArray(minSize) + payload.copyInto(padded, 0) + return padded +} diff --git a/quic/src/commonTest/kotlin/com/vitorpamplona/quic/packet/ShortPayloadHeaderProtectionTest.kt b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/packet/ShortPayloadHeaderProtectionTest.kt new file mode 100644 index 000000000..d3566e205 --- /dev/null +++ b/quic/src/commonTest/kotlin/com/vitorpamplona/quic/packet/ShortPayloadHeaderProtectionTest.kt @@ -0,0 +1,214 @@ +/* + * 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.assertContentEquals +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertTrue + +/** + * RFC 9001 §5.4.2: header protection samples 16 bytes starting 4 bytes after + * the packet number offset. The sender MUST pad short plaintext payloads so + * that pnLen + payload >= 4 — otherwise the encrypted output is too short + * for the sample window and the build path tripped a `require` with + * `packet too short for HP sample`. + * + * The crash was reachable from the application path whenever + * [com.vitorpamplona.quic.connection.QuicConnectionWriter.buildApplicationPacket] + * produced a packet whose only frame was a single-byte PING (e.g. a PTO + * probe with no ACKs queued and no streams to drain) while the packet + * number space was still small enough to encode in 1 byte. + */ +class ShortPayloadHeaderProtectionTest { + 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 short_header_with_one_byte_ping_payload_round_trips() { + // 0x01 is the on-wire encoding of a PING frame. With pnLen=1 this is + // exactly the path that crashed in production (NestRx audio rooms). + val pingPayload = byteArrayOf(0x01) + val plain = + ShortHeaderPlaintextPacket( + dcid = dcid, + packetNumber = 0L, + payload = pingPayload, + ) + val wire = + ShortHeaderPacket.build( + plain = plain, + aead = Aes128Gcm, + key = proto.clientKey, + iv = proto.clientIv, + hp = hp, + hpKey = proto.clientHp, + largestAckedInSpace = -1L, + ) + + val parsed = + 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(parsed) + assertEquals(0L, parsed.packet.packetNumber) + // Plaintext is padded to 3 bytes (pnLen=1 → minPayload=3); the + // original PING frame survives at offset 0 and trailing zeros decode + // as PADDING frames per RFC 9000 §19.1. + assertTrue(parsed.packet.payload.size >= 3) + assertEquals(0x01.toByte(), parsed.packet.payload[0]) + for (i in 1 until parsed.packet.payload.size) { + assertEquals(0x00.toByte(), parsed.packet.payload[i]) + } + } + + @Test + fun short_header_with_empty_payload_round_trips() { + // An empty payload is a degenerate input but should not crash the + // builder — it should pad to satisfy the HP sample window. + val plain = + ShortHeaderPlaintextPacket( + dcid = dcid, + packetNumber = 0L, + payload = ByteArray(0), + ) + val wire = + ShortHeaderPacket.build( + plain = plain, + aead = Aes128Gcm, + key = proto.clientKey, + iv = proto.clientIv, + hp = hp, + hpKey = proto.clientHp, + largestAckedInSpace = -1L, + ) + val parsed = + 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(parsed) + assertEquals(0L, parsed.packet.packetNumber) + } + + @Test + fun long_header_with_short_payload_round_trips() { + // Same hazard at the long-header level: a PING-only Initial or + // Handshake packet would also trip the HP sample require. + val pingPayload = byteArrayOf(0x01) + val wire = + LongHeaderPacket.build( + plain = + LongHeaderPlaintextPacket( + type = LongHeaderType.HANDSHAKE, + dcid = dcid, + scid = scid, + packetNumber = 0L, + payload = pingPayload, + ), + aead = Aes128Gcm, + key = proto.clientKey, + iv = proto.clientIv, + hp = hp, + hpKey = proto.clientHp, + largestAckedInSpace = -1L, + ) + val parsed = + LongHeaderPacket.parseAndDecrypt( + bytes = wire, + offset = 0, + aead = Aes128Gcm, + key = proto.clientKey, + iv = proto.clientIv, + hp = hp, + hpKey = proto.clientHp, + largestReceivedInSpace = -1L, + ) + assertNotNull(parsed) + assertEquals(0L, parsed.packet.packetNumber) + assertTrue(parsed.packet.payload.size >= 3) + assertEquals(0x01.toByte(), parsed.packet.payload[0]) + for (i in 1 until parsed.packet.payload.size) { + assertEquals(0x00.toByte(), parsed.packet.payload[i]) + } + } + + @Test + fun payload_at_or_above_threshold_is_unchanged() { + // pnLen=1, payload=4: already satisfies pnLen+payload >= 4. The + // builder MUST NOT add gratuitous padding — a regression there would + // bloat every outbound 1-RTT packet. + val payload = byteArrayOf(0x01, 0x02, 0x03, 0x04) + val plain = + ShortHeaderPlaintextPacket( + dcid = dcid, + packetNumber = 0L, + payload = payload, + ) + val wire = + ShortHeaderPacket.build( + plain = plain, + aead = Aes128Gcm, + key = proto.clientKey, + iv = proto.clientIv, + hp = hp, + hpKey = proto.clientHp, + largestAckedInSpace = -1L, + ) + val parsed = + 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(parsed) + assertContentEquals(payload, parsed.packet.payload) + } +} From 410123e28143fc7172731b6f5ebe7b4e07dee392 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 5 May 2026 14:17:05 +0000 Subject: [PATCH 2/2] fix(nests): reset CreateNest sheet state after successful publish MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CreateNestViewModel is keyed by user pubkey via `viewModel(key = …)`, so the same instance survives across sheet open/close cycles. After a successful publish the form was left untouched: room name, summary, image URL stayed populated and — most visibly — `isPublishing` was never cleared, so on the second open the Submit button rendered as a spinning progress indicator forever. Reset the form to fresh defaults (re-seeded from the user's saved kind-10112 list) at the end of `publishAndBuildLaunchInfo()`, right before returning the launch info. The sheet is about to dismiss anyway, so the user never sees the in-place reset; the next open behaves like a first open. --- .../nests/create/CreateNestViewModel.kt | 46 +++++++++++++++---- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/nests/create/CreateNestViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/nests/create/CreateNestViewModel.kt index 371b033be..6e46fc53e 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/nests/create/CreateNestViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/nests/create/CreateNestViewModel.kt @@ -70,14 +70,39 @@ class CreateNestViewModel : ViewModel() { fun bindAccountIfMissing(accountViewModel: AccountViewModel) { if (account != null) return account = accountViewModel - // Seed the URL fields from the user's saved kind-10112 list so - // first-time use flows naturally from the Settings screen. If - // the list is empty (or the user hasn't published one yet), keep - // the nostrnests.com defaults already in [FormState.defaults]. + seedDefaultsFromAccount() + } + + /** + * Reset the form to fresh defaults and re-seed the service / endpoint + * URLs from the user's saved kind-10112 list. Called after a + * successful publish so the next open of the sheet doesn't show + * stale fields (room name, summary, image URL) or — worse — a stuck + * `isPublishing=true` Submit button. The ViewModel is keyed by the + * user's pubkey, so without this reset the same instance survives + * across sheet open/close cycles. + */ + fun resetForm() { + _state.value = FormState.defaults() + seedDefaultsFromAccount() + } + + /** + * Seed the URL fields from the user's saved kind-10112 list so + * first-time use flows naturally from the Settings screen. If the + * list is empty (or the user hasn't published one yet), keep the + * nostrnests.com defaults already in [FormState.defaults]. + */ + private fun seedDefaultsFromAccount() { val first = - accountViewModel.account.nestsServers.flow.value - .firstOrNull() - if (first != null && first.relay.startsWith("http") && first.auth.startsWith("http")) { + account + ?.account + ?.nestsServers + ?.flow + ?.value + ?.firstOrNull() + ?: return + if (first.relay.startsWith("http") && first.auth.startsWith("http")) { _state.update { it.copy(serviceUrl = first.auth, endpointUrl = first.relay) } } } @@ -287,9 +312,10 @@ class CreateNestViewModel : ViewModel() { return null } - // Don't reset isPublishing here — the sheet dismisses on success - // and the VM is scoped per-composition so the next open starts - // fresh anyway. + // Reset the form so the next open of the sheet starts fresh — + // the VM is keyed by pubkey via `viewModel(key = …)` and is + // reused across sheet open/close cycles. + resetForm() return RoomLaunchInfo( addressValue = signed.address().toValue(), authBaseUrl = service,