From a30972762423ed5bb9aba401adaaf5fd77619e32 Mon Sep 17 00:00:00 2001 From: Sander Dijkhuis Date: Wed, 31 Jan 2024 16:58:34 +0100 Subject: [PATCH 1/9] test: never apply maximum nonce --- src/test/kotlin/NonceTest.kt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/test/kotlin/NonceTest.kt b/src/test/kotlin/NonceTest.kt index 6f584b1..b73b010 100644 --- a/src/test/kotlin/NonceTest.kt +++ b/src/test/kotlin/NonceTest.kt @@ -24,6 +24,11 @@ class NonceTest { assertNull(Nonce(ULong.MAX_VALUE).increment()) } + @Test + fun `never increment to 2^64-1 which is reserved for other use`() { + assertNull(Nonce(ULong.MAX_VALUE - 1uL).increment()) + } + @Test fun testEncodeLittleEndian() { assertEquals((0uL).toLong(), 0L) From f057fb078c5bdab4b863f1bc2b16cec132db4af0 Mon Sep 17 00:00:00 2001 From: Sander Dijkhuis Date: Wed, 31 Jan 2024 17:00:37 +0100 Subject: [PATCH 2/9] fix: never reach reserved maximum nonce --- src/main/kotlin/cryptography/Nonce.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/kotlin/cryptography/Nonce.kt b/src/main/kotlin/cryptography/Nonce.kt index f055292..f6b62fd 100644 --- a/src/main/kotlin/cryptography/Nonce.kt +++ b/src/main/kotlin/cryptography/Nonce.kt @@ -9,7 +9,7 @@ value class Nonce(val value: ULong) { val bytes: ByteArray get() = SIZE.byteArray { (value shr (it * Byte.SIZE_BITS)).toByte() } - fun increment(): Nonce? = if (value == ULong.MAX_VALUE) null else Nonce(value + 1uL) + fun increment(): Nonce? = if (value >= ULong.MAX_VALUE - 1uL) null else Nonce(value + 1uL) companion object { From 2f13fbfdf79e96fc10f2c679d11643994d58334e Mon Sep 17 00:00:00 2001 From: Sander Dijkhuis Date: Wed, 31 Jan 2024 17:36:43 +0100 Subject: [PATCH 3/9] test: throw upon reaching too high nonces Thanks to @breynders-cb and @tvandriessel-cb for spotting the risk. --- src/test/kotlin/CipherTest.kt | 37 +++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 src/test/kotlin/CipherTest.kt diff --git a/src/test/kotlin/CipherTest.kt b/src/test/kotlin/CipherTest.kt new file mode 100644 index 0000000..78b649d --- /dev/null +++ b/src/test/kotlin/CipherTest.kt @@ -0,0 +1,37 @@ +package nl.sanderdijkhuis.noise + +import nl.sanderdijkhuis.noise.cryptography.AssociatedData +import nl.sanderdijkhuis.noise.cryptography.CipherKey +import nl.sanderdijkhuis.noise.cryptography.Nonce +import nl.sanderdijkhuis.noise.cryptography.Plaintext +import nl.sanderdijkhuis.noise.data.Data +import org.junit.jupiter.api.assertThrows +import kotlin.test.Test + +@OptIn(ExperimentalStdlibApi::class) +class CipherTest { + private val data = AssociatedData(Data.empty) + private val plaintext = Plaintext(Data.empty) + + @Test + fun `throws upon reaching nonce maximum while encrypting`() { + val nonceTooHighToToUse = Nonce(ULong.MAX_VALUE - 1uL) // 2^64-2 + + assertThrows { cipher(nonceTooHighToToUse).encrypt(data, plaintext) } + } + + @Test + fun `throws upon reaching nonce maximum while decrypting`() { + val nonceTooHighToEncrypt = Nonce(ULong.MAX_VALUE - 2uL) // 2^64-3 + val (cipher, ciphertext) = cipher(nonceTooHighToEncrypt).encrypt(data, plaintext) + + assertThrows { cipher.decrypt(data, ciphertext) } + } + + private fun cipher(nonce: Nonce) = + Cipher( + JavaCryptography, + CipherKey(Data("76fef1ab184aa7539e3b62a43019ecafc621248b3ac2f5297dd5814e3bd560d3".hexToByteArray())), + nonce + ) +} From 2795da3d954d655e78dbd67661fbb3defa2f8ee1 Mon Sep 17 00:00:00 2001 From: Sander Dijkhuis Date: Wed, 31 Jan 2024 17:40:59 +0100 Subject: [PATCH 4/9] fix: throw upon reaching too high nonces --- src/main/kotlin/Cipher.kt | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/main/kotlin/Cipher.kt b/src/main/kotlin/Cipher.kt index 161a3d8..344d367 100644 --- a/src/main/kotlin/Cipher.kt +++ b/src/main/kotlin/Cipher.kt @@ -3,18 +3,28 @@ package nl.sanderdijkhuis.noise import nl.sanderdijkhuis.noise.cryptography.* import nl.sanderdijkhuis.noise.data.State -/** Encompasses all Noise protocol cipher state required to encrypt and decrypt data. */ +/** + * Encompasses all Noise protocol cipher state required to encrypt and decrypt data. + * + * Note that as per Noise revision 34 ยง 5.1, [[key]] may be uninitialized. In this case [[encrypt]] and [[decrypt]] + * are identity functions over the plaintext and ciphertext. + * + * Encryption and decryption throw if incrementing [[nonce]] results in its maximum value: it means too many messages + * have been exchanged. Too many is a lot indeed: 2^64-1. + */ data class Cipher(val cryptography: Cryptography, val key: CipherKey? = null, val nonce: Nonce = Nonce.zero) { fun encrypt(associatedData: AssociatedData, plaintext: Plaintext): State = key?.let { k -> - nonce.increment()?.let { - State(copy(nonce = it), cryptography.encrypt(k, nonce, associatedData, plaintext)) + nonce.increment().let { n -> + checkNotNull(n) { "Too many messages" } + State(copy(nonce = n), cryptography.encrypt(k, nonce, associatedData, plaintext)) } } ?: State(this, Ciphertext(plaintext.data)) fun decrypt(associatedData: AssociatedData, ciphertext: Ciphertext): State? = - nonce.increment()?.let { n -> + nonce.increment().let { n -> + checkNotNull(n) { "Too many messages" } key?.let { cryptography.decrypt(it, nonce, associatedData, ciphertext)?.let { p -> State(copy(nonce = n), p) } } ?: State(this, ciphertext.plaintext) From c8ee0d3b997bf18816f033a037e85a5680056dad Mon Sep 17 00:00:00 2001 From: Sander Dijkhuis Date: Wed, 31 Jan 2024 17:44:30 +0100 Subject: [PATCH 5/9] test: do not keep authentication failure silent when decrypting Thanks to @breynders-cb and @tvandriessel-cb for spotting the bug. --- src/test/kotlin/CipherTest.kt | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/test/kotlin/CipherTest.kt b/src/test/kotlin/CipherTest.kt index 78b649d..f33e9e3 100644 --- a/src/test/kotlin/CipherTest.kt +++ b/src/test/kotlin/CipherTest.kt @@ -7,10 +7,12 @@ import nl.sanderdijkhuis.noise.cryptography.Plaintext import nl.sanderdijkhuis.noise.data.Data import org.junit.jupiter.api.assertThrows import kotlin.test.Test +import kotlin.test.assertNull @OptIn(ExperimentalStdlibApi::class) class CipherTest { private val data = AssociatedData(Data.empty) + private val otherData = AssociatedData(Data("other".toByteArray())) private val plaintext = Plaintext(Data.empty) @Test @@ -28,6 +30,13 @@ class CipherTest { assertThrows { cipher.decrypt(data, ciphertext) } } + @Test + fun `signals an error to the caller upon authentication failure during decryption`() { + val (cipher, ciphertext) = cipher(Nonce.zero).encrypt(data, plaintext) + + assertNull(cipher.decrypt(otherData, ciphertext)) + } + private fun cipher(nonce: Nonce) = Cipher( JavaCryptography, From 460205fac97695a92e7837fe5efde422854e58fb Mon Sep 17 00:00:00 2001 From: Sander Dijkhuis Date: Wed, 31 Jan 2024 17:48:09 +0100 Subject: [PATCH 6/9] fix: silent authentication failure --- src/main/kotlin/Cipher.kt | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/kotlin/Cipher.kt b/src/main/kotlin/Cipher.kt index 344d367..b4bf519 100644 --- a/src/main/kotlin/Cipher.kt +++ b/src/main/kotlin/Cipher.kt @@ -25,8 +25,7 @@ data class Cipher(val cryptography: Cryptography, val key: CipherKey? = null, va fun decrypt(associatedData: AssociatedData, ciphertext: Ciphertext): State? = nonce.increment().let { n -> checkNotNull(n) { "Too many messages" } - key?.let { - cryptography.decrypt(it, nonce, associatedData, ciphertext)?.let { p -> State(copy(nonce = n), p) } - } ?: State(this, ciphertext.plaintext) + if (key == null) return State(this, ciphertext.plaintext) + cryptography.decrypt(key, nonce, associatedData, ciphertext)?.let { p -> State(copy(nonce = n), p) } } } From e8aee519e54fcbcf04a46201ba151a83bb86d8cd Mon Sep 17 00:00:00 2001 From: Sander Dijkhuis Date: Wed, 31 Jan 2024 17:52:47 +0100 Subject: [PATCH 7/9] build: update BouncyCastle version --- build.gradle.kts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 6997968..f6c21be 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -22,8 +22,8 @@ repositories { dependencies { testImplementation(kotlin("test")) testImplementation("org.jetbrains.kotlinx:kotlinx-serialization-json:1.6.2") - testImplementation("org.bouncycastle:bcprov-jdk15on:1.70") - testImplementation("org.bouncycastle:bcpkix-jdk15on:1.70") + testImplementation("org.bouncycastle:bcprov-jdk18on:1.77") + testImplementation("org.bouncycastle:bcpkix-jdk18on:1.77") } java { From de2056419b4804127ec0a8fc0f64b64b393634ad Mon Sep 17 00:00:00 2001 From: Sander Dijkhuis Date: Wed, 31 Jan 2024 17:53:23 +0100 Subject: [PATCH 8/9] build: bump to 1.0.1 --- build.gradle.kts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle.kts b/build.gradle.kts index f6c21be..39a91e9 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -13,7 +13,7 @@ plugins { } group = "nl.sanderdijkhuis" -version = "1.1.0-SNAPSHOT" +version = "1.0.1" repositories { mavenCentral() From 3b48677960b7b9c2d9b1dcf8457374294319bff6 Mon Sep 17 00:00:00 2001 From: Sander Dijkhuis Date: Wed, 31 Jan 2024 17:53:42 +0100 Subject: [PATCH 9/9] build: bump to 1.1.0-SNAPSHOT --- build.gradle.kts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle.kts b/build.gradle.kts index 39a91e9..f6c21be 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -13,7 +13,7 @@ plugins { } group = "nl.sanderdijkhuis" -version = "1.0.1" +version = "1.1.0-SNAPSHOT" repositories { mavenCentral()