From 46ca019aa878e364b44480fd6e915639eaa9c690 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Fri, 10 Jul 2026 14:45:45 -0500 Subject: [PATCH] fix(security): derive key-invalidation and cache-guard decisions from seal state, not the encryptCache setting The encryptCache setting is written immediately but the on-disk conversion is deferred to the next cold start, so a transitional window exists (setting off, DB still encrypted, SEALED_AUTH present) in which three components answered from the setting and got it wrong (#479): * AppLockViewModel fed the raw setting into KeyInvalidationPolicy, so removing the device lock inside the window returned DISABLE_APP_LOCK: app-lock silently off, no wipe, SEALED_AUTH orphaned - and the next cold start hung forever in DatabaseKeyStore.resolvePassphrase (session.await() nothing could complete), bricking the app behind the CacheEncryptionGate until a data clear. onForeground now derives the policy input as `encryptCache || hasAuthSealedPassphrase()` (the same gate-on-the-seal fix SettingsViewModel.setAppLock already carries), so the window routes to CLEAR_AND_DISABLE (wipe scheduled, restart) and DISABLE_APP_LOCK is only reachable seal-free. The policy parameter is renamed to `encryptedCacheProtected` to make the contract explicit. * EncryptedCacheGuard.isCacheLocked() derived "locked" from the settings pair, wrong in both transitional states: workers parked forever inside provideDatabase when the setting was off but the DB still auth-sealed (case A), and sync/push/send stalled needlessly while the seal was still MASTER (case B). It now mirrors resolvePassphrase's blocking branches - keyed off DatabaseKeyStore.sealState() plus, for the AUTH-seal-with-setting-off window, a raw header read of the cache file (still never touching Room). * DatabaseProvisioner now releases the orphaned auth seal after the decrypt-on-disable conversion (reseals under the master key, best-effort), closing the window at its source instead of leaving SEALED_AUTH to linger indefinitely. All decision/fallback paths breadcrumb through AppLog (PII-free enums and booleans only). Closes #479 --- .../DatabaseProvisionerInstrumentedTest.kt | 54 +++++ ...WorkerCacheLockDeferralInstrumentedTest.kt | 101 ++++++++- .../FetchGateReceiverInstrumentedTest.kt | 10 +- .../data/local/DatabaseProvisioner.kt | 25 +++ .../data/security/EncryptedCacheGuard.kt | 60 +++++- .../data/security/KeyInvalidationPolicy.kt | 23 +- .../org/libremail/ui/lock/AppLockViewModel.kt | 15 +- .../data/local/DatabaseProvisionerTest.kt | 43 ++++ .../data/security/EncryptedCacheGuardTest.kt | 153 +++++++++++-- .../security/KeyInvalidationPolicyTest.kt | 62 ++++-- .../ui/lock/AppLockViewModelSealStateTest.kt | 204 ++++++++++++++++++ 11 files changed, 688 insertions(+), 62 deletions(-) create mode 100644 app/src/test/kotlin/org/libremail/ui/lock/AppLockViewModelSealStateTest.kt diff --git a/app/src/androidTest/kotlin/org/libremail/data/local/DatabaseProvisionerInstrumentedTest.kt b/app/src/androidTest/kotlin/org/libremail/data/local/DatabaseProvisionerInstrumentedTest.kt index 799ec3d..0c6171d 100644 --- a/app/src/androidTest/kotlin/org/libremail/data/local/DatabaseProvisionerInstrumentedTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/data/local/DatabaseProvisionerInstrumentedTest.kt @@ -3,6 +3,9 @@ package org.libremail.data.local import android.content.Context import android.content.ContextWrapper +import androidx.datastore.preferences.core.PreferenceDataStoreFactory +import androidx.datastore.preferences.core.edit +import androidx.datastore.preferences.core.stringPreferencesKey import androidx.room.Room import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 @@ -15,7 +18,9 @@ import io.mockk.mockkObject import io.mockk.unmockkAll import io.mockk.unmockkObject import io.mockk.verify +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.runBlocking import net.zetetic.database.sqlcipher.SupportOpenHelperFactory @@ -27,7 +32,11 @@ import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.libremail.data.local.entity.MessageEntity +import org.libremail.data.security.DatabaseKeyCipher import org.libremail.data.security.DatabaseKeyStore +import org.libremail.data.security.KeystoreCrypto +import org.libremail.data.security.PassphraseSession +import org.libremail.data.security.SealState import org.libremail.data.settings.AppSettings import org.libremail.data.settings.SettingsRepository import java.io.File @@ -72,6 +81,8 @@ class DatabaseProvisionerInstrumentedTest { clean() coEvery { keyStore.isClearPending() } returns false coEvery { keyStore.resolvePassphrase(any()) } returns passphrase + coEvery { keyStore.hasAuthSealedPassphrase() } returns false + coEvery { keyStore.sealWithMaster() } just Runs coEvery { migrator.migrateIfNeeded() } just Runs } @@ -186,6 +197,49 @@ class DatabaseProvisionerInstrumentedTest { } } + /** + * Issue #479, on real collaborators end to end: the transitional window — encryptCache already + * OFF, the cache still SQLCipher-encrypted on disk, the passphrase still auth-sealed — must not + * merely decrypt the file; it must also RELEASE the orphaned auth seal by resealing under the + * non-auth master key. Otherwise the app stays in the window indefinitely, where losing the + * auth-bound key (device-lock removal / re-enrollment) forces a needless cache wipe. + * + * Uses a REAL [DatabaseKeyStore] over a fresh test DataStore plus real Keystore crypto: the + * sealed-auth blob only needs to EXIST (sealState reads presence), and the passphrase itself + * comes from the unlocked [PassphraseSession] — exactly the state after a real authentication. + */ + @Test + fun encryptionTurnedOffReleasesTheOrphanedAuthSealAfterDecrypting() = runBlocking { + every { settingsRepository.settings } returns flowOf(AppSettings(encryptCache = false, appLock = true)) + seedPlaintextRow() + DatabaseEncryption.ensureEncrypted(dbFile, passphrase) + assertTrue("precondition: the cache starts encrypted", DatabaseEncryption.isEncrypted(dbFile)) + + val session = PassphraseSession().apply { unlock(passphrase) } + val realKeyStore = DatabaseKeyStore(appContext, KeystoreCrypto(), DatabaseKeyCipher(), session) + realKeyStore.dataStore = PreferenceDataStoreFactory.create( + scope = CoroutineScope(Dispatchers.IO + SupervisorJob()), + ) { File(appContext.cacheDir, "provisioner_seal_test_${System.nanoTime()}.preferences_pb") } + realKeyStore.dataStore.edit { it[stringPreferencesKey("sealed_db_key_auth")] = "sealed-by-auth-key" } + assertTrue("precondition: the passphrase is auth-sealed", realKeyStore.hasAuthSealedPassphrase()) + + val provisioner = DatabaseProvisioner(context, realKeyStore, settingsRepository, migrator, Dispatchers.IO) + val mode = provisioner.prepareCache() + + assertEquals(CacheOpenMode.Plaintext, mode) + assertFalse("the cache was decrypted", DatabaseEncryption.isEncrypted(dbFile)) + // The headline: no orphaned auth seal survives the conversion — it was resealed under the + // master key, so the passphrase stays recoverable WITHOUT authentication... + assertFalse("the auth seal must be released", realKeyStore.hasAuthSealedPassphrase()) + assertEquals(SealState.MASTER, realKeyStore.sealState()) + // ...and a later encryptCache re-enable reuses the very same passphrase (real Keystore round-trip). + assertEquals(passphrase, realKeyStore.passphrase()) + openPlaintext().apply { + assertEquals("acct:1", messageDao().getById("acct:1")?.id) + close() + } + } + @Test fun plaintextStartLeavesThePlaintextCacheUntouched() = runBlocking { every { settingsRepository.settings } returns flowOf(AppSettings(encryptCache = false, appLock = false)) diff --git a/app/src/androidTest/kotlin/org/libremail/data/sync/WorkerCacheLockDeferralInstrumentedTest.kt b/app/src/androidTest/kotlin/org/libremail/data/sync/WorkerCacheLockDeferralInstrumentedTest.kt index 07c3985..ad7b274 100644 --- a/app/src/androidTest/kotlin/org/libremail/data/sync/WorkerCacheLockDeferralInstrumentedTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/data/sync/WorkerCacheLockDeferralInstrumentedTest.kt @@ -9,6 +9,7 @@ import androidx.work.WorkerFactory import androidx.work.WorkerParameters import androidx.work.testing.TestListenableWorkerBuilder import dagger.Lazy +import io.mockk.coEvery import io.mockk.every import io.mockk.mockk import io.mockk.unmockkAll @@ -22,17 +23,21 @@ import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Test import org.junit.runner.RunWith +import org.libremail.data.security.DatabaseKeyStore import org.libremail.data.security.EncryptedCacheGuard import org.libremail.data.security.PassphraseSession +import org.libremail.data.security.SealState import org.libremail.data.settings.AppSettings import org.libremail.data.settings.SettingsRepository +import java.io.File /** * On-device proof that [PruneWorker] and [BackfillWorker] defer (`Result.retry()`) while the encrypted * cache is locked, driving them with the REAL [EncryptedCacheGuard] rather than the mocked guard the JVM * `PruneWorkerTest`/`BackfillWorkerTest` use (issue #225). [EncryptedCacheGuard] depends only on - * [SettingsRepository] and [PassphraseSession] — never the Keystore or `BiometricPrompt` — so the locked - * state is reproduced here with no device auth: a fixed [SettingsRepository] (mocked the same way + * [SettingsRepository], [DatabaseKeyStore] (its seal state — issue #479), a raw header read of the + * cache file, and [PassphraseSession] — never the Keystore crypto or `BiometricPrompt` — so the locked + * state is reproduced here with no device auth: fixed settings/seal collaborators (mocked the same way * `DatabaseProvisionerInstrumentedTest` fakes its security/settings collaborators) plus a real, * never-unlocked [PassphraseSession]. * @@ -120,12 +125,98 @@ class WorkerCacheLockDeferralInstrumentedTest { assertFalse(cacheGuard.isCacheLocked()) } - /** A real [EncryptedCacheGuard] over a fixed (mocked) [SettingsRepository] and the real [session]. */ - private fun guardFor(appLock: Boolean, encryptCache: Boolean): EncryptedCacheGuard { + // --- Issue #479: the guard answers from the seal + on-disk state, not the settings pair ------- + + /** + * Transitional case A: encryptCache was toggled OFF but the on-disk decrypt runs only at the next + * cold start — the cache is still auth-sealed and encrypted. The settings-derived formula said + * "unlocked" here, so a worker proceeded into `provideDatabase` and parked forever on the + * passphrase await (holding the provisioner mutex). The real guard must report locked so the + * worker defers with `Result.retry()` instead. + */ + @Test + fun pruneWorkerDefersDuringTheEncryptCacheOffTransitionalWindow() = runBlocking { + val cacheGuard = guardFor( + appLock = true, + encryptCache = false, + sealState = SealState.AUTH, + dbFile = encryptedFixtureFile(), + ) + assertTrue( + "precondition: auth-sealed + still-encrypted must report locked despite the off setting", + cacheGuard.isCacheLocked(), + ) + + val lazyPruner = mockk>() + val worker = TestListenableWorkerBuilder(context) + .setWorkerFactory(pruneWorkerFactory(lazyPruner, cacheGuard)) + .build() + + val result = withTimeout(TIMEOUT_MS) { worker.doWork() } + + assertEquals(Result.retry(), result) + verify(exactly = 0) { lazyPruner.get() } + } + + /** + * Transitional case B: app-lock (or encryptCache) was just enabled mid-session, but the passphrase + * is still MASTER-sealed until the next authentication reseals it — the DB opens auth-free, so + * background sync/push/send must NOT stall on the settings pair (the old formula reported locked). + */ + @Test + fun theRealGuardReportsUnlockedWhileThePassphraseIsStillMasterSealed() = runBlocking { + val cacheGuard = guardFor(appLock = true, encryptCache = true, sealState = SealState.MASTER) + + assertFalse(cacheGuard.isCacheLocked()) + } + + /** + * A lingering (orphaned) auth seal over an already-plaintext cache — the post-conversion tail of + * case A on installs that predate the provisioner's reseal — must not stall background work: the + * plaintext open needs no passphrase. + */ + @Test + fun theRealGuardReportsUnlockedOnceTheCacheIsPlaintextDespiteALingeringAuthSeal() = runBlocking { + val cacheGuard = guardFor( + appLock = true, + encryptCache = false, + sealState = SealState.AUTH, + dbFile = plaintextFixtureFile(), + ) + + assertFalse(cacheGuard.isCacheLocked()) + } + + /** + * A real [EncryptedCacheGuard] over fixed (mocked) [SettingsRepository]/[DatabaseKeyStore] + * collaborators, the real [session], and — for the [SealState.AUTH]-with-setting-off transitional + * window — a real fixture file whose header decides the on-disk half of the answer. + */ + private fun guardFor( + appLock: Boolean, + encryptCache: Boolean, + sealState: SealState = SealState.NONE, + dbFile: File? = null, + ): EncryptedCacheGuard { val settingsRepository = mockk() val settings = AppSettings(appLock = appLock, encryptCache = encryptCache) every { settingsRepository.settings } returns flowOf(settings) - return EncryptedCacheGuard(settingsRepository, session) + val keyStore = mockk() + coEvery { keyStore.sealState() } returns sealState + return EncryptedCacheGuard(context, settingsRepository, keyStore, session) + .apply { if (dbFile != null) this.dbFile = dbFile } + } + + /** A file whose header is NOT the SQLite magic — what a SQLCipher-encrypted cache looks like. */ + private fun encryptedFixtureFile(): File = File.createTempFile("guard_encrypted", ".db", context.cacheDir).apply { + deleteOnExit() + writeBytes(ByteArray(48) { 0x5A }) + } + + /** A file with a genuine plaintext-SQLite 16-byte header (padded past it). */ + private fun plaintextFixtureFile(): File = File.createTempFile("guard_plaintext", ".db", context.cacheDir).apply { + deleteOnExit() + writeBytes("SQLite format 3".toByteArray(Charsets.US_ASCII) + byteArrayOf(0) + ByteArray(32)) } private fun pruneWorkerFactory(lazyPruner: Lazy, cacheGuard: EncryptedCacheGuard) = diff --git a/app/src/androidTest/kotlin/org/libremail/debug/FetchGateReceiverInstrumentedTest.kt b/app/src/androidTest/kotlin/org/libremail/debug/FetchGateReceiverInstrumentedTest.kt index 2777730..c7cab15 100644 --- a/app/src/androidTest/kotlin/org/libremail/debug/FetchGateReceiverInstrumentedTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/debug/FetchGateReceiverInstrumentedTest.kt @@ -27,8 +27,10 @@ import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.junit.runner.RunWith +import org.libremail.data.security.DatabaseKeyStore import org.libremail.data.security.EncryptedCacheGuard import org.libremail.data.security.PassphraseSession +import org.libremail.data.security.SealState import org.libremail.data.settings.AppSettings import org.libremail.data.settings.SettingsRepository import org.libremail.data.sync.BackfillPacer @@ -162,11 +164,15 @@ class FetchGateReceiverInstrumentedTest { return requireNotNull(readBack[0]) { "receiver set no result data" } } - /** A real [EncryptedCacheGuard] reporting UNLOCKED (app-lock off) — so only the gate can defer. */ + /** A real [EncryptedCacheGuard] reporting UNLOCKED (app-lock off, no seal) — so only the gate can defer. */ private fun unlockedGuard(): EncryptedCacheGuard { val settingsRepository = mockk() every { settingsRepository.settings } returns flowOf(AppSettings(appLock = false, encryptCache = true)) - return EncryptedCacheGuard(settingsRepository, session) + // No seal exists (issue #479: the guard answers from the seal state, not the settings pair), + // so the un-armed cache never blocks and only the fetch gate can defer the worker. + val keyStore = mockk() + coEvery { keyStore.sealState() } returns SealState.NONE + return EncryptedCacheGuard(context, settingsRepository, keyStore, session) } private fun backfillWorkerFactory(lazyBackfiller: Lazy, cacheGuard: EncryptedCacheGuard) = diff --git a/app/src/main/kotlin/org/libremail/data/local/DatabaseProvisioner.kt b/app/src/main/kotlin/org/libremail/data/local/DatabaseProvisioner.kt index 409e3eb..f7a7ba7 100644 --- a/app/src/main/kotlin/org/libremail/data/local/DatabaseProvisioner.kt +++ b/app/src/main/kotlin/org/libremail/data/local/DatabaseProvisioner.kt @@ -3,6 +3,7 @@ package org.libremail.data.local import android.content.Context import dagger.hilt.android.qualifiers.ApplicationContext +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.first @@ -183,6 +184,7 @@ class DatabaseProvisioner internal constructor( // Encryption was turned back off — decrypt so the default (unkeyed) open succeeds. val passphrase = keyStore.resolvePassphrase(appLock) DatabaseEncryption.ensurePlaintext(dbFile, passphrase) + releaseOrphanedAuthSeal() CacheOpenMode.Plaintext } @@ -190,6 +192,29 @@ class DatabaseProvisioner internal constructor( } } + /** + * The cache is plaintext now, so a passphrase still sealed by the auth-bound key is an orphan: + * nothing needs it to open the database, but its presence keeps the app in the issue-#479 + * transitional state ([org.libremail.data.security.SealState.AUTH] with `encryptCache` off) — + * where losing the auth key (device-lock removal / re-enrollment) forces a needless cache wipe. + * Reseal it under the non-auth master key (dropping the auth seal and its Keystore key) so the + * passphrase stays recoverable and a later `encryptCache` re-enable reuses it seamlessly. + * Best-effort: the session holds the just-used passphrase on this path, but if the reseal still + * fails the plaintext open must proceed — the lingering seal is handled defensively everywhere + * (EncryptedCacheGuard checks the file, the key-invalidation policy treats it as protected). + */ + private suspend fun releaseOrphanedAuthSeal() { + if (!keyStore.hasAuthSealedPassphrase()) return + try { + keyStore.sealWithMaster() + AppLog.i(TAG, "released orphaned auth seal after decrypt-on-disable (resealed under master key)") + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + AppLog.w(TAG, "failed to release orphaned auth seal after decrypt-on-disable", e) + } + } + private companion object { const val TAG = "DatabaseProvisioner" } diff --git a/app/src/main/kotlin/org/libremail/data/security/EncryptedCacheGuard.kt b/app/src/main/kotlin/org/libremail/data/security/EncryptedCacheGuard.kt index 5fb41cb..8e84cb8 100644 --- a/app/src/main/kotlin/org/libremail/data/security/EncryptedCacheGuard.kt +++ b/app/src/main/kotlin/org/libremail/data/security/EncryptedCacheGuard.kt @@ -1,31 +1,79 @@ // SPDX-License-Identifier: GPL-3.0-or-later package org.libremail.data.security +import android.content.Context +import androidx.annotation.VisibleForTesting +import dagger.hilt.android.qualifiers.ApplicationContext import kotlinx.coroutines.flow.first +import org.libremail.data.local.DatabaseEncryption +import org.libremail.data.local.DatabaseFiles import org.libremail.data.settings.SettingsRepository +import org.libremail.reporting.AppLog +import java.io.File import javax.inject.Inject import javax.inject.Singleton /** * Tells UI-less entry points (background sync/push/send) whether opening the Room cache would block - * on the user authenticating. When app-lock AND encrypted-cache are on, the SQLCipher passphrase is - * sealed by an auth-bound key and only lives in [PassphraseSession] after an unlock — so a worker - * that constructs a DAO before then would park a thread indefinitely inside `provideDatabase`. + * on the user authenticating — a worker that constructs a DAO while it would must defer + * (`Result.retry()`) instead of parking a thread indefinitely inside `provideDatabase`. * - * Depends only on DataStore ([SettingsRepository]) and in-memory state ([PassphraseSession]) — never - * on the Room database — so it is safe to consult *before* touching any DB-backed dependency. + * The answer mirrors [DatabaseKeyStore.resolvePassphrase]'s blocking branches: it is keyed off which + * seal actually EXISTS ([DatabaseKeyStore.sealState]), never off the appLock/encryptCache settings + * alone, which live in a separate DataStore and can lag the on-disk truth (issue #479): + * - [SealState.AUTH]: the passphrase only becomes available after the user authenticates, so the + * cache is locked while the session is — INCLUDING the transitional window where `encryptCache` + * was already toggled off but the deferred decrypt-to-plaintext (next cold start) hasn't run yet. + * Once the file is already plaintext an open no longer needs the passphrase, so a lingering + * (orphaned) auth seal alone does not lock it. + * - [SealState.MASTER]: the passphrase auto-unwraps without authentication — never locked (e.g. + * app-lock was just enabled mid-session and the reseal under the auth key happens only at the + * next authentication; stalling sync/push/send until then would be needless). + * - [SealState.NONE]: locked only while both settings ask for an auth-armed cache that hasn't been + * armed yet (the first-time arm happens at the next authentication). + * + * Depends only on DataStore ([SettingsRepository], [DatabaseKeyStore]), in-memory state + * ([PassphraseSession]) and a raw 16-byte header read of the cache file — never on the Room + * database — so it is safe to consult *before* touching any DB-backed dependency. */ @Singleton class EncryptedCacheGuard @Inject constructor( + @ApplicationContext context: Context, private val settingsRepository: SettingsRepository, + private val keyStore: DatabaseKeyStore, private val session: PassphraseSession, ) { + /** + * The on-disk cache file whose header decides the transitional [SealState.AUTH] case. A + * [VisibleForTesting] seam (mirroring [DatabaseKeyStore.dataStore]) so tests can point the guard + * at a fixture file instead of the app's real cache. + */ + @VisibleForTesting + internal var dbFile: File = context.getDatabasePath(DatabaseFiles.NAME) + /** * True when the encrypted cache is currently locked, i.e. opening the database would suspend * waiting for the user to authenticate. Background work should defer (e.g. `Result.retry()`). */ suspend fun isCacheLocked(): Boolean { + if (session.isUnlocked()) return false val settings = settingsRepository.settings.first() - return settings.appLock && settings.encryptCache && !session.isUnlocked() + return when (keyStore.sealState()) { + SealState.MASTER -> false + SealState.NONE -> settings.appLock && settings.encryptCache + SealState.AUTH -> { + val locked = settings.encryptCache || DatabaseEncryption.isEncrypted(dbFile) + if (!settings.encryptCache) { + // The transitional window (issue #479): the setting is already off but the + // auth seal still exists. Breadcrumb the divergence — booleans only, no PII. + AppLog.i(TAG, "auth seal present with encryptCache off (transitional); cacheLocked=$locked") + } + locked + } + } + } + + private companion object { + const val TAG = "LibreMailCacheGuard" } } diff --git a/app/src/main/kotlin/org/libremail/data/security/KeyInvalidationPolicy.kt b/app/src/main/kotlin/org/libremail/data/security/KeyInvalidationPolicy.kt index b1537f1..41d3c1d 100644 --- a/app/src/main/kotlin/org/libremail/data/security/KeyInvalidationPolicy.kt +++ b/app/src/main/kotlin/org/libremail/data/security/KeyInvalidationPolicy.kt @@ -17,8 +17,9 @@ enum class LockAction { CLEAR_AND_REQUIRE_AUTH, /** - * The device no longer has a secure lock and there is nothing encrypted to protect: silently - * disable app-lock (there is no key to authenticate against) and proceed. + * The device no longer has a secure lock and there is nothing encrypted to protect — the + * encrypted-cache setting is off AND no auth-sealed passphrase exists: silently disable + * app-lock (there is no key to authenticate against) and proceed. */ DISABLE_APP_LOCK, @@ -31,22 +32,32 @@ enum class LockAction { } /** - * Pure decision table reconciling the app-lock / encrypted-cache settings with the current device + * Pure decision table reconciling the app-lock / encrypted-cache state with the current device * security state and Keystore key validity. Extracted from any Android or crypto dependency so the * (security-critical) branch logic — in particular the "clear + re-sync, never corrupt" handling of * lock removal and biometric re-enrollment — is fully unit-tested. */ object KeyInvalidationPolicy { + /** + * [encryptedCacheProtected] must reflect the ACTUAL protection state, not the `encryptCache` + * setting alone: the setting is written immediately but the on-disk conversion is deferred to + * the next cold start, so an auth-sealed (still encrypted) cache can outlive the setting being + * off. Callers derive it as `encryptCache setting || an auth-sealed passphrase exists` + * (issue #479); feeding the raw setting here silently disabled app-lock during that + * transitional window and stranded a permanently unreadable cache. + */ fun decide( appLockEnabled: Boolean, - encryptCacheEnabled: Boolean, + encryptedCacheProtected: Boolean, deviceSecure: Boolean, keyInvalidated: Boolean, ): LockAction = when { !appLockEnabled -> LockAction.PROCEED - !deviceSecure -> if (encryptCacheEnabled) LockAction.CLEAR_AND_DISABLE else LockAction.DISABLE_APP_LOCK - keyInvalidated -> if (encryptCacheEnabled) LockAction.CLEAR_AND_REQUIRE_AUTH else LockAction.REQUIRE_AUTH + !deviceSecure -> + if (encryptedCacheProtected) LockAction.CLEAR_AND_DISABLE else LockAction.DISABLE_APP_LOCK + keyInvalidated -> + if (encryptedCacheProtected) LockAction.CLEAR_AND_REQUIRE_AUTH else LockAction.REQUIRE_AUTH else -> LockAction.REQUIRE_AUTH } } diff --git a/app/src/main/kotlin/org/libremail/ui/lock/AppLockViewModel.kt b/app/src/main/kotlin/org/libremail/ui/lock/AppLockViewModel.kt index 58cfaa8..112aed0 100644 --- a/app/src/main/kotlin/org/libremail/ui/lock/AppLockViewModel.kt +++ b/app/src/main/kotlin/org/libremail/ui/lock/AppLockViewModel.kt @@ -142,9 +142,19 @@ class AppLockViewModel @Inject constructor( // renders before the (async) decision lands. if (_uiState.value == AppLockUiState.Unlocked) _uiState.value = AppLockUiState.Checking val action = withContext(defaultDispatcher) { + // Key the decision off the ACTUAL protection state, not the encryptCache setting + // alone: the setting flips immediately, but the on-disk decrypt runs at the next + // cold start, so an auth-sealed (still encrypted) cache can outlive + // `encryptCache == false`. Losing the auth key in that window must CLEAR (wipe + + // re-sync), never silently disable the lock and strand the seal — the same + // gate-on-the-seal fix SettingsViewModel.setAppLock already carries (issue #479). + val authSealed = databaseKeyStore.hasAuthSealedPassphrase() + if (authSealed && !settings.encryptCache) { + AppLog.i(TAG, "encryptCache off but passphrase still auth-sealed; treating cache as protected") + } KeyInvalidationPolicy.decide( appLockEnabled = true, - encryptCacheEnabled = settings.encryptCache, + encryptedCacheProtected = settings.encryptCache || authSealed, deviceSecure = appLockManager.isDeviceSecure(), keyInvalidated = databaseKeyCipher.isInvalidated(), ) @@ -155,6 +165,9 @@ class AppLockViewModel @Inject constructor( LockAction.PROCEED -> _uiState.value = AppLockUiState.Unlocked LockAction.DISABLE_APP_LOCK -> { + // Only reachable when NO auth-sealed passphrase exists (encryptedCacheProtected + // above ORs the seal in), so dropping the gate here can never orphan a seal or + // strand a still-encrypted cache — those states land on CLEAR_AND_DISABLE. settingsRepository.setAppLock(false) _uiState.value = AppLockUiState.Unlocked } diff --git a/app/src/test/kotlin/org/libremail/data/local/DatabaseProvisionerTest.kt b/app/src/test/kotlin/org/libremail/data/local/DatabaseProvisionerTest.kt index 3844381..cc159d7 100644 --- a/app/src/test/kotlin/org/libremail/data/local/DatabaseProvisionerTest.kt +++ b/app/src/test/kotlin/org/libremail/data/local/DatabaseProvisionerTest.kt @@ -64,6 +64,7 @@ class DatabaseProvisionerTest { // detekt-forbidden, epic #324) so it does not throw "not mocked". mockkStatic(android.util.Log::class) every { android.util.Log.w(any(), any(), any()) } returns 0 + every { android.util.Log.i(any(), any()) } returns 0 every { context.getDatabasePath(any()) } returns File("libremail.db") every { DatabaseFiles.clear(any()) } just Runs @@ -78,6 +79,8 @@ class DatabaseProvisionerTest { coEvery { keyStore.resetSealedPassphrase() } just Runs coEvery { keyStore.clearClearPending() } just Runs coEvery { keyStore.resolvePassphrase(any()) } returns PASSPHRASE + coEvery { keyStore.hasAuthSealedPassphrase() } returns false + coEvery { keyStore.sealWithMaster() } just Runs coEvery { accountDataMigrator.migrateIfNeeded() } just Runs } @@ -278,6 +281,46 @@ class DatabaseProvisionerTest { assertEquals(CacheOpenMode.Plaintext, mode) verify(exactly = 1) { DatabaseEncryption.ensurePlaintext(any(), PASSPHRASE) } verify(exactly = 0) { DatabaseEncryption.ensureEncrypted(any(), any()) } + // No auth seal exists (the passphrase was master-sealed), so nothing must be resealed. + coVerify(exactly = 0) { keyStore.sealWithMaster() } + } + + @Test + fun `decrypt-on-disable releases a lingering auth seal by resealing under the master key`() = runTest { + // Issue #479: after the decrypt-to-plaintext conversion the auth seal is an orphan — nothing + // needs it to open the DB, but its presence keeps the app in the transitional window where + // losing the auth-bound key forces a needless cache wipe. It must be resealed under the + // master key AFTER the file conversion (the passphrase is still in hand on this path). + every { settingsRepository.settings } returns flowOf(AppSettings(encryptCache = false, appLock = true)) + every { DatabaseEncryption.isEncrypted(any()) } returns true + coEvery { keyStore.hasAuthSealedPassphrase() } returns true + + val mode = provisioner().prepareCache() + + assertEquals(CacheOpenMode.Plaintext, mode) + coVerifyOrder { + keyStore.resolvePassphrase(true) + DatabaseEncryption.ensurePlaintext(any(), PASSPHRASE) + keyStore.sealWithMaster() + } + } + + @Test + fun `a failed reseal after decrypt-on-disable is non-fatal and still opens plaintext`() = runTest { + // The reseal is best-effort: the cache is already plaintext, so a Keystore hiccup must not + // fail the open (the lingering seal is handled defensively by the guard and the policy). + every { settingsRepository.settings } returns flowOf(AppSettings(encryptCache = false, appLock = true)) + every { DatabaseEncryption.isEncrypted(any()) } returns true + coEvery { keyStore.hasAuthSealedPassphrase() } returns true + coEvery { keyStore.sealWithMaster() } throws IllegalStateException("keystore busy") + + val mode = provisioner().prepareCache() + + assertEquals(CacheOpenMode.Plaintext, mode) + coVerify(exactly = 1) { keyStore.sealWithMaster() } + // Fail soft, never destructive: the seal is left alone rather than reset/wiped. + coVerify(exactly = 0) { keyStore.resetSealedPassphrase() } + verify(exactly = 0) { DatabaseFiles.clear(any()) } } @Test diff --git a/app/src/test/kotlin/org/libremail/data/security/EncryptedCacheGuardTest.kt b/app/src/test/kotlin/org/libremail/data/security/EncryptedCacheGuardTest.kt index 3e5cd3c..997907c 100644 --- a/app/src/test/kotlin/org/libremail/data/security/EncryptedCacheGuardTest.kt +++ b/app/src/test/kotlin/org/libremail/data/security/EncryptedCacheGuardTest.kt @@ -1,57 +1,172 @@ // SPDX-License-Identifier: GPL-3.0-or-later package org.libremail.data.security +import android.content.Context +import io.mockk.coEvery import io.mockk.every import io.mockk.mockk +import io.mockk.mockkStatic +import io.mockk.unmockkAll import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.runTest +import org.junit.After +import org.junit.Before +import org.junit.Rule import org.junit.Test +import org.junit.rules.TemporaryFolder import org.libremail.data.settings.AppSettings import org.libremail.data.settings.SettingsRepository +import java.io.File import kotlin.test.assertFalse import kotlin.test.assertTrue /** - * [EncryptedCacheGuard.isCacheLocked] is the pure truth table `appLock && encryptCache && !unlocked`: - * only when app-lock AND the encrypted cache are both on AND the passphrase session has not been - * unlocked would opening the Room DB block on authentication, so background work must defer. Every - * other combination is safe to proceed. Both collaborators are DataStore/in-memory only, so this is a - * clean JVM test. + * [EncryptedCacheGuard.isCacheLocked] mirrors [DatabaseKeyStore.resolvePassphrase]'s blocking + * branches: locked iff opening the Room DB would actually suspend on the user authenticating. The + * decision is keyed off which seal EXISTS ([DatabaseKeyStore.sealState]) plus — in the transitional + * [SealState.AUTH]-with-encryptCache-off window (issue #479) — whether the on-disk file is still + * encrypted. Deriving it from the appLock/encryptCache settings alone answered wrongly in both + * transitional states: workers parked forever inside `provideDatabase` when the setting was off but + * the DB still auth-sealed, and sync/push/send stalled needlessly while the seal was still MASTER. + * + * The file-header check runs against real temp files (a genuine 16-byte SQLite header vs. junk + * bytes), so [org.libremail.data.local.DatabaseEncryption.isEncrypted] is exercised for real. */ class EncryptedCacheGuardTest { - private val settingsRepository = mockk() - private val session = mockk() + @get:Rule + val tmp = TemporaryFolder() - private suspend fun cacheLocked(appLock: Boolean, encryptCache: Boolean, unlocked: Boolean): Boolean { + private val settingsRepository = mockk() + private val keyStore = mockk() + private val session = mockk() + private val context = mockk() + + @Before + fun setUp() { + // android.util.Log is a no-op stub that throws "not mocked" in JVM tests; the guard + // breadcrumbs the transitional divergence through AppLog, which always forwards to it. + mockkStatic(android.util.Log::class) + every { android.util.Log.i(any(), any()) } returns 0 + } + + @After + fun tearDown() { + unmockkAll() + } + + private suspend fun cacheLocked( + appLock: Boolean = true, + encryptCache: Boolean = true, + unlocked: Boolean = false, + sealState: SealState = SealState.NONE, + dbFile: File = plaintextDbFile(), + ): Boolean { every { settingsRepository.settings } returns flowOf(AppSettings(appLock = appLock, encryptCache = encryptCache)) every { session.isUnlocked() } returns unlocked - return EncryptedCacheGuard(settingsRepository, session).isCacheLocked() + coEvery { keyStore.sealState() } returns sealState + every { context.getDatabasePath(any()) } returns dbFile + return EncryptedCacheGuard(context, settingsRepository, keyStore, session).isCacheLocked() + } + + /** A file with a genuine plaintext-SQLite 16-byte header (padded past it). */ + private fun plaintextDbFile(): File = tmp.newFile().apply { + writeBytes("SQLite format 3".toByteArray(Charsets.US_ASCII) + byteArrayOf(0) + ByteArray(32)) + } + + /** A file whose header is NOT the SQLite magic — what a SQLCipher-encrypted cache looks like. */ + private fun encryptedDbFile(): File = tmp.newFile().apply { + writeBytes(ByteArray(48) { 0x5A }) + } + + // --- SealState.NONE: the original settings-driven rows are preserved ------------------------- + + @Test + fun `no seal - locked only while both settings ask for an auth-armed cache`() = runTest { + // First-time arm pending: resolvePassphrase(appLock=true) would await the session. + assertTrue(cacheLocked(appLock = true, encryptCache = true, sealState = SealState.NONE)) } @Test - fun `locked only when app-lock and encrypted cache are on and the session is not unlocked`() = runTest { - assertTrue(cacheLocked(appLock = true, encryptCache = true, unlocked = false)) + fun `no seal - not locked once the passphrase session is unlocked`() = runTest { + assertFalse(cacheLocked(appLock = true, encryptCache = true, unlocked = true, sealState = SealState.NONE)) } @Test - fun `not locked once the passphrase session is unlocked`() = runTest { - assertFalse(cacheLocked(appLock = true, encryptCache = true, unlocked = true)) + fun `no seal - not locked when the cache is not encrypted`() = runTest { + assertFalse(cacheLocked(appLock = true, encryptCache = false, sealState = SealState.NONE)) } @Test - fun `not locked when the cache is not encrypted`() = runTest { - assertFalse(cacheLocked(appLock = true, encryptCache = false, unlocked = false)) + fun `no seal - not locked when app-lock is off`() = runTest { + assertFalse(cacheLocked(appLock = false, encryptCache = true, sealState = SealState.NONE)) + } + + // --- SealState.MASTER: auto-unwraps without authentication — never locked -------------------- + + @Test + fun `master seal - not locked even with both settings on and the session locked`() = runTest { + // Issue #479 (transitional case B): app-lock/encryptCache just enabled mid-session; the + // passphrase is still master-sealed until the next authentication reseals it, so the DB + // opens auth-free — background sync/push/send must NOT stall on the settings pair. + assertFalse(cacheLocked(appLock = true, encryptCache = true, sealState = SealState.MASTER)) + } + + // --- SealState.AUTH: locked while the session is, including the setting-off window ----------- + + @Test + fun `auth seal - locked while the setting is on and the session is locked`() = runTest { + assertTrue(cacheLocked(appLock = true, encryptCache = true, sealState = SealState.AUTH)) } @Test - fun `not locked when app-lock is off`() = runTest { - assertFalse(cacheLocked(appLock = false, encryptCache = true, unlocked = false)) + fun `auth seal - not locked once the session is unlocked`() = runTest { + assertFalse(cacheLocked(appLock = true, encryptCache = true, unlocked = true, sealState = SealState.AUTH)) } @Test - fun `not locked when neither app-lock nor encrypted cache is on`() = runTest { - assertFalse(cacheLocked(appLock = false, encryptCache = false, unlocked = true)) + fun `auth seal - locked when the setting is off but the cache file is still encrypted`() = runTest { + // Issue #479 (transitional case A): encryptCache was toggled off but the decrypt runs only + // at the next cold start. Opening now would suspend in resolvePassphrase on session.await(), + // so the guard must report locked — the old settings-derived answer said "unlocked" and let + // workers park forever inside provideDatabase. + assertTrue( + cacheLocked( + appLock = true, + encryptCache = false, + sealState = SealState.AUTH, + dbFile = encryptedDbFile(), + ), + ) + } + + @Test + fun `auth seal - not locked when the setting is off and the cache file is already plaintext`() = runTest { + // A lingering (orphaned) auth seal after the decrypt-on-disable conversion: the plaintext + // open needs no passphrase, so background work must not be stalled by the leftover seal. + assertFalse( + cacheLocked( + appLock = true, + encryptCache = false, + sealState = SealState.AUTH, + dbFile = plaintextDbFile(), + ), + ) + } + + @Test + fun `auth seal - locked even when app-lock is off while the file is still encrypted`() = runTest { + // The desync state the old formula wedged on: app-lock already off but the cache is still + // auth-sealed and encrypted. resolvePassphrase keys off the seal, so an open WOULD suspend; + // deferring (retry) is the only safe answer. + assertTrue( + cacheLocked( + appLock = false, + encryptCache = false, + sealState = SealState.AUTH, + dbFile = encryptedDbFile(), + ), + ) } } diff --git a/app/src/test/kotlin/org/libremail/data/security/KeyInvalidationPolicyTest.kt b/app/src/test/kotlin/org/libremail/data/security/KeyInvalidationPolicyTest.kt index 47a9994..a10a4db 100644 --- a/app/src/test/kotlin/org/libremail/data/security/KeyInvalidationPolicyTest.kt +++ b/app/src/test/kotlin/org/libremail/data/security/KeyInvalidationPolicyTest.kt @@ -6,12 +6,15 @@ import kotlin.test.assertEquals class KeyInvalidationPolicyTest { + // `cacheProtected` is decide()'s `encryptedCacheProtected` input: derived by callers as + // "encryptCache setting ON, or an auth-sealed passphrase still exists" (issue #479) — never the + // raw setting, which can already be off while the on-disk cache is still auth-sealed. private fun decide( appLock: Boolean = true, - encrypt: Boolean = true, + cacheProtected: Boolean = true, secure: Boolean = true, invalidated: Boolean = false, - ) = KeyInvalidationPolicy.decide(appLock, encrypt, secure, invalidated) + ) = KeyInvalidationPolicy.decide(appLock, cacheProtected, secure, invalidated) @Test fun `app-lock off proceeds`() { @@ -25,41 +28,53 @@ class KeyInvalidationPolicyTest { } @Test - fun `lock removed with encrypted cache clears and disables`() { + fun `lock removed with a protected cache clears and disables`() { // The auth-bound passphrase is unrecoverable; wipe the cache and drop the gate. - assertEquals(LockAction.CLEAR_AND_DISABLE, decide(secure = false, encrypt = true)) + assertEquals(LockAction.CLEAR_AND_DISABLE, decide(secure = false, cacheProtected = true)) } @Test - fun `lock removed without encrypted cache just disables`() { - assertEquals(LockAction.DISABLE_APP_LOCK, decide(secure = false, encrypt = false)) + fun `lock removed while the cache is still auth-sealed clears even with the setting off`() { + // The issue-#479 transitional window: encryptCache was just toggled OFF (setting written, + // on-disk decrypt deferred to the next cold start, SEALED_AUTH still present) and the user + // removes the device lock. The caller derives cacheProtected from setting-OR-seal, so this + // state lands on the same protected row — CLEAR_AND_DISABLE — never DISABLE_APP_LOCK, which + // would strand a permanently unreadable cache behind a gate that no longer authenticates. + assertEquals(LockAction.CLEAR_AND_DISABLE, decide(secure = false, cacheProtected = true, invalidated = true)) } @Test - fun `biometric re-enrollment with encrypted cache clears and re-requires auth`() { - assertEquals(LockAction.CLEAR_AND_REQUIRE_AUTH, decide(invalidated = true, encrypt = true)) + fun `lock removed without a protected cache just disables`() { + assertEquals(LockAction.DISABLE_APP_LOCK, decide(secure = false, cacheProtected = false)) } @Test - fun `biometric re-enrollment without encrypted cache re-requires auth without clearing`() { + fun `biometric re-enrollment with a protected cache clears and re-requires auth`() { + assertEquals(LockAction.CLEAR_AND_REQUIRE_AUTH, decide(invalidated = true, cacheProtected = true)) + } + + @Test + fun `biometric re-enrollment without a protected cache re-requires auth without clearing`() { // Nothing encrypted to lose; a fresh key is minted on the next successful unlock. - assertEquals(LockAction.REQUIRE_AUTH, decide(invalidated = true, encrypt = false)) + assertEquals(LockAction.REQUIRE_AUTH, decide(invalidated = true, cacheProtected = false)) } @Test fun `lock removal takes precedence over key-invalidation flag`() { // Both true: the device being insecure dominates (can't authenticate at all). - assertEquals(LockAction.CLEAR_AND_DISABLE, decide(secure = false, invalidated = true, encrypt = true)) + assertEquals(LockAction.CLEAR_AND_DISABLE, decide(secure = false, invalidated = true, cacheProtected = true)) } @Test fun `every one of the 16 input combinations maps to its pinned action`() { - // The complete truth table for decide(appLock, encrypt, secure, invalidated): all 2^4 = 16 rows - // listed explicitly, so a mutation of ANY branch is caught — most importantly the common - // (on, *, secure, valid) rows, whose silent flip to PROCEED would be a lock bypass. The - // completeness guard below fails if a row is ever dropped, keeping the table exhaustive. + // The complete truth table for decide(appLock, cacheProtected, secure, invalidated): all + // 2^4 = 16 rows listed explicitly, so a mutation of ANY branch is caught — most importantly + // the common (on, *, secure, valid) rows, whose silent flip to PROCEED would be a lock + // bypass. The completeness guard below fails if a row is ever dropped, keeping the table + // exhaustive. `cacheProtected` is setting-OR-seal (issue #479), so the "protected" rows also + // pin the transitional encryptCache-off-but-still-auth-sealed states. // - // Columns: appLock, encrypt, secure, invalidated -> expected action. + // Columns: appLock, cacheProtected, secure, invalidated -> expected action. val table = listOf( // App-lock OFF: always PROCEED, whatever the other three inputs are. Case(false, false, false, false, LockAction.PROCEED), @@ -70,12 +85,13 @@ class KeyInvalidationPolicyTest { Case(false, true, false, true, LockAction.PROCEED), Case(false, true, true, false, LockAction.PROCEED), Case(false, true, true, true, LockAction.PROCEED), - // App-lock ON, device NOT secure (lock removed): clear+disable iff a cache exists, else disable. + // App-lock ON, device NOT secure (lock removed): clear+disable iff a cache is protected + // (setting on OR still auth-sealed), else disable. Case(true, true, false, false, LockAction.CLEAR_AND_DISABLE), Case(true, true, false, true, LockAction.CLEAR_AND_DISABLE), Case(true, false, false, false, LockAction.DISABLE_APP_LOCK), Case(true, false, false, true, LockAction.DISABLE_APP_LOCK), - // App-lock ON, secure, key invalidated: clear+re-auth iff a cache exists, else just re-auth. + // App-lock ON, secure, key invalidated: clear+re-auth iff a cache is protected, else just re-auth. Case(true, true, true, true, LockAction.CLEAR_AND_REQUIRE_AUTH), Case(true, false, true, true, LockAction.REQUIRE_AUTH), // App-lock ON, secure, key valid: the common case — require auth, no wipe. @@ -83,19 +99,19 @@ class KeyInvalidationPolicyTest { Case(true, false, true, false, LockAction.REQUIRE_AUTH), ) - // Exhaustiveness: exactly the 16 distinct (appLock, encrypt, secure, invalidated) combinations. + // Exhaustiveness: exactly the 16 distinct (appLock, cacheProtected, secure, invalidated) combinations. assertEquals(16, table.size, "the table must list all 2^4 input combinations") assertEquals( 16, - table.map { listOf(it.appLock, it.encrypt, it.secure, it.invalidated) }.toSet().size, + table.map { listOf(it.appLock, it.cacheProtected, it.secure, it.invalidated) }.toSet().size, "every row must be a distinct input combination", ) for (case in table) { assertEquals( case.expected, - decide(case.appLock, case.encrypt, case.secure, case.invalidated), - "decide(appLock=${case.appLock}, encrypt=${case.encrypt}, " + + decide(case.appLock, case.cacheProtected, case.secure, case.invalidated), + "decide(appLock=${case.appLock}, cacheProtected=${case.cacheProtected}, " + "secure=${case.secure}, invalidated=${case.invalidated})", ) } @@ -103,7 +119,7 @@ class KeyInvalidationPolicyTest { private data class Case( val appLock: Boolean, - val encrypt: Boolean, + val cacheProtected: Boolean, val secure: Boolean, val invalidated: Boolean, val expected: LockAction, diff --git a/app/src/test/kotlin/org/libremail/ui/lock/AppLockViewModelSealStateTest.kt b/app/src/test/kotlin/org/libremail/ui/lock/AppLockViewModelSealStateTest.kt new file mode 100644 index 0000000..79b22b1 --- /dev/null +++ b/app/src/test/kotlin/org/libremail/ui/lock/AppLockViewModelSealStateTest.kt @@ -0,0 +1,204 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.ui.lock + +import android.content.Context +import android.os.SystemClock +import androidx.work.Operation +import com.google.common.util.concurrent.ListenableFuture +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.coVerifyOrder +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkStatic +import io.mockk.unmockkAll +import io.mockk.verify +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.test.setMain +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.libremail.data.security.AppLockGate +import org.libremail.data.security.AppLockManager +import org.libremail.data.security.DatabaseKeyCipher +import org.libremail.data.security.DatabaseKeyStore +import org.libremail.data.security.LockState +import org.libremail.data.security.PassphraseSession +import org.libremail.data.settings.AppSettings +import org.libremail.data.settings.SettingsRepository +import org.libremail.data.sync.SyncScheduler +import org.libremail.reporting.AppLog +import org.libremail.reporting.RingLogBuffer +import org.libremail.restart.ProcessRestarter +import kotlin.test.assertIs +import kotlin.test.assertTrue + +/** + * The issue-#479 seal-present/setting-off matrix: `onForeground` must derive + * [org.libremail.data.security.KeyInvalidationPolicy]'s `encryptedCacheProtected` input from the + * ACTUAL protection state (`encryptCache setting || hasAuthSealedPassphrase()`), never the setting + * alone. The setting flips immediately, but the on-disk decrypt runs only at the next cold start, so + * an auth-sealed (still encrypted) cache can outlive `encryptCache == false` — the transitional + * window in which the old raw-setting input silently chose DISABLE_APP_LOCK on device-lock removal, + * orphaned SEALED_AUTH without a wipe, and bricked every later launch inside + * `DatabaseKeyStore.resolvePassphrase` (a `session.await()` nothing could ever complete). + * + * Unlike `AppLockViewModelTest`'s LockAction-dispatch tests (which stub the policy object), these + * drive the REAL [org.libremail.data.security.KeyInvalidationPolicy] through the ViewModel so the + * derivation itself — not just the dispatch — is pinned. Split into its own class (rather than grown + * onto AppLockViewModelTest) to respect detekt's LargeClass budget there. + */ +@OptIn(ExperimentalCoroutinesApi::class) +class AppLockViewModelSealStateTest { + + private val dispatcher = UnconfinedTestDispatcher() + private val logBuffer = RingLogBuffer() + + @Before + fun setUp() { + Dispatchers.setMain(dispatcher) + // android.util.Log is a throwing stub under plain JVM tests and AppLog always forwards to + // it; stub the levels these paths breadcrumb through, then install a real buffer so the + // breadcrumbs are asserted for real (never `verify { Log... }`) — mirroring AppLockViewModelTest. + mockkStatic(android.util.Log::class) + every { android.util.Log.i(any(), any()) } returns 0 + every { android.util.Log.w(any(), any()) } returns 0 + every { android.util.Log.w(any(), any(), any()) } returns 0 + AppLog.install(logBuffer) + mockkStatic(SystemClock::class) + every { SystemClock.elapsedRealtime() } returns FOREGROUND_AT + } + + @After + fun tearDown() { + Dispatchers.resetMain() + unmockkAll() + } + + @Test + fun `lock removal during the encryptCache-off transitional window clears and disables`() = runTest(dispatcher) { + val fixture = fixture(deviceSecure = false, authSealed = true) + + fixture.vm.onForeground() + advanceUntilIdle() + + // The setting is off but SEALED_AUTH still exists, so the policy must land on + // CLEAR_AND_DISABLE: wipe scheduled, gate dropped, process restarted — never the old + // DISABLE_APP_LOCK, which orphaned the seal without a wipe and hung every later launch + // forever inside DatabaseKeyStore.resolvePassphrase (the app bricked behind the cache gate). + coVerifyOrder { + fixture.databaseKeyStore.setClearPending() + fixture.settingsRepository.setAppLock(false) + fixture.processRestarter.restart() + } + // Issue #479: the seal-overrides-setting derivation and the decision are both breadcrumbed. + val messages = logBuffer.snapshot().map { it.message } + assertTrue("encryptCache off but passphrase still auth-sealed; treating cache as protected" in messages) + assertTrue("app-lock foreground decision: CLEAR_AND_DISABLE" in messages) + } + + @Test + fun `lock removal with no auth seal and the setting off disables without clearing`() = runTest(dispatcher) { + val fixture = fixture(deviceSecure = false, authSealed = false) + + fixture.vm.onForeground() + advanceUntilIdle() + + // Nothing encrypted and nothing sealed: still DISABLE_APP_LOCK — the seal-derived input + // must not turn every lock removal into a destructive wipe. + coVerify { fixture.settingsRepository.setAppLock(false) } + coVerify(exactly = 0) { fixture.databaseKeyStore.setClearPending() } + verify(exactly = 0) { fixture.processRestarter.restart() } + assertIs(fixture.vm.uiState.value) + } + + @Test + fun `key invalidation while still auth-sealed with the setting off clears and keeps the lock`() = + runTest(dispatcher) { + val fixture = fixture(deviceSecure = true, authSealed = true, keyInvalidated = true) + + fixture.vm.onForeground() + advanceUntilIdle() + + // Biometric re-enrollment during the transitional window: the auth-sealed cache is + // unrecoverable, so it is cleared and app-lock stays ON (CLEAR_AND_REQUIRE_AUTH). + coVerify { fixture.databaseKeyStore.setClearPending() } + verify { fixture.processRestarter.restart() } + coVerify(exactly = 0) { fixture.settingsRepository.setAppLock(false) } + } + + @Test + fun `a healthy device still just requires auth during the transitional window`() = runTest(dispatcher) { + val gate = mockk(relaxed = true) + every { gate.state } returns LockState.LOCKED + val fixture = fixture(deviceSecure = true, authSealed = true, gate = gate) + + fixture.vm.onForeground() + advanceUntilIdle() + + // Secure device + valid key: the seal-derived input must not disturb the healthy row — + // REQUIRE_AUTH, no wipe, no restart (the seal is unwrapped after the next auth as before). + verify { gate.onForeground(FOREGROUND_AT, appLockEnabled = true) } + assertIs(fixture.vm.uiState.value) + coVerify(exactly = 0) { fixture.databaseKeyStore.setClearPending() } + verify(exactly = 0) { fixture.processRestarter.restart() } + } + + /** + * A ViewModel over the REAL KeyInvalidationPolicy with app-lock ON and encryptCache OFF (the + * transitional window's setting state), plus mocks pinned to the given device/seal/key state. + */ + private class Fixture( + val vm: AppLockViewModel, + val settingsRepository: SettingsRepository, + val databaseKeyStore: DatabaseKeyStore, + val processRestarter: ProcessRestarter, + ) + + private fun fixture( + deviceSecure: Boolean, + authSealed: Boolean, + keyInvalidated: Boolean = false, + gate: AppLockGate = mockk(relaxed = true), + ): Fixture { + val settingsRepository = mockk(relaxed = true) + every { settingsRepository.settings } returns flowOf(AppSettings(appLock = true, encryptCache = false)) + val databaseKeyStore = mockk(relaxed = true) + coEvery { databaseKeyStore.hasAuthSealedPassphrase() } returns authSealed + val appLockManager = mockk(relaxed = true) + every { appLockManager.isDeviceSecure() } returns deviceSecure + val databaseKeyCipher = mockk(relaxed = true) + every { databaseKeyCipher.isInvalidated() } returns keyInvalidated + val processRestarter = mockk(relaxed = true) + val vm = AppLockViewModel( + context = mockk(relaxed = true), + settingsRepository = settingsRepository, + appLockManager = appLockManager, + databaseKeyStore = databaseKeyStore, + databaseKeyCipher = databaseKeyCipher, + session = mockk(relaxed = true), + syncScheduler = enqueueingScheduler(), + processRestarter = processRestarter, + gate = gate, + ).also { it.defaultDispatcher = dispatcher } + return Fixture(vm, settingsRepository, databaseKeyStore, processRestarter) + } + + /** A [SyncScheduler] whose `syncNow()` returns an [Operation] whose result future resolves. */ + private fun enqueueingScheduler(): SyncScheduler { + val future = mockk>() + every { future.get(any(), any()) } returns Operation.SUCCESS + val operation = mockk { every { result } returns future } + return mockk { every { syncNow() } returns operation } + } + + private companion object { + const val FOREGROUND_AT = 1_000L + } +} -- 2.47.3