Skip to content

Commit

Permalink
PM-15356: Resolve biometrics bypass (#4448)
Browse files Browse the repository at this point in the history
  • Loading branch information
david-livefront authored Jan 9, 2025
1 parent 2f2db1a commit f2c87d1
Show file tree
Hide file tree
Showing 28 changed files with 397 additions and 251 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,16 @@ interface AuthDiskSource {
pendingAuthRequest: PendingAuthRequestJson?,
)

/**
* Gets the biometrics initialization vector for the given [userId].
*/
fun getUserBiometricInitVector(userId: String): ByteArray?

/**
* Stores the biometrics initialization vector for the given [userId].
*/
fun storeUserBiometricInitVector(userId: String, iv: ByteArray?)

/**
* Gets the biometrics key for the given [userId].
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import java.util.UUID
private const val ACCOUNT_TOKENS_KEY = "accountTokens"
private const val AUTHENTICATOR_SYNC_SYMMETRIC_KEY = "authenticatorSyncSymmetric"
private const val AUTHENTICATOR_SYNC_UNLOCK_KEY = "authenticatorSyncUnlock"
private const val BIOMETRICS_INIT_VECTOR_KEY = "biometricInitializationVector"
private const val BIOMETRICS_UNLOCK_KEY = "userKeyBiometricUnlock"
private const val USER_AUTO_UNLOCK_KEY_KEY = "userKeyAutoUnlock"
private const val DEVICE_KEY_KEY = "deviceKey"
Expand Down Expand Up @@ -145,6 +146,7 @@ class AuthDiskSourceImpl(
storePrivateKey(userId = userId, privateKey = null)
storeOrganizationKeys(userId = userId, organizationKeys = null)
storeOrganizations(userId = userId, organizations = null)
storeUserBiometricInitVector(userId = userId, iv = null)
storeUserBiometricUnlockKey(userId = userId, biometricsKey = null)
storeMasterPasswordHash(userId = userId, passwordHash = null)
storePolicies(userId = userId, policies = null)
Expand Down Expand Up @@ -280,6 +282,17 @@ class AuthDiskSourceImpl(
)
}

override fun getUserBiometricInitVector(userId: String): ByteArray? =
getEncryptedString(key = BIOMETRICS_INIT_VECTOR_KEY.appendIdentifier(userId))
?.toByteArray(Charsets.ISO_8859_1)

override fun storeUserBiometricInitVector(userId: String, iv: ByteArray?) {
putEncryptedString(
key = BIOMETRICS_INIT_VECTOR_KEY.appendIdentifier(userId),
value = iv?.toString(Charsets.ISO_8859_1),
)
}

override fun getUserBiometricUnlockKey(userId: String): String? =
getEncryptedString(key = BIOMETRICS_UNLOCK_KEY.appendIdentifier(userId))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,6 @@ interface BiometricsEncryptionManager {
userId: String,
): Cipher?

/**
* Sets up biometrics to ensure future integrity checks work properly. If this method has never
* been called [isBiometricIntegrityValid] will return false.
*/
fun setupBiometrics(userId: String)

/**
* Checks to verify that the biometrics integrity is still valid. This returns `true` if the
* biometrics data has not changed since the app setup biometrics; `false` will be returned if
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ import android.security.keystore.KeyGenParameterSpec
import android.security.keystore.KeyPermanentlyInvalidatedException
import android.security.keystore.KeyProperties
import com.x8bit.bitwarden.BuildConfig
import com.x8bit.bitwarden.data.auth.datasource.disk.AuthDiskSource
import com.x8bit.bitwarden.data.platform.annotation.OmitFromCoverage
import com.x8bit.bitwarden.data.platform.datasource.disk.SettingsDiskSource
import java.io.IOException
import java.security.InvalidAlgorithmParameterException
import java.security.InvalidKeyException
import java.security.KeyStore
Expand All @@ -15,19 +15,20 @@ import java.security.NoSuchAlgorithmException
import java.security.NoSuchProviderException
import java.security.ProviderException
import java.security.UnrecoverableKeyException
import java.security.cert.CertificateException
import java.util.UUID
import javax.crypto.Cipher
import javax.crypto.KeyGenerator
import javax.crypto.NoSuchPaddingException
import javax.crypto.SecretKey
import javax.crypto.spec.IvParameterSpec

/**
* Default implementation of [BiometricsEncryptionManager] for managing Android keystore encryption
* and decryption.
*/
@OmitFromCoverage
class BiometricsEncryptionManagerImpl(
private val authDiskSource: AuthDiskSource,
private val settingsDiskSource: SettingsDiskSource,
) : BiometricsEncryptionManager {
private val keystore = KeyStore
Expand All @@ -50,7 +51,7 @@ class BiometricsEncryptionManagerImpl(
val secretKey: SecretKey = generateKeyOrNull()
?: run {
// user removed all biometrics from the device
settingsDiskSource.systemBiometricIntegritySource = null
destroyBiometrics(userId = userId)
return null
}
val cipher = try {
Expand All @@ -60,37 +61,27 @@ class BiometricsEncryptionManagerImpl(
} catch (_: NoSuchPaddingException) {
return null
}
// Instantiate integrity values.
createIntegrityValues(userId = userId)
// This should never fail to initialize / return false because the cipher is newly generated
initializeCipher(
userId = userId,
cipher = cipher,
secretKey = secretKey,
)
cipher.initializeCipher(userId = userId, secretKey = secretKey)
return cipher
}

override fun getOrCreateCipher(userId: String): Cipher? {
val secretKey = getSecretKeyOrNull()
val secretKey: SecretKey = getSecretKeyOrNull()
?: generateKeyOrNull()
?: run {
// user removed all biometrics from the device
settingsDiskSource.systemBiometricIntegritySource = null
destroyBiometrics(userId = userId)
return null
}

val cipher = Cipher.getInstance(CIPHER_TRANSFORMATION)
val isCipherInitialized = initializeCipher(
userId = userId,
cipher = cipher,
secretKey = secretKey,
)
val isCipherInitialized = cipher.initializeCipher(userId = userId, secretKey = secretKey)
return cipher?.takeIf { isCipherInitialized }
}

override fun setupBiometrics(userId: String) {
createIntegrityValues(userId)
}

override fun isBiometricIntegrityValid(userId: String, cipher: Cipher?): Boolean =
isSystemBiometricIntegrityValid(userId, cipher) && isAccountBiometricIntegrityValid(userId)

Expand All @@ -112,10 +103,7 @@ class BiometricsEncryptionManagerImpl(
*/
private fun generateKeyOrNull(): SecretKey? {
val keyGen = try {
KeyGenerator.getInstance(
KeyProperties.KEY_ALGORITHM_AES,
ENCRYPTION_KEYSTORE_NAME,
)
KeyGenerator.getInstance(KeyProperties.KEY_ALGORITHM_AES, ENCRYPTION_KEYSTORE_NAME)
} catch (_: NoSuchAlgorithmException) {
return null
} catch (_: NoSuchProviderException) {
Expand All @@ -124,40 +112,24 @@ class BiometricsEncryptionManagerImpl(
return null
}

try {
return try {
keyGen.init(keyGenParameterSpec)
keyGen.generateKey()
} catch (_: InvalidAlgorithmParameterException) {
return null
null
} catch (_: ProviderException) {
return null
null
}

return getSecretKeyOrNull()
}

/**
* Returns the [SecretKey] stored in the keystore, or null if there isn't one.
*/
private fun getSecretKeyOrNull(): SecretKey? {
private fun getSecretKeyOrNull(): SecretKey? =
try {
keystore.load(null)
} catch (_: IllegalArgumentException) {
// keystore could not be loaded because [param] is unrecognized.
return null
} catch (_: IOException) {
// keystore data format is invalid or the password is incorrect.
return null
} catch (_: NoSuchAlgorithmException) {
// keystore integrity could not be checked due to missing algorithm.
return null
} catch (_: CertificateException) {
// keystore certificates could not be loaded
return null
}

return try {
keystore.getKey(ENCRYPTION_KEY_NAME, null) as? SecretKey
keystore
.getKey(ENCRYPTION_KEY_NAME, null)
?.let { it as SecretKey }
} catch (_: KeyStoreException) {
// keystore was not loaded
null
Expand All @@ -168,30 +140,31 @@ class BiometricsEncryptionManagerImpl(
// key could not be recovered
null
}
}

/**
* Initialize a [Cipher] and return a boolean indicating whether it is valid.
*/
private fun initializeCipher(
private fun Cipher.initializeCipher(
userId: String,
cipher: Cipher,
secretKey: SecretKey,
): Boolean =
try {
cipher.init(Cipher.ENCRYPT_MODE, secretKey)
authDiskSource
.getUserBiometricInitVector(userId = userId)
?.let { init(Cipher.DECRYPT_MODE, secretKey, IvParameterSpec(it)) }
?: init(Cipher.ENCRYPT_MODE, secretKey)
true
} catch (_: KeyPermanentlyInvalidatedException) {
// Biometric has changed
settingsDiskSource.systemBiometricIntegritySource = null
destroyBiometrics(userId = userId)
false
} catch (_: UnrecoverableKeyException) {
// Biometric was disabled and re-enabled
settingsDiskSource.systemBiometricIntegritySource = null
destroyBiometrics(userId = userId)
false
} catch (_: InvalidKeyException) {
// Fallback for old Bitwarden users without a key
createIntegrityValues(userId)
// User has no key
destroyBiometrics(userId = userId)
true
}

Expand All @@ -201,11 +174,7 @@ class BiometricsEncryptionManagerImpl(
private fun isSystemBiometricIntegrityValid(userId: String, cipher: Cipher?): Boolean {
val secretKey = getSecretKeyOrNull()
return if (cipher != null && secretKey != null) {
initializeCipher(
userId = userId,
cipher = cipher,
secretKey = secretKey,
)
cipher.initializeCipher(userId = userId, secretKey = secretKey)
} else {
false
}
Expand All @@ -215,7 +184,6 @@ class BiometricsEncryptionManagerImpl(
* Creates the initial values to be used for biometrics, including the key from which the
* master [Cipher] will be generated.
*/
@Suppress("TooGenericExceptionCaught")
private fun createIntegrityValues(userId: String) {
val systemBiometricIntegritySource = settingsDiskSource
.systemBiometricIntegritySource
Expand All @@ -226,10 +194,20 @@ class BiometricsEncryptionManagerImpl(
systemBioIntegrityState = systemBiometricIntegritySource,
value = true,
)
}

// Ignore result so biometrics function on devices that are in a state where key generation
// is not functioning
createCipherOrNull(userId)
private fun destroyBiometrics(userId: String) {
settingsDiskSource.systemBiometricIntegritySource?.let { systemBioIntegrityState ->
settingsDiskSource.storeAccountBiometricIntegrityValidity(
userId = userId,
systemBioIntegrityState = systemBioIntegrityState,
value = null,
)
}
settingsDiskSource.systemBiometricIntegritySource = null
authDiskSource.storeUserBiometricUnlockKey(userId = userId, biometricsKey = null)
authDiskSource.storeUserBiometricInitVector(userId = userId, iv = null)
keystore.deleteEntry(ENCRYPTION_KEY_NAME)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,10 @@ object PlatformManagerModule {
@Provides
@Singleton
fun provideBiometricsEncryptionManager(
authDiskSource: AuthDiskSource,
settingsDiskSource: SettingsDiskSource,
): BiometricsEncryptionManager = BiometricsEncryptionManagerImpl(
authDiskSource = authDiskSource,
settingsDiskSource = settingsDiskSource,
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import com.x8bit.bitwarden.ui.platform.feature.settings.appearance.model.AppThem
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.StateFlow
import java.time.Instant
import javax.crypto.Cipher

/**
* Provides an API for observing and modifying settings state.
Expand Down Expand Up @@ -234,7 +235,7 @@ interface SettingsRepository {
* Stores the encrypted user key for biometrics, allowing it to be used to unlock the current
* user's vault.
*/
suspend fun setupBiometricsKey(): BiometricsKeyResult
suspend fun setupBiometricsKey(cipher: Cipher): BiometricsKeyResult

/**
* Stores the given PIN, allowing it to be used to unlock the current user's vault.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import com.x8bit.bitwarden.data.auth.repository.util.policyInformation
import com.x8bit.bitwarden.data.autofill.accessibility.manager.AccessibilityEnabledManager
import com.x8bit.bitwarden.data.autofill.manager.AutofillEnabledManager
import com.x8bit.bitwarden.data.platform.datasource.disk.SettingsDiskSource
import com.x8bit.bitwarden.data.platform.manager.BiometricsEncryptionManager
import com.x8bit.bitwarden.data.platform.manager.PolicyManager
import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager
import com.x8bit.bitwarden.data.platform.repository.model.BiometricsKeyResult
Expand All @@ -37,6 +36,7 @@ import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.launch
import java.time.Instant
import javax.crypto.Cipher

private val DEFAULT_IS_SCREEN_CAPTURE_ALLOWED = BuildConfig.DEBUG

Expand All @@ -50,7 +50,6 @@ class SettingsRepositoryImpl(
private val authDiskSource: AuthDiskSource,
private val settingsDiskSource: SettingsDiskSource,
private val vaultSdkSource: VaultSdkSource,
private val biometricsEncryptionManager: BiometricsEncryptionManager,
accessibilityEnabledManager: AccessibilityEnabledManager,
policyManager: PolicyManager,
dispatcherManager: DispatcherManager,
Expand Down Expand Up @@ -482,13 +481,18 @@ class SettingsRepositoryImpl(
}
}

override suspend fun setupBiometricsKey(): BiometricsKeyResult {
override suspend fun setupBiometricsKey(cipher: Cipher): BiometricsKeyResult {
val userId = activeUserId ?: return BiometricsKeyResult.Error
biometricsEncryptionManager.setupBiometrics(userId)
return vaultSdkSource
.getUserEncryptionKey(userId = userId)
.onSuccess {
authDiskSource.storeUserBiometricUnlockKey(userId = userId, biometricsKey = it)
.onSuccess { biometricsKey ->
authDiskSource.storeUserBiometricUnlockKey(
userId = userId,
biometricsKey = cipher
.doFinal(biometricsKey.encodeToByteArray())
.toString(Charsets.ISO_8859_1),
)
authDiskSource.storeUserBiometricInitVector(userId = userId, iv = cipher.iv)
}
.fold(
onSuccess = { BiometricsKeyResult.Success },
Expand All @@ -498,6 +502,7 @@ class SettingsRepositoryImpl(

override fun clearBiometricsKey() {
val userId = activeUserId ?: return
authDiskSource.storeUserBiometricInitVector(userId = userId, iv = null)
authDiskSource.storeUserBiometricUnlockKey(userId = userId, biometricsKey = null)
}

Expand Down
Loading

0 comments on commit f2c87d1

Please sign in to comment.