Skip to content

Commit 29371bd

Browse files
PM-19389: Handle encoding error when migrating biometric key (#4900)
1 parent 3eed1c1 commit 29371bd

File tree

2 files changed

+58
-15
lines changed

2 files changed

+58
-15
lines changed

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

+30-15
Original file line numberDiff line numberDiff line change
@@ -557,31 +557,46 @@ class VaultRepositoryImpl(
557557
error = MissingPropertyException("Biometric key"),
558558
)
559559
val iv = authDiskSource.getUserBiometricInitVector(userId = userId)
560+
val decryptedUserKey = iv
561+
?.let {
562+
try {
563+
cipher
564+
.doFinal(biometricsKey.toByteArray(Charsets.ISO_8859_1))
565+
.decodeToString()
566+
} catch (e: GeneralSecurityException) {
567+
return VaultUnlockResult.BiometricDecodingError(error = e)
568+
}
569+
}
570+
?: biometricsKey
571+
val encryptedBiometricsKey = if (iv == null) {
572+
// Attempting to setup an encrypted pin before unlocking, if this fails we send back
573+
// the biometrics error and users will need to sign in another way and re-setup
574+
// biometrics.
575+
try {
576+
cipher
577+
.doFinal(biometricsKey.encodeToByteArray())
578+
.toString(Charsets.ISO_8859_1)
579+
} catch (e: GeneralSecurityException) {
580+
return VaultUnlockResult.BiometricDecodingError(error = e)
581+
}
582+
} else {
583+
null
584+
}
560585
return this
561586
.unlockVaultForUser(
562587
userId = userId,
563588
initUserCryptoMethod = InitUserCryptoMethod.DecryptedKey(
564-
decryptedUserKey = iv
565-
?.let {
566-
try {
567-
cipher
568-
.doFinal(biometricsKey.toByteArray(Charsets.ISO_8859_1))
569-
.decodeToString()
570-
} catch (e: GeneralSecurityException) {
571-
return VaultUnlockResult.BiometricDecodingError(error = e)
572-
}
573-
}
574-
?: biometricsKey,
589+
decryptedUserKey = decryptedUserKey,
575590
),
576591
)
577592
.also {
578593
if (it is VaultUnlockResult.Success) {
579-
if (iv == null) {
594+
encryptedBiometricsKey?.let {
595+
// If this key is present, we store it and the associated IV for future use
596+
// since we want to migrate the user to a more secure form of biometrics.
580597
authDiskSource.storeUserBiometricUnlockKey(
581598
userId = userId,
582-
biometricsKey = cipher
583-
.doFinal(biometricsKey.encodeToByteArray())
584-
.toString(Charsets.ISO_8859_1),
599+
biometricsKey = it,
585600
)
586601
authDiskSource.storeUserBiometricInitVector(userId = userId, iv = cipher.iv)
587602
}

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

+28
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ import org.junit.jupiter.api.Test
131131
import retrofit2.HttpException
132132
import java.io.File
133133
import java.net.UnknownHostException
134+
import java.security.GeneralSecurityException
134135
import java.security.MessageDigest
135136
import java.time.Clock
136137
import java.time.Instant
@@ -1296,6 +1297,33 @@ class VaultRepositoryTest {
12961297
}
12971298
}
12981299

1300+
@Suppress("MaxLineLength")
1301+
@Test
1302+
fun `unlockVaultWithBiometrics with failure to encode biometrics key should return BiometricDecodingError`() =
1303+
runTest {
1304+
val userId = MOCK_USER_STATE.activeUserId
1305+
val biometricsKey = "asdf1234"
1306+
val error = GeneralSecurityException()
1307+
val cipher = mockk<Cipher> {
1308+
every { doFinal(any()) } throws error
1309+
}
1310+
fakeAuthDiskSource.userState = MOCK_USER_STATE
1311+
fakeAuthDiskSource.storeUserBiometricUnlockKey(
1312+
userId = userId,
1313+
biometricsKey = biometricsKey,
1314+
)
1315+
1316+
val result = vaultRepository.unlockVaultWithBiometrics(cipher = cipher)
1317+
1318+
assertEquals(
1319+
VaultUnlockResult.BiometricDecodingError(error = error),
1320+
result,
1321+
)
1322+
coVerify(exactly = 0) {
1323+
vaultSdkSource.derivePinProtectedUserKey(any(), any())
1324+
}
1325+
}
1326+
12991327
@Suppress("MaxLineLength")
13001328
@Test
13011329
fun `unlockVaultWithBiometrics with an IV and VaultLockManager Success should store the updated key and IV and unlock for the current user and return Success`() =

0 commit comments

Comments
 (0)