Merge #4a9cd287: Fix Applications screen slowness after envelope-encryp…

Fix Applications screen slowness after envelope-encrypting secrets

nostr:nevent1qqsy48xjslclznph8hy78dyart9cwurd4v3lya9mycqv3r3822nth8qpz3mhxue69uhhyetvv9ujumn8d96zuer9wccae2pp

PR-Author: greenart7c3
nostr:npub1w4uswmv6lu9yel005l3qgheysmr7tk9uvwluddznju3nuxalevvs2d0jr5

PR description:

The GHSA-5fjp-ghh8-wch8 envelope-encryption change made every ApplicationDao read decrypt the secret/localKey columns, and each decryption re-fetched the AMBER_AES_KEY Keystore key (3 sequential binder IPCs per call). The Applications screen paid 2 x rows of those per page plus the TEE cipher op, fully serialized 342200224 several seconds on a populated account.

Two fixes:

1. SecureCryptoHelper caches the SecretKey handle (material never leaves the TEE/StrongBox). All entry points retry exactly once with a re-fetched handle on stale-handle errors (InvalidKeyException/KeyStoreException/UnrecoverableKeyException/ProviderException), so concurrent rotateKey or the unlocked-device policy cannot wedge reads. AEADBadTagException still propagates, preserving the decryptField failure contract.

2. The Applications list now uses a dedicated ApplicationListItem projection (key, name, relays, icon, lastUsed) that never selects the encrypted columns 342200224 the screen renders neither field, so the load performs zero Keystore operations. DecryptingPagingSource removed with its only caller. The Pager is also remember(account.hexKey)-ed instead of being rebuilt on every recomposition.

Also adapts ApplicationsScreen to the NavHostControllerWrapper stability pattern used by the other screens.

Verified: ktlintCheck, lint, test (all variants), assembleFreeDebug, assembleOfflineDebug.

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 <T> 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) {

app/src/main/java/com/greenart7c3/nostrsigner/database/ApplicationDao.kt

@@ -54,14 +54,20 @@ interface ApplicationDao {
 
     suspend fun getAllNotConnected(): List<ApplicationWithPermissions> = getAllNotConnectedRaw().map { it.decryptFromStorage() }
 
-    @Query("SELECT a.* FROM application a WHERE a.pubKey = :pubKey ORDER BY a.lastUsed DESC")
-    fun getAllPagingRaw(pubKey: String): PagingSource<Int, ApplicationEntity>
-
-    fun getAllPaging(pubKey: String): PagingSource<Int, ApplicationEntity> = DecryptingPagingSource(getAllPagingRaw(pubKey))
-
     @Query("SELECT DISTINCT relays FROM application")
     fun getAllRelayLists(): List<RelayListWrapper>
 
+    /**
+     * 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<Int, ApplicationListItem>
+
     @Query("SELECT name FROM application WHERE `key` = :key LIMIT 1")
     suspend fun getAppName(key: String): String?
 

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<NormalizedRelayUrl>,
+    val icon: String,
+    val lastUsed: Long,
+)

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<Int, ApplicationEntity>,
-) : PagingSource<Int, ApplicationEntity>() {
-    override fun getRefreshKey(state: PagingState<Int, ApplicationEntity>): Int? = delegate.getRefreshKey(state)
-
-    override suspend fun load(params: PagingSource.LoadParams<Int>): PagingSource.LoadResult<Int, ApplicationEntity> = 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
-    }
-}

app/src/main/java/com/greenart7c3/nostrsigner/database/CachingApplicationDao.kt

@@ -182,7 +182,7 @@ class CachingApplicationDao(
 
     override suspend fun getAllNotConnected(): List<ApplicationWithPermissions> = delegate.getAllNotConnected()
 
-    override fun getAllPaging(pubKey: String): PagingSource<Int, ApplicationEntity> = delegate.getAllPaging(pubKey)
+    override fun getApplicationListItemsPaging(pubKey: String): PagingSource<Int, ApplicationListItem> = delegate.getApplicationListItemsPaging(pubKey)
 
     override fun getAllRelayLists(): List<RelayListWrapper> = delegate.getAllRelayLists()
 
@@ -245,8 +245,6 @@ class CachingApplicationDao(
 
     override suspend fun getAllNotConnectedRaw(): List<ApplicationWithPermissions> = delegate.getAllNotConnectedRaw()
 
-    override fun getAllPagingRaw(pubKey: String): PagingSource<Int, ApplicationEntity> = delegate.getAllPagingRaw(pubKey)
-
     override suspend fun getByKeyRaw(key: String): ApplicationWithPermissions? = delegate.getByKeyRaw(key)
 
     override fun getByKeySyncRaw(key: String): ApplicationWithPermissions? = delegate.getByKeySyncRaw(key)

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)
                                     }

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,
                             )
                         },
                     )