diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/SecureCryptoHelper.kt b/app/src/main/java/com/greenart7c3/nostrsigner/SecureCryptoHelper.kt index c9bde93c..3f7788c0 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/SecureCryptoHelper.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/SecureCryptoHelper.kt @@ -7,7 +7,11 @@ import android.security.keystore.KeyGenParameterSpec import android.security.keystore.KeyProperties import android.util.Base64 import java.nio.ByteBuffer +import java.security.InvalidKeyException import java.security.KeyStore +import java.security.KeyStoreException +import java.security.ProviderException +import java.security.UnrecoverableKeyException import javax.crypto.Cipher import javax.crypto.KeyGenerator import javax.crypto.SecretKey @@ -24,12 +28,26 @@ object SecureCryptoHelper { private const val TAG_SIZE = 128 // bits private val mutex = Mutex() + /** + * Cached handle to the [KEY_ALIAS] key. An AndroidKeyStore [SecretKey] is + * a lightweight reference (the material never leaves the TEE/StrongBox), + * but fetching it costs a fresh KeyStore load plus `containsAlias`/ + * `getEntry` binder round-trips to the keystore daemon — milliseconds + * each, sequential. Re-fetching it per cipher operation made every + * envelope-encrypted DAO read pay a keystore lookup per field + * (see ApplicationEntityCrypto.kt), which dominated load times. + * Invalidated by [rotateKey] and by [withFreshKeyRetry] when a + * cached handle is found unusable. + */ + @Volatile + private var cachedSecretKey: SecretKey? = null + suspend fun encrypt(plainText: String): String = mutex.withLock { - encryptWithKey(getOrCreateSecretKey(), plainText) + withFreshKeyRetry { key -> encryptWithKey(key, plainText) } } suspend fun decrypt(encryptedText: String): String = mutex.withLock { - decryptWithKey(getOrCreateSecretKey(), encryptedText) + withFreshKeyRetry { key -> decryptWithKey(key, encryptedText) } } /** @@ -41,7 +59,7 @@ object SecureCryptoHelper { * here so blocking callers do not need a [kotlinx.coroutines.runBlocking] * bridge. */ - fun encryptBlocking(plainText: String): String = encryptWithKey(getOrCreateSecretKey(), plainText) + fun encryptBlocking(plainText: String): String = withFreshKeyRetry { key -> encryptWithKey(key, plainText) } private fun encryptWithKey(key: SecretKey, plainText: String): String { val cipher = Cipher.getInstance(TRANSFORMATION) @@ -61,7 +79,7 @@ object SecureCryptoHelper { * Non-suspending equivalent of [decrypt]. See [encryptBlocking] for the * rationale. */ - fun decryptBlocking(encryptedText: String): String = decryptWithKey(getOrCreateSecretKey(), encryptedText) + fun decryptBlocking(encryptedText: String): String = withFreshKeyRetry { key -> decryptWithKey(key, encryptedText) } private fun decryptWithKey(key: SecretKey, encryptedText: String): String { val data = Base64.decode(encryptedText, Base64.NO_WRAP) @@ -78,7 +96,43 @@ object SecureCryptoHelper { return String(plainBytes, Charsets.UTF_8) } + /** + * Runs [block] with the current key handle, retrying exactly once with a + * re-fetched handle when the cached one has gone stale (the entry was + * deleted or invalidated underneath us — e.g. a concurrent [rotateKey], + * or the opt-in unlocked-device policy refusing key use while locked). + * Data errors — chiefly GCM's AEADBadTagException for corrupt or + * wrong-key ciphertext — are not stale-handle errors and propagate to the + * caller on the first attempt, preserving the decryptField failure + * contract in ApplicationEntityCrypto.kt. + */ + private inline fun withFreshKeyRetry(block: (SecretKey) -> T): T { + try { + return block(getOrCreateSecretKey()) + } catch (e: InvalidKeyException) { + AmberLog.w(TAG, "Cached Keystore key unusable, re-fetching once", e) + } catch (e: KeyStoreException) { + AmberLog.w(TAG, "Keystore failure with cached key, re-fetching once", e) + } catch (e: UnrecoverableKeyException) { + AmberLog.w(TAG, "Cached Keystore key unrecoverable, re-fetching once", e) + } catch (e: ProviderException) { + AmberLog.w(TAG, "Keystore provider failure, re-fetching key once", e) + } + cachedSecretKey = null + return block(getOrCreateSecretKey()) + } + private fun getOrCreateSecretKey(): SecretKey { + cachedSecretKey?.let { return it } + return synchronized(this) { + cachedSecretKey?.let { return it } + val key = loadOrCreateSecretKey() + cachedSecretKey = key + key + } + } + + private fun loadOrCreateSecretKey(): SecretKey { val keyStore = KeyStore.getInstance(ANDROID_KEYSTORE).apply { load(null) } if (keyStore.containsAlias(KEY_ALIAS)) { val entry = keyStore.getEntry(KEY_ALIAS, null) as? KeyStore.SecretKeyEntry @@ -144,12 +198,15 @@ object SecureCryptoHelper { * * Uses internal non-locking cipher methods ([encryptWithKey]/ * [decryptWithKey]) and raw DataStore/DAO helpers to avoid mutex - * reentrancy (Kotlin's [Mutex] is not reentrant). + * reentrancy (Kotlin's [Mutex] is not reentrant). The cached-key path + * ([getOrCreateSecretKey]/[withFreshKeyRetry]) is deliberately not used + * here because this function manages the cache itself and must address + * the old/new keys explicitly. */ suspend fun rotateKey(context: Context, requireUnlockedDevice: Boolean) = mutex.withLock { val keyStore = getKeyStore() if (!keyStore.containsAlias(KEY_ALIAS)) { - generateSecretKey(requireUnlockedDevice) + cachedSecretKey = generateSecretKey(requireUnlockedDevice) return@withLock } @@ -224,8 +281,11 @@ object SecureCryptoHelper { } // 2. Delete the old key and generate a new one with the new policy. + // Update the handle cache in the same critical section so concurrent + // blocking callers never observe the deleted old key. keyStore.deleteEntry(KEY_ALIAS) val newKey = generateSecretKey(requireUnlockedDevice) + cachedSecretKey = newKey // 3. Re-encrypt and store all secrets with the new key. for ((npub, keys) in decryptedAccountKeys) { diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/database/ApplicationDao.kt b/app/src/main/java/com/greenart7c3/nostrsigner/database/ApplicationDao.kt index b3bb2eac..da41d27a 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/database/ApplicationDao.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/database/ApplicationDao.kt @@ -54,14 +54,20 @@ interface ApplicationDao { suspend fun getAllNotConnected(): List = getAllNotConnectedRaw().map { it.decryptFromStorage() } - @Query("SELECT a.* FROM application a WHERE a.pubKey = :pubKey ORDER BY a.lastUsed DESC") - fun getAllPagingRaw(pubKey: String): PagingSource - - fun getAllPaging(pubKey: String): PagingSource = DecryptingPagingSource(getAllPagingRaw(pubKey)) - @Query("SELECT DISTINCT relays FROM application") fun getAllRelayLists(): List + /** + * Paged Applications-list projection. Selects only the rendered columns + * ([ApplicationListItem]) and deliberately NOT the envelope-encrypted + * `secret`/`localKey` columns: each Keystore decrypt is a binder + TEE + * round trip, and the list screen renders neither field, so a `SELECT *` + * here made the screen pay 2 × rows cipher operations per page (Paging's + * initial load is 3 × pageSize rows) for values it never reads. + */ + @Query("SELECT `key`, name, relays, icon, lastUsed FROM application WHERE pubKey = :pubKey ORDER BY lastUsed DESC") + fun getApplicationListItemsPaging(pubKey: String): PagingSource + @Query("SELECT name FROM application WHERE `key` = :key LIMIT 1") suspend fun getAppName(key: String): String? diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/database/ApplicationEntity.kt b/app/src/main/java/com/greenart7c3/nostrsigner/database/ApplicationEntity.kt index 225473f8..0e79efb9 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/database/ApplicationEntity.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/database/ApplicationEntity.kt @@ -114,3 +114,18 @@ data class ApplicationKeyName( val key: String, val name: String, ) + +/** + * Row projection for the Applications list screen ([ApplicationDao.getApplicationListItemsPaging]): + * exactly the columns that screen renders. Deliberately excludes the + * envelope-encrypted `secret`/`localKey` columns — every Keystore cipher + * operation is a binder + TEE round trip, so a `SELECT *` paged load made + * the screen pay 2 × rows decrypts per page for fields it never displays. + */ +data class ApplicationListItem( + val key: String, + val name: String, + val relays: List, + val icon: String, + val lastUsed: Long, +) diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/database/ApplicationEntityCrypto.kt b/app/src/main/java/com/greenart7c3/nostrsigner/database/ApplicationEntityCrypto.kt index 98251a73..6d6b3a20 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/database/ApplicationEntityCrypto.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/database/ApplicationEntityCrypto.kt @@ -1,7 +1,5 @@ package com.greenart7c3.nostrsigner.database -import androidx.paging.PagingSource -import androidx.paging.PagingState import com.greenart7c3.nostrsigner.Amber import com.greenart7c3.nostrsigner.AmberLog import com.greenart7c3.nostrsigner.SecureCryptoHelper @@ -65,28 +63,3 @@ fun ApplicationEntity.decryptFromStorage(): ApplicationEntity = copy( /** Decrypts the embedded [ApplicationWithPermissions.application] after a Room read. */ fun ApplicationWithPermissions.decryptFromStorage(): ApplicationWithPermissions = copy(application = application.decryptFromStorage()) - -/** - * Wraps a raw (encrypted-column) [PagingSource] and decrypts every - * [ApplicationEntity] emitted by a successful [LoadResult.Page]. Error and - * Invalid results are passed through unchanged. Used by - * [ApplicationDao.getAllPaging] so the `ApplicationsScreen` Pager sees - * plaintext entities exactly like the non-paging DAO reads. - */ -internal class DecryptingPagingSource( - private val delegate: PagingSource, -) : PagingSource() { - override fun getRefreshKey(state: PagingState): Int? = delegate.getRefreshKey(state) - - override suspend fun load(params: PagingSource.LoadParams): PagingSource.LoadResult = when (val result = delegate.load(params)) { - is PagingSource.LoadResult.Page -> PagingSource.LoadResult.Page( - data = result.data.map { it.decryptFromStorage() }, - prevKey = result.prevKey, - nextKey = result.nextKey, - itemsBefore = result.itemsBefore, - itemsAfter = result.itemsAfter, - ) - is PagingSource.LoadResult.Error -> result - is PagingSource.LoadResult.Invalid -> result - } -} diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/database/CachingApplicationDao.kt b/app/src/main/java/com/greenart7c3/nostrsigner/database/CachingApplicationDao.kt index f5eb8c0e..2f7516cb 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/database/CachingApplicationDao.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/database/CachingApplicationDao.kt @@ -182,7 +182,7 @@ class CachingApplicationDao( override suspend fun getAllNotConnected(): List = delegate.getAllNotConnected() - override fun getAllPaging(pubKey: String): PagingSource = delegate.getAllPaging(pubKey) + override fun getApplicationListItemsPaging(pubKey: String): PagingSource = delegate.getApplicationListItemsPaging(pubKey) override fun getAllRelayLists(): List = delegate.getAllRelayLists() @@ -245,8 +245,6 @@ class CachingApplicationDao( override suspend fun getAllNotConnectedRaw(): List = delegate.getAllNotConnectedRaw() - override fun getAllPagingRaw(pubKey: String): PagingSource = delegate.getAllPagingRaw(pubKey) - override suspend fun getByKeyRaw(key: String): ApplicationWithPermissions? = delegate.getByKeyRaw(key) override fun getByKeySyncRaw(key: String): ApplicationWithPermissions? = delegate.getByKeySyncRaw(key) diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/ui/ApplicationsScreen.kt b/app/src/main/java/com/greenart7c3/nostrsigner/ui/ApplicationsScreen.kt index a812f253..7c44cff3 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/ui/ApplicationsScreen.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/ui/ApplicationsScreen.kt @@ -35,7 +35,6 @@ import androidx.compose.ui.text.withLink import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.sp import androidx.lifecycle.compose.collectAsStateWithLifecycle -import androidx.navigation.NavController import androidx.paging.Pager import androidx.paging.PagingConfig import androidx.paging.compose.collectAsLazyPagingItems @@ -59,16 +58,24 @@ import kotlinx.coroutines.launch fun ApplicationsScreen( modifier: Modifier, account: Account, - navController: NavController, + navControllerWrapper: NavHostControllerWrapper, ) { + // remember: without it every recomposition (killSwitch / backup-warning + // state flips) builds a new Pager + cold flow and re-runs the initial + // load from scratch. Keyed on the account so switching accounts reloads. val pager = - Pager( - PagingConfig( - pageSize = 20, - enablePlaceholders = false, - ), - ) { - Amber.instance.getDatabase(account.npub).dao().getAllPaging(account.hexKey) + remember(account.hexKey) { + Pager( + PagingConfig( + pageSize = 20, + enablePlaceholders = false, + ), + ) { + // Projection excludes the envelope-encrypted secret/localKey + // columns: the list renders neither, and each Keystore decrypt + // is a binder + TEE round trip (2 x rows per page). + Amber.instance.getDatabase(account.npub).dao().getApplicationListItemsPaging(account.hexKey) + } } val lazyPagingItems = pager.flow.collectAsLazyPagingItems() @@ -111,7 +118,7 @@ fun ApplicationsScreen( message = stringResource(R.string.make_backup_message), buttonText = stringResource(R.string.backup), onClick = { - navController.navigate(Route.AccountBackup.route) + navControllerWrapper.navController.navigate(Route.AccountBackup.route) }, ) } @@ -163,7 +170,7 @@ fun ApplicationsScreen( AmberButton( modifier = Modifier.padding(top = 20.dp), onClick = { - navController.navigate(Route.Activities.route) + navControllerWrapper.navController.navigate(Route.Activities.route) }, text = stringResource(R.string.activity), ) @@ -181,9 +188,9 @@ fun ApplicationsScreen( // tap (or a tap landing while the NavHost graph is being recreated) can // otherwise fire navigate() against a graph that no longer contains the // Permission destination, crashing with IllegalArgumentException. - if (navController.currentDestination?.route == Route.Applications.route) { + if (navControllerWrapper.navController.currentDestination?.route == Route.Applications.route) { runCatching { - navController.navigate("Permission/${applicationWithHistory.key}") + navControllerWrapper.navController.navigate("Permission/${applicationWithHistory.key}") }.onFailure { AmberLog.e("ApplicationsScreen", "Failed to open permissions for ${applicationWithHistory.key}", it) } diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/ui/MainScreen.kt b/app/src/main/java/com/greenart7c3/nostrsigner/ui/MainScreen.kt index 7d9434e7..3dcf2750 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/ui/MainScreen.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/ui/MainScreen.kt @@ -423,7 +423,7 @@ fun MainScreen( .padding(horizontal = verticalPadding) .padding(top = verticalPadding * 1.5f), account = account, - navController = navController.navController, + navControllerWrapper = navController, ) }, )