From 2eedff1402f54344555ce5c2e22f830aae00bb10 Mon Sep 17 00:00:00 2001 From: "William K. Santiago" Date: Sat, 22 Aug 2026 14:54:50 -0400 Subject: [PATCH] Isolate FROST integration test onto dedicated keystore alias and prefs namespace --- .../keep/nip55/FrostSigningIntegrationTest.kt | 47 +++++++++++-------- .../keep/storage/AndroidKeystoreStorage.kt | 37 ++++++++++----- 2 files changed, 52 insertions(+), 32 deletions(-) diff --git a/app/src/androidTest/kotlin/io/privkey/keep/nip55/FrostSigningIntegrationTest.kt b/app/src/androidTest/kotlin/io/privkey/keep/nip55/FrostSigningIntegrationTest.kt index 9b4367a8..4438cca6 100644 --- a/app/src/androidTest/kotlin/io/privkey/keep/nip55/FrostSigningIntegrationTest.kt +++ b/app/src/androidTest/kotlin/io/privkey/keep/nip55/FrostSigningIntegrationTest.kt @@ -14,7 +14,6 @@ import io.privkey.keep.uniffi.Nip55Handler import io.privkey.keep.uniffi.Nip55Request import io.privkey.keep.uniffi.Nip55RequestType import org.junit.Assert.* -import org.junit.Assume.assumeFalse import org.junit.Before import org.junit.Test import org.junit.runner.RunWith @@ -29,8 +28,16 @@ class FrostSigningIntegrationTest { private var testNip55Handler: Nip55Handler? = null private companion object { - // Matches AndroidKeystoreStorage.KEYSTORE_ALIAS (the legacy single-share AES key). - const val SHARE_KEY_ALIAS = "keep_frost_share" + // Dedicated test alias, distinct from production's KEYSTORE_ALIAS + // ("keep_frost_share"). The test's no-auth storage and the app's + // auth-gated storage otherwise share one alias, whose auth requirement is + // fixed by whichever caller creates it first; an isolated alias removes + // that race (keep-android-dcq). + const val SHARE_KEY_ALIAS = "keep_frost_share_test" + // Suffix that isolates this test's SharedPreferences (legacy, per-share, + // and registry namespaces) from the app's, so storeShare/deleteShare here + // cannot overwrite or clear real share data. + const val TEST_PREFS_SUFFIX = "_test" } private fun hasBiometricEnrollment(): Boolean { @@ -47,13 +54,18 @@ class FrostSigningIntegrationTest { // Always use no-auth storage for instrumented runs. The app's storage is // auth-per-use (setUserAuthenticationParameters(0, ...)) whenever a device - // has a biometric enrolled, and doFinal cannot succeed unattended. - val storage = AndroidKeystoreStorage(context, requireUserAuth = false) + // has a biometric enrolled, and doFinal cannot succeed unattended. The + // dedicated SHARE_KEY_ALIAS keeps this key off the production alias so the + // app's auth-gated storage can never win the create-race and gate it. + val storage = AndroidKeystoreStorage( + context, + requireUserAuth = false, + keystoreAlias = SHARE_KEY_ALIAS, + prefsSuffix = TEST_PREFS_SUFFIX + ) // requireUserAuth is only honored when the key is created; an existing - // keep_frost_share alias is reused as-is. A prior run on a biometric - // device could leave it auth-gated, which would make importShare's doFinal - // require a biometric. Drop leftover auth-gated key material so a fresh - // non-auth key is generated; if a real share sits behind it, skip instead. + // alias is reused as-is. A prior run could leave the test alias behind, so + // drop leftover auth-gated key material to force a fresh non-auth key. resetIfShareKeyRequiresAuth(storage) val mobile = KeepMobile(storage) testStorage = storage @@ -71,17 +83,12 @@ class FrostSigningIntegrationTest { (factory.getKeySpec(key, KeyInfo::class.java) as KeyInfo).isUserAuthenticationRequired }.getOrDefault(false) if (!authRequired) return - // An auth-gated alias with a share stored behind it is a real user share: - // production always creates this alias with requireUserAuth = true, while this - // test only ever writes no-auth fixture shares, so an auth-gated share is never - // one of ours. deleteShare() would drop both the share and the key that decrypts - // it, so skip instead of destroying user data. - assumeFalse( - "device holds a real auth-gated share; skipping to avoid destroying it", - storage.hasShare() - ) - // Auth-gated key with no share behind it is leftover key material from an - // earlier aborted run; dropping it lets a fresh no-auth key be generated. + // SHARE_KEY_ALIAS is test-exclusive (distinct from production's + // KEYSTORE_ALIAS), so an auth-gated key here is never a real user share — + // only stale material from an earlier run. deleteShare() also clears prefs, + // but TEST_PREFS_SUFFIX isolates those from the production legacy store + // (keep_secure_prefs), so this cannot touch real share data. Drop it so a + // fresh no-auth key is generated. storage.deleteShare() } diff --git a/app/src/main/kotlin/io/privkey/keep/storage/AndroidKeystoreStorage.kt b/app/src/main/kotlin/io/privkey/keep/storage/AndroidKeystoreStorage.kt index 08f7ae80..7eac1d8f 100644 --- a/app/src/main/kotlin/io/privkey/keep/storage/AndroidKeystoreStorage.kt +++ b/app/src/main/kotlin/io/privkey/keep/storage/AndroidKeystoreStorage.kt @@ -38,7 +38,20 @@ import javax.crypto.spec.SecretKeySpec class AndroidKeystoreStorage( private val context: Context, private val requireUserAuth: Boolean = true, - strongBoxUseTimeProbe: (() -> Boolean)? = null + strongBoxUseTimeProbe: (() -> Boolean)? = null, + // Test seam: the Keystore alias backing the legacy single-share AES key. + // Production always uses KEYSTORE_ALIAS ("keep_frost_share"); an instrumented + // test overrides it so its no-auth storage never contends with the app's + // auth-gated storage over the one shared alias, whose auth requirement is + // fixed by whichever caller creates it first (keep-android-dcq). + @get:VisibleForTesting + internal val keystoreAlias: String = KEYSTORE_ALIAS, + // Test seam: suffix appended to every SharedPreferences namespace (legacy + // single-share prefs, per-share prefs, and the multi-share registry). Empty in + // production; an instrumented test sets it so its storeShare/deleteShare write + // and clear an isolated sandbox instead of the app's real share data. + @get:VisibleForTesting + internal val prefsSuffix: String = "" ) : SecureStorage { companion object { @@ -112,9 +125,9 @@ class AndroidKeystoreStorage( private fun createEncryptedPrefs(name: String): SharedPreferences = KeystoreEncryptedPrefs.create(context, name) - private val prefs: SharedPreferences by lazy { createEncryptedPrefs(PREFS_NAME) } + private val prefs: SharedPreferences by lazy { createEncryptedPrefs("$PREFS_NAME$prefsSuffix") } - private val multiSharePrefs: SharedPreferences by lazy { createEncryptedPrefs(MULTI_PREFS_NAME) } + private val multiSharePrefs: SharedPreferences by lazy { createEncryptedPrefs("$MULTI_PREFS_NAME$prefsSuffix") } private fun isMetadataKey(key: String): Boolean = key.startsWith(METADATA_KEY_PREFIX) @@ -175,21 +188,21 @@ class AndroidKeystoreStorage( } private fun getSharePrefs(key: String): SharedPreferences = - createEncryptedPrefs("$PREFS_PREFIX${sanitizeKey(key)}") + createEncryptedPrefs("$PREFS_PREFIX${sanitizeKey(key)}$prefsSuffix") private fun getKeystoreAlias(key: String): String = "$KEYSTORE_PREFIX${sanitizeKey(key)}" @Synchronized - private fun getOrCreateKey(): SecretKey = getOrCreateKeyWithAlias(KEYSTORE_ALIAS, requireUserAuth) + private fun getOrCreateKey(): SecretKey = getOrCreateKeyWithAlias(keystoreAlias, requireUserAuth) private fun isStrongBoxAvailable(): Boolean = runCatching { context.packageManager.hasSystemFeature("android.hardware.strongbox_keystore") }.getOrDefault(false) fun getSecurityLevel(): String { - if (!keyStore.containsAlias(KEYSTORE_ALIAS)) return "none" + if (!keyStore.containsAlias(keystoreAlias)) return "none" val keyInfo = runCatching { - val key = keyStore.getKey(KEYSTORE_ALIAS, null) as? SecretKey ?: return "unknown" + val key = keyStore.getKey(keystoreAlias, null) as? SecretKey ?: return "unknown" val factory = SecretKeyFactory.getInstance(key.algorithm, "AndroidKeyStore") factory.getKeySpec(key, KeyInfo::class.java) as KeyInfo }.getOrNull() ?: return "unknown" @@ -464,8 +477,8 @@ class AndroidKeystoreStorage( throw KeepMobileException.StorageException("Failed to clear share metadata") } try { - if (keyStore.containsAlias(KEYSTORE_ALIAS)) { - keyStore.deleteEntry(KEYSTORE_ALIAS) + if (keyStore.containsAlias(keystoreAlias)) { + keyStore.deleteEntry(keystoreAlias) } } catch (e: Exception) { throw KeepMobileException.StorageException("Failed to delete keystore entry") @@ -484,9 +497,9 @@ class AndroidKeystoreStorage( return keyStore.getKey(legacyAlias, null) as? SecretKey ?: throw KeepMobileException.StorageException("Key $legacyAlias is not a SecretKey") } - if (keyStore.containsAlias(KEYSTORE_ALIAS) && isLegacyAccount(key)) { - return keyStore.getKey(KEYSTORE_ALIAS, null) as? SecretKey - ?: throw KeepMobileException.StorageException("Key $KEYSTORE_ALIAS is not a SecretKey") + if (keyStore.containsAlias(keystoreAlias) && isLegacyAccount(key)) { + return keyStore.getKey(keystoreAlias, null) as? SecretKey + ?: throw KeepMobileException.StorageException("Key $keystoreAlias is not a SecretKey") } return getOrCreateKeyWithAlias(newAlias, requireUserAuth) }