[PM-24971] Sanitize passkey attestation options from AliExpress

app/src/main/kotlin/com/x8bit/bitwarden/data/credentials/di/CredentialProviderModule.kt

@@ -22,6 +22,8 @@ import com.x8bit.bitwarden.data.credentials.processor.CredentialProviderProcesso
 import com.x8bit.bitwarden.data.credentials.processor.CredentialProviderProcessorImpl
 import com.x8bit.bitwarden.data.credentials.repository.PrivilegedAppRepository
 import com.x8bit.bitwarden.data.credentials.repository.PrivilegedAppRepositoryImpl
+import com.x8bit.bitwarden.data.credentials.sanitizer.PasskeyAttestationOptionsSanitizer
+import com.x8bit.bitwarden.data.credentials.sanitizer.PasskeyAttestationOptionsSanitizerImpl
 import com.x8bit.bitwarden.data.platform.manager.AssetManager
 import com.x8bit.bitwarden.data.platform.manager.BiometricsEncryptionManager
 import com.x8bit.bitwarden.data.platform.manager.ciphermatching.CipherMatchingManager
@@ -75,15 +77,17 @@ object CredentialProviderModule {
         dispatcherManager: DispatcherManager,
         credentialEntryBuilder: CredentialEntryBuilder,
         cipherMatchingManager: CipherMatchingManager,
+        passkeyAttestationOptionsSanitizer: PasskeyAttestationOptionsSanitizer,
     ): BitwardenCredentialManager =
         BitwardenCredentialManagerImpl(
             vaultSdkSource = vaultSdkSource,
             fido2CredentialStore = fido2CredentialStore,
+            credentialEntryBuilder = credentialEntryBuilder,
             json = json,
             vaultRepository = vaultRepository,
-            dispatcherManager = dispatcherManager,
-            credentialEntryBuilder = credentialEntryBuilder,
             cipherMatchingManager = cipherMatchingManager,
+            passkeyAttestationOptionsSanitizer = passkeyAttestationOptionsSanitizer,
+            dispatcherManager = dispatcherManager,
         )
 
     @Provides
@@ -139,4 +143,9 @@ object CredentialProviderModule {
         CredentialManagerPendingIntentManagerImpl(
             context = context,
         )
+
+    @Provides
+    @Singleton
+    fun providePasskeyAttestationOptionsSanitizer(): PasskeyAttestationOptionsSanitizer =
+        PasskeyAttestationOptionsSanitizerImpl
 }

app/src/main/kotlin/com/x8bit/bitwarden/data/credentials/manager/BitwardenCredentialManagerImpl.kt

@@ -33,6 +33,7 @@ import com.x8bit.bitwarden.data.credentials.model.GetCredentialsRequest
 import com.x8bit.bitwarden.data.credentials.model.PasskeyAssertionOptions
 import com.x8bit.bitwarden.data.credentials.model.PasskeyAttestationOptions
 import com.x8bit.bitwarden.data.credentials.model.UserVerificationRequirement
+import com.x8bit.bitwarden.data.credentials.sanitizer.PasskeyAttestationOptionsSanitizer
 import com.x8bit.bitwarden.data.platform.manager.ciphermatching.CipherMatchingManager
 import com.x8bit.bitwarden.data.platform.util.getAppOrigin
 import com.x8bit.bitwarden.data.platform.util.getAppSigningSignatureFingerprint
@@ -60,6 +61,7 @@ class BitwardenCredentialManagerImpl(
     private val json: Json,
     private val vaultRepository: VaultRepository,
     private val cipherMatchingManager: CipherMatchingManager,
+    private val passkeyAttestationOptionsSanitizer: PasskeyAttestationOptionsSanitizer,
     dispatcherManager: DispatcherManager,
 ) : BitwardenCredentialManager,
     Fido2CredentialStore by fido2CredentialStore {
@@ -359,31 +361,46 @@ class BitwardenCredentialManagerImpl(
         selectedCipherView: CipherView,
         clientData: ClientData,
         callingPackageName: String,
-    ): Fido2RegisterCredentialResult = vaultSdkSource
-        .registerFido2Credential(
-            request = RegisterFido2CredentialRequest(
-                userId = userId,
-                origin = sdkOrigin,
-                requestJson = """{"publicKey": ${createPublicKeyCredentialRequest.requestJson}}""",
-                clientData = clientData,
-                selectedCipherView = selectedCipherView,
-                // User verification is handled prior to engaging the SDK. We always respond
-                // `true` so that the SDK does not fail if the relying party requests UV.
-                isUserVerificationSupported = true,
-            ),
-            fido2CredentialStore = this,
-        )
-        .map {
-            it.toAndroidAttestationResponse(callingPackageName = callingPackageName)
-        }
-        .mapCatching { json.encodeToString(it) }
-        .fold(
-            onSuccess = { Fido2RegisterCredentialResult.Success(it) },
-            onFailure = {
-                Timber.e(it, "Failed to register FIDO2 credential.")
-                Fido2RegisterCredentialResult.Error.InternalError
-            },
-        )
+    ): Fido2RegisterCredentialResult {
+        val requestJson =
+            getPasskeyAttestationOptionsOrNull(createPublicKeyCredentialRequest.requestJson)
+                ?.let { passkeyAttestationOptionsSanitizer.sanitize(options = it) }
+                ?.runCatching { json.encodeToString(this) }
+                ?.fold(
+                    onSuccess = { it },
+                    onFailure = {
+                        Timber.e(it, "Failed to sanitize passkey attestation options.")
+                        null
+                    },
+                )
+                ?: return Fido2RegisterCredentialResult.Error.InternalError
+
+        return vaultSdkSource
+            .registerFido2Credential(
+                request = RegisterFido2CredentialRequest(
+                    userId = userId,
+                    origin = sdkOrigin,
+                    requestJson = """{"publicKey": $requestJson}""",
+                    clientData = clientData,
+                    selectedCipherView = selectedCipherView,
+                    // User verification is handled prior to engaging the SDK. We always respond
+                    // `true` so that the SDK does not fail if the relying party requests UV.
+                    isUserVerificationSupported = true,
+                ),
+                fido2CredentialStore = this,
+            )
+            .map {
+                it.toAndroidAttestationResponse(callingPackageName = callingPackageName)
+            }
+            .mapCatching { json.encodeToString(it) }
+            .fold(
+                onSuccess = { Fido2RegisterCredentialResult.Success(it) },
+                onFailure = {
+                    Timber.e(it, "Failed to register FIDO2 credential.")
+                    Fido2RegisterCredentialResult.Error.InternalError
+                },
+            )
+    }
 
     private fun List<BeginGetPasswordOption>.toPasswordCredentialEntries(
         userId: String,

app/src/main/kotlin/com/x8bit/bitwarden/data/credentials/sanitizer/PasskeyAttestationOptionsSanitizer.kt

@@ -0,0 +1,22 @@
+package com.x8bit.bitwarden.data.credentials.sanitizer
+
+import com.x8bit.bitwarden.data.credentials.model.PasskeyAttestationOptions
+
+/**
+ * Defines a contract for sanitizing [PasskeyAttestationOptions] received from applications.
+ *
+ * Sanitization applies workarounds for known issues with specific applications'
+ * passkey implementations, ensuring the options are in the correct format before
+ * being used to create a passkey credential.
+ */
+interface PasskeyAttestationOptionsSanitizer {
+
+    /**
+     * Sanitizes the given [PasskeyAttestationOptions] in preparation for use in the
+     * passkey creation process.
+     *
+     * @param options The [PasskeyAttestationOptions] to sanitize.
+     * @return A new, sanitized instance of [PasskeyAttestationOptions].
+     */
+    fun sanitize(options: PasskeyAttestationOptions): PasskeyAttestationOptions
+}

app/src/main/kotlin/com/x8bit/bitwarden/data/credentials/sanitizer/PasskeyAttestationOptionsSanitizerImpl.kt

@@ -0,0 +1,26 @@
+package com.x8bit.bitwarden.data.credentials.sanitizer
+
+import com.x8bit.bitwarden.data.credentials.model.PasskeyAttestationOptions
+
+/**
+ * Default implementation of [PasskeyAttestationOptionsSanitizer].
+ */
+object PasskeyAttestationOptionsSanitizerImpl : PasskeyAttestationOptionsSanitizer {
+    override fun sanitize(options: PasskeyAttestationOptions): PasskeyAttestationOptions {
+        // The AliExpress Android app (com.alibaba.aliexpresshd) incorrectly appends a newline
+        // to the user.id field when creating a passkey. This causes the operation to fail
+        // downstream. As a workaround, we detect this specific scenario, trim the newline, and
+        // re-serialize the JSON request.
+        return if (options.relyingParty.id == ALIEXPRESS_RP_ID &&
+            options.user.id.endsWith("\n")
+        ) {
+            options.copy(
+                user = options.user.copy(id = options.user.id.trimEnd('\n')),
+            )
+        } else {
+            options
+        }
+    }
+}
+
+private const val ALIEXPRESS_RP_ID = "m.aliexpress.com"

app/src/test/kotlin/com/x8bit/bitwarden/data/credentials/manager/BitwardenCredentialManagerTest.kt

@@ -35,6 +35,7 @@ import com.x8bit.bitwarden.data.credentials.model.GetCredentialsRequest
 import com.x8bit.bitwarden.data.credentials.model.PasskeyAssertionOptions
 import com.x8bit.bitwarden.data.credentials.model.PasskeyAttestationOptions
 import com.x8bit.bitwarden.data.credentials.model.UserVerificationRequirement
+import com.x8bit.bitwarden.data.credentials.sanitizer.PasskeyAttestationOptionsSanitizer
 import com.x8bit.bitwarden.data.platform.manager.ciphermatching.CipherMatchingManager
 import com.x8bit.bitwarden.data.platform.util.getAppSigningSignatureFingerprint
 import com.x8bit.bitwarden.data.vault.datasource.sdk.VaultSdkSource
@@ -81,19 +82,23 @@ class BitwardenCredentialManagerTest {
     private val mutableDecryptCipherListResultStateFlow =
         MutableStateFlow<DataState<DecryptCipherListResult>>(DataState.Loading)
 
+    private val mockPasskeyAttestationOptions = createMockPasskeyAttestationOptions(
+        number = 1,
+        relyingPartyId = DEFAULT_HOST,
+    )
     private val json = mockk<Json> {
         every {
             decodeFromStringOrNull<PasskeyAttestationOptions>(any())
-        } returns createMockPasskeyAttestationOptions(
-            number = 1,
-            relyingPartyId = DEFAULT_HOST,
-        )
+        } returns mockPasskeyAttestationOptions
         every {
             decodeFromStringOrNull<PasskeyAssertionOptions>(any())
         } returns createMockPasskeyAssertionOptions(number = 1)
         every {
             decodeFromStringOrNull<PasskeyAssertionOptions>(DEFAULT_FIDO2_AUTH_REQUEST_JSON)
         } returns createMockPasskeyAssertionOptions(number = 1)
+        every {
+            encodeToString(mockPasskeyAttestationOptions)
+        } returns DEFAULT_FIDO2_CREATE_REQUEST_JSON
     }
     private val mockSigningInfo = mockk<SigningInfo> {
         every { apkContentsSigners } returns arrayOf(Signature(DEFAULT_APP_SIGNATURE))
@@ -126,6 +131,9 @@ class BitwardenCredentialManagerTest {
     }
     private val mockCredentialEntryBuilder = mockk<CredentialEntryBuilder>()
     private val mockCipherMatchingManager = mockk<CipherMatchingManager>()
+    private val mockPasskeyAttestationOptionsSanitizer = mockk<PasskeyAttestationOptionsSanitizer> {
+        every { sanitize(any()) } returns mockPasskeyAttestationOptions
+    }
 
     @BeforeEach
     fun setUp() {
@@ -139,11 +147,12 @@ class BitwardenCredentialManagerTest {
         bitwardenCredentialManager = BitwardenCredentialManagerImpl(
             vaultSdkSource = mockVaultSdkSource,
             fido2CredentialStore = mockFido2CredentialStore,
+            credentialEntryBuilder = mockCredentialEntryBuilder,
             json = json,
-            dispatcherManager = FakeDispatcherManager(),
             vaultRepository = mockVaultRepository,
-            credentialEntryBuilder = mockCredentialEntryBuilder,
             cipherMatchingManager = mockCipherMatchingManager,
+            passkeyAttestationOptionsSanitizer = mockPasskeyAttestationOptionsSanitizer,
+            dispatcherManager = FakeDispatcherManager(),
         )
     }
 
@@ -192,6 +201,9 @@ class BitwardenCredentialManagerTest {
             val selectedCipherView = createMockCipherView(number = 1)
             val slot = slot<RegisterFido2CredentialRequest>()
 
+            every {
+                json.encodeToString<PasskeyAttestationOptions>(any())
+            } returns mockCreatePublicKeyCredentialRequest.requestJson
             every {
                 mockCallingAppInfo.getAppSigningSignatureFingerprint()
             } returns DEFAULT_APP_SIGNATURE.toByteArray()
@@ -242,6 +254,9 @@ class BitwardenCredentialManagerTest {
             val slot = slot<RegisterFido2CredentialRequest>()
 
             every { json.encodeToString<Fido2AttestationResponse>(any(), any()) } returns ""
+            every {
+                json.encodeToString<PasskeyAttestationOptions>(any())
+            } returns DEFAULT_FIDO2_CREATE_REQUEST_JSON
             every { mockCallingAppInfo.isOriginPopulated() } returns false
             every { mockCreatePublicKeyCredentialRequest.origin } returns null
             coEvery {
@@ -286,6 +301,9 @@ class BitwardenCredentialManagerTest {
 
             every { Base64.encodeToString(any(), any()) } returns DEFAULT_APP_SIGNATURE
             every { json.encodeToString<Fido2AttestationResponse>(any(), any()) } returns ""
+            every {
+                json.encodeToString<PasskeyAttestationOptions>(any())
+            } returns mockCreatePublicKeyCredentialRequest.requestJson
             every { mockCreatePublicKeyCredentialRequest.origin } returns DEFAULT_WEB_ORIGIN.v1
             val requestCaptureSlot = slot<RegisterFido2CredentialRequest>()
             coEvery {
@@ -416,6 +434,53 @@ class BitwardenCredentialManagerTest {
             )
         }
 
+    @Test
+    fun `registerFido2Credential should sanitize attestation options before registration`() =
+        runTest {
+            val originalOptions = createMockPasskeyAttestationOptions(number = 1)
+            val sanitizedOptions = originalOptions.copy(
+                user = originalOptions.user.copy(id = "sanitized-user-id"),
+            )
+
+            every { mockCallingAppInfo.signingInfo } returns mockSigningInfo
+            every { Base64.encodeToString(any(), any()) } returns DEFAULT_APP_SIGNATURE
+            every {
+                json.encodeToString<Fido2AttestationResponse>(any(), any())
+            } returns ""
+            every {
+                json.decodeFromStringOrNull<PasskeyAttestationOptions>(any())
+            } returns originalOptions
+            every {
+                mockPasskeyAttestationOptionsSanitizer.sanitize(originalOptions)
+            } returns sanitizedOptions
+            every {
+                json.encodeToString(sanitizedOptions)
+            } returns "sanitized-json"
+            every { mockCreatePublicKeyCredentialRequest.origin } returns DEFAULT_WEB_ORIGIN.v1
+            val mockRegistrationResponse = createMockPublicKeyAttestationResponse(number = 1)
+            val requestCaptureSlot = slot<RegisterFido2CredentialRequest>()
+            coEvery {
+                mockVaultSdkSource.registerFido2Credential(
+                    request = capture(requestCaptureSlot),
+                    fido2CredentialStore = any(),
+                )
+            } returns mockRegistrationResponse.asSuccess()
+
+            bitwardenCredentialManager.registerFido2Credential(
+                userId = "mockUserId",
+                createPublicKeyCredentialRequest = mockCreatePublicKeyCredentialRequest,
+                selectedCipherView = createMockCipherView(number = 1),
+                callingAppInfo = mockCallingAppInfo,
+            )
+
+            verify { mockPasskeyAttestationOptionsSanitizer.sanitize(originalOptions) }
+            verify { json.encodeToString(sanitizedOptions) }
+            assertEquals(
+                """{"publicKey": sanitized-json}""",
+                requestCaptureSlot.captured.requestJson,
+            )
+        }
+
     @Test
     fun `registerFido2Credential should return Error when origin is null`() = runTest {
         every { Base64.encodeToString(any(), any()) } returns DEFAULT_APP_SIGNATURE