mirror of
https://github.com/greenart7c3/Amber.git
synced 2026-10-05 19:08:23 +00:00
fix(L1): opt-in unlocked-device requirement for Keystore key (GHSA-8844-q5vh-9j8f)
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.
This commit is contained in:
@@ -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>): 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<String>, 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
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<String, Pair<String?, String?>>()
|
||||
|
||||
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 {
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -555,6 +555,8 @@
|
||||
<string name="name">Name</string>
|
||||
<string name="no_relays_added">No relays added</string>
|
||||
<string name="wss">wss://…</string>
|
||||
<string name="require_unlocked_device">Require unlocked device for key access</string>
|
||||
<string name="require_unlocked_device_description">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.</string>
|
||||
<string name="name_cannot_be_empty">"Name can't be empty "</string>
|
||||
<string name="your_nsec_bunker_has_been_created">Your nsecbunker is ready!</string>
|
||||
<string name="use_this_url_in_your_app">Use this url in your app:</string>
|
||||
|
||||
Reference in New Issue
Block a user