Disable 'Enable biometrics' option if there are not biometric authenticators enrolled. (#6714)

* Disable 'Enable biometrics' option if there are not biometric authenticators enrolled.

* Improve biometric pref enabled check

* Fix changelog issue

* Address review comments. Add extra catch clauses to key migrations.

* Add tests for key migrators
This commit is contained in:
Jorge Martin Espinosa 2022-08-02 15:31:08 +02:00 committed by GitHub
parent 1497650146
commit c848615636
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 57 additions and 15 deletions

1
changelog.d/6713.bugfix Normal file
View File

@ -0,0 +1 @@
Disable 'Enable biometrics' option if there are not biometric authenticators enrolled.

View File

@ -18,6 +18,7 @@ package im.vector.app.features.pin.lockscreen.crypto
import android.os.Build import android.os.Build
import android.security.keystore.KeyPermanentlyInvalidatedException import android.security.keystore.KeyPermanentlyInvalidatedException
import android.security.keystore.UserNotAuthenticatedException
import androidx.test.platform.app.InstrumentationRegistry import androidx.test.platform.app.InstrumentationRegistry
import im.vector.app.TestBuildVersionSdkIntProvider import im.vector.app.TestBuildVersionSdkIntProvider
import io.mockk.every import io.mockk.every
@ -69,10 +70,12 @@ class KeyStoreCryptoTests {
runCatching { keyStoreCrypto.ensureKey() } runCatching { keyStoreCrypto.ensureKey() }
keyStoreCrypto.hasValidKey() shouldBe true keyStoreCrypto.hasValidKey() shouldBe true
val exception = KeyPermanentlyInvalidatedException() val keyInvalidatedException = KeyPermanentlyInvalidatedException()
every { secretStoringUtils.getEncryptCipher(any()) } throws exception every { secretStoringUtils.getEncryptCipher(any()) } throws keyInvalidatedException
keyStoreCrypto.hasValidKey() shouldBe false
runCatching { keyStoreCrypto.ensureKey() } val userNotAuthenticatedException = UserNotAuthenticatedException()
every { secretStoringUtils.getEncryptCipher(any()) } throws userNotAuthenticatedException
keyStoreCrypto.hasValidKey() shouldBe false keyStoreCrypto.hasValidKey() shouldBe false
} }

View File

@ -20,6 +20,7 @@ import android.annotation.SuppressLint
import android.content.Context import android.content.Context
import android.os.Build import android.os.Build
import android.security.keystore.KeyPermanentlyInvalidatedException import android.security.keystore.KeyPermanentlyInvalidatedException
import android.security.keystore.UserNotAuthenticatedException
import android.util.Base64 import android.util.Base64
import androidx.annotation.VisibleForTesting import androidx.annotation.VisibleForTesting
import androidx.biometric.BiometricPrompt import androidx.biometric.BiometricPrompt
@ -117,6 +118,8 @@ class KeyStoreCrypto @AssistedInject constructor(
true true
} catch (e: KeyPermanentlyInvalidatedException) { } catch (e: KeyPermanentlyInvalidatedException) {
false false
} catch (e: UserNotAuthenticatedException) {
false
} }
} else { } else {
keyExists keyExists

View File

@ -19,6 +19,7 @@ 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.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
@ -45,7 +46,9 @@ class MissingSystemKeyMigrator @Inject constructor(
try { try {
keystoreCryptoFactory.provide(systemKeyAlias, true).ensureKey() keystoreCryptoFactory.provide(systemKeyAlias, true).ensureKey()
} catch (e: KeyPermanentlyInvalidatedException) { } catch (e: KeyPermanentlyInvalidatedException) {
Timber.e("Could not automatically create biometric key.", e) Timber.e("Could not automatically create biometric key because it was invalidated.", e)
} catch (e: UserNotAuthenticatedException) {
Timber.e("Could not automatically create biometric key because there are no enrolled biometric authenticators.", e)
} }
} }
} }

View File

@ -17,9 +17,11 @@
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 timber.log.Timber
import java.security.KeyStore import java.security.KeyStore
import javax.inject.Inject import javax.inject.Inject
@ -39,7 +41,11 @@ 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)
systemKeyStoreCrypto.ensureKey() try {
systemKeyStoreCrypto.ensureKey()
} catch (e: UserNotAuthenticatedException) {
Timber.e("Could not migrate v1 biometric key because there are no enrolled biometric authenticators.", e)
}
} }
/** /**

View File

@ -59,7 +59,7 @@ class LockScreenFragment : VectorBaseFragment<FragmentLockScreenBinding>() {
if (state.lockScreenConfiguration.mode == LockScreenMode.CREATE) return@withState if (state.lockScreenConfiguration.mode == LockScreenMode.CREATE) return@withState
viewLifecycleOwner.lifecycleScope.launchWhenResumed { viewLifecycleOwner.lifecycleScope.launchWhenResumed {
if (state.isBiometricKeyInvalidated) { if (state.canUseBiometricAuth && state.isBiometricKeyInvalidated) {
lockScreenListener?.onBiometricKeyInvalidated() lockScreenListener?.onBiometricKeyInvalidated()
} else if (state.showBiometricPromptAutomatically) { } else if (state.showBiometricPromptAutomatically) {
showBiometricPrompt() showBiometricPrompt()

View File

@ -60,17 +60,16 @@ class VectorSettingsPinFragment @Inject constructor(
findPreference<SwitchPreference>(VectorPreferences.SETTINGS_SECURITY_USE_BIOMETRICS_FLAG)!! findPreference<SwitchPreference>(VectorPreferences.SETTINGS_SECURITY_USE_BIOMETRICS_FLAG)!!
} }
private fun shouldCheckBiometricPref(isPinCodeChecked: Boolean): Boolean { private fun updateBiometricPrefState(isPinCodeChecked: Boolean) {
return isPinCodeChecked && // Biometric auth depends on PIN auth // Biometric auth depends on PIN auth
biometricHelper.isSystemAuthEnabledAndValid && useBiometricPref.isEnabled = isPinCodeChecked && biometricHelper.canUseAnySystemAuth
biometricHelper.isSystemKeyValid useBiometricPref.isChecked = isPinCodeChecked && biometricHelper.isSystemAuthEnabledAndValid
} }
override fun onResume() { override fun onResume() {
super.onResume() super.onResume()
useBiometricPref.isEnabled = usePinCodePref.isChecked updateBiometricPrefState(isPinCodeChecked = usePinCodePref.isChecked)
useBiometricPref.isChecked = shouldCheckBiometricPref(usePinCodePref.isChecked)
} }
override fun bindPref() { override fun bindPref() {
@ -78,8 +77,7 @@ class VectorSettingsPinFragment @Inject constructor(
usePinCodePref.setOnPreferenceChangeListener { _, value -> usePinCodePref.setOnPreferenceChangeListener { _, value ->
val isChecked = (value as? Boolean).orFalse() val isChecked = (value as? Boolean).orFalse()
useBiometricPref.isEnabled = isChecked updateBiometricPrefState(isPinCodeChecked = isChecked)
useBiometricPref.isChecked = shouldCheckBiometricPref(isChecked)
if (!isChecked) { if (!isChecked) {
disableBiometricAuthentication() disableBiometricAuthentication()
} }
@ -104,7 +102,7 @@ class VectorSettingsPinFragment @Inject constructor(
}.onFailure { }.onFailure {
showEnableBiometricErrorMessage() showEnableBiometricErrorMessage()
} }
useBiometricPref.isChecked = shouldCheckBiometricPref(usePinCodePref.isChecked) updateBiometricPrefState(isPinCodeChecked = usePinCodePref.isChecked)
} }
false false
} else { } else {

View File

@ -18,6 +18,7 @@ package im.vector.app.features.pin.lockscreen.crypto.migrations
import android.os.Build import android.os.Build
import android.security.keystore.KeyPermanentlyInvalidatedException 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.settings.VectorPreferences import im.vector.app.features.settings.VectorPreferences
import im.vector.app.test.TestBuildVersionSdkIntProvider import im.vector.app.test.TestBuildVersionSdkIntProvider
@ -59,6 +60,17 @@ class MissingSystemKeyMigratorTests {
invoking { missingSystemKeyMigrator.migrate() } shouldNotThrow KeyPermanentlyInvalidatedException::class invoking { missingSystemKeyMigrator.migrate() } shouldNotThrow KeyPermanentlyInvalidatedException::class
} }
@Test
fun migrateHandlesUserNotAuthenticatedExceptions() {
val keyStoreCryptoMock = mockk<KeyStoreCrypto> {
every { ensureKey() } throws UserNotAuthenticatedException()
}
every { keyStoreCryptoFactory.provide(any(), any()) } returns keyStoreCryptoMock
every { vectorPreferences.useBiometricsToUnlock() } returns true
invoking { missingSystemKeyMigrator.migrate() } shouldNotThrow UserNotAuthenticatedException::class
}
@Test @Test
fun migrateReturnsEarlyIfBiometricAuthIsDisabled() { fun migrateReturnsEarlyIfBiometricAuthIsDisabled() {
val keyStoreCryptoMock = mockk<KeyStoreCrypto> { val keyStoreCryptoMock = mockk<KeyStoreCrypto> {

View File

@ -16,11 +16,14 @@
package im.vector.app.features.pin.lockscreen.crypto.migrations package im.vector.app.features.pin.lockscreen.crypto.migrations
import android.security.keystore.UserNotAuthenticatedException
import im.vector.app.features.pin.lockscreen.crypto.KeyStoreCrypto import im.vector.app.features.pin.lockscreen.crypto.KeyStoreCrypto
import io.mockk.every import io.mockk.every
import io.mockk.mockk import io.mockk.mockk
import io.mockk.verify import io.mockk.verify
import org.amshove.kluent.invoking
import org.amshove.kluent.shouldBe import org.amshove.kluent.shouldBe
import org.amshove.kluent.shouldNotThrow
import org.junit.Test import org.junit.Test
import java.security.KeyStore import java.security.KeyStore
@ -39,6 +42,19 @@ class SystemKeyV1MigratorTests {
systemKeyV1Migrator.isMigrationNeeded() shouldBe false systemKeyV1Migrator.isMigrationNeeded() shouldBe false
} }
@Test
fun migrateHandlesUserNotAuthenticatedException() {
val keyStoreCryptoMock = mockk<KeyStoreCrypto> {
every { ensureKey() } throws UserNotAuthenticatedException()
}
every { keyStoreCryptoFactory.provide("vector.system_new", any()) } returns keyStoreCryptoMock
invoking { systemKeyV1Migrator.migrate() } shouldNotThrow UserNotAuthenticatedException::class
verify { keyStore.deleteEntry(SystemKeyV1Migrator.SYSTEM_KEY_ALIAS_V1) }
verify { keyStoreCryptoMock.ensureKey() }
}
@Test @Test
fun migrateDeletesOldEntryAndEnsuresNewKey() { fun migrateDeletesOldEntryAndEnsuresNewKey() {
val keyStoreCryptoMock = mockk<KeyStoreCrypto> { val keyStoreCryptoMock = mockk<KeyStoreCrypto> {