diff --git a/commons/build.gradle.kts b/commons/build.gradle.kts index 4d6a78e368..86c5c10c33 100644 --- a/commons/build.gradle.kts +++ b/commons/build.gradle.kts @@ -167,8 +167,8 @@ kotlin { // Compose UI artifacts before the :commonsUI split. implementation(libs.androidx.core.ktx) - // Secure key storage via Android Keystore - implementation(libs.androidx.security.crypto.ktx) + // Secure key storage talks to the AndroidKeyStore directly through + // SecretEncryption; androidx.security.crypto is gone from this module. } } diff --git a/commons/src/androidMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt b/commons/src/androidMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt index dbb4efb401..f3b08ae257 100644 --- a/commons/src/androidMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt +++ b/commons/src/androidMain/kotlin/com/vitorpamplona/amethyst/commons/keystorage/SecureKeyStorage.kt @@ -21,42 +21,48 @@ package com.vitorpamplona.amethyst.commons.keystorage import android.content.Context -import androidx.core.content.edit -import androidx.security.crypto.EncryptedSharedPreferences -import androidx.security.crypto.MasterKey +import androidx.datastore.preferences.core.PreferenceDataStoreFactory +import androidx.datastore.preferences.core.stringPreferencesKey +import com.vitorpamplona.amethyst.commons.model.preferences.EncryptedDataStore +import com.vitorpamplona.amethyst.commons.model.preferences.SecretEncryption +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.withContext +import kotlinx.coroutines.SupervisorJob +import okio.Path.Companion.toOkioPath +import java.io.File /** - * Android implementation of SecureKeyStorage using EncryptedSharedPreferences - * backed by Android Keystore (AES-256-GCM, hardware-backed when available). + * Android implementation of [SecureKeyStorage]: an encrypted DataStore whose + * values are sealed with a key held in the AndroidKeyStore. * - * ## Security Features + * ## Why not EncryptedSharedPreferences * - * - **Hardware Security:** Uses Android Keystore (hardware-backed on supported devices with StrongBox) - * - **Encryption:** AES-256-GCM for both keys and values - * - **Key Derivation:** AES-256-SIV for preference keys, AES-256-GCM for values - * - **Application Context:** Uses applicationContext to prevent memory leaks - * - **Auto-backup Disabled:** EncryptedSharedPreferences automatically excluded from cloud backups + * This used to be `androidx.security.crypto`, which Google deprecated with no + * drop-in successor. [SecretEncryption] talks to the AndroidKeyStore directly — + * AES-256-GCM, StrongBox-backed where the device offers it — so the key still + * never enters app memory, and the library goes away. * - * **Note:** While the encryption keys are protected by hardware security modules (when available), - * the decrypted private keys returned by [getPrivateKey] are still subject to the String memory - * limitation described in [SecureKeyStorage]. + * Nothing is migrated from the old `amethyst_secure_keys` file because nothing + * ever wrote to it: this class is used by the desktop app, and the Android app + * has its own key storage in LocalPreferences. Were that to change, a migration + * would have to come first. + * + * ## Security note + * + * Only values are encrypted; the key names are not. That reveals which npubs + * this installation holds keys for, but not the keys themselves — the same + * trade-off the rest of the encrypted stores make. + * + * The String memory limitation described on [SecureKeyStorage] still applies: + * a decrypted private key cannot be zeroed from a JVM String. */ actual class SecureKeyStorage private actual constructor() { actual companion object { - private const val PREFS_NAME = "amethyst_secure_keys" + private const val STORE_FILE = "datastore/secure_keys.preferences_pb" private const val KEY_PREFIX = "privkey_" private lateinit var appContext: Context - /** - * Creates a SecureKeyStorage instance for Android. - * - * @param context Android Context (will use applicationContext to avoid leaks) - * @return SecureKeyStorage instance - * @throws IllegalArgumentException if context is null or not a valid Context - */ actual fun create(context: Any?): SecureKeyStorage { require(context is Context) { "Android requires a valid Context" } appContext = context.applicationContext @@ -64,85 +70,61 @@ actual class SecureKeyStorage private actual constructor() { } } - // androidx.security.crypto is deprecated with no drop-in successor; migrating the - // on-disk key store is a separate, security-sensitive effort. - @Suppress("DEPRECATION") - private val masterKey: MasterKey by lazy { - MasterKey - .Builder(appContext, MasterKey.DEFAULT_MASTER_KEY_ALIAS) - .setKeyScheme(MasterKey.KeyScheme.AES256_GCM) - .build() - } + private val scope = CoroutineScope(Dispatchers.IO + SupervisorJob()) - @Suppress("DEPRECATION") - private val encryptedPrefs by lazy { - EncryptedSharedPreferences.create( - appContext, - PREFS_NAME, - masterKey, - EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV, - EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM, + private val store by lazy { + EncryptedDataStore( + PreferenceDataStoreFactory.createWithPath( + scope = scope, + produceFile = { File(appContext.filesDir, STORE_FILE).toOkioPath() }, + ), + SecretEncryption(), + scope = scope, ) } + private fun keyFor(npub: String) = stringPreferencesKey(KEY_PREFIX + npub) + actual suspend fun savePrivateKey( npub: String, privKeyHex: String, ) { - withContext(Dispatchers.IO) { - try { - encryptedPrefs.edit { putString(KEY_PREFIX + npub, privKeyHex) } - } catch (e: Exception) { - throw SecureStorageException("Failed to save private key", e) - } + try { + store.save(keyFor(npub), privKeyHex) + } catch (e: Exception) { + throw SecureStorageException("Failed to save private key", e) } } actual suspend fun getPrivateKey(npub: String): String? = - withContext(Dispatchers.IO) { - try { - encryptedPrefs.getString(KEY_PREFIX + npub, null) - } catch (e: Exception) { - throw SecureStorageException("Failed to retrieve private key", e) - } + try { + store.get(keyFor(npub)) + } catch (e: Exception) { + throw SecureStorageException("Failed to retrieve private key", e) } /** - * Android backend: EncryptedSharedPreferences.contains + getString has no - * ambiguous-error state comparable to macOS Keychain user-cancel/deny, so - * "key not present" and "key present" are the only two null outcomes. - * Any thrown exception is a genuine failure and propagates. + * Unlike [getPrivateKey], this reads through [EncryptedDataStore.getOrThrow] + * so a store that cannot be read raises instead of reporting the key as + * absent. That distinction is the whole point of this method: callers use + * it to decide whether a key needs creating, and treating a transient read + * failure as "no key here" would overwrite a live one. */ actual suspend fun getPrivateKeyOrThrow(npub: String): String? = - withContext(Dispatchers.IO) { - try { - val key = KEY_PREFIX + npub - if (!encryptedPrefs.contains(key)) { - null - } else { - encryptedPrefs.getString(key, null) - } - } catch (e: Exception) { - throw SecureStorageException("Failed to retrieve private key", e) - } + try { + store.getOrThrow(keyFor(npub)) + } catch (e: Exception) { + throw SecureStorageException("Failed to retrieve private key", e) } actual suspend fun deletePrivateKey(npub: String): Boolean = - withContext(Dispatchers.IO) { - try { - val key = KEY_PREFIX + npub - val existed = encryptedPrefs.contains(key) - if (existed) { - encryptedPrefs.edit { remove(key) } - } - existed - } catch (e: Exception) { - throw SecureStorageException("Failed to delete private key", e) - } + try { + val existed = store.get(keyFor(npub)) != null + if (existed) store.remove(keyFor(npub)) + existed + } catch (e: Exception) { + throw SecureStorageException("Failed to delete private key", e) } - actual suspend fun hasPrivateKey(npub: String): Boolean = - withContext(Dispatchers.IO) { - encryptedPrefs.contains(KEY_PREFIX + npub) - } + actual suspend fun hasPrivateKey(npub: String): Boolean = getPrivateKey(npub) != null } diff --git a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/model/preferences/EncryptedDataStore.kt b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/model/preferences/EncryptedDataStore.kt index 710102f222..01b26082bb 100644 --- a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/model/preferences/EncryptedDataStore.kt +++ b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/model/preferences/EncryptedDataStore.kt @@ -26,6 +26,7 @@ import androidx.datastore.preferences.core.edit import androidx.datastore.preferences.core.emptyPreferences import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.catch +import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.firstOrNull import kotlinx.coroutines.flow.map import okio.IOException @@ -58,6 +59,13 @@ class EncryptedDataStore( store.edit { prefs -> prefs[key] = encrypt(value) } } + /** + * The value, or null when the key is absent **or unreadable**. + * + * A read error is reported as absence, which is what most callers want. + * Anything that must not mistake a failure for an empty store — a probe + * deciding whether to create a replacement key, say — needs [getOrThrow]. + */ suspend fun get(key: Preferences.Key): String? = store.data .catch { e -> @@ -66,6 +74,16 @@ class EncryptedDataStore( ?.get(key) ?.let { decrypt(it) } + /** + * The value, or null only when the key is genuinely absent. + * + * Unlike [get], a failure to read propagates rather than being flattened + * into null. The difference matters wherever null means "nothing was ever + * stored" and the caller acts on that — overwriting a key that is present + * but temporarily unreadable is not recoverable. + */ + suspend fun getOrThrow(key: Preferences.Key): String? = store.data.first()[key]?.let { decrypt(it) } + fun getProperty( key: Preferences.Key, parser: (String) -> T, diff --git a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/model/preferences/EncryptedDataStoreTest.kt b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/model/preferences/EncryptedDataStoreTest.kt index 68eddfbf94..05f9e231e1 100644 --- a/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/model/preferences/EncryptedDataStoreTest.kt +++ b/commons/src/jvmTest/kotlin/com/vitorpamplona/amethyst/commons/model/preferences/EncryptedDataStoreTest.kt @@ -25,10 +25,12 @@ import androidx.datastore.preferences.core.stringPreferencesKey import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancel import kotlinx.coroutines.test.runTest import okio.Path.Companion.toOkioPath import org.junit.Assert.assertEquals import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue import org.junit.Rule import org.junit.Test import org.junit.rules.TemporaryFolder @@ -131,4 +133,47 @@ class EncryptedDataStoreTest { assertNull(subject.get(key)) } + + /** + * [EncryptedDataStore.get] flattens a read failure into null; + * [EncryptedDataStore.getOrThrow] does not. + * + * The difference guards a live key: a probe that decides whether to create + * one must not read "absent" from a store it merely failed to open, or it + * overwrites what is already there. + */ + @Test + fun getSwallowsAReadFailureButGetOrThrowDoesNot() = + runTest { + val scope = CoroutineScope(Dispatchers.IO + SupervisorJob()) + val n = seq++ + val dataFile = File(folder.root, "corrupt_$n.preferences_pb") + val keyFile = File(folder.root, "corrupt_$n.key") + val subject = + EncryptedDataStore( + PreferenceDataStoreFactory.createWithPath(scope = scope, produceFile = { dataFile.toOkioPath() }), + SecretEncryption(keyFile), + scope = scope, + ) + subject.save(key, "a real value") + scope.cancel() + + // Truncate the store so opening it fails rather than reading empty. + dataFile.writeBytes(byteArrayOf(0x01, 0x02, 0x03)) + + val readScope = CoroutineScope(Dispatchers.IO + SupervisorJob()) + val reopened = + EncryptedDataStore( + PreferenceDataStoreFactory.createWithPath(scope = readScope, produceFile = { dataFile.toOkioPath() }), + SecretEncryption(keyFile), + scope = readScope, + ) + + assertNull("get() reports the unreadable store as absent", reopened.get(key)) + assertTrue( + "getOrThrow() must not call it absent", + runCatching { reopened.getOrThrow(key) }.isFailure, + ) + readScope.cancel() + } }