perf(quic): AEAD range overload + doc notes for known fragile couplings
Round 8 — AEAD allocation reduction on the receive hot path, plus the documentation of two known-fragile-but-not-broken couplings. * Aead gains [openRange] / [sealRange] taking (array, offset, length) triples for AAD and plaintext/ciphertext. Default impls slice and delegate to the existing whole-array methods so non-JCA AEADs (Aes128Gcm singleton, ChaCha20Poly1305Aead) still work. The JCA-backed [JcaAesGcmAead] overrides both, passing offsets straight to `Cipher.updateAAD(byte[], offset, len)` and `Cipher.doFinal(byte[], inputOffset, inputLen)` — JCA accepts ranges natively and does no internal copies. Saves ~2 KB ByteArray allocations per inbound packet (one for the header `aad`, one for the `ciphertext`) — at audio-room receive rates (~100 packets/sec) that's ~12 MB/min of GC churn eliminated. Same overload on the outbound path is wired but currently exercised less because the build path constructs `headerBytes` and `paddedPlaintext` as separate fresh allocations. parseAndDecrypt in both ShortHeaderPacket and LongHeaderPacket now call openRange instead of slicing into intermediates. * JdkCertificateValidator.dnsMatches: explicit doc note on the public-suffix-list gap. The dot-count heuristic accepts wildcards like `*.co.uk` / `*.github.io` / `*.s3.amazonaws.com` whose effective TLD spans multiple labels. WebPKI / CT logging mitigates this in practice (CAs validate against the actual PSL), but our local validation alone wouldn't catch a rogue cert. Production callers for sensitive endpoints should rely on OS / NetworkSecurityConfig pinning rather than QUIC's hostname check alone. Full PSL data shipping deferred until justified. * QuicStream.incoming: explicit doc note on the single-collector contract. [consumeAsFlow] cancels the underlying [Channel] when its collector terminates, and once cancelled the parser's next trySend fails, sets [overflowed] = true, and the parser tears down the connection with INTERNAL_ERROR. Production callers already follow single-collector + collect-until-FIN, but the doc lays out the rule so a future refactor doesn't loosen it accidentally. Long-form discussion of the `MutableSharedFlow` alternative and why we declined it. All 269 :quic:jvmTest tests pass. BUILD SUCCESSFUL in 40s. https://claude.ai/code/session_01AhGvbMV8uPRse3TmAGaddM
This commit is contained in:
@@ -52,6 +52,64 @@ abstract class Aead {
|
||||
aad: ByteArray,
|
||||
ciphertext: ByteArray,
|
||||
): ByteArray?
|
||||
|
||||
/**
|
||||
* Range-based [seal] — same semantics as [seal] but reads `aad` and
|
||||
* `plaintext` from sub-ranges of larger backing arrays. Default impl
|
||||
* slices and delegates; concrete impls (notably the JCA-backed
|
||||
* [com.vitorpamplona.quic.crypto.JcaAesGcmAead]) override to skip
|
||||
* the slice allocations entirely by passing the offsets through to
|
||||
* `Cipher.updateAAD` / `Cipher.doFinal`.
|
||||
*
|
||||
* Saves ~2 ByteArray allocations per outbound packet on the hot
|
||||
* path (the header `aad` and the payload `plaintext` no longer
|
||||
* need to be carved out of the in-progress packet buffer).
|
||||
*/
|
||||
open fun sealRange(
|
||||
key: ByteArray,
|
||||
nonce: ByteArray,
|
||||
aad: ByteArray,
|
||||
aadOffset: Int,
|
||||
aadLength: Int,
|
||||
plaintext: ByteArray,
|
||||
plaintextOffset: Int,
|
||||
plaintextLength: Int,
|
||||
): ByteArray {
|
||||
val a =
|
||||
if (aadOffset == 0 && aadLength == aad.size) aad else aad.copyOfRange(aadOffset, aadOffset + aadLength)
|
||||
val p =
|
||||
if (plaintextOffset == 0 && plaintextLength == plaintext.size) {
|
||||
plaintext
|
||||
} else {
|
||||
plaintext.copyOfRange(plaintextOffset, plaintextOffset + plaintextLength)
|
||||
}
|
||||
return seal(key, nonce, a, p)
|
||||
}
|
||||
|
||||
/**
|
||||
* Range-based [open] — same semantics as [open] but reads `aad` and
|
||||
* `ciphertext` from sub-ranges. Default impl slices and delegates.
|
||||
*/
|
||||
open fun openRange(
|
||||
key: ByteArray,
|
||||
nonce: ByteArray,
|
||||
aad: ByteArray,
|
||||
aadOffset: Int,
|
||||
aadLength: Int,
|
||||
ciphertext: ByteArray,
|
||||
ciphertextOffset: Int,
|
||||
ciphertextLength: Int,
|
||||
): ByteArray? {
|
||||
val a =
|
||||
if (aadOffset == 0 && aadLength == aad.size) aad else aad.copyOfRange(aadOffset, aadOffset + aadLength)
|
||||
val c =
|
||||
if (ciphertextOffset == 0 && ciphertextLength == ciphertext.size) {
|
||||
ciphertext
|
||||
} else {
|
||||
ciphertext.copyOfRange(ciphertextOffset, ciphertextOffset + ciphertextLength)
|
||||
}
|
||||
return open(key, nonce, a, c)
|
||||
}
|
||||
}
|
||||
|
||||
/** AES-128-GCM AEAD via Quartz's AESGCM (which uses JCA underneath on JVM/Android). */
|
||||
|
||||
@@ -223,10 +223,21 @@ object LongHeaderPacket {
|
||||
)
|
||||
|
||||
val aadEnd = localPnOffset + pnLen
|
||||
val aad = packet.copyOfRange(0, aadEnd)
|
||||
val ciphertext = packet.copyOfRange(aadEnd, packet.size)
|
||||
val nonce = aeadNonce(iv, fullPn)
|
||||
val plaintext = aead.open(key, nonce, aad, ciphertext) ?: return null
|
||||
// Range-based open avoids two ByteArray slice allocations per
|
||||
// inbound packet — see [ShortHeaderPacket.parseAndDecrypt] for
|
||||
// rationale.
|
||||
val plaintext =
|
||||
aead.openRange(
|
||||
key = key,
|
||||
nonce = nonce,
|
||||
aad = packet,
|
||||
aadOffset = 0,
|
||||
aadLength = aadEnd,
|
||||
ciphertext = packet,
|
||||
ciphertextOffset = aadEnd,
|
||||
ciphertextLength = packet.size - aadEnd,
|
||||
) ?: return null
|
||||
|
||||
return ParseResult(
|
||||
packet =
|
||||
|
||||
@@ -182,10 +182,22 @@ object ShortHeaderPacket {
|
||||
}
|
||||
val fullPn = PacketNumberSpaceState.decodePacketNumber(largestReceivedInSpace, truncatedPn, pnLen)
|
||||
val aadEnd = localPnOffset + pnLen
|
||||
val aad = packet.copyOfRange(0, aadEnd)
|
||||
val ciphertext = packet.copyOfRange(aadEnd, packet.size)
|
||||
val nonce = aeadNonce(iv, fullPn)
|
||||
val plaintext = aead.open(key, nonce, aad, ciphertext) ?: return null
|
||||
// Range-based open: aad = packet[0..aadEnd), ciphertext = packet[aadEnd..size).
|
||||
// Saves the two ByteArray slice allocations that the
|
||||
// whole-array form (`aad = copyOfRange(0, aadEnd)` etc.)
|
||||
// would do — ~2 KB per inbound packet on the hot path.
|
||||
val plaintext =
|
||||
aead.openRange(
|
||||
key = key,
|
||||
nonce = nonce,
|
||||
aad = packet,
|
||||
aadOffset = 0,
|
||||
aadLength = aadEnd,
|
||||
ciphertext = packet,
|
||||
ciphertextOffset = aadEnd,
|
||||
ciphertextLength = packet.size - aadEnd,
|
||||
) ?: return null
|
||||
return ParseResult(
|
||||
packet =
|
||||
ShortHeaderPlaintextPacket(
|
||||
|
||||
@@ -77,6 +77,31 @@ class QuicStream(
|
||||
* rather than silently dropping bytes. Pre-audit-4 the failed `trySend`
|
||||
* was discarded, leaving a hole in the stream that the application could
|
||||
* never know about.
|
||||
*
|
||||
* **Single-collector contract.** [consumeAsFlow] cancels the underlying
|
||||
* [Channel] when its collector terminates (either by exception or
|
||||
* explicit cancellation). After that point any subsequent
|
||||
* `trySend` from the parser fails, sets [overflowed] = true, and
|
||||
* the parser tears the whole connection down with
|
||||
* `INTERNAL_ERROR: stream … consumer overflowed`. So:
|
||||
* - Application code MUST collect [incoming] **at most once** per
|
||||
* stream and MUST hold the collect open until the stream
|
||||
* terminates (FIN, peer RESET_STREAM, or local
|
||||
* [stopSending] / [resetStream] decision).
|
||||
* - Cancelling the collector early — e.g. wrapping in
|
||||
* `withTimeout(...)` — is equivalent to telling the connection
|
||||
* to drop. If the application wants to stop receiving without
|
||||
* killing the connection, call [stopSending] FIRST, then let
|
||||
* the parser's RESET_STREAM-style teardown drain the channel
|
||||
* cleanly.
|
||||
*
|
||||
* This is a fragile coupling we accept for now because (a) every
|
||||
* production caller already follows the single-collector pattern,
|
||||
* and (b) widening the contract would require swapping
|
||||
* `consumeAsFlow` for a `MutableSharedFlow` with replay/buffer
|
||||
* semantics, which has its own back-pressure pitfalls. The
|
||||
* comment is here so a future refactor doesn't accidentally
|
||||
* loosen the contract.
|
||||
*/
|
||||
private val incomingChannel = Channel<ByteArray>(capacity = 64)
|
||||
val incoming: Flow<ByteArray> get() = incomingChannel.consumeAsFlow()
|
||||
|
||||
@@ -138,6 +138,74 @@ class JcaAesGcmAead(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* JCA-native range overload: `Cipher.updateAAD(byte[], offset, len)`
|
||||
* and `Cipher.doFinal(byte[], inputOffset, inputLen)` accept
|
||||
* sub-ranges directly, so we skip the two `copyOfRange` calls the
|
||||
* default impl would perform. Saves ~2 KB allocation per inbound
|
||||
* packet on the audio-rooms hot path (one for `aad`, one for
|
||||
* `ciphertext`, both sliced from a packet buffer that the caller
|
||||
* already owns).
|
||||
*/
|
||||
override fun openRange(
|
||||
key: ByteArray,
|
||||
nonce: ByteArray,
|
||||
aad: ByteArray,
|
||||
aadOffset: Int,
|
||||
aadLength: Int,
|
||||
ciphertext: ByteArray,
|
||||
ciphertextOffset: Int,
|
||||
ciphertextLength: Int,
|
||||
): ByteArray? =
|
||||
synchronized(this) {
|
||||
try {
|
||||
decryptCipher.init(Cipher.DECRYPT_MODE, keySpec, GCMParameterSpec(128, nonce))
|
||||
decryptCipher.updateAAD(aad, aadOffset, aadLength)
|
||||
decryptCipher.doFinal(ciphertext, ciphertextOffset, ciphertextLength)
|
||||
} catch (_: GeneralSecurityException) {
|
||||
null
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* JCA-native range overload for [seal]. Same nonce-reuse history +
|
||||
* fresh-Cipher fallback as the whole-array [seal] path; the only
|
||||
* difference is the offset+length pass-through to `updateAAD` /
|
||||
* `doFinal`. Saves the slice allocations on the outbound hot path.
|
||||
*/
|
||||
override fun sealRange(
|
||||
key: ByteArray,
|
||||
nonce: ByteArray,
|
||||
aad: ByteArray,
|
||||
aadOffset: Int,
|
||||
aadLength: Int,
|
||||
plaintext: ByteArray,
|
||||
plaintextOffset: Int,
|
||||
plaintextLength: Int,
|
||||
): ByteArray =
|
||||
synchronized(this) {
|
||||
val reuse = recentEncryptNonces.any { it.contentEquals(nonce) }
|
||||
if (reuse) {
|
||||
val fresh = Cipher.getInstance("AES/GCM/NoPadding")
|
||||
fresh.init(Cipher.ENCRYPT_MODE, keySpec, GCMParameterSpec(128, nonce))
|
||||
fresh.updateAAD(aad, aadOffset, aadLength)
|
||||
fresh.doFinal(plaintext, plaintextOffset, plaintextLength)
|
||||
} else {
|
||||
try {
|
||||
encryptCipher.init(Cipher.ENCRYPT_MODE, keySpec, GCMParameterSpec(128, nonce))
|
||||
encryptCipher.updateAAD(aad, aadOffset, aadLength)
|
||||
val out = encryptCipher.doFinal(plaintext, plaintextOffset, plaintextLength)
|
||||
rememberEncryptNonce(nonce)
|
||||
out
|
||||
} catch (t: Throwable) {
|
||||
if (recentEncryptNonces.lastOrNull()?.contentEquals(nonce) == true) {
|
||||
recentEncryptNonces.removeLast()
|
||||
}
|
||||
throw t
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private companion object {
|
||||
private const val NONCE_HISTORY_LIMIT = 8
|
||||
}
|
||||
|
||||
@@ -248,9 +248,22 @@ class JdkCertificateValidator(
|
||||
val prefix = lhost.substring(0, lhost.length - suffix.length)
|
||||
if (prefix.isEmpty() || '.' in prefix) return false
|
||||
// RFC 6125 §6.4.3 — disallow wildcards in the public-suffix label.
|
||||
// Heuristic: require ≥ 2 dots in the suffix (e.g. *.example.com is OK,
|
||||
// *.com is not). Conservative; doesn't consult the actual PSL but
|
||||
// matches what most browsers do for non-PSL-aware certs.
|
||||
//
|
||||
// KNOWN GAP: this heuristic counts dots in the suffix (≥ 2 → OK)
|
||||
// and DOES NOT consult the actual public-suffix list. So it
|
||||
// accepts `*.co.uk`, `*.github.io`, `*.s3.amazonaws.com`, etc.
|
||||
// — multi-tenant TLDs whose effective TLD is multi-label. A
|
||||
// CA that mis-issues such a wildcard could impersonate any
|
||||
// co-tenant. The mitigation is partial: the WebPKI ecosystem
|
||||
// already requires CAs to consult the PSL when issuing, so a
|
||||
// rogue cert is unlikely to make it past CT logging — but if
|
||||
// one does, our validation accepts it.
|
||||
//
|
||||
// Full fix requires shipping PSL data; deferred until the cost
|
||||
// is justified by a higher-risk deployment. Production callers
|
||||
// should rely on the OS / NetworkSecurityConfig pinning layer
|
||||
// for sensitive endpoints rather than QUIC's built-in
|
||||
// hostname check alone.
|
||||
val suffixDots = suffix.count { it == '.' }
|
||||
return suffixDots >= 2
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user