diff --git a/app/src/main/java/to/bitkit/data/keychain/AndroidKeyStore.kt b/app/src/main/java/to/bitkit/data/keychain/AndroidKeyStore.kt index 3391df16e2..f7cfff3803 100644 --- a/app/src/main/java/to/bitkit/data/keychain/AndroidKeyStore.kt +++ b/app/src/main/java/to/bitkit/data/keychain/AndroidKeyStore.kt @@ -87,4 +87,6 @@ class AndroidKeyStore( } generateKey() } + + fun containsAlias(): Boolean = keyStore.containsAlias(alias) } diff --git a/app/src/main/java/to/bitkit/data/keychain/Keychain.kt b/app/src/main/java/to/bitkit/data/keychain/Keychain.kt index 9b777174a3..486039e8f3 100644 --- a/app/src/main/java/to/bitkit/data/keychain/Keychain.kt +++ b/app/src/main/java/to/bitkit/data/keychain/Keychain.kt @@ -19,8 +19,11 @@ import to.bitkit.ext.fromBase64 import to.bitkit.ext.toBase64 import to.bitkit.utils.AppError import to.bitkit.utils.Logger +import java.util.Collections +import java.util.concurrent.ConcurrentHashMap import javax.inject.Inject import javax.inject.Singleton +import kotlin.coroutines.cancellation.CancellationException private val Context.keychainDataStore: DataStore by preferencesDataStore( name = "keychain" @@ -32,59 +35,76 @@ class Keychain @Inject constructor( @ApplicationContext private val context: Context, @IoDispatcher private val dispatcher: CoroutineDispatcher, ) : BaseCoroutineScope(dispatcher) { + companion object { + private const val TAG = "Keychain" + private const val CAUSE_CHAIN_DEPTH = 4 + } + private val keyStore by lazy { AndroidKeyStore(alias = "keychain") } @Suppress("MemberNameEqualsClassName") private val keychain = context.keychainDataStore + private val loadDiagnosticsEmitted: MutableSet = + Collections.newSetFromMap(ConcurrentHashMap()) + val snapshot get() = runBlocking(this.coroutineContext) { keychain.data.first() } fun loadString(key: String): String? = load(key)?.decodeToString() - @Suppress("TooGenericExceptionCaught", "SwallowedException") + @Suppress("TooGenericExceptionCaught") fun load(key: String): ByteArray? { try { return snapshot[key.indexed]?.fromBase64()?.let { keyStore.decrypt(it) } - } catch (_: Exception) { - throw KeychainError.FailedToLoad(key) + } catch (c: CancellationException) { + throw c + } catch (t: Throwable) { + emitLoadDiagnosticsOnce(key, t) + throw KeychainError.FailedToLoad(key, cause = t) } } suspend fun saveString(key: String, value: String) = save(key, value.toByteArray()) - @Suppress("TooGenericExceptionCaught", "SwallowedException") + @Suppress("TooGenericExceptionCaught", "ThrowsCount") suspend fun save(key: String, value: ByteArray) { if (exists(key)) throw KeychainError.FailedToSaveAlreadyExists(key) try { val encryptedValue = keyStore.encrypt(value) keychain.edit { it[key.indexed] = encryptedValue.toBase64() } - } catch (_: Exception) { - throw KeychainError.FailedToSave(key) + } catch (c: CancellationException) { + throw c + } catch (t: Throwable) { + throw KeychainError.FailedToSave(key, cause = t) } Logger.info("Saved to keychain: $key") } /** Inserts or replaces a string value associated with a given key in the keychain. */ - @Suppress("TooGenericExceptionCaught", "SwallowedException") + @Suppress("TooGenericExceptionCaught") suspend fun upsertString(key: String, value: String) { try { val encryptedValue = keyStore.encrypt(value.toByteArray()) keychain.edit { it[key.indexed] = encryptedValue.toBase64() } - } catch (_: Exception) { - throw KeychainError.FailedToSave(key) + } catch (c: CancellationException) { + throw c + } catch (t: Throwable) { + throw KeychainError.FailedToSave(key, cause = t) } Logger.info("Upsert in keychain: $key") } - @Suppress("TooGenericExceptionCaught", "SwallowedException") + @Suppress("TooGenericExceptionCaught") suspend fun delete(key: String) { try { keychain.edit { it.remove(key.indexed) } - } catch (_: Exception) { - throw KeychainError.FailedToDelete(key) + } catch (c: CancellationException) { + throw c + } catch (t: Throwable) { + throw KeychainError.FailedToDelete(key, cause = t) } Logger.debug("Deleted from keychain: $key") } @@ -120,6 +140,32 @@ class Keychain @Inject constructor( .map { string -> string?.toIntOrNull() } } + private fun emitLoadDiagnosticsOnce(key: String, cause: Throwable) { + if (!loadDiagnosticsEmitted.add(key)) return + + val aliasPresent = probe { keyStore.containsAlias() } + val entryPresent = probe { snapshot.contains(key.indexed) } + val walletIndex = probe { + runBlocking { db.configDao().getAll().first() }.firstOrNull()?.walletIndex ?: 0L + } + val causeChain = generateSequence(cause) { it.cause } + .take(CAUSE_CHAIN_DEPTH) + .joinToString(separator = " <- ") { it.javaClass.simpleName } + + Logger.warn( + "Decrypt failed for key='$key' walletIndex='$walletIndex' " + + "aliasPresent='$aliasPresent' entryPresent='$entryPresent' " + + "causeChain='$causeChain'", + context = TAG, + ) + } + + private inline fun probe(block: () -> T): String = + runCatching(block).fold( + onSuccess = { it.toString() }, + onFailure = { "error:${it.javaClass.simpleName}" }, + ) + enum class Key { PUSH_NOTIFICATION_TOKEN, PUSH_NOTIFICATION_PRIVATE_KEY, @@ -130,10 +176,19 @@ class Keychain @Inject constructor( } } -sealed class KeychainError(message: String) : AppError(message) { - class FailedToDelete(key: String) : KeychainError("Failed to delete $key from keychain.") - class FailedToLoad(key: String) : KeychainError("Failed to load $key from keychain.") - class FailedToSave(key: String) : KeychainError("Failed to save to $key keychain.") +sealed class KeychainError(message: String, cause: Throwable? = null) : AppError(message, cause) { + class FailedToDelete(key: String, cause: Throwable? = null) : + KeychainError("Failed to delete $key from keychain${cause.causeSuffix()}", cause) + + class FailedToLoad(val key: String, cause: Throwable? = null) : + KeychainError("Failed to load $key from keychain${cause.causeSuffix()}", cause) + + class FailedToSave(key: String, cause: Throwable? = null) : + KeychainError("Failed to save to $key keychain${cause.causeSuffix()}", cause) + class FailedToSaveAlreadyExists(key: String) : KeychainError("Key $key already exists in keychain. Explicitly delete key before attempting to update value.") } + +private fun Throwable?.causeSuffix(): String = + this?.let { " (cause='${it.javaClass.simpleName}')" } ?: "." diff --git a/app/src/test/java/to/bitkit/data/keychain/KeychainErrorTest.kt b/app/src/test/java/to/bitkit/data/keychain/KeychainErrorTest.kt new file mode 100644 index 0000000000..f365aef06f --- /dev/null +++ b/app/src/test/java/to/bitkit/data/keychain/KeychainErrorTest.kt @@ -0,0 +1,96 @@ +package to.bitkit.data.keychain + +import org.junit.Test +import java.io.IOException +import javax.crypto.AEADBadTagException +import javax.crypto.BadPaddingException +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNull +import kotlin.test.assertSame + +class KeychainErrorTest { + + @Test + fun `FailedToLoad without cause renders key and terminating period`() { + val error = KeychainError.FailedToLoad(key = "BIP39_MNEMONIC") + + assertEquals("Failed to load BIP39_MNEMONIC from keychain.", error.message) + assertNull(error.cause) + } + + @Test + fun `FailedToLoad preserves cause and embeds its simple class name`() { + val cause = AEADBadTagException("leak-probe-9f3a") + val error = KeychainError.FailedToLoad(key = "BIP39_MNEMONIC", cause = cause) + + assertSame(cause, error.cause) + assertEquals( + "Failed to load BIP39_MNEMONIC from keychain (cause='AEADBadTagException')", + error.message, + ) + } + + @Test + fun `FailedToLoad does not leak underlying cause message`() { + val probe = "leak-probe-9f3a" + val causes = listOf( + AEADBadTagException(probe), + BadPaddingException(probe), + ClassCastException(probe), + IllegalArgumentException(probe), + IOException(probe), + ) + + causes.forEach { cause -> + val error = KeychainError.FailedToLoad(key = "BIP39_MNEMONIC", cause = cause) + assertFalse( + actual = error.message.orEmpty().contains(probe), + message = "message leaked cause.message for ${cause.javaClass.simpleName}", + ) + } + } + + @Test + fun `FailedToLoad exposes the failing key`() { + val error = KeychainError.FailedToLoad(key = "BIP39_PASSPHRASE", cause = IOException()) + + assertEquals("BIP39_PASSPHRASE", error.key) + } + + @Test + fun `FailedToSave preserves cause and renders simple class name`() { + val cause = IOException("probe") + val error = KeychainError.FailedToSave(key = "BIP39_MNEMONIC", cause = cause) + + assertSame(cause, error.cause) + assertEquals( + "Failed to save to BIP39_MNEMONIC keychain (cause='IOException')", + error.message, + ) + } + + @Test + fun `FailedToDelete preserves cause and renders simple class name`() { + val cause = IOException("probe") + val error = KeychainError.FailedToDelete(key = "BIP39_MNEMONIC", cause = cause) + + assertSame(cause, error.cause) + assertEquals( + "Failed to delete BIP39_MNEMONIC from keychain (cause='IOException')", + error.message, + ) + } + + @Test + fun `FailedToSaveAlreadyExists has no cause and static message`() { + val error = KeychainError.FailedToSaveAlreadyExists(key = "BIP39_MNEMONIC") + + assertNull(error.cause) + assertEquals( + "Key BIP39_MNEMONIC already exists in keychain. " + + "Explicitly delete key before attempting to update value.", + error.message, + ) + } +} diff --git a/app/src/test/java/to/bitkit/repositories/WalletRepoTest.kt b/app/src/test/java/to/bitkit/repositories/WalletRepoTest.kt index 63625d275d..4140a80b63 100644 --- a/app/src/test/java/to/bitkit/repositories/WalletRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/WalletRepoTest.kt @@ -155,6 +155,17 @@ class WalletRepoTest : BaseUnitTest() { assertFalse(result) } + @Test + fun `walletExists relies on exists and does not touch load paths`() = test { + whenever(keychain.exists(Keychain.Key.BIP39_MNEMONIC.name)).thenReturn(true) + + val result = sut.walletExists() + + assertTrue(result) + verify(keychain, never()).loadString(Keychain.Key.BIP39_MNEMONIC.name) + verify(keychain, never()).load(Keychain.Key.BIP39_MNEMONIC.name) + } + @Test fun `setWalletExistsState should update walletState with current existence status`() = test { whenever(keychain.exists(Keychain.Key.BIP39_MNEMONIC.name)).thenReturn(true)