PM-17968: Create unique secret keys per user and handle decoding error (#4696)

app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt

@@ -1088,6 +1088,7 @@ class AuthRepositoryImpl(
                     }
 
                     is VaultUnlockResult.AuthenticationError,
+                    VaultUnlockResult.BiometricDecodingError,
                     VaultUnlockResult.InvalidStateError,
                     VaultUnlockResult.GenericError,
                         -> {

app/src/main/java/com/x8bit/bitwarden/data/auth/repository/model/LoginResultExtensions.kt

@@ -9,6 +9,7 @@ import com.x8bit.bitwarden.data.vault.repository.model.VaultUnlockResult
  */
 fun VaultUnlockError.toLoginErrorResult(): LoginResult.Error = when (this) {
     is VaultUnlockResult.AuthenticationError -> LoginResult.Error(this.message)
+    VaultUnlockResult.BiometricDecodingError,
     VaultUnlockResult.GenericError,
     VaultUnlockResult.InvalidStateError,
         -> LoginResult.Error(errorMessage = null)

app/src/main/java/com/x8bit/bitwarden/data/platform/manager/BiometricsEncryptionManager.kt

@@ -13,6 +13,11 @@ interface BiometricsEncryptionManager {
         userId: String,
     ): Cipher?
 
+    /**
+     * Clears the data associated with the users biometrics.
+     */
+    fun clearBiometrics(userId: String)
+
     /**
      * Gets the [Cipher] built from a keystore, or creates one if it doesn't already exist.
      */

app/src/main/java/com/x8bit/bitwarden/data/platform/manager/BiometricsEncryptionManagerImpl.kt

@@ -26,6 +26,7 @@ import javax.crypto.spec.IvParameterSpec
  * Default implementation of [BiometricsEncryptionManager] for managing Android keystore encryption
  * and decryption.
  */
+@Suppress("TooManyFunctions")
 @OmitFromCoverage
 class BiometricsEncryptionManagerImpl(
     private val authDiskSource: AuthDiskSource,
@@ -35,20 +36,8 @@ class BiometricsEncryptionManagerImpl(
         .getInstance(ENCRYPTION_KEYSTORE_NAME)
         .also { it.load(null) }
 
-    private val keyGenParameterSpec: KeyGenParameterSpec
-        get() = KeyGenParameterSpec
-            .Builder(
-                ENCRYPTION_KEY_NAME,
-                KeyProperties.PURPOSE_ENCRYPT or KeyProperties.PURPOSE_DECRYPT,
-            )
-            .setBlockModes(KeyProperties.BLOCK_MODE_CBC)
-            .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_PKCS7)
-            .setUserAuthenticationRequired(true)
-            .setInvalidatedByBiometricEnrollment(true)
-            .build()
-
     override fun createCipherOrNull(userId: String): Cipher? {
-        val secretKey: SecretKey = generateKeyOrNull()
+        val secretKey: SecretKey = generateKeyOrNull(userId = userId)
             ?: run {
                 // user removed all biometrics from the device
                 destroyBiometrics(userId = userId)
@@ -68,9 +57,25 @@ class BiometricsEncryptionManagerImpl(
         return cipher
     }
 
+    override fun clearBiometrics(userId: String) {
+        settingsDiskSource.systemBiometricIntegritySource?.let { systemBioIntegrityState ->
+            settingsDiskSource.storeAccountBiometricIntegrityValidity(
+                userId = userId,
+                systemBioIntegrityState = systemBioIntegrityState,
+                value = null,
+            )
+        }
+        authDiskSource.storeUserBiometricUnlockKey(userId = userId, biometricsKey = null)
+        authDiskSource.storeUserBiometricInitVector(userId = userId, iv = null)
+        keystore.deleteEntry(encryptionKeyName(userId = userId))
+    }
+
     override fun getOrCreateCipher(userId: String): Cipher? {
-        val secretKey: SecretKey = getSecretKeyOrNull()
-            ?: generateKeyOrNull()
+        // Attempt to get the user scoped key. If that fails, we check to see if a legacy key
+        // is available. If neither succeeds, then we need to generate a new one.
+        val secretKey: SecretKey = getSecretKeyOrNull(userId = userId)
+            ?: getSecretKeyOrNull(userId = null)
+            ?: generateKeyOrNull(userId = userId)
             ?: run {
                 // user removed all biometrics from the device
                 destroyBiometrics(userId = userId)
@@ -97,11 +102,26 @@ class BiometricsEncryptionManagerImpl(
             ?: false
     }
 
+    private fun getKeyGenParameterSpec(userId: String): KeyGenParameterSpec =
+        KeyGenParameterSpec
+            .Builder(
+                encryptionKeyName(userId = userId),
+                KeyProperties.PURPOSE_ENCRYPT or KeyProperties.PURPOSE_DECRYPT,
+            )
+            .setBlockModes(KeyProperties.BLOCK_MODE_CBC)
+            .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_PKCS7)
+            .setUserAuthenticationRequired(true)
+            .setInvalidatedByBiometricEnrollment(true)
+            .build()
+
+    private fun encryptionKeyName(userId: String?): String =
+        "${BuildConfig.APPLICATION_ID}.biometric_integrity${userId?.let { ".$it" }.orEmpty()}"
+
     /**
      * Generates a [SecretKey] from which the [Cipher] will be generated, or `null` if a key cannot
      * be generated.
      */
-    private fun generateKeyOrNull(): SecretKey? {
+    private fun generateKeyOrNull(userId: String): SecretKey? {
         val keyGen = try {
             KeyGenerator.getInstance(KeyProperties.KEY_ALGORITHM_AES, ENCRYPTION_KEYSTORE_NAME)
         } catch (_: NoSuchAlgorithmException) {
@@ -113,7 +133,7 @@ class BiometricsEncryptionManagerImpl(
         }
 
         return try {
-            keyGen.init(keyGenParameterSpec)
+            keyGen.init(getKeyGenParameterSpec(userId = userId))
             keyGen.generateKey()
         } catch (_: InvalidAlgorithmParameterException) {
             null
@@ -125,10 +145,10 @@ class BiometricsEncryptionManagerImpl(
     /**
      * Returns the [SecretKey] stored in the keystore, or null if there isn't one.
      */
-    private fun getSecretKeyOrNull(): SecretKey? =
+    private fun getSecretKeyOrNull(userId: String?): SecretKey? =
         try {
             keystore
-                .getKey(ENCRYPTION_KEY_NAME, null)
+                .getKey(encryptionKeyName(userId = userId), null)
                 ?.let { it as SecretKey }
         } catch (_: KeyStoreException) {
             // keystore was not loaded
@@ -172,7 +192,9 @@ class BiometricsEncryptionManagerImpl(
      * Validates the keystore key and decrypts it using the user-provided [cipher].
      */
     private fun isSystemBiometricIntegrityValid(userId: String, cipher: Cipher?): Boolean {
-        val secretKey = getSecretKeyOrNull()
+        // Attempt to get the user scoped key. If that fails, we check to see if a legacy key
+        // is available.
+        val secretKey = getSecretKeyOrNull(userId = userId) ?: getSecretKeyOrNull(userId = null)
         return if (cipher != null && secretKey != null) {
             cipher.initializeCipher(userId = userId, secretKey = secretKey)
         } else {
@@ -197,22 +219,12 @@ class BiometricsEncryptionManagerImpl(
     }
 
     private fun destroyBiometrics(userId: String) {
-        settingsDiskSource.systemBiometricIntegritySource?.let { systemBioIntegrityState ->
-            settingsDiskSource.storeAccountBiometricIntegrityValidity(
-                userId = userId,
-                systemBioIntegrityState = systemBioIntegrityState,
-                value = null,
-            )
-        }
+        clearBiometrics(userId = userId)
         settingsDiskSource.systemBiometricIntegritySource = null
-        authDiskSource.storeUserBiometricUnlockKey(userId = userId, biometricsKey = null)
-        authDiskSource.storeUserBiometricInitVector(userId = userId, iv = null)
-        keystore.deleteEntry(ENCRYPTION_KEY_NAME)
     }
 }
 
 private const val ENCRYPTION_KEYSTORE_NAME: String = "AndroidKeyStore"
-private const val ENCRYPTION_KEY_NAME: String = "${BuildConfig.APPLICATION_ID}.biometric_integrity"
 private const val CIPHER_TRANSFORMATION =
     KeyProperties.KEY_ALGORITHM_AES + "/" +
         KeyProperties.BLOCK_MODE_CBC + "/" +

app/src/main/java/com/x8bit/bitwarden/data/platform/repository/AuthenticatorBridgeRepositoryImpl.kt

@@ -71,6 +71,7 @@ class AuthenticatorBridgeRepositoryImpl(
 
                     when (unlockResult) {
                         is VaultUnlockResult.AuthenticationError,
+                        VaultUnlockResult.BiometricDecodingError,
                         VaultUnlockResult.GenericError,
                         VaultUnlockResult.InvalidStateError,
                             -> {

app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepository.kt

@@ -231,11 +231,6 @@ interface SettingsRepository {
      */
     fun storePullToRefreshEnabled(isPullToRefreshEnabled: Boolean)
 
-    /**
-     * Clears any previously stored encrypted user key used with biometrics for the current user.
-     */
-    fun clearBiometricsKey()
-
     /**
      * Stores the encrypted user key for biometrics, allowing it to be used to unlock the current
      * user's vault.

app/src/main/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryImpl.kt

@@ -510,12 +510,6 @@ class SettingsRepositoryImpl(
             )
     }
 
-    override fun clearBiometricsKey() {
-        val userId = activeUserId ?: return
-        authDiskSource.storeUserBiometricInitVector(userId = userId, iv = null)
-        authDiskSource.storeUserBiometricUnlockKey(userId = userId, biometricsKey = null)
-    }
-
     override fun storeUnlockPin(
         pin: String,
         shouldRequireMasterPasswordOnRestart: Boolean,

app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt

@@ -114,6 +114,7 @@ import kotlinx.coroutines.flow.stateIn
 import kotlinx.coroutines.flow.update
 import kotlinx.coroutines.launch
 import retrofit2.HttpException
+import java.security.GeneralSecurityException
 import java.time.Clock
 import java.time.temporal.ChronoUnit
 import javax.crypto.Cipher
@@ -555,9 +556,13 @@ class VaultRepositoryImpl(
                 initUserCryptoMethod = InitUserCryptoMethod.DecryptedKey(
                     decryptedUserKey = iv
                         ?.let {
-                            cipher
-                                .doFinal(biometricsKey.toByteArray(Charsets.ISO_8859_1))
-                                .decodeToString()
+                            try {
+                                cipher
+                                    .doFinal(biometricsKey.toByteArray(Charsets.ISO_8859_1))
+                                    .decodeToString()
+                            } catch (_: GeneralSecurityException) {
+                                return VaultUnlockResult.BiometricDecodingError
+                            }
                         }
                         ?: biometricsKey,
                 ),

app/src/main/java/com/x8bit/bitwarden/data/vault/repository/model/VaultUnlockResult.kt

@@ -17,6 +17,11 @@ sealed class VaultUnlockResult {
         val message: String? = null,
     ) : VaultUnlockResult(), VaultUnlockError
 
+    /**
+     * Unable to decode biometrics key.
+     */
+    data object BiometricDecodingError : VaultUnlockResult(), VaultUnlockError
+
     /**
      * Unable to access user state information.
      */

app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupUnlockViewModel.kt

@@ -132,7 +132,7 @@ class SetupUnlockViewModel @Inject constructor(
     }
 
     private fun handleUnlockWithBiometricToggleDisabled() {
-        settingsRepository.clearBiometricsKey()
+        biometricsEncryptionManager.clearBiometrics(userId = state.userId)
         mutableStateFlow.update { it.copy(isUnlockWithBiometricsEnabled = false) }
     }
 

app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModel.kt

@@ -330,6 +330,19 @@ class VaultUnlockViewModel @Inject constructor(
                 }
             }
 
+            VaultUnlockResult.BiometricDecodingError -> {
+                biometricsEncryptionManager.clearBiometrics(userId = state.userId)
+                mutableStateFlow.update {
+                    it.copy(
+                        isBiometricsValid = false,
+                        dialog = VaultUnlockState.VaultUnlockDialog.Error(
+                            title = R.string.biometrics_failed.asText(),
+                            message = R.string.biometrics_decoding_failure.asText(),
+                        ),
+                    )
+                }
+            }
+
             VaultUnlockResult.GenericError,
             VaultUnlockResult.InvalidStateError,
                 -> {
@@ -381,6 +394,7 @@ class VaultUnlockViewModel @Inject constructor(
             val accountSummaries = userState.toAccountSummaries()
             val activeAccountSummary = userState.toActiveAccountSummary()
             it.copy(
+                userId = userState.activeUserId,
                 initials = activeAccountSummary.initials,
                 avatarColorString = activeAccountSummary.avatarColorHex,
                 accountSummaries = accountSummaries,

app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityViewModel.kt

@@ -318,7 +318,7 @@ class AccountSecurityViewModel @Inject constructor(
     }
 
     private fun handleUnlockWithBiometricToggleDisabled() {
-        settingsRepository.clearBiometricsKey()
+        biometricsEncryptionManager.clearBiometrics(userId = state.userId)
         mutableStateFlow.update { it.copy(isUnlockWithBiometricsEnabled = false) }
         validateVaultTimeoutAction()
     }

app/src/main/res/values/strings.xml

@@ -497,6 +497,8 @@ Scanning will happen automatically.</string>
   <string name="login_expired">Your login session has expired.</string>
   <string name="biometrics_direction">Biometric verification</string>
   <string name="biometrics">Biometrics</string>
+  <string name="biometrics_failed">Biometrics Failed</string>
+  <string name="biometrics_decoding_failure">Log in with your Master Password or PIN then re-enable biometric login in Settings.</string>
   <string name="use_biometrics_to_unlock">Use biometrics to unlock</string>
   <string name="accessibility_overlay_permission_alert">Bitwarden needs attention - See \"Autofill Accessibility Service\" from Bitwarden settings</string>
   <string name="bitwarden_autofill_service_overlay_permission">3. On the Android App Settings screen for Bitwarden, go to the \"Display over other apps\" options (under Advanced) and tap the toggle to allow overlay support.</string>

app/src/test/java/com/x8bit/bitwarden/data/auth/repository/model/LoginResultExtensionsTest.kt

@@ -25,9 +25,11 @@ class LoginResultExtensionsTest {
     fun `VaultUnlockResult with no message value are mapped to LoginResult with null error message`() {
         val invalidStateResult = VaultUnlockResult.InvalidStateError.toLoginErrorResult()
         val genericErrorResult = VaultUnlockResult.GenericError.toLoginErrorResult()
+        val biometricErrorResult = VaultUnlockResult.BiometricDecodingError.toLoginErrorResult()
         val expectedResult = LoginResult.Error(errorMessage = null)
 
         assertEquals(expectedResult, invalidStateResult)
         assertEquals(expectedResult, genericErrorResult)
+        assertEquals(expectedResult, biometricErrorResult)
     }
 }

app/src/test/java/com/x8bit/bitwarden/data/platform/repository/SettingsRepositoryTest.kt

@@ -806,18 +806,6 @@ class SettingsRepositoryTest {
         assertEquals(true, fakeSettingsDiskSource.getPullToRefreshEnabled(userId = USER_ID))
     }
 
-    @Test
-    fun `clearBiometricsKey should remove the stored biometrics key`() {
-        fakeAuthDiskSource.userState = MOCK_USER_STATE
-        fakeAuthDiskSource.storeUserBiometricUnlockKey(userId = USER_ID, "fake key")
-        fakeAuthDiskSource.storeUserBiometricInitVector(userId = USER_ID, byteArrayOf(1))
-
-        settingsRepository.clearBiometricsKey()
-
-        fakeAuthDiskSource.assertBiometricsKey(userId = USER_ID, biometricsKey = null)
-        fakeAuthDiskSource.assertBiometricInitVector(userId = USER_ID, iv = null)
-    }
-
     @Test
     fun `setupBiometricsKey with missing user state should return BiometricsKeyResult Error`() =
         runTest {

app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt

@@ -134,6 +134,7 @@ import java.time.ZoneId
 import java.time.ZoneOffset
 import java.time.ZonedDateTime
 import java.time.temporal.ChronoUnit
+import javax.crypto.BadPaddingException
 import javax.crypto.Cipher
 
 @Suppress("LargeClass")
@@ -1244,6 +1245,32 @@ class VaultRepositoryTest {
             }
         }
 
+    @Suppress("MaxLineLength")
+    @Test
+    fun `unlockVaultWithBiometrics with failure to decode biometrics key should return BiometricDecodingError`() =
+        runTest {
+            val userId = MOCK_USER_STATE.activeUserId
+            val privateKey = "mockPrivateKey-1"
+            val biometricsKey = "asdf1234"
+            fakeAuthDiskSource.userState = MOCK_USER_STATE
+            val initVector = byteArrayOf(2, 2)
+            val cipher = mockk<Cipher> {
+                every { doFinal(any()) } throws BadPaddingException()
+            }
+            fakeAuthDiskSource.apply {
+                storeUserBiometricInitVector(userId = userId, iv = initVector)
+                storeUserBiometricUnlockKey(userId = userId, biometricsKey = biometricsKey)
+                storePrivateKey(userId = userId, privateKey = privateKey)
+            }
+
+            val result = vaultRepository.unlockVaultWithBiometrics(cipher = cipher)
+
+            assertEquals(VaultUnlockResult.BiometricDecodingError, result)
+            coVerify(exactly = 0) {
+                vaultSdkSource.derivePinProtectedUserKey(any(), any())
+            }
+        }
+
     @Suppress("MaxLineLength")
     @Test
     fun `unlockVaultWithBiometrics with an IV and VaultLockManager Success should store the updated key and IV and unlock for the current user and return Success`() =

app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupUnlockViewModelTest.kt

@@ -153,7 +153,7 @@ class SetupUnlockViewModelTest : BaseViewModelTest() {
     fun `on UnlockWithBiometricToggleDisabled should call clearBiometricsKey and update the state`() {
         val initialState = DEFAULT_STATE.copy(isUnlockWithBiometricsEnabled = true)
         every { settingsRepository.isUnlockWithBiometricsEnabled } returns true
-        every { settingsRepository.clearBiometricsKey() } just runs
+        every { biometricsEncryptionManager.clearBiometrics(userId = DEFAULT_USER_ID) } just runs
         val viewModel = createViewModel(initialState)
         assertEquals(initialState, viewModel.stateFlow.value)
 
@@ -164,7 +164,7 @@ class SetupUnlockViewModelTest : BaseViewModelTest() {
             viewModel.stateFlow.value,
         )
         verify(exactly = 1) {
-            settingsRepository.clearBiometricsKey()
+            biometricsEncryptionManager.clearBiometrics(userId = DEFAULT_USER_ID)
         }
     }
 

app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/vaultunlock/VaultUnlockViewModelTest.kt

@@ -1095,6 +1095,37 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
         }
     }
 
+    @Suppress("MaxLineLength")
+    @Test
+    fun `on BiometricsUnlockSuccess should disable biometrics and display error dialog on unlockVaultWithBiometrics BiometricDecodingError`() {
+        val initialState = DEFAULT_STATE.copy(isBiometricEnabled = true)
+        mutableUserStateFlow.value = DEFAULT_USER_STATE.copy(
+            accounts = listOf(DEFAULT_ACCOUNT.copy(isBiometricsEnabled = true)),
+        )
+        val viewModel = createViewModel(state = initialState)
+        coEvery {
+            vaultRepository.unlockVaultWithBiometrics(cipher = CIPHER)
+        } returns VaultUnlockResult.BiometricDecodingError
+        every { encryptionManager.clearBiometrics(userId = USER_ID) } just runs
+
+        viewModel.trySendAction(VaultUnlockAction.BiometricsUnlockSuccess(CIPHER))
+
+        assertEquals(
+            initialState.copy(
+                isBiometricsValid = false,
+                dialog = VaultUnlockState.VaultUnlockDialog.Error(
+                    title = R.string.biometrics_failed.asText(),
+                    message = R.string.biometrics_decoding_failure.asText(),
+                ),
+            ),
+            viewModel.stateFlow.value,
+        )
+        coVerify(exactly = 1) {
+            encryptionManager.clearBiometrics(userId = USER_ID)
+            vaultRepository.unlockVaultWithBiometrics(cipher = CIPHER)
+        }
+    }
+
     @Suppress("MaxLineLength")
     @Test
     fun `on BiometricsUnlockSuccess should display error dialog on unlockVaultWithBiometrics InvalidStateError`() {

app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityViewModelTest.kt

@@ -432,7 +432,9 @@ class AccountSecurityViewModelTest : BaseViewModelTest() {
         runTest {
             val initialState = DEFAULT_STATE.copy(isUnlockWithBiometricsEnabled = true)
             every { settingsRepository.isUnlockWithBiometricsEnabled } returns true
-            every { settingsRepository.clearBiometricsKey() } just runs
+            every {
+                biometricsEncryptionManager.clearBiometrics(userId = DEFAULT_USER_ID)
+            } just runs
             val viewModel = createViewModel(initialState)
             assertEquals(initialState, viewModel.stateFlow.value)
 
@@ -443,7 +445,7 @@ class AccountSecurityViewModelTest : BaseViewModelTest() {
                 viewModel.stateFlow.value,
             )
             verify(exactly = 1) {
-                settingsRepository.clearBiometricsKey()
+                biometricsEncryptionManager.clearBiometrics(userId = DEFAULT_USER_ID)
             }
         }
 
@@ -456,7 +458,9 @@ class AccountSecurityViewModelTest : BaseViewModelTest() {
                 isUnlockWithBiometricsEnabled = true,
             )
             every { settingsRepository.isUnlockWithBiometricsEnabled } returns true
-            every { settingsRepository.clearBiometricsKey() } just runs
+            every {
+                biometricsEncryptionManager.clearBiometrics(userId = DEFAULT_USER_ID)
+            } just runs
             every { settingsRepository.vaultTimeoutAction = VaultTimeoutAction.LOGOUT } just runs
             val viewModel = createViewModel(initialState)
             assertEquals(initialState, viewModel.stateFlow.value)
@@ -471,7 +475,7 @@ class AccountSecurityViewModelTest : BaseViewModelTest() {
                 viewModel.stateFlow.value,
             )
             verify(exactly = 1) {
-                settingsRepository.clearBiometricsKey()
+                biometricsEncryptionManager.clearBiometrics(userId = DEFAULT_USER_ID)
                 settingsRepository.vaultTimeoutAction = VaultTimeoutAction.LOGOUT
             }
         }
@@ -863,8 +867,9 @@ class AccountSecurityViewModelTest : BaseViewModelTest() {
 private val CIPHER = mockk<Cipher>()
 private const val FINGERPRINT: String = "fingerprint"
 
+private const val DEFAULT_USER_ID: String = "activeUserId"
 private val DEFAULT_USER_STATE = UserState(
-    activeUserId = "activeUserId",
+    activeUserId = DEFAULT_USER_ID,
     accounts = listOf(
         UserState.Account(
             userId = "activeUserId",
@@ -895,7 +900,7 @@ private val DEFAULT_STATE: AccountSecurityState = AccountSecurityState(
     isUnlockWithBiometricsEnabled = false,
     isUnlockWithPasswordEnabled = true,
     isUnlockWithPinEnabled = false,
-    userId = DEFAULT_USER_STATE.activeUserId,
+    userId = DEFAULT_USER_ID,
     vaultTimeout = VaultTimeout.ThirtyMinutes,
     vaultTimeoutAction = VaultTimeoutAction.LOCK,
     vaultTimeoutPolicyMinutes = null,