From 802427a290508a82fdf7c0608453ec232d3ff652 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Fri, 2 Oct 2026 03:50:32 +0000 Subject: [PATCH] [PM-44191] fix: Passkey allowCredentials and excludeCredentials never match (#7431) --- .../sdk/model/Fido2CredentialStoreImpl.kt | 33 +++- .../sdk/model/Fido2CredentialStoreImplTest.kt | 159 ++++++++++++++++++ 2 files changed, 189 insertions(+), 3 deletions(-) create mode 100644 app/src/test/kotlin/com/x8bit/bitwarden/data/vault/datasource/sdk/model/Fido2CredentialStoreImplTest.kt diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/datasource/sdk/model/Fido2CredentialStoreImpl.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/datasource/sdk/model/Fido2CredentialStoreImpl.kt index c16f0a6dac..e4a8328b50 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/datasource/sdk/model/Fido2CredentialStoreImpl.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/vault/datasource/sdk/model/Fido2CredentialStoreImpl.kt @@ -15,6 +15,12 @@ import com.x8bit.bitwarden.data.vault.repository.VaultRepository import com.x8bit.bitwarden.data.vault.repository.model.CreateCipherResult import com.x8bit.bitwarden.data.vault.repository.model.UpdateCipherResult import timber.log.Timber +import java.nio.ByteBuffer +import java.util.Base64 +import java.util.UUID + +private const val B64_PREFIX: String = "b64." +private const val UUID_BYTE_COUNT: Int = 16 /** * Primary implementation of [Fido2CredentialStore]. @@ -138,18 +144,39 @@ class Fido2CredentialStoreImpl( val fido2CredentialIds = cipherListView.login ?.fido2Credentials .orEmpty() - .map { it.credentialId.toByteArray() } + .mapNotNull { it.credentialId.toGuidBytesOrNull() } val hasIntersectingCredentials = credentialIds - ?.intersect(fido2CredentialIds) .orEmpty() - .isNotEmpty() + .any { id -> fido2CredentialIds.any { it.contentEquals(id) } } hasMatchingRpId && (skipCredentialIdFiltering || hasIntersectingCredentials) } } + /** + * Convert a stored credential ID to its raw bytes, mirroring the SDK's + * `string_to_guid_bytes`. Ids prefixed with "b64." are Base64 URL-safe encoded; all others + * are UUIDs (16 big-endian bytes). Returns null when the id is invalid. + */ + private fun String.toGuidBytesOrNull(): ByteArray? { + return if (startsWith(B64_PREFIX)) { + runCatching { Base64.getUrlDecoder().decode(removePrefix(B64_PREFIX)) } + .getOrNull() + } else { + runCatching { UUID.fromString(this) } + .map { + ByteBuffer + .allocate(UUID_BYTE_COUNT) + .putLong(it.mostSignificantBits) + .putLong(it.leastSignificantBits) + .array() + } + .getOrNull() + } + } + private fun GetCipherResult.toCipherViewOrNull(): CipherView? { return when (this) { GetCipherResult.CipherNotFound -> { diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/datasource/sdk/model/Fido2CredentialStoreImplTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/datasource/sdk/model/Fido2CredentialStoreImplTest.kt new file mode 100644 index 0000000000..d16f128407 --- /dev/null +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/vault/datasource/sdk/model/Fido2CredentialStoreImplTest.kt @@ -0,0 +1,159 @@ +package com.x8bit.bitwarden.data.vault.datasource.sdk.model + +import com.bitwarden.core.data.repository.model.DataState +import com.bitwarden.vault.CipherListViewType +import com.bitwarden.vault.CipherView +import com.bitwarden.vault.DecryptCipherListResult +import com.x8bit.bitwarden.data.auth.repository.AuthRepository +import com.x8bit.bitwarden.data.vault.datasource.sdk.VaultSdkSource +import com.x8bit.bitwarden.data.vault.manager.model.GetCipherResult +import com.x8bit.bitwarden.data.vault.repository.VaultRepository +import io.mockk.coEvery +import io.mockk.every +import io.mockk.mockk +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.test.runTest +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import java.nio.ByteBuffer +import java.util.Base64 +import java.util.UUID + +class Fido2CredentialStoreImplTest { + + private val mutableDecryptCipherListResultStateFlow = + MutableStateFlow>(DataState.Loading) + private val authRepository: AuthRepository = mockk() + private val vaultSdkSource: VaultSdkSource = mockk() + private val vaultRepository: VaultRepository = mockk { + every { decryptCipherListResultStateFlow } returns mutableDecryptCipherListResultStateFlow + } + + private val fido2CredentialStore = Fido2CredentialStoreImpl( + authRepository = authRepository, + vaultSdkSource = vaultSdkSource, + vaultRepository = vaultRepository, + ) + + @BeforeEach + fun setUp() { + mutableDecryptCipherListResultStateFlow.value = DataState.Loaded( + data = createMockDecryptCipherListResult( + number = 1, + successes = listOf( + createFido2CipherListView(number = 1, credentialId = UUID_CREDENTIAL_ID), + createFido2CipherListView(number = 2, credentialId = B64_CREDENTIAL_ID), + ), + ), + ) + coEvery { + vaultRepository.getCipher(cipherId = "mockId-1") + } returns GetCipherResult.Success(cipherView = UUID_CIPHER_VIEW) + coEvery { + vaultRepository.getCipher(cipherId = "mockId-2") + } returns GetCipherResult.Success(cipherView = B64_CIPHER_VIEW) + } + + @Test + fun `findCredentials should match UUID credential id against fresh raw bytes`() = runTest { + val result = fido2CredentialStore.findCredentials( + ids = listOf(UUID.fromString(UUID_CREDENTIAL_ID).toRawBytes()), + ripId = RP_ID, + userHandle = null, + ) + + assertEquals(listOf(UUID_CIPHER_VIEW), result) + } + + @Test + fun `findCredentials should exclude ciphers with non-matching credential ids`() = runTest { + val result = fido2CredentialStore.findCredentials( + ids = listOf(UUID.randomUUID().toRawBytes()), + ripId = RP_ID, + userHandle = null, + ) + + assertEquals(emptyList(), result) + } + + @Test + fun `findCredentials should match b64 credential id against decoded bytes`() = runTest { + val result = fido2CredentialStore.findCredentials( + ids = listOf(B64_CREDENTIAL_BYTES.copyOf()), + ripId = RP_ID, + userHandle = null, + ) + + assertEquals(listOf(B64_CIPHER_VIEW), result) + } + + @Test + fun `findCredentials should match excludeCredentials ids during registration`() = runTest { + // Registration passes the exclude list with the new user's handle; a match must be found + // so the authenticator reports the credential as already registered. + val result = fido2CredentialStore.findCredentials( + ids = listOf(UUID.fromString(UUID_CREDENTIAL_ID).toRawBytes()), + ripId = RP_ID, + userHandle = byteArrayOf(1, 2, 3), + ) + + assertEquals(listOf(UUID_CIPHER_VIEW), result) + } + + @Test + fun `findCredentials should skip credential id filtering when ids are null`() = runTest { + val result = fido2CredentialStore.findCredentials( + ids = null, + ripId = RP_ID, + userHandle = null, + ) + + assertEquals(listOf(UUID_CIPHER_VIEW, B64_CIPHER_VIEW), result) + } + + @Test + fun `findCredentials should skip credential id filtering when ids are empty`() = runTest { + val result = fido2CredentialStore.findCredentials( + ids = emptyList(), + ripId = RP_ID, + userHandle = null, + ) + + assertEquals(listOf(UUID_CIPHER_VIEW, B64_CIPHER_VIEW), result) + } +} + +private const val RP_ID = "mockRpId" +private const val UUID_CREDENTIAL_ID = "3c5d6a1e-8f2b-4c7d-9e0a-1b2c3d4e5f60" +private val B64_CREDENTIAL_BYTES = byteArrayOf(1, 2, 3, 4, 5, 6, 7, 8, 9, 10) +private val B64_CREDENTIAL_ID = + "b64." + Base64.getUrlEncoder().withoutPadding().encodeToString(B64_CREDENTIAL_BYTES) +private val UUID_CIPHER_VIEW = createMockCipherView(number = 1) +private val B64_CIPHER_VIEW = createMockCipherView(number = 2) + +private fun UUID.toRawBytes(): ByteArray = ByteBuffer + .allocate(16) + .putLong(mostSignificantBits) + .putLong(leastSignificantBits) + .array() + +private fun createFido2CipherListView( + number: Int, + credentialId: String, +) = createMockCipherListView( + number = number, + type = CipherListViewType.Login( + createMockLoginListView( + number = number, + hasFido2 = true, + fido2Credentials = listOf( + createMockFido2CredentialListView( + number = number, + credentialId = credentialId, + rpId = RP_ID, + ), + ), + ), + ), +)