From d424abc6d31f98f9d2414d411b73c84b91272b53 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Thu, 2 Jul 2026 12:21:09 -0500 Subject: [PATCH] refactor(security): de-duplicate AES-GCM Keystore plumbing and unify authenticator policy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract the AES-256-GCM Android Keystore plumbing that `KeystoreCrypto` and `DatabaseKeyCipher` copy-pasted (~60 lines) into a shared alias-parameterized base, `AesGcmKeystoreCipher`: the encrypt/decrypt bodies, existing-key lookup / get-or-create under a lock, key deletion, and the 5 identical GCM constants now live in ONE place. Each cipher keeps only its delta — the `KeyGenParameterSpec` (via `keySpecBuilder()`) and, for the auth-bound key, the invalidation handling. Preserve — deliberately — the two ciphers' different missing-key-on-decrypt behavior via a `generateKeyOnDecrypt` policy parameter, documented on the base: - master key (`KeystoreCrypto`, true): auto-generates on a missing alias, correct for a first-run key with nothing sealed yet. - auth-bound cache key (`DatabaseKeyCipher`, false): fails fast, because a missing auth-bound key means it was INVALIDATED and silently regenerating it would re-arm the lock against a cache that can no longer be decrypted. Also map the opaque `AEADBadTagException` (thrown when the master path generates a fresh key then can't decrypt old data) to a clear `GeneralSecurityException`, while leaving `KeyPermanentlyInvalidatedException` to propagate unwrapped. Unify the accepted-authenticator policy behind one source of truth, `AuthenticatorPolicy.ACCEPTED`, mapped into each API's vocabulary (`AppLockManager.AUTHENTICATORS` for BiometricManager / BiometricPrompt, `DatabaseKeyCipher.keySpec` for KeyProperties / KeyGenParameterSpec) so the two can no longer drift — a drift that yields a prompt that succeeds but a key that throws `UserNotAuthenticatedException` at use. Add JVM tests for the shared base (both `generateKeyOnDecrypt` modes + the AES-GCM error mapping) and for the authenticator mapping. #100's seal-exchange and policy-table safety net stays green. Closes #102 Co-Authored-By: Claude Fable 5 --- .../data/security/AesGcmKeystoreCipher.kt | 143 ++++++++++++++++++ .../libremail/data/security/AppLockManager.kt | 11 +- .../data/security/AuthenticatorPolicy.kt | 48 ++++++ .../data/security/DatabaseKeyCipher.kt | 83 +++------- .../libremail/data/security/KeystoreCrypto.kt | 64 ++------ .../data/security/AesGcmKeystoreCipherTest.kt | 121 +++++++++++++++ .../data/security/AuthenticatorPolicyTest.kt | 60 ++++++++ 7 files changed, 407 insertions(+), 123 deletions(-) create mode 100644 app/src/main/kotlin/org/libremail/data/security/AesGcmKeystoreCipher.kt create mode 100644 app/src/main/kotlin/org/libremail/data/security/AuthenticatorPolicy.kt create mode 100644 app/src/test/kotlin/org/libremail/data/security/AesGcmKeystoreCipherTest.kt create mode 100644 app/src/test/kotlin/org/libremail/data/security/AuthenticatorPolicyTest.kt diff --git a/app/src/main/kotlin/org/libremail/data/security/AesGcmKeystoreCipher.kt b/app/src/main/kotlin/org/libremail/data/security/AesGcmKeystoreCipher.kt new file mode 100644 index 0000000..be37397 --- /dev/null +++ b/app/src/main/kotlin/org/libremail/data/security/AesGcmKeystoreCipher.kt @@ -0,0 +1,143 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.security + +import android.security.keystore.KeyGenParameterSpec +import android.security.keystore.KeyProperties +import android.util.Base64 +import java.security.GeneralSecurityException +import java.security.KeyStore +import javax.crypto.AEADBadTagException +import javax.crypto.Cipher +import javax.crypto.KeyGenerator +import javax.crypto.SecretKey +import javax.crypto.spec.GCMParameterSpec + +/** + * Shared AES-256-GCM plumbing backed by a non-exportable key in the Android Keystore, parameterized + * by key [alias] and a missing-key-on-decrypt policy ([generateKeyOnDecrypt]). + * + * Encapsulates everything the two Keystore users have in common — the GCM transform and its + * constants, the alias-scoped key lookup/creation guarded by a lock, `Base64(iv || ciphertext)` + * framing, and key deletion — so a change to the crypto (StrongBox opt-in, IV handling, error + * mapping) is made in ONE place instead of being copy-pasted and drifting. Subclasses supply only + * their delta: the [keySpec] that mints the key (extend [keySpecBuilder] for the common + * AES-256-GCM base) and, via [generateKeyOnDecrypt], how a decrypt behaves when the alias is absent. + * + * That missing-key policy is **deliberately different** between the two users and MUST stay + * different: + * - The non-auth master key ([KeystoreCrypto], `generateKeyOnDecrypt = true`) silently generates a + * key on a missing alias — correct for a first-run master key that has nothing sealed yet. + * - The auth-bound cache key ([DatabaseKeyCipher], `generateKeyOnDecrypt = false`) fails fast, + * because a MISSING auth-bound key means it was INVALIDATED (biometric re-enrollment or lock + * removal). Silently regenerating it would defeat the security model — quietly re-arming a lock + * against a cache that can no longer be decrypted — so the absence must surface, not self-heal. + * + * The key operations touch the Android Keystore, so the two concrete ciphers are exercised on-device; + * the alias/policy wiring above is unit-tested against this base with fake keys (see the test seams + * [existingKey], [getOrCreateKey], and [decryptWithKey]). + */ +abstract class AesGcmKeystoreCipher(private val alias: String, private val generateKeyOnDecrypt: Boolean) { + + private val keyLock = Any() + + /** Returns `Base64(iv || ciphertext)`, generating the key under [alias] on first use. */ + open fun encrypt(plaintext: String): String = doEncrypt(getOrCreateKey(), plaintext) + + /** + * Decrypts a blob produced by [encrypt]. Missing-key handling follows [generateKeyOnDecrypt] + * (see the class KDoc). An AES-GCM tag mismatch — the ciphertext no longer matches the key, e.g. + * the alias was cleared and regenerated underneath a still-persisted blob — is surfaced as a + * clear [GeneralSecurityException] instead of an opaque [AEADBadTagException]. A key-invalidation + * failure ([android.security.keystore.KeyPermanentlyInvalidatedException]) is thrown from + * `Cipher.init` and propagates unwrapped, so callers can still classify it. + */ + fun decrypt(encoded: String): String { + val key = decryptionKey() + return try { + decryptWithKey(key, encoded) + } catch (e: AEADBadTagException) { + throw GeneralSecurityException( + "AES-GCM authentication failed for Keystore alias '$alias': the ciphertext no longer " + + "matches the current key (the key was cleared and regenerated, or the data is corrupt)", + e, + ) + } + } + + /** Resolves the key a decrypt should use, applying the [generateKeyOnDecrypt] policy. */ + private fun decryptionKey(): SecretKey = + if (generateKeyOnDecrypt) getOrCreateKey() else existingKey() ?: onMissingDecryptionKey() + + /** + * Invoked when a decrypt finds no key and the policy forbids minting one. The default fails with + * a generic message; auth-bound subclasses override it to explain that the absence means the key + * was invalidated. + */ + protected open fun onMissingDecryptionKey(): Nothing = error("Keystore key for alias '$alias' is missing") + + private fun doEncrypt(key: SecretKey, plaintext: String): String { + val cipher = initEncryptCipher(key) + val iv = cipher.iv + val ciphertext = cipher.doFinal(plaintext.toByteArray(Charsets.UTF_8)) + return Base64.encodeToString(iv + ciphertext, Base64.NO_WRAP) + } + + /** Test seam separating the Keystore-backed cipher call from the decrypt policy/error mapping. */ + protected open fun decryptWithKey(key: SecretKey, encoded: String): String { + val bytes = Base64.decode(encoded, Base64.NO_WRAP) + val iv = bytes.copyOfRange(0, IV_LENGTH) + val ciphertext = bytes.copyOfRange(IV_LENGTH, bytes.size) + val cipher = Cipher.getInstance(TRANSFORMATION) + cipher.init(Cipher.DECRYPT_MODE, key, GCMParameterSpec(TAG_BITS, iv)) + return String(cipher.doFinal(ciphertext), Charsets.UTF_8) + } + + /** + * Initializes an encrypt-mode [Cipher] with [key]. Shared by [doEncrypt] and reused by auth-bound + * subclasses to probe whether a key is still usable (a bare `init` throws if it was invalidated). + */ + protected fun initEncryptCipher(key: SecretKey): Cipher = + Cipher.getInstance(TRANSFORMATION).apply { init(Cipher.ENCRYPT_MODE, key) } + + /** True when a key exists under [alias]. */ + protected fun keyExists(): Boolean = existingKey() != null + + /** Deletes the key so a fresh one is generated on the next [encrypt]. */ + protected fun deleteKeyEntry(): Unit = synchronized(keyLock) { + KeyStore.getInstance(ANDROID_KEYSTORE).apply { load(null) }.deleteEntry(alias) + } + + protected open fun existingKey(): SecretKey? = synchronized(keyLock) { + val keyStore = KeyStore.getInstance(ANDROID_KEYSTORE).apply { load(null) } + (keyStore.getEntry(alias, null) as? KeyStore.SecretKeyEntry)?.secretKey + } + + // Synchronized so two concurrent first-run encrypts can't both generate a key under the same + // alias — the second would overwrite the first, leaving the first secret undecryptable. + protected open fun getOrCreateKey(): SecretKey = synchronized(keyLock) { + existingKey()?.let { return it } + val generator = KeyGenerator.getInstance(KeyProperties.KEY_ALGORITHM_AES, ANDROID_KEYSTORE) + generator.init(keySpec()) + generator.generateKey() + } + + /** The alias-bound [KeyGenParameterSpec] for this key; subclasses extend [keySpecBuilder]. */ + protected abstract fun keySpec(): KeyGenParameterSpec + + /** The common AES-256-GCM builder (encrypt + decrypt, GCM, no padding, 256-bit) to extend. */ + protected fun keySpecBuilder(): KeyGenParameterSpec.Builder = KeyGenParameterSpec.Builder( + alias, + KeyProperties.PURPOSE_ENCRYPT or KeyProperties.PURPOSE_DECRYPT, + ) + .setBlockModes(KeyProperties.BLOCK_MODE_GCM) + .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_NONE) + .setKeySize(AES_KEY_SIZE_BITS) + + private companion object { + const val ANDROID_KEYSTORE = "AndroidKeyStore" + const val TRANSFORMATION = "AES/GCM/NoPadding" + const val IV_LENGTH = 12 + const val TAG_BITS = 128 + const val AES_KEY_SIZE_BITS = 256 + } +} diff --git a/app/src/main/kotlin/org/libremail/data/security/AppLockManager.kt b/app/src/main/kotlin/org/libremail/data/security/AppLockManager.kt index c20a8ae..bdf8278 100644 --- a/app/src/main/kotlin/org/libremail/data/security/AppLockManager.kt +++ b/app/src/main/kotlin/org/libremail/data/security/AppLockManager.kt @@ -3,8 +3,6 @@ package org.libremail.data.security import android.app.KeyguardManager import android.content.Context -import androidx.biometric.BiometricManager.Authenticators.BIOMETRIC_STRONG -import androidx.biometric.BiometricManager.Authenticators.DEVICE_CREDENTIAL import dagger.hilt.android.qualifiers.ApplicationContext import javax.inject.Inject import javax.inject.Singleton @@ -19,8 +17,13 @@ interface AppLockManager { fun isDeviceSecure(): Boolean companion object { - /** Accept a strong biometric OR the device credential (PIN/pattern/password) as fallback. */ - const val AUTHENTICATORS: Int = BIOMETRIC_STRONG or DEVICE_CREDENTIAL + /** + * `BiometricPrompt` authenticators (a strong biometric OR the device credential). Derived + * from the single [AuthenticatorPolicy] source of truth so it can never drift from the + * auth-bound Keystore key's authenticators in [DatabaseKeyCipher] — a drift that would let a + * prompt succeed against an authenticator the key rejects with `UserNotAuthenticatedException`. + */ + val AUTHENTICATORS: Int = AuthenticatorPolicy.biometricPromptAuthenticators } } diff --git a/app/src/main/kotlin/org/libremail/data/security/AuthenticatorPolicy.kt b/app/src/main/kotlin/org/libremail/data/security/AuthenticatorPolicy.kt new file mode 100644 index 0000000..a535914 --- /dev/null +++ b/app/src/main/kotlin/org/libremail/data/security/AuthenticatorPolicy.kt @@ -0,0 +1,48 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.security + +import android.security.keystore.KeyProperties +import androidx.biometric.BiometricManager + +/** A user-presence proof the app accepts to unlock the app lock and authorize the auth-bound key. */ +enum class AppAuthenticator { STRONG_BIOMETRIC, DEVICE_CREDENTIAL } + +/** + * THE single source of truth for which authenticators gate the app lock. The same [ACCEPTED] set is + * mapped into each Android API's own flag vocabulary — androidx [BiometricManager] (for + * `BiometricPrompt.setAllowedAuthenticators`, via [AppLockManager.AUTHENTICATORS]) and platform + * [KeyProperties] (for `KeyGenParameterSpec.setUserAuthenticationParameters`, via + * [DatabaseKeyCipher]) — because the two APIs use *different* bit constants for the same concept + * (`BIOMETRIC_STRONG` is `0xF` here, `AUTH_BIOMETRIC_STRONG` is `1` there). + * + * Deriving both flag sets from one [ACCEPTED] set — with an exhaustive `when` that the compiler forces + * to cover every [AppAuthenticator] — makes it impossible to loosen or tighten one without the other. + * That drift is otherwise invisible until a device hits it: the `BiometricPrompt` would accept an + * authenticator the key does not, so the prompt succeeds but the key then throws + * `UserNotAuthenticatedException` at use. + */ +object AuthenticatorPolicy { + + /** Accept a strong biometric OR the device credential (PIN / pattern / password) as fallback. */ + val ACCEPTED: Set = + setOf(AppAuthenticator.STRONG_BIOMETRIC, AppAuthenticator.DEVICE_CREDENTIAL) + + /** [ACCEPTED] as androidx `BiometricManager.Authenticators` flags for a `BiometricPrompt`. */ + val biometricPromptAuthenticators: Int = ACCEPTED.toFlags { + when (it) { + AppAuthenticator.STRONG_BIOMETRIC -> BiometricManager.Authenticators.BIOMETRIC_STRONG + AppAuthenticator.DEVICE_CREDENTIAL -> BiometricManager.Authenticators.DEVICE_CREDENTIAL + } + } + + /** [ACCEPTED] as platform `KeyProperties.AUTH_*` flags for a `KeyGenParameterSpec`. */ + val keyGenAuthenticators: Int = ACCEPTED.toFlags { + when (it) { + AppAuthenticator.STRONG_BIOMETRIC -> KeyProperties.AUTH_BIOMETRIC_STRONG + AppAuthenticator.DEVICE_CREDENTIAL -> KeyProperties.AUTH_DEVICE_CREDENTIAL + } + } + + private inline fun Set.toFlags(flagOf: (AppAuthenticator) -> Int): Int = + fold(0) { acc, authenticator -> acc or flagOf(authenticator) } +} diff --git a/app/src/main/kotlin/org/libremail/data/security/DatabaseKeyCipher.kt b/app/src/main/kotlin/org/libremail/data/security/DatabaseKeyCipher.kt index f0aa1ec..41fe348 100644 --- a/app/src/main/kotlin/org/libremail/data/security/DatabaseKeyCipher.kt +++ b/app/src/main/kotlin/org/libremail/data/security/DatabaseKeyCipher.kt @@ -4,15 +4,8 @@ package org.libremail.data.security import android.os.Build import android.security.keystore.KeyGenParameterSpec import android.security.keystore.KeyPermanentlyInvalidatedException -import android.security.keystore.KeyProperties import android.security.keystore.UserNotAuthenticatedException -import android.util.Base64 import android.util.Log -import java.security.KeyStore -import javax.crypto.Cipher -import javax.crypto.KeyGenerator -import javax.crypto.SecretKey -import javax.crypto.spec.GCMParameterSpec import javax.inject.Inject import javax.inject.Singleton @@ -30,47 +23,29 @@ import javax.inject.Singleton * lock — permanently invalidates the key; [decrypt] then throws [KeyPermanentlyInvalidatedException], * which the caller treats as "cache unrecoverable -> clear + re-sync". * + * Reuses the shared [AesGcmKeystoreCipher] plumbing; its delta is the auth-bound [keySpec] and the + * invalidation handling below. It is created with `generateKeyOnDecrypt = false` on purpose: a + * missing auth-bound key means it was INVALIDATED, so [decrypt] fails fast (via [onMissingDecryptionKey]) + * rather than silently minting a new key and re-arming the lock against a cache it can never decrypt. + * * DEVICE-ONLY: auth-bound Keystore keys and BiometricPrompt cannot be exercised in JVM unit tests; * this class is covered by on-device instrumentation / manual validation only. */ @Singleton -class DatabaseKeyCipher @Inject constructor() { - - private val keyLock = Any() +class DatabaseKeyCipher @Inject constructor() : + AesGcmKeystoreCipher(alias = KEY_ALIAS, generateKeyOnDecrypt = false) { /** * Returns Base64(iv || ciphertext). Requires a valid auth window (call right after unlock). * Self-heals a stale, permanently-invalidated key by replacing it and retrying once, so sealing a * fresh passphrase after a re-enrollment doesn't fail. */ - fun encrypt(plaintext: String): String = try { - doEncrypt(getOrCreateKey(), plaintext) + override fun encrypt(plaintext: String): String = try { + super.encrypt(plaintext) } catch (e: KeyPermanentlyInvalidatedException) { Log.d(TAG, "replacing invalidated auth-bound key before sealing", e) deleteKey() - doEncrypt(getOrCreateKey(), plaintext) - } - - private fun doEncrypt(key: SecretKey, plaintext: String): String { - val cipher = Cipher.getInstance(TRANSFORMATION) - cipher.init(Cipher.ENCRYPT_MODE, key) - val iv = cipher.iv - val ciphertext = cipher.doFinal(plaintext.toByteArray(Charsets.UTF_8)) - return Base64.encodeToString(iv + ciphertext, Base64.NO_WRAP) - } - - /** - * Decrypts a blob produced by [encrypt]. Requires a valid auth window. Throws - * [KeyPermanentlyInvalidatedException] if the key was invalidated by re-enrollment / lock removal. - */ - fun decrypt(encoded: String): String { - val key = existingKey() ?: error("auth-bound database key is missing") - val bytes = Base64.decode(encoded, Base64.NO_WRAP) - val iv = bytes.copyOfRange(0, IV_LENGTH) - val ciphertext = bytes.copyOfRange(IV_LENGTH, bytes.size) - val cipher = Cipher.getInstance(TRANSFORMATION) - cipher.init(Cipher.DECRYPT_MODE, key, GCMParameterSpec(TAG_BITS, iv)) - return String(cipher.doFinal(ciphertext), Charsets.UTF_8) + super.encrypt(plaintext) } /** @@ -82,7 +57,7 @@ class DatabaseKeyCipher @Inject constructor() { fun isInvalidated(): Boolean { val key = existingKey() ?: return false return try { - Cipher.getInstance(TRANSFORMATION).init(Cipher.ENCRYPT_MODE, key) + initEncryptCipher(key) false } catch (e: KeyPermanentlyInvalidatedException) { Log.d(TAG, "auth-bound database key invalidated", e) @@ -99,39 +74,22 @@ class DatabaseKeyCipher @Inject constructor() { } } - fun hasKey(): Boolean = existingKey() != null + fun hasKey(): Boolean = keyExists() /** Deletes the auth-bound key so a fresh one is generated on the next [encrypt]. */ - fun deleteKey(): Unit = synchronized(keyLock) { - KeyStore.getInstance(ANDROID_KEYSTORE).apply { load(null) }.deleteEntry(KEY_ALIAS) - } + fun deleteKey(): Unit = deleteKeyEntry() - private fun existingKey(): SecretKey? = synchronized(keyLock) { - val keyStore = KeyStore.getInstance(ANDROID_KEYSTORE).apply { load(null) } - (keyStore.getEntry(KEY_ALIAS, null) as? KeyStore.SecretKeyEntry)?.secretKey - } + /** A missing auth-bound key means it was invalidated; surface that instead of regenerating. */ + override fun onMissingDecryptionKey(): Nothing = error("auth-bound database key is missing") - private fun getOrCreateKey(): SecretKey = synchronized(keyLock) { - existingKey()?.let { return it } - val generator = KeyGenerator.getInstance(KeyProperties.KEY_ALGORITHM_AES, ANDROID_KEYSTORE) - generator.init(buildSpec()) - generator.generateKey() - } - - private fun buildSpec(): KeyGenParameterSpec { - val builder = KeyGenParameterSpec.Builder( - KEY_ALIAS, - KeyProperties.PURPOSE_ENCRYPT or KeyProperties.PURPOSE_DECRYPT, - ) - .setBlockModes(KeyProperties.BLOCK_MODE_GCM) - .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_NONE) - .setKeySize(AES_KEY_SIZE_BITS) + override fun keySpec(): KeyGenParameterSpec { + val builder = keySpecBuilder() .setUserAuthenticationRequired(true) .setInvalidatedByBiometricEnrollment(true) if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) { builder.setUserAuthenticationParameters( AUTH_VALIDITY_SECONDS, - KeyProperties.AUTH_BIOMETRIC_STRONG or KeyProperties.AUTH_DEVICE_CREDENTIAL, + AuthenticatorPolicy.keyGenAuthenticators, ) } else { @Suppress("DEPRECATION") @@ -141,12 +99,7 @@ class DatabaseKeyCipher @Inject constructor() { } private companion object { - const val ANDROID_KEYSTORE = "AndroidKeyStore" const val KEY_ALIAS = "libremail.dbkey.auth" - const val TRANSFORMATION = "AES/GCM/NoPadding" - const val IV_LENGTH = 12 - const val TAG_BITS = 128 - const val AES_KEY_SIZE_BITS = 256 const val AUTH_VALIDITY_SECONDS = 15 const val TAG = "LibreMailDbKeyAuth" } diff --git a/app/src/main/kotlin/org/libremail/data/security/KeystoreCrypto.kt b/app/src/main/kotlin/org/libremail/data/security/KeystoreCrypto.kt index 5d8cd05..8844203 100644 --- a/app/src/main/kotlin/org/libremail/data/security/KeystoreCrypto.kt +++ b/app/src/main/kotlin/org/libremail/data/security/KeystoreCrypto.kt @@ -2,69 +2,25 @@ package org.libremail.data.security import android.security.keystore.KeyGenParameterSpec -import android.security.keystore.KeyProperties -import android.util.Base64 -import java.security.KeyStore -import javax.crypto.Cipher -import javax.crypto.KeyGenerator -import javax.crypto.SecretKey -import javax.crypto.spec.GCMParameterSpec import javax.inject.Inject import javax.inject.Singleton /** - * AES-256-GCM encryption backed by a non-exportable key in the Android Keystore. Secrets - * (OAuth tokens, IMAP passwords) are encrypted at rest so they never touch disk in plaintext. + * AES-256-GCM encryption backed by a non-exportable key in the Android Keystore. Secrets (OAuth + * tokens, IMAP passwords) are encrypted at rest so they never touch disk in plaintext. + * + * The non-auth-bound **master** key: usable in the background without a user-presence prompt, so + * credential access keeps working while the app is locked. As the master key it is minted lazily on a + * missing-alias decrypt (`generateKeyOnDecrypt = true`) — correct for a first run that has nothing + * sealed yet. Contrast the auth-bound [DatabaseKeyCipher], whose absent key means invalidation and so + * fails fast; the shared [AesGcmKeystoreCipher] documents why the two must differ. */ @Singleton -class KeystoreCrypto @Inject constructor() { +class KeystoreCrypto @Inject constructor() : AesGcmKeystoreCipher(alias = KEY_ALIAS, generateKeyOnDecrypt = true) { - private val keyLock = Any() - - // Synchronized so two concurrent first-run encrypts can't both generate a key under the same - // alias — the second would overwrite the first, leaving the first secret undecryptable. - private fun secretKey(): SecretKey = synchronized(keyLock) { - val keyStore = KeyStore.getInstance(ANDROID_KEYSTORE).apply { load(null) } - (keyStore.getEntry(KEY_ALIAS, null) as? KeyStore.SecretKeyEntry)?.let { return it.secretKey } - - val generator = KeyGenerator.getInstance(KeyProperties.KEY_ALGORITHM_AES, ANDROID_KEYSTORE) - generator.init( - KeyGenParameterSpec.Builder( - KEY_ALIAS, - KeyProperties.PURPOSE_ENCRYPT or KeyProperties.PURPOSE_DECRYPT, - ) - .setBlockModes(KeyProperties.BLOCK_MODE_GCM) - .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_NONE) - .setKeySize(AES_KEY_SIZE_BITS) - .build(), - ) - generator.generateKey() - } - - /** Returns Base64(iv || ciphertext). */ - fun encrypt(plaintext: String): String { - val cipher = Cipher.getInstance(TRANSFORMATION) - cipher.init(Cipher.ENCRYPT_MODE, secretKey()) - val iv = cipher.iv - val ciphertext = cipher.doFinal(plaintext.toByteArray(Charsets.UTF_8)) - return Base64.encodeToString(iv + ciphertext, Base64.NO_WRAP) - } - - fun decrypt(encoded: String): String { - val bytes = Base64.decode(encoded, Base64.NO_WRAP) - val iv = bytes.copyOfRange(0, IV_LENGTH) - val ciphertext = bytes.copyOfRange(IV_LENGTH, bytes.size) - val cipher = Cipher.getInstance(TRANSFORMATION) - cipher.init(Cipher.DECRYPT_MODE, secretKey(), GCMParameterSpec(TAG_BITS, iv)) - return String(cipher.doFinal(ciphertext), Charsets.UTF_8) - } + override fun keySpec(): KeyGenParameterSpec = keySpecBuilder().build() private companion object { - const val ANDROID_KEYSTORE = "AndroidKeyStore" const val KEY_ALIAS = "libremail.master.key" - const val TRANSFORMATION = "AES/GCM/NoPadding" - const val IV_LENGTH = 12 - const val TAG_BITS = 128 - const val AES_KEY_SIZE_BITS = 256 } } diff --git a/app/src/test/kotlin/org/libremail/data/security/AesGcmKeystoreCipherTest.kt b/app/src/test/kotlin/org/libremail/data/security/AesGcmKeystoreCipherTest.kt new file mode 100644 index 0000000..b6438e8 --- /dev/null +++ b/app/src/test/kotlin/org/libremail/data/security/AesGcmKeystoreCipherTest.kt @@ -0,0 +1,121 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.security + +import android.security.keystore.KeyGenParameterSpec +import org.junit.Test +import java.security.GeneralSecurityException +import javax.crypto.AEADBadTagException +import javax.crypto.SecretKey +import javax.crypto.spec.SecretKeySpec +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertSame +import kotlin.test.assertTrue + +/** + * JVM coverage for the shared [AesGcmKeystoreCipher] wiring — specifically the deliberately different + * missing-key-on-decrypt policy the two production ciphers depend on, plus the AES-GCM error mapping. + * The Keystore-backed operations (real key generation and the GCM cipher) are device-only, so they + * are replaced here through the [existingKey], [getOrCreateKey], and [decryptWithKey] seams; what is + * pinned is the base's control flow: which key a decrypt resolves under each `generateKeyOnDecrypt` + * mode, and how a tag mismatch is surfaced. + */ +class AesGcmKeystoreCipherTest { + + @Test + fun `generateKeyOnDecrypt true mints a key for a missing alias (master-key behavior)`() { + val cipher = FakeCipher(generateKeyOnDecrypt = true, storedKey = null) + + assertEquals("plain:blob", cipher.decrypt("blob")) + assertEquals(1, cipher.generatedKeys, "a missing master alias is generated on decrypt") + } + + @Test + fun `generateKeyOnDecrypt true reuses an existing key without regenerating`() { + val cipher = FakeCipher(generateKeyOnDecrypt = true, storedKey = newAesKey()) + + assertEquals("plain:blob", cipher.decrypt("blob")) + assertEquals(0, cipher.generatedKeys) + } + + @Test + fun `generateKeyOnDecrypt false fails fast for a missing alias (auth-bound behavior)`() { + val cipher = FakeCipher(generateKeyOnDecrypt = false, storedKey = null) + + val error = assertFailsWith { cipher.decrypt("blob") } + assertTrue(error.message!!.contains("test.alias")) + assertEquals(0, cipher.generatedKeys, "an absent auth-bound key must NOT be silently regenerated") + assertEquals(0, cipher.decryptCalls, "decrypt short-circuits before touching the cipher") + } + + @Test + fun `generateKeyOnDecrypt false decrypts with the existing key`() { + val cipher = FakeCipher(generateKeyOnDecrypt = false, storedKey = newAesKey()) + + assertEquals("plain:blob", cipher.decrypt("blob")) + assertEquals(0, cipher.generatedKeys) + } + + @Test + fun `an AES-GCM tag mismatch is remapped to a clear GeneralSecurityException`() { + val badTag = AEADBadTagException("tag mismatch") + val cipher = FakeCipher( + generateKeyOnDecrypt = true, + storedKey = newAesKey(), + onDecrypt = { _, _ -> throw badTag }, + ) + + val error = assertFailsWith { cipher.decrypt("blob") } + assertSame(badTag, error.cause) + assertTrue(error.message!!.contains("test.alias")) + } + + @Test + fun `a non-AEAD failure propagates unwrapped so key invalidation still surfaces`() { + // Only AEADBadTagException is remapped; every other cipher failure — including the + // KeyPermanentlyInvalidatedException a real init throws on an invalidated key — must propagate + // unchanged so callers can classify it. + val boom = IllegalArgumentException("boom") + val cipher = FakeCipher( + generateKeyOnDecrypt = false, + storedKey = newAesKey(), + onDecrypt = { _, _ -> throw boom }, + ) + + assertSame(boom, assertFailsWith { cipher.decrypt("blob") }) + } + + /** + * A JVM-only [AesGcmKeystoreCipher] whose Keystore seams are replaced by in-memory fakes so the + * base's key-resolution policy and error mapping run without a device. [storedKey] models the key + * present under the alias (null = absent); [onDecrypt] models the GCM cipher operation. + */ + private class FakeCipher( + generateKeyOnDecrypt: Boolean, + private val storedKey: SecretKey?, + private val onDecrypt: (SecretKey, String) -> String = { _, encoded -> "plain:$encoded" }, + ) : AesGcmKeystoreCipher(alias = "test.alias", generateKeyOnDecrypt = generateKeyOnDecrypt) { + + var generatedKeys = 0 + private set + var decryptCalls = 0 + private set + + override fun existingKey(): SecretKey? = storedKey + + override fun getOrCreateKey(): SecretKey = existingKey() ?: newAesKey().also { generatedKeys++ } + + override fun decryptWithKey(key: SecretKey, encoded: String): String { + decryptCalls++ + return onDecrypt(key, encoded) + } + + override fun keySpec(): KeyGenParameterSpec = error("keySpec is not exercised in the JVM base test") + } + + private companion object { + const val KEY_BYTES = 32 + + fun newAesKey(): SecretKey = SecretKeySpec(ByteArray(KEY_BYTES) { it.toByte() }, "AES") + } +} diff --git a/app/src/test/kotlin/org/libremail/data/security/AuthenticatorPolicyTest.kt b/app/src/test/kotlin/org/libremail/data/security/AuthenticatorPolicyTest.kt new file mode 100644 index 0000000..5babb5b --- /dev/null +++ b/app/src/test/kotlin/org/libremail/data/security/AuthenticatorPolicyTest.kt @@ -0,0 +1,60 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.security + +import android.security.keystore.KeyProperties +import androidx.biometric.BiometricManager +import org.junit.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotEquals + +/** + * Pins the single-source-of-truth authenticator mapping. [AuthenticatorPolicy.ACCEPTED] is the one + * definition; the two derived flag sets translate it into each Android API's own vocabulary. If a + * future change loosens or tightens one, both must move together — these assertions catch the drift + * that is otherwise only observable on a device (a BiometricPrompt that succeeds but a Keystore key + * that then throws UserNotAuthenticatedException at use). + * + * The referenced SDK constants are Java compile-time constants, so they inline into this test on the + * plain JVM — no Android runtime is needed to compare the values. + */ +class AuthenticatorPolicyTest { + + @Test + fun `accepted set is a strong biometric or the device credential`() { + assertEquals( + setOf(AppAuthenticator.STRONG_BIOMETRIC, AppAuthenticator.DEVICE_CREDENTIAL), + AuthenticatorPolicy.ACCEPTED, + ) + } + + @Test + fun `maps to the BiometricManager vocabulary for the BiometricPrompt`() { + assertEquals( + BiometricManager.Authenticators.BIOMETRIC_STRONG or BiometricManager.Authenticators.DEVICE_CREDENTIAL, + AuthenticatorPolicy.biometricPromptAuthenticators, + ) + } + + @Test + fun `maps to the KeyProperties vocabulary for the KeyGenParameterSpec`() { + assertEquals( + KeyProperties.AUTH_BIOMETRIC_STRONG or KeyProperties.AUTH_DEVICE_CREDENTIAL, + AuthenticatorPolicy.keyGenAuthenticators, + ) + } + + @Test + fun `AppLockManager AUTHENTICATORS is the biometric-prompt mapping, not an independent copy`() { + assertEquals(AuthenticatorPolicy.biometricPromptAuthenticators, AppLockManager.AUTHENTICATORS) + } + + @Test + fun `the two API vocabularies are genuinely different bit sets`() { + // Why a single shared Int would be a bug: the same concept has different flag values in each + // API, so the policy has to be mapped, not copied. + assertNotEquals( + AuthenticatorPolicy.biometricPromptAuthenticators, + AuthenticatorPolicy.keyGenAuthenticators, + ) + } +}