feat(android): keep the session cookie in Keystore-encrypted storage (M462 #4985)
release / govulncheck (push) Successful in 18s
release / web (push) Successful in 1m18s
release / go (push) Successful in 1m40s
release / integration (push) Successful in 4m29s
release / android (push) Successful in 5m17s
release / Build signed APK (releases and dev) (push) Successful in 5m20s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 16s
release / Verify release artifacts (tag releases only) (push) Skipped
release / govulncheck (push) Successful in 18s
release / web (push) Successful in 1m18s
release / go (push) Successful in 1m40s
release / integration (push) Successful in 4m29s
release / android (push) Successful in 5m17s
release / Build signed APK (releases and dev) (push) Successful in 5m20s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 16s
release / Verify release artifacts (tag releases only) (push) Skipped
The session cookie is a bearer credential, and it sat in plain text in the Room auth_session row. It now lives in a SessionVault: AES-256-GCM under a key held in the Android Keystore, with only the ciphertext in a private prefs file. A copy of the app's files no longer yields a usable session. Platform APIs only, no new dependency (androidx.security-crypto is deprecated). Nobody is signed out by the upgrade. On first launch AuthStore moves a cookie still in the row into the vault and clears the column. If the Keystore can't be used on a device, the cookie stays in the row as before rather than being lost. A sign-in or 401 that lands during the move wins over the value it read, and the move never throws. The auth gate now waits for this before choosing Login or Home, with a 10s deadline so a wedged Keystore can't leave the start screen spinning. Tests: AuthStoreSessionVaultTest (upgrade move, vault-only load, Keystore fallback, sign-in/out, hydration race) and SealedBoxTest (round trip, fresh IV, tamper and wrong-key rejection). The real Keystore path needs a device; the first launch after updating is that check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -5,11 +5,17 @@ import com.fabledsword.minstrel.cache.db.dao.AuthSessionDao
|
||||
import com.fabledsword.minstrel.cache.db.entities.AuthSessionEntity
|
||||
import com.fabledsword.minstrel.di.ApplicationScope
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.Deferred
|
||||
import kotlinx.coroutines.async
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.StateFlow
|
||||
import kotlinx.coroutines.flow.asStateFlow
|
||||
import kotlinx.coroutines.launch
|
||||
import kotlinx.coroutines.sync.Mutex
|
||||
import kotlinx.coroutines.sync.withLock
|
||||
import kotlinx.coroutines.withTimeoutOrNull
|
||||
import kotlinx.serialization.json.Json
|
||||
import timber.log.Timber
|
||||
import javax.inject.Inject
|
||||
import javax.inject.Singleton
|
||||
|
||||
@@ -27,6 +33,14 @@ import javax.inject.Singleton
|
||||
* in-memory state changes synchronously so the next interceptor read
|
||||
* sees the new value immediately; the DAO write coroutine catches up
|
||||
* shortly after.
|
||||
*
|
||||
* **The session cookie is the exception** (M462 #4985): it is persisted
|
||||
* through [SessionVault] (Keystore-encrypted), not the Room row. On the
|
||||
* first launch after the upgrade, a cookie still in the row is moved into
|
||||
* the vault and the column cleared, so nobody is signed out by the change.
|
||||
* If the Keystore cannot be used on a device, the cookie stays in the row
|
||||
* as before rather than being lost. [awaitSessionHydrated] lets a caller
|
||||
* that needs a definitive answer (the auth gate) wait for this.
|
||||
*/
|
||||
// AuthStore is the single-row facade over auth_session (de-facto
|
||||
// app_preferences — see entity comment). It legitimately owns one
|
||||
@@ -39,6 +53,7 @@ import javax.inject.Singleton
|
||||
@Singleton
|
||||
class AuthStore @Inject constructor(
|
||||
private val dao: AuthSessionDao,
|
||||
private val vault: SessionVault,
|
||||
@ApplicationScope private val scope: CoroutineScope,
|
||||
) {
|
||||
private val sessionCookieState = MutableStateFlow<String?>(null)
|
||||
@@ -64,10 +79,20 @@ class AuthStore @Inject constructor(
|
||||
|
||||
private val json = Json { ignoreUnknownKeys = true }
|
||||
|
||||
// Serialises every cookie persist with the one-time hydration, so a
|
||||
// sign-in or a 401 that lands while hydration runs is never overwritten
|
||||
// by the stale value hydration read.
|
||||
private val cookieLock = Mutex()
|
||||
|
||||
// Set by setSessionCookie. Once something has written the cookie this
|
||||
// process, that value wins over whatever hydration finds on disk.
|
||||
@Volatile private var cookieTouched = false
|
||||
|
||||
private val cookieHydration: Deferred<Unit> = scope.async { hydrateSessionCookie() }
|
||||
|
||||
init {
|
||||
scope.launch {
|
||||
dao.observe().collect { row ->
|
||||
sessionCookieState.value = row?.sessionCookie
|
||||
baseUrlState.value = row?.baseUrl ?: DEFAULT_BASE_URL
|
||||
userJsonState.value = row?.userJson
|
||||
themeModeState.value = row?.themeMode
|
||||
@@ -85,9 +110,50 @@ class AuthStore @Inject constructor(
|
||||
}.getOrDefault(CacheSettings.DEFAULT)
|
||||
}
|
||||
|
||||
/**
|
||||
* Suspends until the stored session cookie has been loaded into
|
||||
* [sessionCookie], or [HYDRATION_DEADLINE_MS] passes (rule 156: a wedged
|
||||
* Keystore must not leave the start screen spinning). Returns false on
|
||||
* the deadline; the caller then decides from whatever has loaded, and a
|
||||
* late hydration still lands in [sessionCookie].
|
||||
*/
|
||||
suspend fun awaitSessionHydrated(): Boolean {
|
||||
val done = withTimeoutOrNull(HYDRATION_DEADLINE_MS) { cookieHydration.await() } != null
|
||||
if (!done) Timber.w("auth store: session hydration passed its deadline; deciding without it")
|
||||
return done
|
||||
}
|
||||
|
||||
fun setSessionCookie(value: String?) {
|
||||
cookieTouched = true
|
||||
sessionCookieState.value = value
|
||||
scope.launch { persistCookie(value) }
|
||||
scope.launch { cookieLock.withLock { storeCookie(value) } }
|
||||
}
|
||||
|
||||
private suspend fun hydrateSessionCookie() = cookieLock.withLock {
|
||||
val legacy = runCatching { dao.get()?.sessionCookie }.getOrNull()
|
||||
if (cookieTouched) return@withLock
|
||||
// A cookie in the row is the newer one when both exist: the row is
|
||||
// only written when the vault failed, and an install upgrading from
|
||||
// before the vault has nothing in the vault yet.
|
||||
val cookie = legacy ?: vault.read()
|
||||
// Best-effort: if moving it fails, the session still loads this time
|
||||
// and the move is retried on the next launch. Hydration must never
|
||||
// throw, or awaitSessionHydrated would leave the auth gate stuck.
|
||||
if (legacy != null) {
|
||||
runCatching { storeCookie(legacy) }
|
||||
.onFailure { Timber.w(it, "auth store: could not move the session cookie into the vault") }
|
||||
}
|
||||
sessionCookieState.value = cookie
|
||||
}
|
||||
|
||||
// Vault first; the Room row only when the Keystore is unusable, so a
|
||||
// broken Keystore degrades to the old storage rather than a sign-out.
|
||||
private suspend fun storeCookie(value: String?) {
|
||||
if (vault.write(value)) {
|
||||
if (dao.get()?.sessionCookie != null) dao.setSessionCookie(null)
|
||||
} else {
|
||||
persistLegacyCookie(value)
|
||||
}
|
||||
}
|
||||
|
||||
fun setBaseUrl(value: String) {
|
||||
@@ -121,7 +187,7 @@ class AuthStore @Inject constructor(
|
||||
scope.launch { persistDiagnosticsOptOut(value) }
|
||||
}
|
||||
|
||||
private suspend fun persistCookie(value: String?) {
|
||||
private suspend fun persistLegacyCookie(value: String?) {
|
||||
if (dao.get() == null) {
|
||||
dao.upsert(currentEntity().copy(sessionCookie = value))
|
||||
} else {
|
||||
@@ -179,7 +245,9 @@ class AuthStore @Inject constructor(
|
||||
|
||||
private fun currentEntity(): AuthSessionEntity = AuthSessionEntity(
|
||||
id = ROW_ID,
|
||||
sessionCookie = sessionCookieState.value,
|
||||
// Never copied into the row: the cookie lives in the vault, and
|
||||
// persistLegacyCookie sets it explicitly on the fallback path.
|
||||
sessionCookie = null,
|
||||
baseUrl = baseUrlState.value,
|
||||
userJson = userJsonState.value,
|
||||
themeMode = themeModeState.value,
|
||||
@@ -193,6 +261,10 @@ class AuthStore @Inject constructor(
|
||||
|
||||
companion object {
|
||||
const val DEFAULT_BASE_URL: String = "http://localhost:8080"
|
||||
|
||||
// Generous on purpose: hydration is one local row read and one
|
||||
// Keystore decrypt, normally milliseconds. This only bounds "never".
|
||||
const val HYDRATION_DEADLINE_MS: Long = 10_000
|
||||
private const val ROW_ID = 0
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,147 @@
|
||||
package com.fabledsword.minstrel.auth
|
||||
|
||||
import android.content.Context
|
||||
import android.security.keystore.KeyGenParameterSpec
|
||||
import android.security.keystore.KeyProperties
|
||||
import dagger.Binds
|
||||
import dagger.Module
|
||||
import dagger.hilt.InstallIn
|
||||
import dagger.hilt.android.qualifiers.ApplicationContext
|
||||
import dagger.hilt.components.SingletonComponent
|
||||
import timber.log.Timber
|
||||
import java.security.KeyStore
|
||||
import java.util.Base64
|
||||
import javax.crypto.Cipher
|
||||
import javax.crypto.KeyGenerator
|
||||
import javax.crypto.SecretKey
|
||||
import javax.crypto.spec.GCMParameterSpec
|
||||
import javax.inject.Inject
|
||||
import javax.inject.Singleton
|
||||
|
||||
/**
|
||||
* Where the session cookie lives at rest (M462 #4985).
|
||||
*
|
||||
* The cookie is a bearer credential: anyone holding it is signed in as the
|
||||
* user until the server expires or revokes it. It used to sit in plain text
|
||||
* in the Room `auth_session` row, readable from any copy of the app's data
|
||||
* directory (a rooted device, an adb backup of a debuggable build, a
|
||||
* forensic image). Now only ciphertext is stored, under an AES key that
|
||||
* lives in the Android Keystore and never leaves it, so a copy of the
|
||||
* files alone yields nothing usable.
|
||||
*/
|
||||
interface SessionVault {
|
||||
/** The stored cookie, or null when none is stored or it can't be decrypted. */
|
||||
fun read(): String?
|
||||
|
||||
/**
|
||||
* Stores [value], or clears the stored cookie when null. Returns false
|
||||
* when the Keystore could not be used, so the caller can fall back
|
||||
* rather than lose the session.
|
||||
*/
|
||||
fun write(value: String?): Boolean
|
||||
}
|
||||
|
||||
/**
|
||||
* AES-GCM sealing of a short string, framed as base64(iv || ciphertext+tag).
|
||||
* Kept apart from the Keystore so the framing can be unit-tested on the JVM
|
||||
* with an ordinary key; the Android Keystore has no JVM implementation.
|
||||
*/
|
||||
internal object SealedBox {
|
||||
private const val TRANSFORMATION = "AES/GCM/NoPadding"
|
||||
private const val TAG_BITS = 128
|
||||
private const val IV_BYTES = 12
|
||||
|
||||
// Binds a sealed value to its purpose: a blob sealed for something else
|
||||
// under the same key will not open as a session cookie.
|
||||
private val AAD = "minstrel-session-cookie-v1".toByteArray(Charsets.UTF_8)
|
||||
|
||||
fun seal(key: SecretKey, plaintext: String): String {
|
||||
val cipher = Cipher.getInstance(TRANSFORMATION)
|
||||
// No IV passed: the provider generates a fresh random one. Keystore
|
||||
// keys refuse a caller-chosen IV for encryption by default.
|
||||
cipher.init(Cipher.ENCRYPT_MODE, key)
|
||||
cipher.updateAAD(AAD)
|
||||
val sealed = cipher.iv + cipher.doFinal(plaintext.toByteArray(Charsets.UTF_8))
|
||||
return Base64.getEncoder().encodeToString(sealed)
|
||||
}
|
||||
|
||||
fun open(key: SecretKey, sealed: String): String {
|
||||
val bytes = Base64.getDecoder().decode(sealed)
|
||||
require(bytes.size > IV_BYTES) { "sealed value too short" }
|
||||
val cipher = Cipher.getInstance(TRANSFORMATION)
|
||||
cipher.init(Cipher.DECRYPT_MODE, key, GCMParameterSpec(TAG_BITS, bytes, 0, IV_BYTES))
|
||||
cipher.updateAAD(AAD)
|
||||
return String(cipher.doFinal(bytes, IV_BYTES, bytes.size - IV_BYTES), Charsets.UTF_8)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* [SessionVault] backed by a Keystore AES key and a private prefs file that
|
||||
* holds only the sealed value.
|
||||
*
|
||||
* A value that will not open (the key was wiped by a factory-reset of the
|
||||
* Keystore, or the file was restored onto another device; app backup is off,
|
||||
* but a vendor transfer tool may still copy files) is discarded and reported
|
||||
* as absent. The user signs in again, which is the right outcome for a
|
||||
* credential that no longer verifies.
|
||||
*/
|
||||
@Singleton
|
||||
class KeystoreSessionVault @Inject constructor(
|
||||
@ApplicationContext context: Context,
|
||||
) : SessionVault {
|
||||
private val prefs = context.getSharedPreferences(PREFS_NAME, Context.MODE_PRIVATE)
|
||||
|
||||
override fun read(): String? {
|
||||
val sealed = prefs.getString(KEY_COOKIE, null) ?: return null
|
||||
return runCatching { SealedBox.open(key(), sealed) }
|
||||
.onFailure {
|
||||
Timber.w(it, "session vault: stored cookie would not decrypt; discarding it")
|
||||
prefs.edit().remove(KEY_COOKIE).commit()
|
||||
}
|
||||
.getOrNull()
|
||||
}
|
||||
|
||||
override fun write(value: String?): Boolean = runCatching {
|
||||
val editor = prefs.edit()
|
||||
if (value == null) {
|
||||
editor.remove(KEY_COOKIE)
|
||||
} else {
|
||||
editor.putString(KEY_COOKIE, SealedBox.seal(key(), value))
|
||||
}
|
||||
editor.commit()
|
||||
}.onFailure {
|
||||
Timber.w(it, "session vault: Keystore unavailable; cookie not stored in the vault")
|
||||
}.getOrDefault(false)
|
||||
|
||||
private fun key(): SecretKey {
|
||||
val keyStore = KeyStore.getInstance(ANDROID_KEYSTORE).apply { load(null) }
|
||||
(keyStore.getKey(KEY_ALIAS, null) as? SecretKey)?.let { return it }
|
||||
val generator = KeyGenerator.getInstance(KeyProperties.KEY_ALGORITHM_AES, ANDROID_KEYSTORE)
|
||||
generator.init(
|
||||
KeyGenParameterSpec.Builder(
|
||||
KEY_ALIAS,
|
||||
KeyProperties.PURPOSE_ENCRYPT or KeyProperties.PURPOSE_DECRYPT,
|
||||
)
|
||||
.setBlockModes(KeyProperties.BLOCK_MODE_GCM)
|
||||
.setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_NONE)
|
||||
.setKeySize(KEY_BITS)
|
||||
.build(),
|
||||
)
|
||||
return generator.generateKey()
|
||||
}
|
||||
|
||||
private companion object {
|
||||
const val ANDROID_KEYSTORE = "AndroidKeyStore"
|
||||
const val KEY_ALIAS = "minstrel_session_cookie"
|
||||
const val KEY_BITS = 256
|
||||
const val PREFS_NAME = "session_vault"
|
||||
const val KEY_COOKIE = "sealed_cookie"
|
||||
}
|
||||
}
|
||||
|
||||
@Module
|
||||
@InstallIn(SingletonComponent::class)
|
||||
abstract class SessionVaultModule {
|
||||
@Binds
|
||||
abstract fun bindSessionVault(impl: KeystoreSessionVault): SessionVault
|
||||
}
|
||||
@@ -16,10 +16,11 @@ import javax.inject.Inject
|
||||
|
||||
/**
|
||||
* Computes the initial startDestination for the root NavHost based on
|
||||
* persisted auth state. Sits on top of AuthSessionDao directly rather
|
||||
* than AuthStore's StateFlow because the StateFlow defaults to null
|
||||
* until Room's first async emission — we need a definitive answer
|
||||
* before drawing any nav graph.
|
||||
* persisted auth state. Reads the row from AuthSessionDao directly, and
|
||||
* the session cookie only after [AuthStore.awaitSessionHydrated]: both
|
||||
* StateFlows default to null until their async load lands, and we need a
|
||||
* definitive answer before drawing any nav graph. The cookie is no longer
|
||||
* in the row (it lives in the Keystore-backed SessionVault, #4985).
|
||||
*
|
||||
* - no row at all → ServerUrl (first launch)
|
||||
* - row with baseUrl, no cookie → Login (URL configured, not yet signed in)
|
||||
@@ -31,6 +32,7 @@ import javax.inject.Inject
|
||||
@HiltViewModel
|
||||
class AuthGateViewModel @Inject constructor(
|
||||
private val dao: AuthSessionDao,
|
||||
private val authStore: AuthStore,
|
||||
) : ViewModel() {
|
||||
|
||||
private val internal = MutableStateFlow<Any?>(null)
|
||||
@@ -38,12 +40,13 @@ class AuthGateViewModel @Inject constructor(
|
||||
|
||||
init {
|
||||
viewModelScope.launch {
|
||||
authStore.awaitSessionHydrated()
|
||||
val signedIn = !authStore.sessionCookie.value.isNullOrEmpty()
|
||||
val row = dao.get()
|
||||
internal.value = when {
|
||||
row == null -> ServerUrl
|
||||
row.baseUrl == AuthStore.DEFAULT_BASE_URL && row.sessionCookie.isNullOrEmpty() ->
|
||||
ServerUrl
|
||||
row.sessionCookie.isNullOrEmpty() -> Login
|
||||
row.baseUrl == AuthStore.DEFAULT_BASE_URL && !signedIn -> ServerUrl
|
||||
!signedIn -> Login
|
||||
else -> Home
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user