mirrors/bitwarden-android
1
0
Fork 0
mirror of https://github.com/bitwarden/android.git synced 2024-10-31 07:05:35 +03:00
Code Issues Projects Releases Packages Wiki Activity

[PM-10884] Catch ProviderException when generating a secure key (#3733)

Browse source
This commit is contained in:
Patrick Honkonen 2024-08-16 15:13:41 -04:00 committed by GitHub
parent 7dbfcfdea2
commit 5a7dc198dd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 340 additions and 47 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -9,9 +9,9 @@ interface BiometricsEncryptionManager {
/**
* Creates a [Cipher] built from a keystore.
*/
fun createCipher(
fun createCipherOrNull(
userId: String,
): Cipher
): Cipher?
/**
* Gets the [Cipher] built from a keystore, or creates one if it doesn't already exist.

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

@ -6,13 +6,20 @@ import android.security.keystore.KeyProperties
import com.x8bit.bitwarden.BuildConfig
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
import java.security.KeyStoreException
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
/**
@ -39,9 +46,20 @@ class BiometricsEncryptionManagerImpl(
.setInvalidatedByBiometricEnrollment(true)
.build()
override fun createCipher(userId: String): Cipher {
val secretKey: SecretKey = generateKey()
val cipher = Cipher.getInstance(CIPHER_TRANSFORMATION)
override fun createCipherOrNull(userId: String): Cipher? {
val secretKey: SecretKey = generateKeyOrNull()
?: run {
// user removed all biometrics from the device
settingsDiskSource.systemBiometricIntegritySource = null
return null
}
val cipher = try {
Cipher.getInstance(CIPHER_TRANSFORMATION)
} catch (e: NoSuchAlgorithmException) {
return null
} catch (e: NoSuchPaddingException) {
return null
}
// This should never fail to initialize / return false because the cipher is newly generated
initializeCipher(
userId = userId,
@ -52,13 +70,14 @@ class BiometricsEncryptionManagerImpl(
}
override fun getOrCreateCipher(userId: String): Cipher? {
val secretKey = try {
getSecretKey() ?: generateKey()
} catch (e: InvalidAlgorithmParameterException) {
// user removed all biometrics from the device
settingsDiskSource.systemBiometricIntegritySource = null
return null
}
val secretKey = getSecretKeyOrNull()
?: generateKeyOrNull()
?: run {
// user removed all biometrics from the device
settingsDiskSource.systemBiometricIntegritySource = null
return null
}
val cipher = Cipher.getInstance(CIPHER_TRANSFORMATION)
val isCipherInitialized = initializeCipher(
userId = userId,
@ -88,24 +107,67 @@ class BiometricsEncryptionManagerImpl(
}
/**
* Generates a [SecretKey] from which the [Cipher] will be generated.
* Generates a [SecretKey] from which the [Cipher] will be generated, or `null` if a key cannot
* be generated.
*/
private fun generateKey(): SecretKey {
val keyGen = KeyGenerator.getInstance(
KeyProperties.KEY_ALGORITHM_AES,
ENCRYPTION_KEYSTORE_NAME,
)
keyGen.init(keyGenParameterSpec)
keyGen.generateKey()
return requireNotNull(getSecretKey())
private fun generateKeyOrNull(): SecretKey? {
val keyGen = try {
KeyGenerator.getInstance(
KeyProperties.KEY_ALGORITHM_AES,
ENCRYPTION_KEYSTORE_NAME,
)
} catch (e: NoSuchAlgorithmException) {
return null
} catch (e: NoSuchProviderException) {
return null
} catch (e: IllegalArgumentException) {
return null
}
try {
keyGen.init(keyGenParameterSpec)
keyGen.generateKey()
} catch (e: InvalidAlgorithmParameterException) {
return null
} catch (e: ProviderException) {
return null
}
return getSecretKeyOrNull()
}
/**
* Returns the [SecretKey] stored in the keystore, or null if there isn't one.
*/
private fun getSecretKey(): SecretKey? {
keystore.load(null)
return keystore.getKey(ENCRYPTION_KEY_NAME, null) as? SecretKey
private fun getSecretKeyOrNull(): SecretKey? {
try {
keystore.load(null)
} catch (e: IllegalArgumentException) {
// keystore could not be loaded because [param] is unrecognized.
return null
} catch (e: IOException) {
// keystore data format is invalid or the password is incorrect.
return null
} catch (e: NoSuchAlgorithmException) {
// keystore integrity could not be checked due to missing algorithm.
return null
} catch (e: CertificateException) {
// keystore certificates could not be loaded
return null
}
return try {
keystore.getKey(ENCRYPTION_KEY_NAME, null) as? SecretKey
} catch (e: KeyStoreException) {
// keystore was not loaded
null
} catch (e: NoSuchAlgorithmException) {
// keystore algorithm cannot be found
null
} catch (e: UnrecoverableKeyException) {
// key could not be recovered
null
}
}
/**
@ -137,7 +199,7 @@ class BiometricsEncryptionManagerImpl(
* Validates the keystore key and decrypts it using the user-provided [cipher].
*/
private fun isSystemBiometricIntegrityValid(userId: String, cipher: Cipher?): Boolean {
val secretKey = getSecretKey()
val secretKey = getSecretKeyOrNull()
return if (cipher != null && secretKey != null) {
initializeCipher(
userId = userId,
@ -165,12 +227,9 @@ class BiometricsEncryptionManagerImpl(
value = true,
)
try {
createCipher(userId)
} catch (e: Exception) {
// Catch silently to allow biometrics to function on devices that are in
// a state where key generation is not functioning
}
// Ignore result so biometrics function on devices that are in a state where key generation
// is not functioning
createCipherOrNull(userId)
}
}

18
app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupUnlockScreen.kt
View file

@ -43,6 +43,8 @@ import com.x8bit.bitwarden.ui.platform.base.util.standardHorizontalMargin
import com.x8bit.bitwarden.ui.platform.components.appbar.BitwardenTopAppBar
import com.x8bit.bitwarden.ui.platform.components.button.BitwardenFilledButton
import com.x8bit.bitwarden.ui.platform.components.button.BitwardenTextButton
import com.x8bit.bitwarden.ui.platform.components.dialog.BasicDialogState
import com.x8bit.bitwarden.ui.platform.components.dialog.BitwardenBasicDialog
import com.x8bit.bitwarden.ui.platform.components.dialog.BitwardenLoadingDialog
import com.x8bit.bitwarden.ui.platform.components.dialog.BitwardenTwoButtonDialog
import com.x8bit.bitwarden.ui.platform.components.dialog.LoadingDialogState
@ -86,7 +88,12 @@ fun SetupUnlockScreen(
}
}
SetupUnlockScreenDialogs(dialogState = state.dialogState)
SetupUnlockScreenDialogs(
dialogState = state.dialogState,
onDismissRequest = remember(viewModel) {
{ viewModel.trySendAction(SetupUnlockAction.DismissDialog) }
},
)
val scrollBehavior = TopAppBarDefaults.pinnedScrollBehavior(rememberTopAppBarState())
BitwardenScaffold(
@ -292,12 +299,21 @@ private fun SetupUnlockHeaderLandscape(
@Composable
private fun SetupUnlockScreenDialogs(
dialogState: SetupUnlockState.DialogState?,
onDismissRequest: () -> Unit,
) {
when (dialogState) {
is SetupUnlockState.DialogState.Loading -> BitwardenLoadingDialog(
visibilityState = LoadingDialogState.Shown(text = dialogState.title),
)
is SetupUnlockState.DialogState.Error -> BitwardenBasicDialog(
visibilityState = BasicDialogState.Shown(
title = dialogState.title,
message = dialogState.message,
),
onDismissRequest = onDismissRequest,
)
null -> Unit
}
}

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

@ -56,6 +56,7 @@ class SetupUnlockViewModel @Inject constructor(
SetupUnlockAction.ContinueClick -> handleContinueClick()
SetupUnlockAction.EnableBiometricsClick -> handleEnableBiometricsClick()
SetupUnlockAction.SetUpLaterClick -> handleSetUpLaterClick()
SetupUnlockAction.DismissDialog -> handleDismissDialog()
is SetupUnlockAction.UnlockWithBiometricToggle -> {
handleUnlockWithBiometricToggle(action)
}
@ -70,18 +71,38 @@ class SetupUnlockViewModel @Inject constructor(
}
private fun handleEnableBiometricsClick() {
sendEvent(
SetupUnlockEvent.ShowBiometricsPrompt(
// Generate a new key in case the previous one was invalidated
cipher = biometricsEncryptionManager.createCipher(userId = state.userId),
),
)
biometricsEncryptionManager
.createCipherOrNull(userId = state.userId)
?.let {
sendEvent(
SetupUnlockEvent.ShowBiometricsPrompt(
// Generate a new key in case the previous one was invalidated
cipher = it,
),
)
}
?: run {
mutableStateFlow.update {
it.copy(
dialogState = SetupUnlockState.DialogState.Error(
title = R.string.an_error_has_occurred.asText(),
message = R.string.generic_error_message.asText(),
),
)
}
}
}
private fun handleSetUpLaterClick() {
sendEvent(SetupUnlockEvent.NavigateToSetupAutofill)
}
private fun handleDismissDialog() {
mutableStateFlow.update {
it.copy(dialogState = null)
}
}
private fun handleUnlockWithBiometricToggle(
action: SetupUnlockAction.UnlockWithBiometricToggle,
) {
@ -182,6 +203,15 @@ data class SetupUnlockState(
data class Loading(
val title: Text,
) : DialogState()
/**
* Displays an error dialog with a title, message, and an acknowledgement button.
*/
@Parcelize
data class Error(
val title: Text,
val message: Text,
) : DialogState()
}
}
@ -235,6 +265,11 @@ sealed class SetupUnlockAction {
*/
data object SetUpLaterClick : SetupUnlockAction()
/**
* The user has dismissed the dialog.
*/
data object DismissDialog : SetupUnlockAction()
/**
* Models actions that can be sent by the view model itself.
*/

8
app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreen.kt
View file

@ -376,6 +376,14 @@ private fun AccountSecurityDialogs(
visibilityState = LoadingDialogState.Shown(text = dialogState.message),
)
is AccountSecurityDialog.Error -> BitwardenBasicDialog(
visibilityState = BasicDialogState.Shown(
title = dialogState.title,
message = dialogState.message,
),
onDismissRequest = onDismissRequest,
)
null -> Unit
}
}

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

@ -158,14 +158,26 @@ class AccountSecurityViewModel @Inject constructor(
}
private fun handleEnableBiometricsClick() {
sendEvent(
AccountSecurityEvent.ShowBiometricsPrompt(
// Generate a new key in case the previous one was invalidated
cipher = biometricsEncryptionManager.createCipher(
userId = state.userId,
),
),
)
biometricsEncryptionManager
.createCipherOrNull(userId = state.userId)
?.let {
sendEvent(
AccountSecurityEvent.ShowBiometricsPrompt(
// Generate a new key in case the previous one was invalidated
cipher = it,
),
)
}
?: run {
mutableStateFlow.update {
it.copy(
dialog = AccountSecurityDialog.Error(
title = R.string.an_error_has_occurred.asText(),
message = R.string.generic_error_message.asText(),
),
)
}
}
}
private fun handleFingerPrintLearnMoreClick() {
@ -407,6 +419,15 @@ sealed class AccountSecurityDialog : Parcelable {
data class Loading(
val message: Text,
) : AccountSecurityDialog()
/**
* Displays an error dialog with a title and message.
*/
@Parcelize
data class Error(
val title: Text,
val message: Text,
) : AccountSecurityDialog()
}
/**

34
app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupUnlockScreenTest.kt
View file

@ -586,6 +586,40 @@ class SetupUnlockScreenTest : BaseComposeTest() {
mutableStateFlow.update { it.copy(dialogState = null) }
composeTestRule.assertNoDialogExists()
}
@Suppress("MaxLineLength")
@Test
fun `Error Dialog should be displayed according to state and send DismissDialog action on click`() {
val title = "title"
val message = "message"
composeTestRule.assertNoDialogExists()
mutableStateFlow.update {
it.copy(
dialogState = SetupUnlockState.DialogState.Error(
title = title.asText(),
message = message.asText(),
),
)
}
composeTestRule
.onAllNodesWithText(text = title)
.filterToOne(hasAnyAncestor(isDialog()))
.assertIsDisplayed()
composeTestRule
.onAllNodesWithText("Ok")
.filterToOne(hasAnyAncestor(isDialog()))
.performClick()
verify {
viewModel.trySendAction(SetupUnlockAction.DismissDialog)
}
mutableStateFlow.update { it.copy(dialogState = null) }
composeTestRule.assertNoDialogExists()
}
}
private const val DEFAULT_USER_ID: String = "user_id"

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

@ -40,6 +40,7 @@ class SetupUnlockViewModelTest : BaseViewModelTest() {
every {
isBiometricIntegrityValid(userId = DEFAULT_USER_ID, cipher = CIPHER)
} returns false
every { createCipherOrNull(DEFAULT_USER_ID) } returns CIPHER
}
@Test
@ -210,6 +211,56 @@ class SetupUnlockViewModelTest : BaseViewModelTest() {
}
}
@Test
fun `on DismissDialog should hide dialog`() {
val viewModel = createViewModel()
viewModel.trySendAction(SetupUnlockAction.DismissDialog)
assertEquals(
DEFAULT_STATE.copy(dialogState = null),
viewModel.stateFlow.value,
)
}
@Test
fun `EnableBiometricsClick action should create a new biometrics cipher and emit result`() =
runTest {
val viewModel = createViewModel()
viewModel.trySendAction(SetupUnlockAction.EnableBiometricsClick)
verify {
biometricsEncryptionManager.getOrCreateCipher(DEFAULT_USER_ID)
}
viewModel.eventFlow.test {
assertEquals(
SetupUnlockEvent.ShowBiometricsPrompt(CIPHER),
awaitItem(),
)
}
}
@Test
fun `EnableBiometricsClick actin should show error dialog when cipher is null`() {
every {
biometricsEncryptionManager.createCipherOrNull(DEFAULT_USER_ID)
} returns null
val viewModel = createViewModel()
viewModel.trySendAction(SetupUnlockAction.EnableBiometricsClick)
assertEquals(
DEFAULT_STATE.copy(
dialogState = SetupUnlockState.DialogState.Error(
title = R.string.an_error_has_occurred.asText(),
message = R.string.generic_error_message.asText(),
),
),
viewModel.stateFlow.value,
)
}
private fun createViewModel(
state: SetupUnlockState? = null,
): SetupUnlockViewModel =

49
app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/AccountSecurityScreenTest.kt
View file

@ -1287,6 +1287,55 @@ class AccountSecurityScreenTest : BaseComposeTest() {
verify { viewModel.trySendAction(AccountSecurityAction.DismissDialog) }
}
@Test
fun `Error dialog should be shown or hidden according to state`() {
val title = "title"
val message = "message"
composeTestRule.assertNoDialogExists()
mutableStateFlow.update {
it.copy(
dialog = AccountSecurityDialog.Error(
title = title.asText(),
message = message.asText(),
),
)
}
composeTestRule
.onNodeWithText(title)
.assert(hasAnyAncestor(isDialog()))
.assertIsDisplayed()
composeTestRule
.onNodeWithText(message)
.assert(hasAnyAncestor(isDialog()))
.assertIsDisplayed()
mutableStateFlow.update { it.copy(dialog = null) }
composeTestRule.assertNoDialogExists()
}
@Test
fun `Error dialog dismiss should send DismissDialog`() {
val title = "title"
val message = "message"
mutableStateFlow.update {
it.copy(
dialog = AccountSecurityDialog.Error(
title = title.asText(),
message = message.asText(),
),
)
}
composeTestRule
.onAllNodesWithText("Ok")
.filterToOne(hasAnyAncestor(isDialog()))
.performClick()
verify { viewModel.trySendAction(AccountSecurityAction.DismissDialog) }
}
@Test
fun `fingerprint phrase dialog should be shown or hidden according to the state`() {
composeTestRule.onNode(isDialog()).assertDoesNotExist()

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

@ -57,7 +57,7 @@ class AccountSecurityViewModelTest : BaseViewModelTest() {
}
private val mutableActivePolicyFlow = bufferedMutableSharedFlow<List<SyncResponseJson.Policy>>()
private val biometricsEncryptionManager: BiometricsEncryptionManager = mockk {
every { createCipher(DEFAULT_USER_STATE.activeUserId) } returns CIPHER
every { createCipherOrNull(DEFAULT_USER_STATE.activeUserId) } returns CIPHER
}
private val policyManager: PolicyManager = mockk {
every {
@ -341,6 +341,26 @@ class AccountSecurityViewModelTest : BaseViewModelTest() {
}
}
@Test
fun `on EnableBiometricsClick should show Error dialog when cipher is null`() {
every {
biometricsEncryptionManager.createCipherOrNull(DEFAULT_USER_STATE.activeUserId)
} returns null
val viewModel = createViewModel()
viewModel.trySendAction(AccountSecurityAction.EnableBiometricsClick)
assertEquals(
DEFAULT_STATE.copy(
dialog = AccountSecurityDialog.Error(
title = R.string.an_error_has_occurred.asText(),
message = R.string.generic_error_message.asText(),
),
),
viewModel.stateFlow.value,
)
}
@Test
fun `on UnlockWithBiometricToggle false should call clearBiometricsKey and update the state`() =
runTest {