Avoid crashes from unknown exceptions on lockscreen key migration.

This commit is contained in:
Jorge Martín 2022-08-09 09:53:08 +02:00 committed by Jorge Martin Espinosa
parent 58d47df37b
commit 6e1e31bac1
7 changed files with 28 additions and 23 deletions

1
changelog.d/6769.bugfix Normal file
View file

@ -0,0 +1 @@
Catch all exceptions on lockscreen system key migrations.

View file

@ -40,7 +40,7 @@ class LockScreenKeysMigrator @Inject constructor(
suspend fun migrateIfNeeded() { suspend fun migrateIfNeeded() {
if (legacyPinCodeMigrator.isMigrationNeeded()) { if (legacyPinCodeMigrator.isMigrationNeeded()) {
legacyPinCodeMigrator.migrate() legacyPinCodeMigrator.migrate()
missingSystemKeyMigrator.migrate() missingSystemKeyMigrator.migrateIfNeeded()
} }
if (systemKeyV1Migrator.isMigrationNeeded() && versionProvider.get() >= Build.VERSION_CODES.M) { if (systemKeyV1Migrator.isMigrationNeeded() && versionProvider.get() >= Build.VERSION_CODES.M) {

View file

@ -18,8 +18,6 @@ package im.vector.app.features.pin.lockscreen.crypto.migrations
import android.annotation.SuppressLint import android.annotation.SuppressLint
import android.os.Build import android.os.Build
import android.security.keystore.KeyPermanentlyInvalidatedException
import android.security.keystore.UserNotAuthenticatedException
import im.vector.app.features.pin.lockscreen.crypto.KeyStoreCrypto import im.vector.app.features.pin.lockscreen.crypto.KeyStoreCrypto
import im.vector.app.features.pin.lockscreen.di.BiometricKeyAlias import im.vector.app.features.pin.lockscreen.di.BiometricKeyAlias
import im.vector.app.features.settings.VectorPreferences import im.vector.app.features.settings.VectorPreferences
@ -41,14 +39,15 @@ class MissingSystemKeyMigrator @Inject constructor(
* If user had biometric auth enabled, ensure system key exists, creating one if needed. * If user had biometric auth enabled, ensure system key exists, creating one if needed.
*/ */
@SuppressLint("NewApi") @SuppressLint("NewApi")
fun migrate() { fun migrateIfNeeded() {
if (buildVersionSdkIntProvider.get() >= Build.VERSION_CODES.M && vectorPreferences.useBiometricsToUnlock()) { if (buildVersionSdkIntProvider.get() >= Build.VERSION_CODES.M && vectorPreferences.useBiometricsToUnlock()) {
try { val systemKeyStoreCrypto = keystoreCryptoFactory.provide(systemKeyAlias, true)
keystoreCryptoFactory.provide(systemKeyAlias, true).ensureKey() runCatching {
} catch (e: KeyPermanentlyInvalidatedException) { systemKeyStoreCrypto.ensureKey()
Timber.e("Could not automatically create biometric key because it was invalidated.", e) }.onFailure { e ->
} catch (e: UserNotAuthenticatedException) { Timber.e(e, "Could not automatically create biometric key. Biometric authentication will be disabled.")
Timber.e("Could not automatically create biometric key because there are no enrolled biometric authenticators.", e) systemKeyStoreCrypto.deleteKey()
vectorPreferences.setUseBiometricToUnlock(false)
} }
} }
} }

View file

@ -17,10 +17,10 @@
package im.vector.app.features.pin.lockscreen.crypto.migrations package im.vector.app.features.pin.lockscreen.crypto.migrations
import android.os.Build import android.os.Build
import android.security.keystore.UserNotAuthenticatedException
import androidx.annotation.RequiresApi import androidx.annotation.RequiresApi
import im.vector.app.features.pin.lockscreen.crypto.KeyStoreCrypto import im.vector.app.features.pin.lockscreen.crypto.KeyStoreCrypto
import im.vector.app.features.pin.lockscreen.di.BiometricKeyAlias import im.vector.app.features.pin.lockscreen.di.BiometricKeyAlias
import im.vector.app.features.settings.VectorPreferences
import timber.log.Timber import timber.log.Timber
import java.security.KeyStore import java.security.KeyStore
import javax.inject.Inject import javax.inject.Inject
@ -32,6 +32,7 @@ class SystemKeyV1Migrator @Inject constructor(
@BiometricKeyAlias private val systemKeyAlias: String, @BiometricKeyAlias private val systemKeyAlias: String,
private val keyStore: KeyStore, private val keyStore: KeyStore,
private val keystoreCryptoFactory: KeyStoreCrypto.Factory, private val keystoreCryptoFactory: KeyStoreCrypto.Factory,
private val vectorPreferences: VectorPreferences,
) { ) {
/** /**
@ -41,10 +42,12 @@ class SystemKeyV1Migrator @Inject constructor(
fun migrate() { fun migrate() {
keyStore.deleteEntry(SYSTEM_KEY_ALIAS_V1) keyStore.deleteEntry(SYSTEM_KEY_ALIAS_V1)
val systemKeyStoreCrypto = keystoreCryptoFactory.provide(systemKeyAlias, keyNeedsUserAuthentication = true) val systemKeyStoreCrypto = keystoreCryptoFactory.provide(systemKeyAlias, keyNeedsUserAuthentication = true)
try { runCatching {
systemKeyStoreCrypto.ensureKey() systemKeyStoreCrypto.ensureKey()
} catch (e: UserNotAuthenticatedException) { }.onFailure { e ->
Timber.e("Could not migrate v1 biometric key because there are no enrolled biometric authenticators.", e) Timber.e(e, "Could not migrate v1 biometric key. Biometric authentication will be disabled.")
systemKeyStoreCrypto.deleteKey()
vectorPreferences.setUseBiometricToUnlock(false)
} }
} }

View file

@ -26,7 +26,7 @@ import io.mockk.verify
import kotlinx.coroutines.runBlocking import kotlinx.coroutines.runBlocking
import org.junit.Test import org.junit.Test
class LockScreenTestMigratorTests { class LockScreenKeysMigratorTests {
private val legacyPinCodeMigrator = mockk<LegacyPinCodeMigrator>(relaxed = true) private val legacyPinCodeMigrator = mockk<LegacyPinCodeMigrator>(relaxed = true)
private val missingSystemKeyMigrator = mockk<MissingSystemKeyMigrator>(relaxed = true) private val missingSystemKeyMigrator = mockk<MissingSystemKeyMigrator>(relaxed = true)
@ -42,7 +42,7 @@ class LockScreenTestMigratorTests {
runBlocking { migrator.migrateIfNeeded() } runBlocking { migrator.migrateIfNeeded() }
coVerify(exactly = 0) { legacyPinCodeMigrator.migrate() } coVerify(exactly = 0) { legacyPinCodeMigrator.migrate() }
verify(exactly = 0) { missingSystemKeyMigrator.migrate() } verify(exactly = 0) { missingSystemKeyMigrator.migrateIfNeeded() }
// When migration is needed // When migration is needed
every { legacyPinCodeMigrator.isMigrationNeeded() } returns true every { legacyPinCodeMigrator.isMigrationNeeded() } returns true
@ -50,7 +50,7 @@ class LockScreenTestMigratorTests {
runBlocking { migrator.migrateIfNeeded() } runBlocking { migrator.migrateIfNeeded() }
coVerify { legacyPinCodeMigrator.migrate() } coVerify { legacyPinCodeMigrator.migrate() }
verify { missingSystemKeyMigrator.migrate() } verify { missingSystemKeyMigrator.migrateIfNeeded() }
} }
@Test @Test

View file

@ -44,7 +44,7 @@ class MissingSystemKeyMigratorTests {
every { keyStoreCryptoFactory.provide(any(), any()) } returns keyStoreCryptoMock every { keyStoreCryptoFactory.provide(any(), any()) } returns keyStoreCryptoMock
every { vectorPreferences.useBiometricsToUnlock() } returns true every { vectorPreferences.useBiometricsToUnlock() } returns true
missingSystemKeyMigrator.migrate() missingSystemKeyMigrator.migrateIfNeeded()
verify { keyStoreCryptoMock.ensureKey() } verify { keyStoreCryptoMock.ensureKey() }
} }
@ -57,7 +57,7 @@ class MissingSystemKeyMigratorTests {
every { keyStoreCryptoFactory.provide(any(), any()) } returns keyStoreCryptoMock every { keyStoreCryptoFactory.provide(any(), any()) } returns keyStoreCryptoMock
every { vectorPreferences.useBiometricsToUnlock() } returns true every { vectorPreferences.useBiometricsToUnlock() } returns true
invoking { missingSystemKeyMigrator.migrate() } shouldNotThrow KeyPermanentlyInvalidatedException::class invoking { missingSystemKeyMigrator.migrateIfNeeded() } shouldNotThrow KeyPermanentlyInvalidatedException::class
} }
@Test @Test
@ -68,7 +68,7 @@ class MissingSystemKeyMigratorTests {
every { keyStoreCryptoFactory.provide(any(), any()) } returns keyStoreCryptoMock every { keyStoreCryptoFactory.provide(any(), any()) } returns keyStoreCryptoMock
every { vectorPreferences.useBiometricsToUnlock() } returns true every { vectorPreferences.useBiometricsToUnlock() } returns true
invoking { missingSystemKeyMigrator.migrate() } shouldNotThrow UserNotAuthenticatedException::class invoking { missingSystemKeyMigrator.migrateIfNeeded() } shouldNotThrow UserNotAuthenticatedException::class
} }
@Test @Test
@ -79,7 +79,7 @@ class MissingSystemKeyMigratorTests {
every { keyStoreCryptoFactory.provide(any(), any()) } returns keyStoreCryptoMock every { keyStoreCryptoFactory.provide(any(), any()) } returns keyStoreCryptoMock
every { vectorPreferences.useBiometricsToUnlock() } returns false every { vectorPreferences.useBiometricsToUnlock() } returns false
missingSystemKeyMigrator.migrate() missingSystemKeyMigrator.migrateIfNeeded()
verify(exactly = 0) { keyStoreCryptoMock.ensureKey() } verify(exactly = 0) { keyStoreCryptoMock.ensureKey() }
} }
@ -93,7 +93,7 @@ class MissingSystemKeyMigratorTests {
every { keyStoreCryptoFactory.provide(any(), any()) } returns keyStoreCryptoMock every { keyStoreCryptoFactory.provide(any(), any()) } returns keyStoreCryptoMock
every { vectorPreferences.useBiometricsToUnlock() } returns false every { vectorPreferences.useBiometricsToUnlock() } returns false
missingSystemKeyMigrator.migrate() missingSystemKeyMigrator.migrateIfNeeded()
verify(exactly = 0) { keyStoreCryptoMock.ensureKey() } verify(exactly = 0) { keyStoreCryptoMock.ensureKey() }
} }

View file

@ -18,6 +18,7 @@ package im.vector.app.features.pin.lockscreen.crypto.migrations
import android.security.keystore.UserNotAuthenticatedException import android.security.keystore.UserNotAuthenticatedException
import im.vector.app.features.pin.lockscreen.crypto.KeyStoreCrypto import im.vector.app.features.pin.lockscreen.crypto.KeyStoreCrypto
import im.vector.app.features.settings.VectorPreferences
import io.mockk.every import io.mockk.every
import io.mockk.mockk import io.mockk.mockk
import io.mockk.verify import io.mockk.verify
@ -31,7 +32,8 @@ class SystemKeyV1MigratorTests {
private val keyStoreCryptoFactory = mockk<KeyStoreCrypto.Factory>() private val keyStoreCryptoFactory = mockk<KeyStoreCrypto.Factory>()
private val keyStore = mockk<KeyStore>(relaxed = true) private val keyStore = mockk<KeyStore>(relaxed = true)
private val systemKeyV1Migrator = SystemKeyV1Migrator("vector.system_new", keyStore, keyStoreCryptoFactory) private val vectorPreferences = mockk<VectorPreferences>(relaxed = true)
private val systemKeyV1Migrator = SystemKeyV1Migrator("vector.system_new", keyStore, keyStoreCryptoFactory, vectorPreferences)
@Test @Test
fun isMigrationNeededReturnsTrueIfV1KeyExists() { fun isMigrationNeededReturnsTrueIfV1KeyExists() {