nick/amethyst
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fix(quic): finish P1 + A2 audit follow-ups (PR #2873)

Browse Source 
P1 (header protection — eliminate remaining HP allocations):
Add HeaderProtection.maskInto(hpKey, src, srcOffset, scratch16, dstMask)
that writes the 5-byte mask into a caller-owned buffer using a
caller-owned 16-byte AES scratch. Add `hpScratch` (16 bytes) and
`hpMask` (5 bytes) to PacketProtection — same per-direction
single-threaded analysis as nonceScratch from the prior commit. Thread
both buffers through Short/LongHeaderPacket build / parseAndDecrypt /
peekKeyPhase as optional parameters paired with nonceScratch; production
call sites in QuicConnectionWriter / QuicConnectionParser pass
`proto.hpScratch` + `proto.hpMask`. With both this commit and 09b28b8d
the AES-128-GCM hot path now allocates ZERO bytes for HP + nonce per
packet (down from 16-byte sample slice + 16-byte AES output + 5-byte
mask + 12-byte nonce = 49 bytes/pkt).

ChaCha20HeaderProtection.maskInto routes through the same shape but
still allocates a fresh 5-byte ciphertext + 12-byte nonce slice
internally — Quartz's `ChaCha20Core.chaCha20Xor` SPI takes a
standalone nonce ByteArray. Documented as a future cleanup; AES is
the dominant path in production.

A2 (TlsRunningSha256 — lazy fallback accumulator):
Probe `digest.clone()` ONCE at construction. Conscrypt's clone
support is a build-time property (native bridge presence), not state-
dependent, so a single probe is a reliable signal. Devices where the
probe succeeds skip the byte accumulator entirely (zero overhead);
devices where it fails get the byte-accumulator path from the very
first update so the first snapshot already has the complete
transcript. Replaces the previous "always accumulate" shape that
wasted a few KB per handshake on every device, including the
overwhelming majority that support clone.

All quic JVM unit tests pass; spotless applied.

https://claude.ai/code/session_01EBHtGLy5o7FUR5qfcpHUUx
This commit is contained in:
Claude
2026-05-13 19:27:47 +00:00
parent 09b28b8d78
commit 8a3bd52631
7 changed files with 203 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files
quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/EncryptionLevel.kt
+17
View File
@@ -46,6 +46,23 @@ class PacketProtection(
* invocation.
*/
val nonceScratch: ByteArray = ByteArray(iv.size)
/**
* Per-direction header-protection scratch — 16 bytes of AES-ECB output
* for [hp.maskInto][com.vitorpamplona.quic.crypto.HeaderProtection.maskInto].
* Reused per packet so the AES output no longer allocates (round-5 #P1).
* Same single-direction thread-safety rationale as [nonceScratch].
*/
val hpScratch: ByteArray = ByteArray(16)
/**
* Per-direction header-protection mask buffer — 5 bytes per RFC 9001
* §5.4.3. Filled by [hp.maskInto][com.vitorpamplona.quic.crypto.HeaderProtection.maskInto]
* and immediately consumed by [com.vitorpamplona.quic.crypto.applyHeaderProtectionMask]
* (write path) or by inline first-byte / packet-number unmasking
* (parse path). Consume before the next mask call on this instance.
*/
val hpMask: ByteArray = ByteArray(5)
}
/** All four encryption levels we ever see in a QUIC client connection. */
quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionParser.kt
+10
View File
@@ -321,6 +321,8 @@ private fun feedLongHeaderPacket(
hpKey = proto.hpKey,
largestReceivedInSpace = state.pnSpace.largestReceived,
nonceScratch = proto.nonceScratch,
hpScratch = proto.hpScratch,
hpMask = proto.hpMask,
)
if (parsed == null) {
conn.qlogObserver.onPacketDropped(
@@ -393,6 +395,8 @@ private fun feedShortHeaderPacket(
dcidLen = conn.sourceConnectionId.length,
hp = live.hp,
hpKey = live.hpKey,
hpScratch = live.hpScratch,
hpMask = live.hpMask,
)
if (peek == null) {
conn.qlogObserver.onPacketDropped(
@@ -444,6 +448,8 @@ private fun feedShortHeaderPacket(
hpKey = live.hpKey,
largestReceivedInSpace = state.pnSpace.largestReceived,
nonceScratch = live.nonceScratch,
hpScratch = live.hpScratch,
hpMask = live.hpMask,
)
rotateOnSuccess = null
} else {
@@ -463,6 +469,8 @@ private fun feedShortHeaderPacket(
hpKey = prev.hpKey,
largestReceivedInSpace = state.pnSpace.largestReceived,
nonceScratch = prev.nonceScratch,
hpScratch = prev.hpScratch,
hpMask = prev.hpMask,
)
}
if (priorTry != null) {
@@ -493,6 +501,8 @@ private fun feedShortHeaderPacket(
hpKey = nextPhase.hpKey,
largestReceivedInSpace = state.pnSpace.largestReceived,
nonceScratch = nextPhase.nonceScratch,
hpScratch = nextPhase.hpScratch,
hpMask = nextPhase.hpMask,
)
rotateOnSuccess = nextPhase
}
quic/src/commonMain/kotlin/com/vitorpamplona/quic/connection/QuicConnectionWriter.kt
+10
View File
@@ -360,6 +360,8 @@ private fun buildBestLevelPacket(
proto.hpKey,
largestAckedInSpace = -1L,
nonceScratch = proto.nonceScratch,
hpScratch = proto.hpScratch,
hpMask = proto.hpMask,
)
emitQlogSent(conn, EncryptionLevel.APPLICATION, pn, built.size, frames)
return built
@@ -407,6 +409,8 @@ private fun buildLongHeaderPacket(
proto.hpKey,
largestAckedInSpace = -1L,
nonceScratch = proto.nonceScratch,
hpScratch = proto.hpScratch,
hpMask = proto.hpMask,
)
emitQlogSent(conn, level, pn, built.size, frames)
return built
@@ -562,6 +566,8 @@ private fun buildLongHeaderFromFrames(
proto.hpKey,
largestAckedInSpace = -1L,
nonceScratch = proto.nonceScratch,
hpScratch = proto.hpScratch,
hpMask = proto.hpMask,
)
// Step E: retain the packet for RFC 9002 retransmit. Initial /
// Handshake packets carry CRYPTO frames; loss detection runs at
@@ -916,6 +922,8 @@ private fun buildApplicationPacket(
proto.hpKey,
largestAckedInSpace = -1L,
nonceScratch = proto.nonceScratch,
hpScratch = proto.hpScratch,
hpMask = proto.hpMask,
)
} else {
// 0-RTT — long header type=0x01. Same Application packet
@@ -938,6 +946,8 @@ private fun buildApplicationPacket(
proto.hpKey,
largestAckedInSpace = -1L,
nonceScratch = proto.nonceScratch,
hpScratch = proto.hpScratch,
hpMask = proto.hpMask,
)
}
}
quic/src/commonMain/kotlin/com/vitorpamplona/quic/crypto/HeaderProtection.kt
+65 -13
View File
@@ -32,7 +32,8 @@ sealed class HeaderProtection {
/**
* Compute the HP mask from a 16-byte standalone sample buffer. Retained
* for callers (mostly tests) that already have a heap-allocated sample
* blob — the production hot path uses [maskAt] to avoid the slice.
* blob — the production hot path uses [maskInto] to avoid every per-
* packet allocation.
*/
abstract fun mask(
hpKey: ByteArray,
@@ -42,15 +43,36 @@ sealed class HeaderProtection {
/**
* Compute the HP mask from a 16-byte sample window inside [src] starting
* at [srcOffset]. Avoids the `copyOfRange` of the sample on every
* outbound and inbound packet (round-5 #P1). Returns a freshly allocated
* 5-byte mask — the per-packet allocation budget drops from
* `16 (sample) + 16 (cipher output) + 5 (mask)` to `16 + 5` for AES-ECB.
* outbound and inbound packet (round-5 #P1) but still allocates a
* fresh 5-byte mask + an internal 16-byte AES scratch on each call.
* Use [maskInto] when caller-owned scratch + mask buffers are
* available (the production writer/parser path).
*/
abstract fun maskAt(
hpKey: ByteArray,
src: ByteArray,
srcOffset: Int,
): ByteArray
/**
* Allocation-free HP mask: write the 5 mask bytes into [dstMask], using
* [scratch16] (caller-owned 16-byte buffer) for the AES-ECB output
* intermediate. Returns [dstMask] for fluent use.
*
* Both buffers are MUTATED — callers must consume the mask before the
* next [maskInto] call on the same buffers. The hot QUIC writer/parser
* path in `Short/LongHeaderPacket.build` + `parseAndDecrypt` keeps a
* per-PacketProtection scratch + mask pair and consumes the mask
* immediately during [com.vitorpamplona.quic.crypto.applyHeaderProtectionMask],
* so the lifetime is trivially safe.
*/
abstract fun maskInto(
hpKey: ByteArray,
src: ByteArray,
srcOffset: Int,
scratch16: ByteArray,
dstMask: ByteArray,
): ByteArray
}
/** AES-128-ECB header protection. Implemented via the platform AES helper. */
@@ -71,15 +93,23 @@ class AesEcbHeaderProtection(
hpKey: ByteArray,
src: ByteArray,
srcOffset: Int,
): ByteArray = maskInto(hpKey, src, srcOffset, ByteArray(16), ByteArray(5))
override fun maskInto(
hpKey: ByteArray,
src: ByteArray,
srcOffset: Int,
scratch16: ByteArray,
dstMask: ByteArray,
): ByteArray {
require(srcOffset >= 0 && srcOffset + 16 <= src.size) { "HP sample window out of range" }
require(hpKey.size in setOf(16, 24, 32)) { "AES-ECB key must be 16/24/32 bytes" }
val scratch = ByteArray(16)
aesEncryptOneBlock.encryptInto(hpKey, src, srcOffset, scratch, 0)
require(scratch16.size == 16) { "AES scratch must be 16 bytes" }
require(dstMask.size >= 5) { "HP mask buffer must be at least 5 bytes" }
aesEncryptOneBlock.encryptInto(hpKey, src, srcOffset, scratch16, 0)
// Mask is the first 5 bytes per RFC 9001 §5.4.3.
val mask = ByteArray(5)
scratch.copyInto(mask, 0, 0, 5)
return mask
scratch16.copyInto(dstMask, 0, 0, 5)
return dstMask
}
}
@@ -99,6 +129,32 @@ class ChaCha20HeaderProtection(
hpKey: ByteArray,
src: ByteArray,
srcOffset: Int,
): ByteArray = chacha20Mask(hpKey, src, srcOffset)
override fun maskInto(
hpKey: ByteArray,
src: ByteArray,
srcOffset: Int,
scratch16: ByteArray,
dstMask: ByteArray,
): ByteArray {
require(dstMask.size >= 5) { "HP mask buffer must be at least 5 bytes" }
// ChaCha20 HP unavoidably allocates a fresh 5-byte ciphertext via the
// [ChaCha20BlockEncrypt] SPI, plus a 12-byte nonce slice (Quartz's
// `ChaCha20Core.chaCha20Xor` takes a standalone nonce). Copy the
// result into the caller's [dstMask] so call-site shape matches the
// AES-ECB path; a future pass could push src+offset through the
// ChaCha20 SPI to fully eliminate the slice. [scratch16] is unused
// here.
val mask = chacha20Mask(hpKey, src, srcOffset)
mask.copyInto(dstMask, 0, 0, 5)
return dstMask
}
private fun chacha20Mask(
hpKey: ByteArray,
src: ByteArray,
srcOffset: Int,
): ByteArray {
require(srcOffset >= 0 && srcOffset + 16 <= src.size) { "ChaCha20 HP sample window out of range" }
require(hpKey.size == 32) { "ChaCha20 HP key must be 32 bytes" }
@@ -107,10 +163,6 @@ class ChaCha20HeaderProtection(
((src[srcOffset + 1].toInt() and 0xFF) shl 8) or
((src[srcOffset + 2].toInt() and 0xFF) shl 16) or
((src[srcOffset + 3].toInt() and 0xFF) shl 24)
// The nonce slice is unavoidable as long as we delegate to Quartz's
// `ChaCha20Core.chaCha20Xor(plaintext, key, nonce, counter)` which
// takes a standalone nonce ByteArray. A future pass could thread
// src + offset all the way down.
val nonce = src.copyOfRange(srcOffset + 4, srcOffset + 16)
return chacha20Encrypt.encrypt(hpKey, nonce, counter, ByteArray(5))
}
quic/src/commonMain/kotlin/com/vitorpamplona/quic/packet/LongHeaderPacket.kt
+25 -8
View File
@@ -73,9 +73,12 @@ object LongHeaderPacket {
hp: HeaderProtection,
hpKey: ByteArray,
largestAckedInSpace: Long,
// Round-5 #P2: optional 12-byte scratch the writer can reuse across
// packets. Default = allocate fresh (preserves test ergonomics).
// Round-5 #P2 + #P1: optional caller-owned scratch buffers. When
// provided the build path allocates nothing for nonce / HP work.
// Default = allocate fresh on each call (preserves tests).
nonceScratch: ByteArray? = null,
hpScratch: ByteArray? = null,
hpMask: ByteArray? = null,
): ByteArray {
val pnLen =
com.vitorpamplona.quic.connection.PacketNumberSpaceState.encodeLength(
@@ -142,11 +145,17 @@ object LongHeaderPacket {
)
// Apply header protection. Sample is 16 bytes starting 4 bytes after pnOffset.
// Round-5 #P1: read the sample window directly from `packet` instead
// of allocating a 16-byte copyOfRange per outbound long-header packet.
// Round-5 #P1: read the sample window directly from `packet`. When
// caller-owned hp scratch + mask are provided the HP path allocates
// zero bytes per packet; otherwise allocate fresh (test path).
val sampleStart = pnOffset + 4
require(sampleStart + 16 <= packet.size) { "packet too short for HP sample" }
val mask = hp.maskAt(hpKey, packet, sampleStart)
val mask =
if (hpScratch != null && hpMask != null) {
hp.maskInto(hpKey, packet, sampleStart, hpScratch, hpMask)
} else {
hp.maskAt(hpKey, packet, sampleStart)
}
applyHeaderProtectionMask(packet, firstByteOffset, pnOffset, pnLen, mask)
return packet
@@ -169,9 +178,12 @@ object LongHeaderPacket {
hp: HeaderProtection,
hpKey: ByteArray,
largestReceivedInSpace: Long,
// Round-5 #P2: optional 12-byte scratch the parser reuses across
// inbound packets. Default = allocate fresh (preserves tests).
// Round-5 #P2 + #P1: optional caller-owned scratch buffers. When
// provided the parse path allocates nothing for nonce / HP work.
// Default = allocate fresh on each call (preserves tests).
nonceScratch: ByteArray? = null,
hpScratch: ByteArray? = null,
hpMask: ByteArray? = null,
): ParseResult? {
val packetStart = offset
val r = QuicReader(bytes, offset)
@@ -209,7 +221,12 @@ object LongHeaderPacket {
// Sample for HP starts at pnOffset + 4.
val sampleStart = pnOffset + 4
if (sampleStart + 16 > bytes.size) return null
val mask = hp.maskAt(hpKey, bytes, sampleStart)
val mask =
if (hpScratch != null && hpMask != null) {
hp.maskInto(hpKey, bytes, sampleStart, hpScratch, hpMask)
} else {
hp.maskAt(hpKey, bytes, sampleStart)
}
// Make a private copy of the packet so we can mutate the header in place.
val packetEnd = pnOffset + length
quic/src/commonMain/kotlin/com/vitorpamplona/quic/packet/ShortHeaderPacket.kt
+39 -9
View File
@@ -54,9 +54,16 @@ object ShortHeaderPacket {
hp: HeaderProtection,
hpKey: ByteArray,
largestAckedInSpace: Long,
// Round-5 #P2: optional 12-byte scratch the writer can reuse across
// packets. Default = allocate fresh (preserves test ergonomics).
// Round-5 #P2: optional 12-byte AEAD-nonce scratch the writer can
// reuse across packets. Default = allocate fresh (preserves tests).
nonceScratch: ByteArray? = null,
// Round-5 #P1: optional 16-byte AES-ECB scratch + 5-byte mask
// buffer for header protection. When BOTH are provided the call
// site reuses them across packets and the HP path allocates
// nothing per packet. Either-null (or both-null) falls back to
// the maskAt path which allocates fresh (preserves tests).
hpScratch: ByteArray? = null,
hpMask: ByteArray? = null,
): ByteArray {
val pnLen = PacketNumberSpaceState.encodeLength(plain.packetNumber, largestAckedInSpace)
require(pnLen in 1..4)
@@ -107,9 +114,15 @@ object ShortHeaderPacket {
val sampleStart = pnOffset + 4
require(sampleStart + 16 <= packet.size) { "packet too short for HP sample" }
// Round-5 #P1: read the sample window directly from `packet` — pre-
// fix this allocated a fresh 16-byte ByteArray on every outbound.
val mask = hp.maskAt(hpKey, packet, sampleStart)
// Round-5 #P1: when caller-owned HP scratch + mask buffers are
// provided, write the mask in-place — zero per-packet allocation.
// Otherwise allocate fresh (test path).
val mask =
if (hpScratch != null && hpMask != null) {
hp.maskInto(hpKey, packet, sampleStart, hpScratch, hpMask)
} else {
hp.maskAt(hpKey, packet, sampleStart)
}
applyHeaderProtectionMask(packet, firstByteOffset, pnOffset, pnLen, mask)
return packet
}
@@ -136,6 +149,10 @@ object ShortHeaderPacket {
dcidLen: Int,
hp: HeaderProtection,
hpKey: ByteArray,
// Round-5 #P1: optional caller-owned HP scratch + mask. Both null
// = allocate fresh (test path); both non-null = zero-alloc.
hpScratch: ByteArray? = null,
hpMask: ByteArray? = null,
): Peek? {
if (offset >= bytes.size) return null
val first = bytes[offset].toInt() and 0xFF
@@ -148,7 +165,12 @@ object ShortHeaderPacket {
val pnOffset = offset + 1 + dcidLen
val sampleStart = pnOffset + 4
if (sampleStart + 16 > bytes.size) return null
val mask = hp.maskAt(hpKey, bytes, sampleStart)
val mask =
if (hpScratch != null && hpMask != null) {
hp.maskInto(hpKey, bytes, sampleStart, hpScratch, hpMask)
} else {
hp.maskAt(hpKey, bytes, sampleStart)
}
val unprotectedFirst = first xor (mask[0].toInt() and 0x1F)
return Peek(
keyPhase = (unprotectedFirst and 0x04) != 0,
@@ -172,9 +194,12 @@ object ShortHeaderPacket {
hp: HeaderProtection,
hpKey: ByteArray,
largestReceivedInSpace: Long,
// Round-5 #P2: optional 12-byte scratch the parser reuses across
// inbound packets. Default = allocate fresh (preserves tests).
// Round-5 #P2 + #P1: optional caller-owned scratch buffers. When
// provided the parse path allocates nothing for nonce / HP work.
// Default = allocate fresh on each call (preserves tests).
nonceScratch: ByteArray? = null,
hpScratch: ByteArray? = null,
hpMask: ByteArray? = null,
): ParseResult? {
if (offset >= bytes.size) return null
val first = bytes[offset].toInt() and 0xFF
@@ -185,7 +210,12 @@ object ShortHeaderPacket {
val pnOffset = offset + 1 + dcidLen
val sampleStart = pnOffset + 4
if (sampleStart + 16 > bytes.size) return null
val mask = hp.maskAt(hpKey, bytes, sampleStart)
val mask =
if (hpScratch != null && hpMask != null) {
hp.maskInto(hpKey, bytes, sampleStart, hpScratch, hpMask)
} else {
hp.maskAt(hpKey, bytes, sampleStart)
}
val packetEnd = bytes.size
val packet = bytes.copyOfRange(offset, packetEnd)
val localPnOffset = pnOffset - offset
quic/src/jvmAndroid/kotlin/com/vitorpamplona/quic/tls/TlsRunningSha256.kt
+37 -23
View File
@@ -28,9 +28,14 @@ import java.security.MessageDigest
* stock JDK SHA-256 providers AND by Android's Conscrypt
* `OpenSSLMessageDigestJDK` on shipping releases — but a handful of API
* 26–28 builds have been reported to throw `CloneNotSupportedException`
* from the native digest. Detect that on first snapshot and fall back to
* one-shot SHA-256 over an accumulated byte buffer; we keep accumulating
* regardless so the fallback always has the complete transcript to hash.
* from the native digest.
*
* We probe `clone()` ONCE at construction: if it works the JCA-clone path
* runs for the lifetime of the instance with zero side-buffer overhead;
* if it doesn't we switch to byte-accumulator-then-one-shot-SHA-256 mode
* from the start, so the very first snapshot already has the complete
* transcript. Working devices (the overwhelming majority) pay nothing
* for the fallback.
*
* Single-thread per instance: the TlsClient state machine is the sole caller,
* driven by the QUIC connection's lock, so no synchronization is needed.
@@ -38,36 +43,45 @@ import java.security.MessageDigest
actual class TlsRunningSha256 actual constructor() {
private val digest: MessageDigest = MessageDigest.getInstance("SHA-256")
// Parallel byte accumulator — small (a few KB for a TLS transcript),
// and only consulted on the cloneable-digest fallback path. Keeping it
// populated unconditionally costs one `ByteArrayOutputStream.write` per
// [update] but avoids a "first snapshot fails, we have no history"
// failure mode on the broken-clone Android builds.
private val accumulator = ByteArrayOutputStream(512)
private var cloneable = true
/**
* Result of the one-time `digest.clone()` probe. Conscrypt's clone
* support is a property of the build (native bridge presence), not of
* the digest's internal state, so a single probe is a reliable signal
* — repeated clones on the same instance behave identically.
*/
private val cloneable: Boolean =
try {
digest.clone()
true
} catch (_: CloneNotSupportedException) {
false
}
/**
* Byte accumulator allocated only on the broken-clone fallback path.
* Holds every byte fed to [update] so [snapshot] can one-shot
* SHA-256 the full transcript. On clone-capable devices this stays
* `null` and [update] never touches it.
*/
private val accumulator: ByteArrayOutputStream? =
if (cloneable) null else ByteArrayOutputStream(512)
actual fun update(bytes: ByteArray) {
digest.update(bytes)
accumulator.write(bytes)
accumulator?.write(bytes)
}
actual fun snapshot(): ByteArray {
if (cloneable) {
try {
// Cloning the digest is the cheap path — independent digest
// object with the current internal state, no consume.
val clone = digest.clone() as MessageDigest
return clone.digest()
} catch (_: CloneNotSupportedException) {
// Latch the fallback so we don't pay the JCA throw on every
// subsequent snapshot.
cloneable = false
}
// Cheap path — independent digest object with current internal
// state, no consume.
val clone = digest.clone() as MessageDigest
return clone.digest()
}
// Fallback: one-shot SHA-256 over the accumulated transcript bytes.
// `MessageDigest.getInstance("SHA-256")` is mandated on every JCA
// provider — only the `.clone()` capability varies — so this is
// provider — only `.clone()` capability varies — so this is
// guaranteed to work on the same device that rejected the clone.
return MessageDigest.getInstance("SHA-256").digest(accumulator.toByteArray())
return MessageDigest.getInstance("SHA-256").digest(accumulator!!.toByteArray())
}
}