chore: merge nonce and decryption fixes (#47)

Author: sander-cb

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 {

src/main/kotlin/Cipher.kt

@@ -3,20 +3,29 @@ 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<Cipher, Ciphertext> =
         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<Cipher, Plaintext>? =
-        nonce.increment()?.let { n ->
-            key?.let {
-                cryptography.decrypt(it, nonce, associatedData, ciphertext)?.let { p -> State(copy(nonce = n), p) }
-            } ?: State(this, ciphertext.plaintext)
+        nonce.increment().let { n ->
+            checkNotNull(n) { "Too many messages" }
+            if (key == null) return State(this, ciphertext.plaintext)
+            cryptography.decrypt(key, nonce, associatedData, ciphertext)?.let { p -> State(copy(nonce = n), p) }
         }
 }

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 {
 

src/test/kotlin/CipherTest.kt

@@ -0,0 +1,46 @@
+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
+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
+    fun `throws upon reaching nonce maximum while encrypting`() {
+        val nonceTooHighToToUse = Nonce(ULong.MAX_VALUE - 1uL) // 2^64-2
+
+        assertThrows<IllegalStateException> { 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<IllegalStateException> { 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,
+            CipherKey(Data("76fef1ab184aa7539e3b62a43019ecafc621248b3ac2f5297dd5814e3bd560d3".hexToByteArray())),
+            nonce
+        )
+}

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)