diff --git a/changelog.d/6522.feature b/changelog.d/6522.feature new file mode 100644 index 0000000000..fb5e535108 --- /dev/null +++ b/changelog.d/6522.feature @@ -0,0 +1 @@ +Improve lock screen implementation with extra security measures diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/securestorage/SecretStoringUtils.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/securestorage/SecretStoringUtils.kt index bd2a1078b2..e701e0f3ba 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/securestorage/SecretStoringUtils.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/securestorage/SecretStoringUtils.kt @@ -180,11 +180,11 @@ class SecretStoringUtils @Inject constructor( is KeyStore.PrivateKeyEntry -> keyEntry.certificate.publicKey else -> throw IllegalStateException("Unknown KeyEntry type.") } - val cipherMode = when { + val cipherAlgorithm = when { buildVersionSdkIntProvider.get() >= Build.VERSION_CODES.M -> AES_MODE else -> RSA_MODE } - val cipher = Cipher.getInstance(cipherMode) + val cipher = Cipher.getInstance(cipherAlgorithm) cipher.init(Cipher.ENCRYPT_MODE, key) return cipher } @@ -204,13 +204,17 @@ class SecretStoringUtils @Inject constructor( .setBlockModes(KeyProperties.BLOCK_MODE_GCM) .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_NONE) .setKeySize(128) + .setUserAuthenticationRequired(keyNeedsUserAuthentication) .apply { - setUserAuthenticationRequired(keyNeedsUserAuthentication) - if (buildVersionSdkIntProvider.get() >= Build.VERSION_CODES.N) { - setInvalidatedByBiometricEnrollment(true) + if (keyNeedsUserAuthentication) { + buildVersionSdkIntProvider.whenAtLeast(Build.VERSION_CODES.N) { + setInvalidatedByBiometricEnrollment(true) + } + buildVersionSdkIntProvider.whenAtLeast(Build.VERSION_CODES.P) { + setUnlockedDeviceRequired(true) + } } } - .setUserAuthenticationRequired(keyNeedsUserAuthentication) .build() generator.init(keyGenSpec) return generator.generateKey() diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/util/BuildVersionSdkIntProvider.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/util/BuildVersionSdkIntProvider.kt index b7ea187ec5..900a2e237f 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/util/BuildVersionSdkIntProvider.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/util/BuildVersionSdkIntProvider.kt @@ -21,4 +21,14 @@ interface BuildVersionSdkIntProvider { * Return the current version of the Android SDK. */ fun get(): Int + + /** + * Checks the if the current OS version is equal or greater than [version]. + * @return A `non-null` result if true, `null` otherwise. + */ + fun whenAtLeast(version: Int, result: () -> T): T? { + return if (get() >= version) { + result() + } else null + } } diff --git a/vector/src/androidTest/java/im/vector/app/features/pin/lockscreen/biometrics/BiometricHelperTests.kt b/vector/src/androidTest/java/im/vector/app/features/pin/lockscreen/biometrics/BiometricHelperTests.kt index b519d2f623..ac9b8a58bf 100644 --- a/vector/src/androidTest/java/im/vector/app/features/pin/lockscreen/biometrics/BiometricHelperTests.kt +++ b/vector/src/androidTest/java/im/vector/app/features/pin/lockscreen/biometrics/BiometricHelperTests.kt @@ -24,6 +24,7 @@ import androidx.biometric.BiometricManager.Authenticators.BIOMETRIC_WEAK import androidx.biometric.BiometricManager.Authenticators.DEVICE_CREDENTIAL import androidx.biometric.BiometricManager.BIOMETRIC_ERROR_NONE_ENROLLED import androidx.biometric.BiometricManager.BIOMETRIC_SUCCESS +import androidx.biometric.BiometricPrompt import androidx.lifecycle.lifecycleScope import androidx.test.core.app.ActivityScenario import androidx.test.platform.app.InstrumentationRegistry @@ -31,6 +32,7 @@ import im.vector.app.TestBuildVersionSdkIntProvider import im.vector.app.features.pin.lockscreen.configuration.LockScreenConfiguration import im.vector.app.features.pin.lockscreen.configuration.LockScreenConfiguratorProvider import im.vector.app.features.pin.lockscreen.configuration.LockScreenMode +import im.vector.app.features.pin.lockscreen.crypto.LockScreenCryptoConstants import im.vector.app.features.pin.lockscreen.crypto.LockScreenKeyRepository import im.vector.app.features.pin.lockscreen.tests.LockScreenTestActivity import im.vector.app.features.pin.lockscreen.ui.fallbackprompt.FallbackBiometricDialogFragment @@ -56,6 +58,9 @@ import org.amshove.kluent.shouldBeTrue import org.junit.Before import org.junit.Ignore import org.junit.Test +import org.matrix.android.sdk.api.securestorage.SecretStoringUtils +import java.security.KeyStore +import java.util.UUID import java.util.concurrent.CountDownLatch import java.util.concurrent.TimeUnit @@ -64,6 +69,13 @@ class BiometricHelperTests { private val biometricManager = mockk(relaxed = true) private val lockScreenKeyRepository = mockk(relaxed = true) private val buildVersionSdkIntProvider = TestBuildVersionSdkIntProvider() + private val keyStore = KeyStore.getInstance(LockScreenCryptoConstants.ANDROID_KEY_STORE).also { it.load(null) } + private val secretStoringUtils = SecretStoringUtils( + InstrumentationRegistry.getInstrumentation().targetContext, + keyStore, + buildVersionSdkIntProvider, + false, + ) @Before fun setup() { @@ -190,6 +202,7 @@ class BiometricHelperTests { @OptIn(ExperimentalCoroutinesApi::class) @Test fun authenticateInDeviceWithIssuesShowsFallbackPromptDialog() = runTest { + buildVersionSdkIntProvider.value = Build.VERSION_CODES.M mockkStatic("kotlinx.coroutines.flow.FlowKt") val mockAuthChannel: Channel = mockk(relaxed = true) { // Empty flow to keep the dialog open @@ -201,6 +214,9 @@ class BiometricHelperTests { mockkObject(DevicePromptCheck) every { DevicePromptCheck.isDeviceWithNoBiometricUI } returns true every { lockScreenKeyRepository.isSystemKeyValid() } returns true + + val keyAlias = UUID.randomUUID().toString() + every { biometricUtils.getAuthCryptoObject() } returns BiometricPrompt.CryptoObject(secretStoringUtils.getEncryptCipher(keyAlias)) val latch = CountDownLatch(1) val intent = Intent(InstrumentationRegistry.getInstrumentation().targetContext, LockScreenTestActivity::class.java) with(ActivityScenario.launch(intent)) { @@ -214,6 +230,7 @@ class BiometricHelperTests { } } latch.await(1, TimeUnit.SECONDS) + keyStore.deleteEntry(keyAlias) unmockkObject(DevicePromptCheck) unmockkStatic("kotlinx.coroutines.flow.FlowKt") } diff --git a/vector/src/androidTest/java/im/vector/app/features/pin/lockscreen/crypto/KeyStoreCryptoTests.kt b/vector/src/androidTest/java/im/vector/app/features/pin/lockscreen/crypto/KeyStoreCryptoTests.kt index 68e1244791..6e02cc0262 100644 --- a/vector/src/androidTest/java/im/vector/app/features/pin/lockscreen/crypto/KeyStoreCryptoTests.kt +++ b/vector/src/androidTest/java/im/vector/app/features/pin/lockscreen/crypto/KeyStoreCryptoTests.kt @@ -43,7 +43,9 @@ class KeyStoreCryptoTests { private val versionProvider = TestBuildVersionSdkIntProvider().also { it.value = Build.VERSION_CODES.M } private val secretStoringUtils = spyk(SecretStoringUtils(context, keyStore, versionProvider)) private val keyStoreCrypto = spyk( - KeyStoreCrypto(alias, false, context, versionProvider, keyStore, secretStoringUtils) + KeyStoreCrypto(alias, false, context, versionProvider, keyStore).also { + it.secretStoringUtils = secretStoringUtils + } ) @After @@ -146,10 +148,10 @@ class KeyStoreCryptoTests { @Test fun getCryptoObjectUsesCipherFromSecretStoringUtils() { - keyStoreCrypto.getCryptoObject() + keyStoreCrypto.getAuthCryptoObject() verify { secretStoringUtils.getEncryptCipher(any()) } every { secretStoringUtils.getEncryptCipher(any()) } throws KeyPermanentlyInvalidatedException() - invoking { keyStoreCrypto.getCryptoObject() } shouldThrow KeyPermanentlyInvalidatedException::class + invoking { keyStoreCrypto.getAuthCryptoObject() } shouldThrow KeyPermanentlyInvalidatedException::class } } diff --git a/vector/src/androidTest/java/im/vector/app/features/pin/lockscreen/crypto/LockScreenKeyRepositoryTests.kt b/vector/src/androidTest/java/im/vector/app/features/pin/lockscreen/crypto/LockScreenKeyRepositoryTests.kt index 23eefe6577..924dbfee9e 100644 --- a/vector/src/androidTest/java/im/vector/app/features/pin/lockscreen/crypto/LockScreenKeyRepositoryTests.kt +++ b/vector/src/androidTest/java/im/vector/app/features/pin/lockscreen/crypto/LockScreenKeyRepositoryTests.kt @@ -16,21 +16,16 @@ package im.vector.app.features.pin.lockscreen.crypto -import android.security.keystore.KeyPermanentlyInvalidatedException import androidx.test.platform.app.InstrumentationRegistry +import im.vector.app.features.pin.lockscreen.crypto.migrations.LegacyPinCodeMigrator import im.vector.app.features.settings.VectorPreferences import io.mockk.clearAllMocks -import io.mockk.coVerify import io.mockk.every import io.mockk.mockk import io.mockk.spyk -import io.mockk.verify -import kotlinx.coroutines.test.runTest -import org.amshove.kluent.coInvoking import org.amshove.kluent.shouldBeEqualTo import org.amshove.kluent.shouldBeFalse import org.amshove.kluent.shouldBeTrue -import org.amshove.kluent.shouldNotThrow import org.junit.After import org.junit.Before import org.junit.Test @@ -49,7 +44,7 @@ class LockScreenKeyRepositoryTests { } private lateinit var lockScreenKeyRepository: LockScreenKeyRepository - private val pinCodeMigrator: PinCodeMigrator = mockk(relaxed = true) + private val legacyPinCodeMigrator: LegacyPinCodeMigrator = mockk(relaxed = true) private val vectorPreferences: VectorPreferences = mockk(relaxed = true) private val keyStore: KeyStore by lazy { @@ -58,7 +53,7 @@ class LockScreenKeyRepositoryTests { @Before fun setup() { - lockScreenKeyRepository = spyk(LockScreenKeyRepository("base", pinCodeMigrator, vectorPreferences, keyStoreCryptoFactory)) + lockScreenKeyRepository = spyk(LockScreenKeyRepository("base.pin_code", "base.system", keyStoreCryptoFactory)) } @After @@ -141,44 +136,4 @@ class LockScreenKeyRepositoryTests { lockScreenKeyRepository.hasPinCodeKey().shouldBeFalse() } - - @Test - fun migrateKeysIfNeededReturnsEarlyIfNotNeeded() = runTest { - every { pinCodeMigrator.isMigrationNeeded() } returns false - - lockScreenKeyRepository.migrateKeysIfNeeded() - - coVerify(exactly = 0) { pinCodeMigrator.migrate(any()) } - } - - @Test - fun migrateKeysIfNeededWillMigratePinCodeAndKeys() = runTest { - every { pinCodeMigrator.isMigrationNeeded() } returns true - - lockScreenKeyRepository.migrateKeysIfNeeded() - - coVerify { pinCodeMigrator.migrate(any()) } - } - - @Test - fun migrateKeysIfNeededWillCreateSystemKeyIfNeeded() = runTest { - every { pinCodeMigrator.isMigrationNeeded() } returns true - every { vectorPreferences.useBiometricsToUnlock() } returns true - every { lockScreenKeyRepository.ensureSystemKey() } returns mockk() - - lockScreenKeyRepository.migrateKeysIfNeeded() - - verify { lockScreenKeyRepository.ensureSystemKey() } - } - - @Test - fun migrateKeysIfNeededWillHandleKeyPermanentlyInvalidatedException() = runTest { - every { pinCodeMigrator.isMigrationNeeded() } returns true - every { vectorPreferences.useBiometricsToUnlock() } returns true - every { lockScreenKeyRepository.ensureSystemKey() } throws KeyPermanentlyInvalidatedException() - - coInvoking { lockScreenKeyRepository.migrateKeysIfNeeded() } shouldNotThrow KeyPermanentlyInvalidatedException::class - - verify { lockScreenKeyRepository.ensureSystemKey() } - } } diff --git a/vector/src/androidTest/java/im/vector/app/features/pin/lockscreen/crypto/PinCodeMigratorTests.kt b/vector/src/androidTest/java/im/vector/app/features/pin/lockscreen/crypto/migrations/LegacyPinCodeMigratorTests.kt similarity index 89% rename from vector/src/androidTest/java/im/vector/app/features/pin/lockscreen/crypto/PinCodeMigratorTests.kt rename to vector/src/androidTest/java/im/vector/app/features/pin/lockscreen/crypto/migrations/LegacyPinCodeMigratorTests.kt index 297793c7a4..44c5db89c8 100644 --- a/vector/src/androidTest/java/im/vector/app/features/pin/lockscreen/crypto/PinCodeMigratorTests.kt +++ b/vector/src/androidTest/java/im/vector/app/features/pin/lockscreen/crypto/migrations/LegacyPinCodeMigratorTests.kt @@ -16,7 +16,7 @@ @file:Suppress("DEPRECATION") -package im.vector.app.features.pin.lockscreen.crypto +package im.vector.app.features.pin.lockscreen.crypto.migrations import android.os.Build import android.security.KeyPairGeneratorSpec @@ -57,7 +57,7 @@ import javax.crypto.spec.PSource import javax.security.auth.x500.X500Principal import kotlin.math.abs -class PinCodeMigratorTests { +class LegacyPinCodeMigratorTests { private val alias = UUID.randomUUID().toString() @@ -72,7 +72,9 @@ class PinCodeMigratorTests { private val secretStoringUtils: SecretStoringUtils = spyk( SecretStoringUtils(context, keyStore, buildVersionSdkIntProvider) ) - private val pinCodeMigrator = spyk(PinCodeMigrator(pinCodeStore, keyStore, secretStoringUtils, buildVersionSdkIntProvider)) + private val legacyPinCodeMigrator = spyk( + LegacyPinCodeMigrator(alias, pinCodeStore, keyStore, secretStoringUtils, buildVersionSdkIntProvider) + ) @After fun tearDown() { @@ -87,21 +89,21 @@ class PinCodeMigratorTests { @Test fun isMigrationNeededReturnsTrueIfLegacyKeyExists() { - pinCodeMigrator.isMigrationNeeded() shouldBe false + legacyPinCodeMigrator.isMigrationNeeded() shouldBe false generateLegacyKey() - pinCodeMigrator.isMigrationNeeded() shouldBe true + legacyPinCodeMigrator.isMigrationNeeded() shouldBe true } @Test fun migrateWillReturnEarlyIfPinCodeDoesNotExist() = runTest { - every { pinCodeMigrator.isMigrationNeeded() } returns false + every { legacyPinCodeMigrator.isMigrationNeeded() } returns false coEvery { pinCodeStore.getPinCode() } returns null - pinCodeMigrator.migrate(alias) + legacyPinCodeMigrator.migrate() - coVerify(exactly = 0) { pinCodeMigrator.getDecryptedPinCode() } + coVerify(exactly = 0) { legacyPinCodeMigrator.getDecryptedPinCode() } verify(exactly = 0) { secretStoringUtils.securelyStoreBytes(any(), any()) } coVerify(exactly = 0) { pinCodeStore.savePinCode(any()) } verify(exactly = 0) { keyStore.deleteEntry(LEGACY_PIN_CODE_KEY_ALIAS) } @@ -109,13 +111,13 @@ class PinCodeMigratorTests { @Test fun migrateWillReturnEarlyIfIsNotNeeded() = runTest { - every { pinCodeMigrator.isMigrationNeeded() } returns false - coEvery { pinCodeMigrator.getDecryptedPinCode() } returns "1234" + every { legacyPinCodeMigrator.isMigrationNeeded() } returns false + coEvery { legacyPinCodeMigrator.getDecryptedPinCode() } returns "1234" every { secretStoringUtils.securelyStoreBytes(any(), any()) } returns ByteArray(0) - pinCodeMigrator.migrate(alias) + legacyPinCodeMigrator.migrate() - coVerify(exactly = 0) { pinCodeMigrator.getDecryptedPinCode() } + coVerify(exactly = 0) { legacyPinCodeMigrator.getDecryptedPinCode() } verify(exactly = 0) { secretStoringUtils.securelyStoreBytes(any(), any()) } coVerify(exactly = 0) { pinCodeStore.savePinCode(any()) } verify(exactly = 0) { keyStore.deleteEntry(LEGACY_PIN_CODE_KEY_ALIAS) } @@ -126,9 +128,9 @@ class PinCodeMigratorTests { val pinCode = "1234" saveLegacyPinCode(pinCode) - pinCodeMigrator.migrate(alias) + legacyPinCodeMigrator.migrate() - coVerify { pinCodeMigrator.getDecryptedPinCode() } + coVerify { legacyPinCodeMigrator.getDecryptedPinCode() } verify { secretStoringUtils.securelyStoreBytes(any(), any()) } coVerify { pinCodeStore.savePinCode(any()) } verify { keyStore.deleteEntry(LEGACY_PIN_CODE_KEY_ALIAS) } @@ -145,9 +147,9 @@ class PinCodeMigratorTests { every { buildVersionSdkIntProvider.get() } returns Build.VERSION_CODES.LOLLIPOP saveLegacyPinCode(pinCode) - pinCodeMigrator.migrate(alias) + legacyPinCodeMigrator.migrate() - coVerify { pinCodeMigrator.getDecryptedPinCode() } + coVerify { legacyPinCodeMigrator.getDecryptedPinCode() } verify { secretStoringUtils.securelyStoreBytes(any(), any()) } coVerify { pinCodeStore.savePinCode(any()) } verify { keyStore.deleteEntry(LEGACY_PIN_CODE_KEY_ALIAS) } diff --git a/vector/src/main/java/im/vector/app/features/pin/lockscreen/biometrics/BiometricHelper.kt b/vector/src/main/java/im/vector/app/features/pin/lockscreen/biometrics/BiometricHelper.kt index a34b284193..ae4fa637b4 100644 --- a/vector/src/main/java/im/vector/app/features/pin/lockscreen/biometrics/BiometricHelper.kt +++ b/vector/src/main/java/im/vector/app/features/pin/lockscreen/biometrics/BiometricHelper.kt @@ -53,6 +53,7 @@ import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.launch import org.matrix.android.sdk.api.util.BuildVersionSdkIntProvider import java.security.KeyStore +import javax.crypto.Cipher import javax.inject.Inject import kotlin.coroutines.CoroutineContext @@ -74,22 +75,19 @@ class BiometricHelper @Inject constructor( * Returns true if a weak biometric method (i.e.: some face or iris unlock implementations) can be used. */ val canUseWeakBiometricAuth: Boolean - get() = - configuration.isWeakBiometricsEnabled && biometricManager.canAuthenticate(BIOMETRIC_WEAK) == BIOMETRIC_SUCCESS + get() = configuration.isWeakBiometricsEnabled && biometricManager.canAuthenticate(BIOMETRIC_WEAK) == BIOMETRIC_SUCCESS /** * Returns true if a strong biometric method (i.e.: fingerprint, some face or iris unlock implementations) can be used. */ val canUseStrongBiometricAuth: Boolean - get() = - configuration.isStrongBiometricsEnabled && biometricManager.canAuthenticate(BIOMETRIC_STRONG) == BIOMETRIC_SUCCESS + get() = configuration.isStrongBiometricsEnabled && biometricManager.canAuthenticate(BIOMETRIC_STRONG) == BIOMETRIC_SUCCESS /** * Returns true if the device credentials can be used to unlock (system pin code, password, pattern, etc.). */ val canUseDeviceCredentialsAuth: Boolean - get() = - configuration.isDeviceCredentialUnlockEnabled && biometricManager.canAuthenticate(DEVICE_CREDENTIAL) == BIOMETRIC_SUCCESS + get() = configuration.isDeviceCredentialUnlockEnabled && biometricManager.canAuthenticate(DEVICE_CREDENTIAL) == BIOMETRIC_SUCCESS /** * Returns true if any system authentication method (biometric weak/strong or device credentials) can be used. @@ -120,7 +118,7 @@ class BiometricHelper @Inject constructor( */ @MainThread fun enableAuthentication(activity: FragmentActivity): Flow { - return authenticateInternal(activity, checkSystemKeyExists = false, cryptoObject = null) + return authenticateInternal(activity, checkSystemKeyExists = false, cryptoObject = getAuthCryptoObject()) } /** @@ -140,7 +138,7 @@ class BiometricHelper @Inject constructor( */ @MainThread fun authenticate(activity: FragmentActivity): Flow { - return authenticateInternal(activity, checkSystemKeyExists = true, cryptoObject = null) + return authenticateInternal(activity, checkSystemKeyExists = true, cryptoObject = getAuthCryptoObject()) } /** @@ -157,9 +155,9 @@ class BiometricHelper @Inject constructor( @SuppressLint("NewApi") @OptIn(ExperimentalCoroutinesApi::class) private fun authenticateInternal( - activity: FragmentActivity, - checkSystemKeyExists: Boolean, - cryptoObject: BiometricPrompt.CryptoObject? = null, + activity: FragmentActivity, + checkSystemKeyExists: Boolean, + cryptoObject: BiometricPrompt.CryptoObject, ): Flow { if (checkSystemKeyExists && !isSystemAuthEnabledAndValid) return flowOf(false) @@ -193,9 +191,9 @@ class BiometricHelper @Inject constructor( @VisibleForTesting(otherwise = PRIVATE) internal fun authenticateWithPromptInternal( - activity: FragmentActivity, - cryptoObject: BiometricPrompt.CryptoObject? = null, - channel: Channel, + activity: FragmentActivity, + cryptoObject: BiometricPrompt.CryptoObject, + channel: Channel, ): BiometricPrompt { val executor = ContextCompat.getMainExecutor(context) val callback = createSuspendingAuthCallback(channel, executor.asCoroutineDispatcher()) @@ -214,15 +212,12 @@ class BiometricHelper @Inject constructor( } .setAllowedAuthenticators(authenticators) .build() + return BiometricPrompt(activity, executor, callback).also { showFallbackFragmentIfNeeded(activity, channel.receiveAsFlow(), executor.asCoroutineDispatcher()) { // For some reason this seems to be needed unless we want to receive a fragment transaction exception delay(1L) - if (cryptoObject != null) { - it.authenticate(promptInfo, cryptoObject) - } else { - it.authenticate(promptInfo) - } + it.authenticate(promptInfo, cryptoObject) } } } @@ -270,13 +265,28 @@ class BiometricHelper @Inject constructor( } override fun onAuthenticationSucceeded(result: BiometricPrompt.AuthenticationResult) { - scope.launch { - channel.send(true) - // Success is a terminal event, should close both the Channel and the CoroutineScope to free resources. - channel.close() - scope.cancel() + val cipher = result.cryptoObject?.cipher + if (isCipherValid(cipher)) { + scope.launch { + channel.send(true) + // Success is a terminal event, should close both the Channel and the CoroutineScope to free resources. + channel.close() + scope.cancel() + } + } else { + scope.launch { + channel.close(IllegalStateException("System key was not valid after authentication.")) + scope.cancel() + } } } + + private fun isCipherValid(cipher: Cipher?): Boolean { + if (cipher == null) return false + return runCatching { + cipher.doFinal("biometric_challenge".toByteArray()) + }.isSuccess + } } /** @@ -321,6 +331,9 @@ class BiometricHelper @Inject constructor( @VisibleForTesting(otherwise = PRIVATE) internal fun createAuthChannel(): Channel = Channel(capacity = 1, onBufferOverflow = BufferOverflow.DROP_OLDEST) + @VisibleForTesting(otherwise = PRIVATE) + internal fun getAuthCryptoObject(): BiometricPrompt.CryptoObject = lockScreenKeyRepository.getSystemKeyAuthCryptoObject() + companion object { private const val FALLBACK_BIOMETRIC_FRAGMENT_TAG = "fragment.biometric_fallback" } diff --git a/vector/src/main/java/im/vector/app/features/pin/lockscreen/crypto/KeyStoreCrypto.kt b/vector/src/main/java/im/vector/app/features/pin/lockscreen/crypto/KeyStoreCrypto.kt index 95266d7663..4c52fccbaa 100644 --- a/vector/src/main/java/im/vector/app/features/pin/lockscreen/crypto/KeyStoreCrypto.kt +++ b/vector/src/main/java/im/vector/app/features/pin/lockscreen/crypto/KeyStoreCrypto.kt @@ -18,11 +18,11 @@ package im.vector.app.features.pin.lockscreen.crypto import android.annotation.SuppressLint import android.content.Context -import android.hardware.biometrics.BiometricPrompt import android.os.Build import android.security.keystore.KeyPermanentlyInvalidatedException import android.util.Base64 -import androidx.annotation.RequiresApi +import androidx.annotation.VisibleForTesting +import androidx.biometric.BiometricPrompt import dagger.assisted.Assisted import dagger.assisted.AssistedFactory import dagger.assisted.AssistedInject @@ -40,8 +40,6 @@ class KeyStoreCrypto @AssistedInject constructor( context: Context, private val buildVersionSdkIntProvider: BuildVersionSdkIntProvider, private val keyStore: KeyStore, - // It's easier to test it this way - private val secretStoringUtils: SecretStoringUtils = SecretStoringUtils(context, keyStore, buildVersionSdkIntProvider, keyNeedsUserAuthentication) ) { @AssistedFactory @@ -49,6 +47,9 @@ class KeyStoreCrypto @AssistedInject constructor( fun provide(alias: String, keyNeedsUserAuthentication: Boolean): KeyStoreCrypto } + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + internal var secretStoringUtils: SecretStoringUtils = SecretStoringUtils(context, keyStore, buildVersionSdkIntProvider, keyNeedsUserAuthentication) + /** * Ensures a [Key] for the [alias] exists and validates it. * @throws KeyPermanentlyInvalidatedException if key is not valid. @@ -137,6 +138,5 @@ class KeyStoreCrypto @AssistedInject constructor( * @throws KeyPermanentlyInvalidatedException if key is invalidated. */ @Throws(KeyPermanentlyInvalidatedException::class) - @RequiresApi(Build.VERSION_CODES.P) - fun getCryptoObject() = BiometricPrompt.CryptoObject(secretStoringUtils.getEncryptCipher(alias)) + fun getAuthCryptoObject() = BiometricPrompt.CryptoObject(secretStoringUtils.getEncryptCipher(alias)) } diff --git a/vector/src/main/java/im/vector/app/features/pin/lockscreen/crypto/LockScreenKeyRepository.kt b/vector/src/main/java/im/vector/app/features/pin/lockscreen/crypto/LockScreenKeyRepository.kt index 4a7bce8a52..2581951789 100644 --- a/vector/src/main/java/im/vector/app/features/pin/lockscreen/crypto/LockScreenKeyRepository.kt +++ b/vector/src/main/java/im/vector/app/features/pin/lockscreen/crypto/LockScreenKeyRepository.kt @@ -19,22 +19,22 @@ package im.vector.app.features.pin.lockscreen.crypto import android.os.Build import android.security.keystore.KeyPermanentlyInvalidatedException import androidx.annotation.RequiresApi -import im.vector.app.features.settings.VectorPreferences -import timber.log.Timber +import androidx.biometric.BiometricPrompt +import im.vector.app.features.pin.lockscreen.di.BiometricKeyAlias +import im.vector.app.features.pin.lockscreen.di.PinCodeKeyAlias +import javax.inject.Inject +import javax.inject.Singleton /** * Class in charge of managing both the PIN code key and the system authentication keys. */ -class LockScreenKeyRepository( - baseName: String, - private val pinCodeMigrator: PinCodeMigrator, - private val vectorPreferences: VectorPreferences, +@Singleton +class LockScreenKeyRepository @Inject constructor( + @PinCodeKeyAlias private val pinCodeKeyAlias: String, + @BiometricKeyAlias private val systemKeyAlias: String, private val keyStoreCryptoFactory: KeyStoreCrypto.Factory, ) { - private val pinCodeKeyAlias = "$baseName.pin_code" - private val systemKeyAlias = "$baseName.system" - private val pinCodeCrypto: KeyStoreCrypto by lazy { keyStoreCryptoFactory.provide(pinCodeKeyAlias, keyNeedsUserAuthentication = false) } @@ -86,19 +86,7 @@ class LockScreenKeyRepository( fun isSystemKeyValid() = systemKeyCrypto.hasValidKey() /** - * Migrates the PIN code key and encrypted value to use a more secure cipher, also creates a new system key if needed. + * Returns a [BiometricPrompt.CryptoObject] for the system key. */ - suspend fun migrateKeysIfNeeded() { - if (pinCodeMigrator.isMigrationNeeded()) { - pinCodeMigrator.migrate(pinCodeKeyAlias) - - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M && vectorPreferences.useBiometricsToUnlock()) { - try { - ensureSystemKey() - } catch (e: KeyPermanentlyInvalidatedException) { - Timber.e("Could not automatically create biometric key.", e) - } - } - } - } + fun getSystemKeyAuthCryptoObject() = systemKeyCrypto.getAuthCryptoObject() } diff --git a/vector/src/main/java/im/vector/app/features/pin/lockscreen/crypto/LockScreenKeysMigrator.kt b/vector/src/main/java/im/vector/app/features/pin/lockscreen/crypto/LockScreenKeysMigrator.kt new file mode 100644 index 0000000000..68acfcebf3 --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/pin/lockscreen/crypto/LockScreenKeysMigrator.kt @@ -0,0 +1,50 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.pin.lockscreen.crypto + +import android.annotation.SuppressLint +import android.os.Build +import im.vector.app.features.pin.lockscreen.crypto.migrations.LegacyPinCodeMigrator +import im.vector.app.features.pin.lockscreen.crypto.migrations.MissingSystemKeyMigrator +import im.vector.app.features.pin.lockscreen.crypto.migrations.SystemKeyV1Migrator +import org.matrix.android.sdk.api.util.BuildVersionSdkIntProvider +import javax.inject.Inject + +/** + * Performs all migrations needed for the lock screen keys. + */ +class LockScreenKeysMigrator @Inject constructor( + private val legacyPinCodeMigrator: LegacyPinCodeMigrator, + private val missingSystemKeyMigrator: MissingSystemKeyMigrator, + private val systemKeyV1Migrator: SystemKeyV1Migrator, + private val versionProvider: BuildVersionSdkIntProvider, +) { + /** + * Performs any needed migrations in order. + */ + @SuppressLint("NewApi") + suspend fun migrateIfNeeded() { + if (legacyPinCodeMigrator.isMigrationNeeded()) { + legacyPinCodeMigrator.migrate() + missingSystemKeyMigrator.migrate() + } + + if (systemKeyV1Migrator.isMigrationNeeded() && versionProvider.get() >= Build.VERSION_CODES.M) { + systemKeyV1Migrator.migrate() + } + } +} diff --git a/vector/src/main/java/im/vector/app/features/pin/lockscreen/crypto/PinCodeMigrator.kt b/vector/src/main/java/im/vector/app/features/pin/lockscreen/crypto/migrations/LegacyPinCodeMigrator.kt similarity index 88% rename from vector/src/main/java/im/vector/app/features/pin/lockscreen/crypto/PinCodeMigrator.kt rename to vector/src/main/java/im/vector/app/features/pin/lockscreen/crypto/migrations/LegacyPinCodeMigrator.kt index 976ee48e5f..5d790ba01a 100644 --- a/vector/src/main/java/im/vector/app/features/pin/lockscreen/crypto/PinCodeMigrator.kt +++ b/vector/src/main/java/im/vector/app/features/pin/lockscreen/crypto/migrations/LegacyPinCodeMigrator.kt @@ -14,13 +14,14 @@ * limitations under the License. */ -package im.vector.app.features.pin.lockscreen.crypto +package im.vector.app.features.pin.lockscreen.crypto.migrations import android.os.Build import android.util.Base64 import androidx.annotation.VisibleForTesting import im.vector.app.features.pin.PinCodeStore import im.vector.app.features.pin.lockscreen.crypto.LockScreenCryptoConstants.LEGACY_PIN_CODE_KEY_ALIAS +import im.vector.app.features.pin.lockscreen.di.PinCodeKeyAlias import org.matrix.android.sdk.api.securestorage.SecretStoringUtils import org.matrix.android.sdk.api.util.BuildVersionSdkIntProvider import java.security.Key @@ -31,7 +32,8 @@ import javax.inject.Inject /** * Used to migrate from the old PIN code key ciphers to a more secure ones. */ -class PinCodeMigrator @Inject constructor( +class LegacyPinCodeMigrator @Inject constructor( + @PinCodeKeyAlias private val pinCodeKeyAlias: String, private val pinCodeStore: PinCodeStore, private val keyStore: KeyStore, private val secretStoringUtils: SecretStoringUtils, @@ -41,13 +43,13 @@ class PinCodeMigrator @Inject constructor( private val legacyKey: Key get() = keyStore.getKey(LEGACY_PIN_CODE_KEY_ALIAS, null) /** - * Migrates from the old ciphers and [LEGACY_PIN_CODE_KEY_ALIAS] to the [newAlias]. + * Migrates from the old ciphers and renames [LEGACY_PIN_CODE_KEY_ALIAS] to [pinCodeKeyAlias]. */ - suspend fun migrate(newAlias: String) { + suspend fun migrate() { if (!keyStore.containsAlias(LEGACY_PIN_CODE_KEY_ALIAS)) return val pinCode = getDecryptedPinCode() ?: return - val encryptedBytes = secretStoringUtils.securelyStoreBytes(pinCode.toByteArray(), newAlias) + val encryptedBytes = secretStoringUtils.securelyStoreBytes(pinCode.toByteArray(), pinCodeKeyAlias) val encryptedPinCode = Base64.encodeToString(encryptedBytes, Base64.NO_WRAP) pinCodeStore.savePinCode(encryptedPinCode) keyStore.deleteEntry(LEGACY_PIN_CODE_KEY_ALIAS) diff --git a/vector/src/main/java/im/vector/app/features/pin/lockscreen/crypto/migrations/MissingSystemKeyMigrator.kt b/vector/src/main/java/im/vector/app/features/pin/lockscreen/crypto/migrations/MissingSystemKeyMigrator.kt new file mode 100644 index 0000000000..f8d3520047 --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/pin/lockscreen/crypto/migrations/MissingSystemKeyMigrator.kt @@ -0,0 +1,52 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.pin.lockscreen.crypto.migrations + +import android.annotation.SuppressLint +import android.os.Build +import android.security.keystore.KeyPermanentlyInvalidatedException +import im.vector.app.features.pin.lockscreen.crypto.KeyStoreCrypto +import im.vector.app.features.pin.lockscreen.di.BiometricKeyAlias +import im.vector.app.features.settings.VectorPreferences +import org.matrix.android.sdk.api.util.BuildVersionSdkIntProvider +import timber.log.Timber +import javax.inject.Inject + +/** + * Creates a new system/biometric key when migrating from the old PFLockScreen implementation. + */ +class MissingSystemKeyMigrator @Inject constructor( + @BiometricKeyAlias private val systemKeyAlias: String, + private val keystoreCryptoFactory: KeyStoreCrypto.Factory, + private val vectorPreferences: VectorPreferences, + private val buildVersionSdkIntProvider: BuildVersionSdkIntProvider, +) { + + /** + * If user had biometric auth enabled, ensure system key exists, creating one if needed. + */ + @SuppressLint("NewApi") + fun migrate() { + if (buildVersionSdkIntProvider.get() >= Build.VERSION_CODES.M && vectorPreferences.useBiometricsToUnlock()) { + try { + keystoreCryptoFactory.provide(systemKeyAlias, true).ensureKey() + } catch (e: KeyPermanentlyInvalidatedException) { + Timber.e("Could not automatically create biometric key.", e) + } + } + } +} diff --git a/vector/src/main/java/im/vector/app/features/pin/lockscreen/crypto/migrations/SystemKeyV1Migrator.kt b/vector/src/main/java/im/vector/app/features/pin/lockscreen/crypto/migrations/SystemKeyV1Migrator.kt new file mode 100644 index 0000000000..25e9d24272 --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/pin/lockscreen/crypto/migrations/SystemKeyV1Migrator.kt @@ -0,0 +1,53 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.pin.lockscreen.crypto.migrations + +import android.os.Build +import androidx.annotation.RequiresApi +import im.vector.app.features.pin.lockscreen.crypto.KeyStoreCrypto +import im.vector.app.features.pin.lockscreen.di.BiometricKeyAlias +import java.security.KeyStore +import javax.inject.Inject + +/** + * Migrates from the v1 of the biometric/system key to the new format, adding extra security measures to the new key. + */ +class SystemKeyV1Migrator @Inject constructor( + @BiometricKeyAlias private val systemKeyAlias: String, + private val keyStore: KeyStore, + private val keystoreCryptoFactory: KeyStoreCrypto.Factory, +) { + + /** + * Removes the old v1 system key and creates a new system key. + */ + @RequiresApi(Build.VERSION_CODES.M) + fun migrate() { + keyStore.deleteEntry(SYSTEM_KEY_ALIAS_V1) + val systemKeyStoreCrypto = keystoreCryptoFactory.provide(systemKeyAlias, keyNeedsUserAuthentication = true) + systemKeyStoreCrypto.ensureKey() + } + + /** + * Checks if an entry with [SYSTEM_KEY_ALIAS_V1] exists in the [keyStore]. + */ + fun isMigrationNeeded() = keyStore.containsAlias(SYSTEM_KEY_ALIAS_V1) + + companion object { + internal const val SYSTEM_KEY_ALIAS_V1 = "vector.system" + } +} diff --git a/vector/src/main/java/im/vector/app/features/pin/lockscreen/di/LockScreenModule.kt b/vector/src/main/java/im/vector/app/features/pin/lockscreen/di/LockScreenModule.kt index ef8f03b9e5..fb333b96bb 100644 --- a/vector/src/main/java/im/vector/app/features/pin/lockscreen/di/LockScreenModule.kt +++ b/vector/src/main/java/im/vector/app/features/pin/lockscreen/di/LockScreenModule.kt @@ -30,35 +30,32 @@ import im.vector.app.core.di.MavericksViewModelKey import im.vector.app.features.pin.PinCodeStore import im.vector.app.features.pin.lockscreen.configuration.LockScreenConfiguration import im.vector.app.features.pin.lockscreen.configuration.LockScreenMode -import im.vector.app.features.pin.lockscreen.crypto.KeyStoreCrypto -import im.vector.app.features.pin.lockscreen.crypto.LockScreenKeyRepository -import im.vector.app.features.pin.lockscreen.crypto.PinCodeMigrator +import im.vector.app.features.pin.lockscreen.crypto.migrations.LegacyPinCodeMigrator import im.vector.app.features.pin.lockscreen.pincode.EncryptedPinCodeStorage import im.vector.app.features.pin.lockscreen.ui.LockScreenViewModel -import im.vector.app.features.settings.VectorPreferences import org.matrix.android.sdk.api.securestorage.SecretStoringUtils import org.matrix.android.sdk.api.util.BuildVersionSdkIntProvider import org.matrix.android.sdk.api.util.DefaultBuildVersionSdkIntProvider import java.security.KeyStore -import javax.inject.Singleton @Module @InstallIn(SingletonComponent::class) object LockScreenModule { + @Provides + @PinCodeKeyAlias + fun providePinCodeKeyAlias(): String = "vector.pin_code" + + @Provides + @BiometricKeyAlias + fun provideSystemKeyAlias(): String = "vector.system_v2" + @Provides fun provideKeyStore(): KeyStore = KeyStore.getInstance("AndroidKeyStore").also { it.load(null) } @Provides fun provideBuildVersionSdkIntProvider(): BuildVersionSdkIntProvider = DefaultBuildVersionSdkIntProvider() - @Provides - fun provideSecretStoringUtils( - @ApplicationContext context: Context, - keyStore: KeyStore, - buildVersionSdkIntProvider: BuildVersionSdkIntProvider, - ) = SecretStoringUtils(context, keyStore, buildVersionSdkIntProvider) - @Provides fun provideLockScreenConfig() = LockScreenConfiguration( mode = LockScreenMode.VERIFY, @@ -70,20 +67,22 @@ object LockScreenModule { ) @Provides - @Singleton - fun provideKeyRepository( - pinCodeMigrator: PinCodeMigrator, - vectorPreferences: VectorPreferences, - keyStoreCryptoFactory: KeyStoreCrypto.Factory, - ) = LockScreenKeyRepository( - baseName = "vector", - pinCodeMigrator, - vectorPreferences, - keyStoreCryptoFactory, - ) + fun provideBiometricManager(@ApplicationContext context: Context) = BiometricManager.from(context) @Provides - fun provideBiometricManager(@ApplicationContext context: Context) = BiometricManager.from(context) + fun provideLegacyPinCodeMigrator( + @PinCodeKeyAlias pinCodeKeyAlias: String, + context: Context, + pinCodeStore: PinCodeStore, + keyStore: KeyStore, + buildVersionSdkIntProvider: BuildVersionSdkIntProvider, + ) = LegacyPinCodeMigrator( + pinCodeKeyAlias, + pinCodeStore, + keyStore, + SecretStoringUtils(context, keyStore, buildVersionSdkIntProvider), + buildVersionSdkIntProvider, + ) } @Module diff --git a/vector/src/main/java/im/vector/app/features/pin/lockscreen/di/LockScreenQualifiers.kt b/vector/src/main/java/im/vector/app/features/pin/lockscreen/di/LockScreenQualifiers.kt new file mode 100644 index 0000000000..0954772772 --- /dev/null +++ b/vector/src/main/java/im/vector/app/features/pin/lockscreen/di/LockScreenQualifiers.kt @@ -0,0 +1,27 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.pin.lockscreen.di + +import javax.inject.Qualifier + +@Qualifier +@Retention(AnnotationRetention.BINARY) +annotation class PinCodeKeyAlias + +@Qualifier +@Retention(AnnotationRetention.BINARY) +annotation class BiometricKeyAlias diff --git a/vector/src/main/java/im/vector/app/features/pin/lockscreen/pincode/PinCodeHelper.kt b/vector/src/main/java/im/vector/app/features/pin/lockscreen/pincode/PinCodeHelper.kt index bb2861dcda..9b2c2efda5 100644 --- a/vector/src/main/java/im/vector/app/features/pin/lockscreen/pincode/PinCodeHelper.kt +++ b/vector/src/main/java/im/vector/app/features/pin/lockscreen/pincode/PinCodeHelper.kt @@ -55,11 +55,4 @@ class PinCodeHelper @Inject constructor( encryptedStorage.deletePinCode() lockScreenKeyRepository.deletePinCodeKey() } - - /** - * Migrates the PIN code key and encrypted value to use a more secure cipher. - */ - suspend fun migratePinCodeIfNeeded() { - lockScreenKeyRepository.migrateKeysIfNeeded() - } } diff --git a/vector/src/main/java/im/vector/app/features/pin/lockscreen/ui/LockScreenViewModel.kt b/vector/src/main/java/im/vector/app/features/pin/lockscreen/ui/LockScreenViewModel.kt index 39d0937323..79c1967670 100644 --- a/vector/src/main/java/im/vector/app/features/pin/lockscreen/ui/LockScreenViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/pin/lockscreen/ui/LockScreenViewModel.kt @@ -34,6 +34,7 @@ import im.vector.app.features.pin.lockscreen.biometrics.BiometricHelper import im.vector.app.features.pin.lockscreen.configuration.LockScreenConfiguration import im.vector.app.features.pin.lockscreen.configuration.LockScreenConfiguratorProvider import im.vector.app.features.pin.lockscreen.configuration.LockScreenMode +import im.vector.app.features.pin.lockscreen.crypto.LockScreenKeysMigrator import im.vector.app.features.pin.lockscreen.pincode.PinCodeHelper import kotlinx.coroutines.flow.catch import kotlinx.coroutines.flow.emitAll @@ -47,6 +48,7 @@ class LockScreenViewModel @AssistedInject constructor( @Assisted val initialState: LockScreenViewState, private val pinCodeHelper: PinCodeHelper, private val biometricHelper: BiometricHelper, + private val lockScreenKeysMigrator: LockScreenKeysMigrator, private val configuratorProvider: LockScreenConfiguratorProvider, private val buildVersionSdkIntProvider: BuildVersionSdkIntProvider, ) : VectorViewModel(initialState) { @@ -85,7 +87,7 @@ class LockScreenViewModel @AssistedInject constructor( init { // We need this to run synchronously before we start reading the configurations - runBlocking { pinCodeHelper.migratePinCodeIfNeeded() } + runBlocking { lockScreenKeysMigrator.migrateIfNeeded() } configuratorProvider.configurationFlow .onEach { updateConfiguration(it) } diff --git a/vector/src/test/java/im/vector/app/features/pin/lockscreen/crypto/migrations/LockScreenTestMigratorTests.kt b/vector/src/test/java/im/vector/app/features/pin/lockscreen/crypto/migrations/LockScreenTestMigratorTests.kt new file mode 100644 index 0000000000..73f71dbf2b --- /dev/null +++ b/vector/src/test/java/im/vector/app/features/pin/lockscreen/crypto/migrations/LockScreenTestMigratorTests.kt @@ -0,0 +1,81 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.pin.lockscreen.crypto.migrations + +import android.os.Build +import im.vector.app.features.pin.lockscreen.crypto.LockScreenKeysMigrator +import im.vector.app.test.TestBuildVersionSdkIntProvider +import io.mockk.coVerify +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import kotlinx.coroutines.runBlocking +import org.junit.Test + +class LockScreenTestMigratorTests { + + private val legacyPinCodeMigrator = mockk(relaxed = true) + private val missingSystemKeyMigrator = mockk(relaxed = true) + private val systemKeyV1Migrator = mockk(relaxed = true) + private val versionProvider = TestBuildVersionSdkIntProvider() + private val migrator = LockScreenKeysMigrator(legacyPinCodeMigrator, missingSystemKeyMigrator, systemKeyV1Migrator, versionProvider) + + @Test + fun `When legacy pin code migration is needed, both legacyPinCodeMigrator and missingSystemKeyMigrator will be run`() { + // When no migration is needed + every { legacyPinCodeMigrator.isMigrationNeeded() } returns false + + runBlocking { migrator.migrateIfNeeded() } + + coVerify(exactly = 0) { legacyPinCodeMigrator.migrate() } + verify(exactly = 0) { missingSystemKeyMigrator.migrate() } + + // When migration is needed + every { legacyPinCodeMigrator.isMigrationNeeded() } returns true + + runBlocking { migrator.migrateIfNeeded() } + + coVerify { legacyPinCodeMigrator.migrate() } + verify { missingSystemKeyMigrator.migrate() } + } + + @Test + fun `System key from v1 migration will not be run for versions that don't support biometrics`() { + versionProvider.value = Build.VERSION_CODES.LOLLIPOP + every { systemKeyV1Migrator.isMigrationNeeded() } returns true + + runBlocking { migrator.migrateIfNeeded() } + + verify(exactly = 0) { systemKeyV1Migrator.migrate() } + } + + @Test + fun `When system key from v1 migration is needed it will be run`() { + versionProvider.value = Build.VERSION_CODES.M + every { systemKeyV1Migrator.isMigrationNeeded() } returns false + + runBlocking { migrator.migrateIfNeeded() } + + verify(exactly = 0) { systemKeyV1Migrator.migrate() } + + every { systemKeyV1Migrator.isMigrationNeeded() } returns true + + runBlocking { migrator.migrateIfNeeded() } + + verify { systemKeyV1Migrator.migrate() } + } +} diff --git a/vector/src/test/java/im/vector/app/features/pin/lockscreen/crypto/migrations/MissingSystemKeyMigratorTests.kt b/vector/src/test/java/im/vector/app/features/pin/lockscreen/crypto/migrations/MissingSystemKeyMigratorTests.kt new file mode 100644 index 0000000000..d65c3da2d2 --- /dev/null +++ b/vector/src/test/java/im/vector/app/features/pin/lockscreen/crypto/migrations/MissingSystemKeyMigratorTests.kt @@ -0,0 +1,88 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.pin.lockscreen.crypto.migrations + +import android.os.Build +import android.security.keystore.KeyPermanentlyInvalidatedException +import im.vector.app.features.pin.lockscreen.crypto.KeyStoreCrypto +import im.vector.app.features.settings.VectorPreferences +import im.vector.app.test.TestBuildVersionSdkIntProvider +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import org.amshove.kluent.invoking +import org.amshove.kluent.shouldNotThrow +import org.junit.Test + +class MissingSystemKeyMigratorTests { + + private val keyStoreCryptoFactory = mockk() + private val vectorPreferences = mockk(relaxed = true) + private val versionProvider = TestBuildVersionSdkIntProvider().also { it.value = Build.VERSION_CODES.M } + private val missingSystemKeyMigrator = MissingSystemKeyMigrator("vector.system", keyStoreCryptoFactory, vectorPreferences, versionProvider) + + @Test + fun migrateEnsuresSystemKeyExistsIfBiometricAuthIsEnabledAndSupported() { + val keyStoreCryptoMock = mockk { + every { ensureKey() } returns mockk() + } + every { keyStoreCryptoFactory.provide(any(), any()) } returns keyStoreCryptoMock + every { vectorPreferences.useBiometricsToUnlock() } returns true + + missingSystemKeyMigrator.migrate() + + verify { keyStoreCryptoMock.ensureKey() } + } + + @Test + fun migrateHandlesKeyPermanentlyInvalidatedExceptions() { + val keyStoreCryptoMock = mockk { + every { ensureKey() } throws KeyPermanentlyInvalidatedException() + } + every { keyStoreCryptoFactory.provide(any(), any()) } returns keyStoreCryptoMock + every { vectorPreferences.useBiometricsToUnlock() } returns true + + invoking { missingSystemKeyMigrator.migrate() } shouldNotThrow KeyPermanentlyInvalidatedException::class + } + + @Test + fun migrateReturnsEarlyIfBiometricAuthIsDisabled() { + val keyStoreCryptoMock = mockk { + every { ensureKey() } returns mockk() + } + every { keyStoreCryptoFactory.provide(any(), any()) } returns keyStoreCryptoMock + every { vectorPreferences.useBiometricsToUnlock() } returns false + + missingSystemKeyMigrator.migrate() + + verify(exactly = 0) { keyStoreCryptoMock.ensureKey() } + } + + @Test + fun migrateReturnsEarlyIfAndroidVersionCantHandleBiometrics() { + versionProvider.value = Build.VERSION_CODES.LOLLIPOP + val keyStoreCryptoMock = mockk { + every { ensureKey() } returns mockk() + } + every { keyStoreCryptoFactory.provide(any(), any()) } returns keyStoreCryptoMock + every { vectorPreferences.useBiometricsToUnlock() } returns false + + missingSystemKeyMigrator.migrate() + + verify(exactly = 0) { keyStoreCryptoMock.ensureKey() } + } +} diff --git a/vector/src/test/java/im/vector/app/features/pin/lockscreen/crypto/migrations/SystemKeyV1MigratorTests.kt b/vector/src/test/java/im/vector/app/features/pin/lockscreen/crypto/migrations/SystemKeyV1MigratorTests.kt new file mode 100644 index 0000000000..a519251398 --- /dev/null +++ b/vector/src/test/java/im/vector/app/features/pin/lockscreen/crypto/migrations/SystemKeyV1MigratorTests.kt @@ -0,0 +1,54 @@ +/* + * Copyright (c) 2022 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package im.vector.app.features.pin.lockscreen.crypto.migrations + +import im.vector.app.features.pin.lockscreen.crypto.KeyStoreCrypto +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import org.amshove.kluent.shouldBe +import org.junit.Test +import java.security.KeyStore + +class SystemKeyV1MigratorTests { + + private val keyStoreCryptoFactory = mockk() + private val keyStore = mockk(relaxed = true) + private val systemKeyV1Migrator = SystemKeyV1Migrator("vector.system_new", keyStore, keyStoreCryptoFactory) + + @Test + fun isMigrationNeededReturnsTrueIfV1KeyExists() { + every { keyStore.containsAlias(SystemKeyV1Migrator.SYSTEM_KEY_ALIAS_V1) } returns true + systemKeyV1Migrator.isMigrationNeeded() shouldBe true + + every { keyStore.containsAlias(SystemKeyV1Migrator.SYSTEM_KEY_ALIAS_V1) } returns false + systemKeyV1Migrator.isMigrationNeeded() shouldBe false + } + + @Test + fun migrateDeletesOldEntryAndEnsuresNewKey() { + val keyStoreCryptoMock = mockk { + every { ensureKey() } returns mockk() + } + every { keyStoreCryptoFactory.provide("vector.system_new", any()) } returns keyStoreCryptoMock + + systemKeyV1Migrator.migrate() + + verify { keyStore.deleteEntry(SystemKeyV1Migrator.SYSTEM_KEY_ALIAS_V1) } + verify { keyStoreCryptoMock.ensureKey() } + } +} diff --git a/vector/src/test/java/im/vector/app/features/pin/lockscreen/fragment/LockScreenViewModelTests.kt b/vector/src/test/java/im/vector/app/features/pin/lockscreen/fragment/LockScreenViewModelTests.kt index e551b7f563..9e80bb490b 100644 --- a/vector/src/test/java/im/vector/app/features/pin/lockscreen/fragment/LockScreenViewModelTests.kt +++ b/vector/src/test/java/im/vector/app/features/pin/lockscreen/fragment/LockScreenViewModelTests.kt @@ -25,6 +25,7 @@ import im.vector.app.features.pin.lockscreen.biometrics.BiometricHelper import im.vector.app.features.pin.lockscreen.configuration.LockScreenConfiguration import im.vector.app.features.pin.lockscreen.configuration.LockScreenConfiguratorProvider import im.vector.app.features.pin.lockscreen.configuration.LockScreenMode +import im.vector.app.features.pin.lockscreen.crypto.LockScreenKeysMigrator import im.vector.app.features.pin.lockscreen.pincode.PinCodeHelper import im.vector.app.features.pin.lockscreen.ui.AuthMethod import im.vector.app.features.pin.lockscreen.ui.LockScreenAction @@ -56,7 +57,8 @@ class LockScreenViewModelTests { private val pinCodeHelper = mockk(relaxed = true) private val biometricHelper = mockk(relaxed = true) - private val buildVersionSdkIntProvider = TestBuildVersionSdkIntProvider() + private val keysMigrator = mockk(relaxed = true) + private val versionProvider = TestBuildVersionSdkIntProvider() @Before fun setup() { @@ -67,9 +69,9 @@ class LockScreenViewModelTests { fun `init migrates old keys to new ones if needed`() { val initialState = createViewState() val configProvider = LockScreenConfiguratorProvider(createDefaultConfiguration()) - LockScreenViewModel(initialState, pinCodeHelper, biometricHelper, configProvider, buildVersionSdkIntProvider) + LockScreenViewModel(initialState, pinCodeHelper, biometricHelper, keysMigrator, configProvider, versionProvider) - coVerify { pinCodeHelper.migratePinCodeIfNeeded() } + coVerify { keysMigrator.migrateIfNeeded() } } @Test @@ -78,7 +80,7 @@ class LockScreenViewModelTests { val configProvider = LockScreenConfiguratorProvider(createDefaultConfiguration()) // This should set canUseBiometricAuth to true every { biometricHelper.isSystemAuthEnabledAndValid } returns true - val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelper, configProvider, buildVersionSdkIntProvider) + val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelper, keysMigrator, configProvider, versionProvider) val newState = withState(viewModel) { it } initialState shouldNotBeEqualTo newState } @@ -87,7 +89,7 @@ class LockScreenViewModelTests { fun `when onPinCodeEntered is called in VERIFY mode, the code is verified and the result is emitted as a ViewEvent`() = runTest { val initialState = createViewState() val configProvider = LockScreenConfiguratorProvider(createDefaultConfiguration()) - val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelper, configProvider, buildVersionSdkIntProvider) + val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelper, keysMigrator, configProvider, versionProvider) coEvery { pinCodeHelper.verifyPinCode(any()) } returns true val events = viewModel.test().viewEvents @@ -112,7 +114,7 @@ class LockScreenViewModelTests { val configuration = createDefaultConfiguration(mode = LockScreenMode.CREATE, needsNewCodeValidation = false) val initialState = createViewState(lockScreenConfiguration = configuration) val configProvider = LockScreenConfiguratorProvider(configuration) - val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelper, configProvider, buildVersionSdkIntProvider) + val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelper, keysMigrator, configProvider, versionProvider) val events = viewModel.test().viewEvents events.assertNoValues() @@ -128,7 +130,7 @@ class LockScreenViewModelTests { val configuration = createDefaultConfiguration(mode = LockScreenMode.CREATE, needsNewCodeValidation = true) val configProvider = LockScreenConfiguratorProvider(configuration) val initialState = createViewState(lockScreenConfiguration = configuration) - val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelper, configProvider, buildVersionSdkIntProvider) + val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelper, keysMigrator, configProvider, versionProvider) val events = viewModel.test().viewEvents events.assertNoValues() @@ -148,7 +150,7 @@ class LockScreenViewModelTests { val configuration = createDefaultConfiguration(mode = LockScreenMode.CREATE, needsNewCodeValidation = true) val initialState = createViewState(lockScreenConfiguration = configuration) val configProvider = LockScreenConfiguratorProvider(configuration) - val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelper, configProvider, buildVersionSdkIntProvider) + val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelper, keysMigrator, configProvider, versionProvider) val events = viewModel.test().viewEvents events.assertNoValues() @@ -169,7 +171,7 @@ class LockScreenViewModelTests { fun `onPinCodeEntered handles exceptions`() = runTest { val initialState = createViewState() val configProvider = LockScreenConfiguratorProvider(createDefaultConfiguration()) - val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelper, configProvider, buildVersionSdkIntProvider) + val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelper, keysMigrator, configProvider, versionProvider) val exception = IllegalStateException("Something went wrong") coEvery { pinCodeHelper.verifyPinCode(any()) } throws exception @@ -183,7 +185,7 @@ class LockScreenViewModelTests { @Test fun `when showBiometricPrompt catches a KeyPermanentlyInvalidatedException it disables biometric authentication`() = runTest { - buildVersionSdkIntProvider.value = Build.VERSION_CODES.M + versionProvider.value = Build.VERSION_CODES.M every { biometricHelper.isSystemAuthEnabledAndValid } returns true every { biometricHelper.isSystemKeyValid } returns true @@ -199,7 +201,7 @@ class LockScreenViewModelTests { isBiometricKeyInvalidated = false, lockScreenConfiguration = configuration ) - val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelper, configProvider, buildVersionSdkIntProvider) + val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelper, keysMigrator, configProvider, versionProvider) val events = viewModel.test().viewEvents events.assertNoValues() @@ -217,7 +219,7 @@ class LockScreenViewModelTests { @Test fun `when showBiometricPrompt receives an event it propagates it as a ViewEvent`() = runTest { val configProvider = LockScreenConfiguratorProvider(createDefaultConfiguration()) - val viewModel = LockScreenViewModel(createViewState(), pinCodeHelper, biometricHelper, configProvider, buildVersionSdkIntProvider) + val viewModel = LockScreenViewModel(createViewState(), pinCodeHelper, biometricHelper, keysMigrator, configProvider, versionProvider) coEvery { biometricHelper.authenticate(any()) } returns flowOf(false, true) val events = viewModel.test().viewEvents @@ -231,7 +233,7 @@ class LockScreenViewModelTests { @Test fun `showBiometricPrompt handles exceptions`() = runTest { val configProvider = LockScreenConfiguratorProvider(createDefaultConfiguration()) - val viewModel = LockScreenViewModel(createViewState(), pinCodeHelper, biometricHelper, configProvider, buildVersionSdkIntProvider) + val viewModel = LockScreenViewModel(createViewState(), pinCodeHelper, biometricHelper, keysMigrator, configProvider, versionProvider) val exception = IllegalStateException("Something went wrong") coEvery { biometricHelper.authenticate(any()) } throws exception