From 286f8416e5bba068df467510d776917bea6d446d Mon Sep 17 00:00:00 2001 From: greenart7c3 Date: Mon, 10 Aug 2026 16:03:55 -0300 Subject: [PATCH] fix(L1): opt-in unlocked-device requirement for Keystore key (GHSA-8844-q5vh-9j8f) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AMBER_AES_KEY (AES-256-GCM, optional StrongBox) was built without setUnlockedDeviceRequired, so any code in Amber's process could decrypt every account key while the screen was locked — the PIN/biometric lock is UI-only. The advisory suggests an opt-in user-auth-bound key for at least interactive signing, acknowledging the trade-off with background NIP-46 signing. Add an opt-in toggle on the Security screen: when enabled, the Keystore key is generated with setUnlockedDeviceRequired(true) (API 28+), so the TEE refuses key use while the device is locked. This does NOT prompt for biometrics per operation — it only refuses use while locked, so foreground NIP-46 signing keeps working. The flag is set at key-generation time, so toggling requires key rotation: decrypt all stored secrets (per-account DataStore NOSTR_PRIVKEY/SEED_WORDS, app DataStore PIN, WebDAV password) with the old key, delete it, generate a new one with the new policy, and re-encrypt everything. The rotation holds SecureCryptoHelper's mutex for the entire operation and uses internal non-locking cipher methods + raw DataStore helpers to avoid mutex reentrancy (Kotlin's Mutex is not reentrant). The rotation is safe-by-design: all secrets are decrypted before the old key is deleted. --- .../nostrsigner/DataStoreAccess.kt | 35 +++++ .../nostrsigner/LocalPreferences.kt | 41 +++++- .../nostrsigner/SecureCryptoHelper.kt | 123 +++++++++++++++++- .../nostrsigner/models/AmberSettings.kt | 1 + .../nostrsigner/ui/SecurityScreen.kt | 39 ++++++ app/src/main/res/values/strings.xml | 2 + 6 files changed, 235 insertions(+), 6 deletions(-) diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/DataStoreAccess.kt b/app/src/main/java/com/greenart7c3/nostrsigner/DataStoreAccess.kt index 8dc04942..d6044d6c 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/DataStoreAccess.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/DataStoreAccess.kt @@ -59,6 +59,28 @@ object DataStoreAccess { return SecureCryptoHelper.decrypt(encrypted) } + /** + * Raw read: returns the still-encrypted value without decrypting. Used by + * [SecureCryptoHelper.rotateKey] to avoid mutex reentrancy (the public + * [getEncryptedKey] calls SecureCryptoHelper.decrypt which acquires the + * mutex that rotateKey already holds). + */ + suspend fun getEncryptedRaw(context: Context, npub: String, key: Preferences.Key): String? { + val prefs = getDataStore(context, npub).data.first() + if (prefs.asMap().keys.isEmpty()) return null + return prefs[key] + } + + /** + * Raw write: stores an already-encrypted value without encrypting. Used by + * [SecureCryptoHelper.rotateKey] to avoid mutex reentrancy. + */ + suspend fun saveEncryptedRaw(context: Context, npub: String, key: Preferences.Key, encryptedValue: String) { + getDataStore(context, npub).edit { prefs -> + prefs[key] = encryptedValue + } + } + suspend fun clearCacheForNpub(context: Context, npub: String) { getDataStore(context, npub).edit { prefs -> prefs.clear() @@ -80,4 +102,17 @@ object DataStoreAccess { val encrypted = prefs[PIN] ?: return null return SecureCryptoHelper.decrypt(encrypted) } + + /** Raw read for the PIN — see [getEncryptedRaw] for rationale. */ + suspend fun getPinRaw(context: Context): String? { + val prefs = getAppDataStore(context).data.first() + return prefs[PIN] + } + + /** Raw write for the PIN — see [saveEncryptedRaw] for rationale. */ + suspend fun savePinRaw(context: Context, encryptedValue: String) { + getAppDataStore(context).edit { prefs -> + prefs[PIN] = encryptedValue + } + } } diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/LocalPreferences.kt b/app/src/main/java/com/greenart7c3/nostrsigner/LocalPreferences.kt index 97d73df2..2e8c9d9f 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/LocalPreferences.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/LocalPreferences.kt @@ -78,6 +78,7 @@ private enum class SettingsKeys(val key: String) { RATE_LIMIT_WINDOW_SECONDS("rate_limit_window_seconds"), PROFILE_FETCH_INTERVAL("profile_fetch_interval"), TRUST_SCORE_ENABLED("trust_score_enabled"), + REQUIRE_UNLOCKED_DEVICE("require_unlocked_device"), } @Immutable @@ -158,6 +159,7 @@ object LocalPreferences { putInt(SettingsKeys.RATE_LIMIT_MAX_PER_WINDOW.key, settings.rateLimitMaxPerWindow) putInt(SettingsKeys.RATE_LIMIT_WINDOW_SECONDS.key, settings.rateLimitWindowSeconds) putBoolean(SettingsKeys.TRUST_SCORE_ENABLED.key, settings.trustScoreEnabled) + putBoolean(SettingsKeys.REQUIRE_UNLOCKED_DEVICE.key, settings.requireUnlockedDevice) } } } @@ -322,6 +324,7 @@ object LocalPreferences { ProfileFetchInterval.FIFTEEN_MINUTES }, trustScoreEnabled = getBoolean(SettingsKeys.TRUST_SCORE_ENABLED.key, true), + requireUnlockedDevice = getBoolean(SettingsKeys.REQUIRE_UNLOCKED_DEVICE.key, false), ) } } @@ -614,6 +617,23 @@ object LocalPreferences { } } + /** + * Toggles the opt-in "require unlocked device" key policy (GHSA-8844-q5vh-9j8f, L1). + * The Keystore flag is set at key-generation time, so changing the setting + * requires rotating the AMBER_AES_KEY: decrypt all stored secrets with the + * old key, delete it, generate a new one with/without + * setUnlockedDeviceRequired, and re-encrypt everything. + */ + suspend fun updateRequireUnlockedDevice(context: Context, enabled: Boolean) { + SecureCryptoHelper.rotateKey(context, requireUnlockedDevice = enabled) + sharedPrefs(context).edit { + apply { + putBoolean(SettingsKeys.REQUIRE_UNLOCKED_DEVICE.key, enabled) + } + } + Amber.instance.settings = loadSettingsFromEncryptedStorage() + } + fun updateUpdateCheckFrequency(context: Context, frequency: UpdateCheckFrequency) { sharedPrefs(context).edit { apply { @@ -659,7 +679,7 @@ object LocalPreferences { fun getWebDavFilename(context: Context): String = sharedPrefs(context).getString(SettingsKeys.WEBDAV_FILENAME.key, "amber_backup.txt") ?: "amber_backup.txt" suspend fun getWebDavPassword(context: Context): String { - val encrypted = sharedPrefs(context).getString(SettingsKeys.WEBDAV_PASSWORD_ENCRYPTED.key, "") ?: "" + val encrypted = getWebDavPasswordEncrypted(context) return if (encrypted.isBlank()) { "" } else { @@ -671,6 +691,25 @@ object LocalPreferences { } } + /** + * Returns the still-encrypted WebDAV password (raw SharedPreferences + * value). Used by [SecureCryptoHelper.rotateKey] to avoid mutex + * reentrancy. + */ + fun getWebDavPasswordEncrypted(context: Context): String = sharedPrefs(context).getString(SettingsKeys.WEBDAV_PASSWORD_ENCRYPTED.key, "") ?: "" + + /** + * Stores an already-encrypted WebDAV password. Used by + * [SecureCryptoHelper.rotateKey]. + */ + fun saveWebDavPasswordEncrypted(context: Context, encryptedPassword: String) { + sharedPrefs(context).edit { + apply { + putString(SettingsKeys.WEBDAV_PASSWORD_ENCRYPTED.key, encryptedPassword) + } + } + } + suspend fun saveWebDavSettings( context: Context, url: String, diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/SecureCryptoHelper.kt b/app/src/main/java/com/greenart7c3/nostrsigner/SecureCryptoHelper.kt index 8613fc24..38d1b78b 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/SecureCryptoHelper.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/SecureCryptoHelper.kt @@ -24,11 +24,11 @@ object SecureCryptoHelper { private val mutex = Mutex() suspend fun encrypt(plainText: String): String = mutex.withLock { - encryptBlocking(plainText) + encryptWithKey(getOrCreateSecretKey(), plainText) } suspend fun decrypt(encryptedText: String): String = mutex.withLock { - decryptBlocking(encryptedText) + decryptWithKey(getOrCreateSecretKey(), encryptedText) } /** @@ -41,7 +41,10 @@ object SecureCryptoHelper { * bridge. */ fun encryptBlocking(plainText: String): String { - val key = getOrCreateSecretKey() + return encryptWithKey(getOrCreateSecretKey(), plainText) + } + + private fun encryptWithKey(key: SecretKey, plainText: String): String { val cipher = Cipher.getInstance(TRANSFORMATION) cipher.init(Cipher.ENCRYPT_MODE, key) val iv = cipher.iv @@ -60,7 +63,10 @@ object SecureCryptoHelper { * rationale. */ fun decryptBlocking(encryptedText: String): String { - val key = getOrCreateSecretKey() + return decryptWithKey(getOrCreateSecretKey(), encryptedText) + } + + private fun decryptWithKey(key: SecretKey, encryptedText: String): String { val data = Base64.decode(encryptedText, Base64.NO_WRAP) val buffer = ByteBuffer.wrap(data) @@ -84,6 +90,12 @@ object SecureCryptoHelper { } } + return generateSecretKey(Amber.instance.settings.requireUnlockedDevice) + } + + private fun getKeyStore(): KeyStore = KeyStore.getInstance(ANDROID_KEYSTORE).apply { load(null) } + + private fun generateSecretKey(requireUnlockedDevice: Boolean): SecretKey { val keyGenerator = KeyGenerator.getInstance(KeyProperties.KEY_ALGORITHM_AES, ANDROID_KEYSTORE) val paramsBuilder = KeyGenParameterSpec.Builder( KEY_ALIAS, @@ -93,7 +105,15 @@ object SecureCryptoHelper { .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_NONE) .setKeySize(256) - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) { + // Defense-in-depth (GHSA-8844-q5vh-9j8f, L1): opt-in toggle — when + // enabled, require the device to be unlocked at the time the key + // material is used, so a process that runs while the screen is + // locked cannot decrypt stored account keys. Does not prompt for + // biometrics per operation (which would break background NIP-46 + // signing); it only refuses key use while the device is locked. + // Available from API 28 (KeyGenParameterSpec.Builder). + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P && requireUnlockedDevice) { + paramsBuilder.setUnlockedDeviceRequired(true) try { if (Amber.instance.hasStrongBox()) { paramsBuilder.setIsStrongBoxBacked(true) @@ -111,6 +131,99 @@ object SecureCryptoHelper { return keyGenerator.generateKey() } } + + /** + * Rotates the [KEY_ALIAS] Keystore key, re-encrypting every stored secret + * (per-account DataStore keys, app DataStore PIN, and WebDAV password). + * + * The Keystore `setUnlockedDeviceRequired` flag is set at key-generation + * time, so toggling the opt-in policy requires destroying and recreating + * the key. The rotation is safe-by-design: all secrets are decrypted + * with the old key BEFORE the old key is deleted, so a mid-rotation crash + * leaves the old key intact (the delete is the only destructive step and + * it runs after all decryptions succeed). + * + * Uses internal non-locking cipher methods ([encryptWithKey]/ + * [decryptWithKey]) and raw DataStore helpers to avoid mutex reentrancy + * (Kotlin's [Mutex] is not reentrant). + */ + suspend fun rotateKey(context: Context, requireUnlockedDevice: Boolean) = mutex.withLock { + val keyStore = getKeyStore() + if (!keyStore.containsAlias(KEY_ALIAS)) { + generateSecretKey(requireUnlockedDevice) + return@withLock + } + + // 1. Get the old key and decrypt all stored secrets (before deleting). + val oldEntry = keyStore.getEntry(KEY_ALIAS, null) as KeyStore.SecretKeyEntry + val oldKey = oldEntry.secretKey + + val accounts = LocalPreferences.allSavedAccounts(context) + val decryptedAccountKeys = mutableMapOf>() + + for (acc in accounts) { + val rawPrivKey = DataStoreAccess.getEncryptedRaw(context, acc.npub, DataStoreAccess.NOSTR_PRIVKEY) + val rawSeedWords = DataStoreAccess.getEncryptedRaw(context, acc.npub, DataStoreAccess.SEED_WORDS) + decryptedAccountKeys[acc.npub] = Pair( + rawPrivKey?.let { + try { + decryptWithKey(oldKey, it) + } catch (_: Exception) { + null + } + }, + rawSeedWords?.let { + try { + decryptWithKey(oldKey, it) + } catch (_: Exception) { + null + } + }, + ) + } + + val rawPin = DataStoreAccess.getPinRaw(context) + val decryptedPin = rawPin?.let { + try { + decryptWithKey(oldKey, it) + } catch (_: Exception) { + null + } + } + + val encryptedWebDavPassword = LocalPreferences.getWebDavPasswordEncrypted(context) + val decryptedWebDavPassword = if (encryptedWebDavPassword.isNotBlank()) { + try { + decryptWithKey(oldKey, encryptedWebDavPassword) + } catch (_: Exception) { + null + } + } else { + null + } + + // 2. Delete the old key and generate a new one with the new policy. + keyStore.deleteEntry(KEY_ALIAS) + val newKey = generateSecretKey(requireUnlockedDevice) + + // 3. Re-encrypt and store all secrets with the new key. + for ((npub, keys) in decryptedAccountKeys) { + keys.first?.let { + DataStoreAccess.saveEncryptedRaw(context, npub, DataStoreAccess.NOSTR_PRIVKEY, encryptWithKey(newKey, it)) + } + keys.second?.let { + DataStoreAccess.saveEncryptedRaw(context, npub, DataStoreAccess.SEED_WORDS, encryptWithKey(newKey, it)) + } + } + decryptedPin?.let { + DataStoreAccess.savePinRaw(context, encryptWithKey(newKey, it)) + } + decryptedWebDavPassword?.let { + LocalPreferences.saveWebDavPasswordEncrypted(context, encryptWithKey(newKey, it)) + } + + AmberLog.d("SecureCryptoHelper", "Key rotation complete (requireUnlockedDevice=$requireUnlockedDevice)") + } } fun Context.hasStrongBox(): Boolean { diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/models/AmberSettings.kt b/app/src/main/java/com/greenart7c3/nostrsigner/models/AmberSettings.kt index ae856f5b..d0ed5978 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/models/AmberSettings.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/models/AmberSettings.kt @@ -40,6 +40,7 @@ data class AmberSettings( val rateLimitWindowSeconds: Int = 30, val profileFetchInterval: ProfileFetchInterval = ProfileFetchInterval.FIFTEEN_MINUTES, val trustScoreEnabled: Boolean = true, + val requireUnlockedDevice: Boolean = false, ) { val useProxy: Boolean get() = torMode != TorMode.DISABLED } diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/ui/SecurityScreen.kt b/app/src/main/java/com/greenart7c3/nostrsigner/ui/SecurityScreen.kt index b135a125..bd5b6ed1 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/ui/SecurityScreen.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/ui/SecurityScreen.kt @@ -51,6 +51,7 @@ fun SecurityScreen( var enableBiometrics by remember { mutableStateOf(Amber.instance.settings.useAuth) } val setupPin by remember { mutableStateOf(Amber.instance.settings.usePin) } var privacyMode by remember { mutableStateOf(Amber.instance.settings.privacyMode) } + var requireUnlockedDevice by remember { mutableStateOf(Amber.instance.settings.requireUnlockedDevice) } var biometricsIndex by remember { mutableIntStateOf(Amber.instance.settings.biometricsTimeType.screenCode) } @@ -121,6 +122,44 @@ fun SecurityScreen( ) } + // GHSA-8844-q5vh-9j8f, L1: opt-in toggle + Row( + horizontalArrangement = Arrangement.SpaceBetween, + verticalAlignment = Alignment.CenterVertically, + modifier = Modifier + .fillMaxWidth() + .padding(horizontal = 8.dp, vertical = 4.dp) + .clickable { + val newValue = !requireUnlockedDevice + requireUnlockedDevice = newValue + // Use the application-scoped IOScope, not the + // composition scope: key rotation must complete + // even if the user leaves the screen/app, or + // stored secrets could be left inaccessible. + Amber.instance.applicationIOScope.launch(Dispatchers.IO) { + LocalPreferences.updateRequireUnlockedDevice(context, newValue) + } + }, + ) { + Column(modifier = Modifier.weight(1f)) { + Text(text = stringResource(R.string.require_unlocked_device)) + Text( + text = stringResource(R.string.require_unlocked_device_description), + style = MaterialTheme.typography.bodySmall, + color = Color.Gray, + ) + } + Switch( + checked = requireUnlockedDevice, + onCheckedChange = { enabled -> + requireUnlockedDevice = enabled + Amber.instance.applicationIOScope.launch(Dispatchers.IO) { + LocalPreferences.updateRequireUnlockedDevice(context, enabled) + } + }, + ) + } + Row( horizontalArrangement = Arrangement.SpaceBetween, verticalAlignment = Alignment.CenterVertically, diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 60120709..e10b9d2d 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -555,6 +555,8 @@ Name No relays added wss://… + Require unlocked device for key access + When enabled, the Keystore key used to decrypt your stored account keys cannot be used while the device is locked. This prevents any code running in Amber\'s process from decrypting your keys while the screen is locked. Note: this disables background NIP-46 signing while the device is locked. Toggling this requires re-encrypting all stored keys. "Name can't be empty " Your nsecbunker is ready! Use this url in your app: