diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/crypto/X25519.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/crypto/X25519.kt index 702fb3d8b4..ea6f18ae98 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/crypto/X25519.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/marmot/mls/crypto/X25519.kt @@ -25,9 +25,14 @@ package com.vitorpamplona.quartz.marmot.mls.crypto * * Used in TreeKEM for path secret encryption via HPKE. * - * Platform-specific implementations: - * - JVM/Android: java.security XDH (Java 11+, Android API 31+) - * - Native: expect/actual with platform crypto + * All platform actuals (jvmAndroid, apple, linux) are the SAME pure-Kotlin + * Montgomery-ladder implementation over [Curve25519Field] (a TweetNaCl port) — + * they intentionally do NOT delegate to a platform provider such as + * `java.security` XDH, because Android's KeyStore/JCA integration for raw X25519 + * keys is unreliable across API levels. `dh()` rejects an all-zero shared secret + * per RFC 7748 §6.1; note the ladder relies on branch-uniform (mask-based) + * selection but, being pure Kotlin on the JVM/ART, carries no hardware + * constant-time guarantee. */ expect object X25519 { /** diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip44Encryption/Nip44v2.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip44Encryption/Nip44v2.kt index 7dffa7b13a..98baa80452 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip44Encryption/Nip44v2.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip44Encryption/Nip44v2.kt @@ -25,6 +25,7 @@ import com.vitorpamplona.quartz.nip44Encryption.crypto.ChaCha20 import com.vitorpamplona.quartz.nip44Encryption.crypto.Hkdf import com.vitorpamplona.quartz.utils.RandomInstance import com.vitorpamplona.quartz.utils.Secp256k1Instance +import com.vitorpamplona.quartz.utils.equalsConstantTime import kotlinx.coroutines.CancellationException import kotlin.io.encoding.Base64 import kotlin.math.floor @@ -104,7 +105,7 @@ class Nip44v2 { ) { val calculatedMac = hmacAad(messageKey.hmacKey, decoded.ciphertext, decoded.nonce) - check(calculatedMac.contentEquals(decoded.mac)) { + check(calculatedMac.equalsConstantTime(decoded.mac)) { "Invalid Mac: Calculated ${calculatedMac.toHexKey()}, decoded: ${decoded.mac.toHexKey()}" } } diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip44Encryption/SharedKeyCache.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip44Encryption/SharedKeyCache.kt index 3d510ca11e..a74614dda6 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip44Encryption/SharedKeyCache.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip44Encryption/SharedKeyCache.kt @@ -23,32 +23,56 @@ package com.vitorpamplona.quartz.nip44Encryption import androidx.collection.LruCache class SharedKeyCache { - private val sharedKeyCache = LruCache(200) + // Keyed by the full (privateKey, pubKey) content via [CacheKey], NOT by a bare + // 32-bit hashCode. Using a hashCode as the whole map key silently collides: + // two distinct peers whose (priv, pub) bytes hash to the same Int would share a + // slot, so `get` could return one peer's conversation key for a message meant + // for the other — a silent wrong-key encrypt/decrypt (and the polynomial hash + // that was used collides independently of the private key, so a pubkey aliasing + // a victim's contact is grindable). [CacheKey] keeps the cheap Int hash only as + // a bucket selector and disambiguates collisions with a full contentEquals, so + // it stays correct while avoiding the per-lookup allocation of a hex String key. + private val sharedKeyCache = LruCache(200) fun clearCache() { sharedKeyCache.evictAll() } - fun combinedHashCode( - a: ByteArray, - b: ByteArray, - ): Int { - var result = 1 - for (element in a) result = 31 * result + element - for (element in b) result = 31 * result + element - return result - } - fun get( privateKey: ByteArray, pubKey: ByteArray, - ): ByteArray? = sharedKeyCache[combinedHashCode(privateKey, pubKey)] + ): ByteArray? = sharedKeyCache[CacheKey(privateKey, pubKey)] fun add( privateKey: ByteArray, pubKey: ByteArray, secret: ByteArray, ) { - sharedKeyCache.put(combinedHashCode(privateKey, pubKey), secret) + sharedKeyCache.put(CacheKey(privateKey, pubKey), secret) + } + + /** + * Content-addressed cache key holding the raw key references (no byte copy, no + * hex string). The precomputed [hash] is only a bucket selector — [equals] does + * the authoritative full-content comparison, so hash collisions can never return + * the wrong peer's secret. Callers must treat the passed arrays as immutable + * (the same value-type contract [com.vitorpamplona.quartz.marmot.mls.crypto.X25519KeyPair] + * relies on when used as a map key). + */ + private class CacheKey( + val privateKey: ByteArray, + val pubKey: ByteArray, + ) { + private val hash = privateKey.contentHashCode() * 31 + pubKey.contentHashCode() + + override fun hashCode(): Int = hash + + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (other !is CacheKey) return false + return hash == other.hash && + privateKey.contentEquals(other.privateKey) && + pubKey.contentEquals(other.pubKey) + } } } diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip44Encryption/crypto/Hkdf.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip44Encryption/crypto/Hkdf.kt index 9ede033861..e117245b2f 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip44Encryption/crypto/Hkdf.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip44Encryption/crypto/Hkdf.kt @@ -21,6 +21,7 @@ package com.vitorpamplona.quartz.nip44Encryption.crypto import com.vitorpamplona.quartz.nip01Core.core.toHexKey +import com.vitorpamplona.quartz.utils.equalsConstantTime import com.vitorpamplona.quartz.utils.mac.MacInstance class Hkdf( @@ -194,7 +195,7 @@ class Hkdf( mac.update(ciphertext) val calculatedMac = mac.doFinal() - check(calculatedMac.contentEquals(ciphertextMac)) { + check(calculatedMac.equalsConstantTime(ciphertextMac)) { "Invalid Mac: Calculated ${calculatedMac.toHexKey()}, decoded: ${ciphertextMac.toHexKey()}" } } diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/utils/ConstantTime.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/utils/ConstantTime.kt new file mode 100644 index 0000000000..b9b0ccb757 --- /dev/null +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/utils/ConstantTime.kt @@ -0,0 +1,43 @@ +/* + * 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.quartz.utils + +/** + * Constant-time byte-array equality for MAC / authentication-tag comparison. + * + * Unlike [ByteArray.contentEquals], this does not short-circuit on the first + * differing byte, so its running time does not depend on how many leading bytes + * of a candidate MAC/tag are correct. Comparing MACs with an early-exit check + * exposes a timing side channel that can, in principle, let an attacker recover a + * valid tag byte-by-byte and forge messages (the classic Keyczar/XBox-360 class + * of bug). Use this for every secret-dependent equality check (HMAC tags, Poly1305 + * tags, reset tokens); ordinary non-secret comparisons can keep `contentEquals`. + * + * The length check leaks only the (public, fixed) tag length, not its contents. + */ +fun ByteArray.equalsConstantTime(other: ByteArray): Boolean { + if (this.size != other.size) return false + var diff = 0 + for (i in indices) { + diff = diff or (this[i].toInt() xor other[i].toInt()) + } + return diff == 0 +} diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip44Encryption/SharedKeyCacheTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip44Encryption/SharedKeyCacheTest.kt new file mode 100644 index 0000000000..e0e97e64e4 --- /dev/null +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip44Encryption/SharedKeyCacheTest.kt @@ -0,0 +1,86 @@ +/* + * 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.quartz.nip44Encryption + +import kotlin.test.Test +import kotlin.test.assertNull +import kotlin.test.assertTrue + +class SharedKeyCacheTest { + /** + * The cache maps (privateKey, pubKey) to a shared/conversation key. If two + * DISTINCT peer pubkeys can map to the same cache slot, the cache returns one + * peer's secret for a message intended for the other — a silent wrong-key + * encryption/decryption (confidentiality break). + * + * `pubA` and `pubB` below are engineered to collide under a 31-multiplier + * polynomial hash of their bytes: bumping byte[31] by +31 and byte[30] by -1 + * leaves `31 * acc + byte` unchanged (31^1 * (-1) + 31^0 * (+31) == 0). They + * are otherwise different keys, so a correct cache MUST treat them as distinct. + */ + @Test + fun collidingPubKeysMustNotShareCacheEntry() { + val cache = SharedKeyCache() + val priv = ByteArray(32) { 1 } + + val pubA = + ByteArray(32).also { + it[30] = 0x10 + it[31] = 0x00 + } + val pubB = + ByteArray(32).also { + it[30] = 0x0F + it[31] = 0x1F + } + + // Sanity: these are genuinely different peer public keys. + assertTrue(!pubA.contentEquals(pubB), "test setup: pubkeys must differ") + + val secretForA = ByteArray(32) { 0xAA.toByte() } + cache.add(priv, pubA, secretForA) + + // The secret cached for peer A must never be handed back for peer B. + assertNull( + cache.get(priv, pubB), + "cache returned peer A's shared secret when asked for peer B (hash collision leaks the wrong key)", + ) + + // The legitimate lookup must still work. + assertTrue(secretForA.contentEquals(cache.get(priv, pubA)!!)) + } + + @Test + fun distinctPeersGetDistinctEntries() { + val cache = SharedKeyCache() + val priv = ByteArray(32) { 7 } + val pub1 = ByteArray(32) { 2 } + val pub2 = ByteArray(32) { 3 } + val s1 = ByteArray(32) { 0x11 } + val s2 = ByteArray(32) { 0x22 } + + cache.add(priv, pub1, s1) + cache.add(priv, pub2, s2) + + assertTrue(s1.contentEquals(cache.get(priv, pub1)!!)) + assertTrue(s2.contentEquals(cache.get(priv, pub2)!!)) + } +}