Recover from an undecryptable auth token keyset instead of crashing (#960)

* Recover from an undecryptable auth token keyset instead of crashing

Exclude the encrypted token prefs from backups, and attach the R8 mappings
to GitHub releases.

* Reset the Keystore master key along with the token prefs

.github/workflows/prepare-release.yml

@@ -465,6 +465,9 @@ jobs:
 
           echo "File renamed successfully."
 
+      - name: Rename R8 Mapping
+        run: cp "$GITHUB_WORKSPACE/android/build/outputs/mapping/release/mapping.txt" "$GITHUB_WORKSPACE/android/build/outputs/apk/release/hammer-apk-mapping.txt"
+
       - name: Create Release
         uses: ncipollo/release-action@339a81892b84b4eeb0f6e744e4574d79d0d9b8dd # v1.21.0
         env:
@@ -474,7 +477,7 @@ jobs:
           prerelease: true
           artifactErrorsFailBuild: true
           allowUpdates: true
-          artifacts: "${{ github.workspace }}/android/build/outputs/apk/release/hammer.apk"
+          artifacts: "${{ github.workspace }}/android/build/outputs/apk/release/hammer.apk,${{ github.workspace }}/android/build/outputs/apk/release/hammer-apk-mapping.txt"
 
   snap:
     needs: tests

.github/workflows/publish-google-play.yml

@@ -89,3 +89,15 @@ jobs:
           esac
           echo "Running fastlane $LANE for track=$TRACK"
           bundle exec fastlane "$LANE"
+
+      # Keeps the mappings for the exact Play builds next to the release, so prod
+      # crash traces can be retraced without the Play Console.
+      - name: Attach R8 mappings to the GitHub release
+        if: github.event_name == 'release'
+        env:
+          GH_TOKEN: ${{ github.token }}
+          TAG: ${{ github.event.release.tag_name }}
+        run: |
+          cp android/build/outputs/mapping/release/mapping.txt hammer-google-play-mapping.txt
+          cp wear/build/outputs/mapping/release/mapping.txt hammer-wear-mapping.txt
+          gh release upload "$TAG" hammer-google-play-mapping.txt hammer-wear-mapping.txt --clobber

.github/workflows/publish-release.yml

@@ -11,6 +11,8 @@ jobs:
   publish-google-play:
     if: ${{ !contains(github.event.release.tag_name, '+') || contains(github.event.release.tag_name, '+google-play') }}
     uses: ./.github/workflows/publish-google-play.yml
+    permissions:
+      contents: write
     secrets: inherit
     with:
       track: production

android/src/main/res/xml/backup_rules.xml

@@ -3,4 +3,5 @@
 	<exclude domain="file" path="HammerProjects/server.json"/>
 	<exclude domain="file" path="HammerProjects/sync.json"/>
 	<exclude domain="file" path="HammerProjects/.backups"/>
+	<exclude domain="sharedpref" path="hammer_auth_tokens.xml"/>
 </full-backup-content>

android/src/main/res/xml/data_extraction_rules.xml

@@ -4,10 +4,12 @@
 		<exclude domain="file" path="HammerProjects/server.json"/>
 		<exclude domain="file" path="HammerProjects/sync.json"/>
 		<exclude domain="file" path="HammerProjects/.backups"/>
+		<exclude domain="sharedpref" path="hammer_auth_tokens.xml"/>
 	</cloud-backup>
 	<device-transfer>
 		<exclude domain="file" path="HammerProjects/server.json"/>
 		<exclude domain="file" path="HammerProjects/sync.json"/>
 		<exclude domain="file" path="HammerProjects/.backups"/>
+		<exclude domain="sharedpref" path="hammer_auth_tokens.xml"/>
 	</device-transfer>
 </data-extraction-rules>

common/src/androidHostTest/kotlin/com/darkrockstudios/apps/hammer/common/data/globalsettings/datasource/EncryptedSharedPrefsAuthTokenStoreTest.kt

@@ -0,0 +1,150 @@
+package com.darkrockstudios.apps.hammer.common.data.globalsettings.datasource
+
+import android.content.SharedPreferences
+import kotlinx.serialization.json.Json
+import javax.crypto.AEADBadTagException
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertNull
+
+class EncryptedSharedPrefsAuthTokenStoreTest {
+
+	private val disk = FakeEncryptedPrefsDisk()
+
+	private fun createStore() = EncryptedSharedPrefsAuthTokenStore(
+		json = Json,
+		openPrefs = disk::open,
+		resetPrefs = disk::reset,
+	)
+
+	@Test
+	fun `tokens round trip`() {
+		val store = createStore()
+
+		store.put(URL, USER_ID, TOKENS)
+
+		assertEquals(TOKENS, createStore().get(URL, USER_ID))
+	}
+
+	@Test
+	fun `keyset the master key cannot decrypt is treated as no tokens`() {
+		disk.keysetDecryptable = false
+		val store = createStore()
+
+		val tokens = store.get(URL, USER_ID)
+
+		assertNull(tokens)
+	}
+
+	@Test
+	fun `store is usable again after an undecryptable keyset`() {
+		disk.keysetDecryptable = false
+		createStore().get(URL, USER_ID)
+
+		createStore().put(URL, USER_ID, TOKENS)
+
+		assertEquals(TOKENS, createStore().get(URL, USER_ID))
+	}
+
+	@Test
+	fun `value that cannot be decrypted is treated as no tokens`() {
+		createStore().put(URL, USER_ID, TOKENS)
+		disk.valuesDecryptable = false
+
+		val tokens = createStore().get(URL, USER_ID)
+
+		assertNull(tokens)
+	}
+
+	@Test
+	fun `put replaces a value that cannot be decrypted`() {
+		createStore().put(URL, USER_ID, TOKENS)
+		disk.valuesDecryptable = false
+		val replacement = AuthTokens(bearerToken = "new-bearer", refreshToken = "new-refresh")
+
+		createStore().put(URL, USER_ID, replacement)
+
+		assertEquals(replacement, createStore().get(URL, USER_ID))
+	}
+
+	companion object {
+		private const val URL = "https://hammer.example"
+		private const val USER_ID = 7L
+		private val TOKENS = AuthTokens(bearerToken = "bearer", refreshToken = "refresh")
+	}
+}
+
+/**
+ * Models the encrypted prefs file: [open] fails like EncryptedSharedPreferences.create
+ * when the Keystore master key cannot decrypt the stored keyset, and [reset] removes
+ * the file (keyset included) so the next open generates a fresh one.
+ */
+private class FakeEncryptedPrefsDisk {
+	val values = mutableMapOf<String, String>()
+	var keysetDecryptable = true
+	var valuesDecryptable = true
+
+	fun open(): SharedPreferences {
+		if (!keysetDecryptable) throw AEADBadTagException()
+		return FakePrefs()
+	}
+
+	fun reset() {
+		values.clear()
+		keysetDecryptable = true
+		valuesDecryptable = true
+	}
+
+	private inner class FakePrefs : SharedPreferences {
+		override fun getString(key: String, defValue: String?): String? {
+			val stored = values[key] ?: return defValue
+			if (!valuesDecryptable) throw SecurityException("Could not decrypt value")
+			return stored
+		}
+
+		override fun contains(key: String): Boolean = key in values
+		override fun getAll(): MutableMap<String, *> = values.toMutableMap()
+		override fun edit(): SharedPreferences.Editor = FakeEditor()
+
+		override fun getStringSet(key: String, defValues: MutableSet<String>?) = unsupported()
+		override fun getInt(key: String, defValue: Int): Int = unsupported()
+		override fun getLong(key: String, defValue: Long): Long = unsupported()
+		override fun getFloat(key: String, defValue: Float): Float = unsupported()
+		override fun getBoolean(key: String, defValue: Boolean): Boolean = unsupported()
+		override fun registerOnSharedPreferenceChangeListener(
+			listener: SharedPreferences.OnSharedPreferenceChangeListener
+		) = unsupported()
+
+		override fun unregisterOnSharedPreferenceChangeListener(
+			listener: SharedPreferences.OnSharedPreferenceChangeListener
+		) = unsupported()
+	}
+
+	private inner class FakeEditor : SharedPreferences.Editor {
+		private val pending = mutableMapOf<String, String?>()
+		private var clearAll = false
+
+		override fun putString(key: String, value: String?) = apply { pending[key] = value }
+		override fun remove(key: String) = apply { pending[key] = null }
+		override fun clear() = apply { clearAll = true }
+
+		override fun commit(): Boolean {
+			if (clearAll) values.clear()
+			pending.forEach { (key, value) -> if (value == null) values.remove(key) else values[key] = value }
+			valuesDecryptable = true
+			return true
+		}
+
+		override fun apply() {
+			commit()
+		}
+
+		override fun putStringSet(key: String, values: MutableSet<String>?) = unsupported()
+		override fun putInt(key: String, value: Int) = unsupported()
+		override fun putLong(key: String, value: Long) = unsupported()
+		override fun putFloat(key: String, value: Float) = unsupported()
+		override fun putBoolean(key: String, value: Boolean) = unsupported()
+	}
+}
+
+private fun unsupported(): Nothing = throw UnsupportedOperationException()

common/src/androidMain/kotlin/com/darkrockstudios/apps/hammer/common/data/globalsettings/datasource/EncryptedSharedPrefsAuthTokenStore.kt

@@ -9,31 +9,46 @@ import io.github.aakira.napier.Napier
 import kotlinx.atomicfu.locks.reentrantLock
 import kotlinx.atomicfu.locks.withLock
 import kotlinx.serialization.json.Json
+import java.io.IOException
+import java.security.GeneralSecurityException
+import java.security.KeyStore
 
 /**
  * [AuthTokenStore] backed by [EncryptedSharedPreferences] with an Android
  * Keystore-backed master key. The account-keyed token map is stored as JSON in a
  * single encrypted preferences entry.
  */
-class EncryptedSharedPrefsAuthTokenStore(
-	context: Context,
+class EncryptedSharedPrefsAuthTokenStore internal constructor(
 	private val json: Json,
+	private val openPrefs: () -> SharedPreferences,
+	private val resetPrefs: () -> Unit,
 ) : AuthTokenStore {
 
+	constructor(context: Context, json: Json) : this(
+		json = json,
+		openPrefs = { openEncryptedPrefs(context) },
+		resetPrefs = { resetEncryptedPrefs(context) },
+	)
+
 	private val lock = reentrantLock()
 
 	private val prefs: SharedPreferences by lazy {
-		val masterKey = MasterKey.Builder(context)
-			.setKeyScheme(MasterKey.KeyScheme.AES256_GCM)
-			.build()
+		try {
+			openPrefs()
+		} catch (e: GeneralSecurityException) {
+			recreatePrefs(e)
+		} catch (e: IOException) {
+			recreatePrefs(e)
+		}
+	}
 
-		EncryptedSharedPreferences.create(
-			context,
-			PREFS_NAME,
-			masterKey,
-			EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV,
-			EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM,
-		)
+	// The keyset lives in the prefs file but its master key lives in the Keystore, which is
+	// never backed up, so a restored or orphaned file is undecryptable, and some keystores
+	// leave the master key itself unusable. Start over with both (forces re-login).
+	private fun recreatePrefs(cause: Exception): SharedPreferences {
+		Napier.w("Auth token keyset is unreadable; recreating the store", cause)
+		resetPrefs()
+		return openPrefs()
 	}
 
 	override fun get(url: String, userId: Long): AuthTokens? = lock.withLock {
@@ -54,11 +69,11 @@ class EncryptedSharedPrefsAuthTokenStore(
 	}
 
 	private fun loadMap(): Map<String, AuthTokens> {
-		val stored = prefs.getString(TOKENS_KEY, null) ?: return emptyMap()
 		return try {
+			val stored = prefs.getString(TOKENS_KEY, null) ?: return emptyMap()
 			json.decodeFromString<Map<String, AuthTokens>>(stored)
 		} catch (@Suppress("TooGenericExceptionCaught") e: Exception) {
-			Napier.w("Failed to parse encrypted auth tokens; treating as empty", e)
+			Napier.w("Failed to read encrypted auth tokens; treating as empty", e)
 			emptyMap()
 		}
 	}
@@ -70,5 +85,26 @@ class EncryptedSharedPrefsAuthTokenStore(
 	companion object {
 		private const val PREFS_NAME = "hammer_auth_tokens"
 		private const val TOKENS_KEY = "tokens"
+		private const val ANDROID_KEYSTORE = "AndroidKeyStore"
+
+		private fun openEncryptedPrefs(context: Context): SharedPreferences {
+			val masterKey = MasterKey.Builder(context)
+				.setKeyScheme(MasterKey.KeyScheme.AES256_GCM)
+				.build()
+
+			return EncryptedSharedPreferences.create(
+				context,
+				PREFS_NAME,
+				masterKey,
+				EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV,
+				EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM,
+			)
+		}
+
+		private fun resetEncryptedPrefs(context: Context) {
+			context.deleteSharedPreferences(PREFS_NAME)
+			KeyStore.getInstance(ANDROID_KEYSTORE).apply { load(null) }
+				.deleteEntry(MasterKey.DEFAULT_MASTER_KEY_ALIAS)
+		}
 	}
 }