From 5360577ad15408f1f442d172f49843c858b47a26 Mon Sep 17 00:00:00 2001 From: Patrick Honkonen Date: Thu, 3 Oct 2024 13:58:50 -0400 Subject: [PATCH] [PM-13101] Validate FIDO2 privileged apps against community allow list This commit introduces a two-step validation process for FIDO2 privileged applications. First, the app is validated against the Google allow list. If the validation fails, the app is then validated against the community allow list. This change ensures that apps can be validated against both lists, improving the security and flexibility of the FIDO2 privileged app validation process. --- ...list.json => fido2_privileged_google.json} | 0 .../manager/Fido2CredentialManagerImpl.kt | 45 +++++++++++++++++-- .../manager/Fido2CredentialManagerTest.kt | 40 +++++++++++++++-- 3 files changed, 78 insertions(+), 7 deletions(-) rename app/src/main/assets/{fido2_privileged_allow_list.json => fido2_privileged_google.json} (100%) diff --git a/app/src/main/assets/fido2_privileged_allow_list.json b/app/src/main/assets/fido2_privileged_google.json similarity index 100% rename from app/src/main/assets/fido2_privileged_allow_list.json rename to app/src/main/assets/fido2_privileged_google.json diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/manager/Fido2CredentialManagerImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/manager/Fido2CredentialManagerImpl.kt index 6c808920e16..73702bb5e1b 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/manager/Fido2CredentialManagerImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/manager/Fido2CredentialManagerImpl.kt @@ -31,7 +31,8 @@ import kotlinx.serialization.SerializationException import kotlinx.serialization.encodeToString import kotlinx.serialization.json.Json -private const val ALLOW_LIST_FILE_NAME = "fido2_privileged_allow_list.json" +private const val GOOGLE_ALLOW_LIST_FILE_NAME = "fido2_privileged_google.json" +private const val COMMUNITY_ALLOW_LIST_FILE_NAME = "fido2_privileged_community.json" /** * Primary implementation of [Fido2CredentialManager]. @@ -203,7 +204,8 @@ class Fido2CredentialManagerImpl( callingAppInfo: CallingAppInfo, relyingPartyId: String, ): Fido2ValidateOriginResult { - return digitalAssetLinkService.getDigitalAssetLinkForRp(relyingParty = relyingPartyId) + return digitalAssetLinkService + .getDigitalAssetLinkForRp(relyingParty = relyingPartyId) .onFailure { return Fido2ValidateOriginResult.Error.AssetLinkNotFound } @@ -215,7 +217,8 @@ class Fido2CredentialManagerImpl( ?: return Fido2ValidateOriginResult.Error.ApplicationNotFound } .map { matchingStatements -> - callingAppInfo.getSignatureFingerprintAsHexString() + callingAppInfo + .getSignatureFingerprintAsHexString() ?.let { certificateFingerprint -> matchingStatements .filterMatchingAppSignaturesOrNull( @@ -236,9 +239,43 @@ class Fido2CredentialManagerImpl( private suspend fun validatePrivilegedAppOrigin( callingAppInfo: CallingAppInfo, + ): Fido2ValidateOriginResult { + + val googleAllowListResult = + validatePrivilegedAppSignatureWithGoogleList(callingAppInfo) + if (googleAllowListResult is Fido2ValidateOriginResult.Success) { + // Application was found and successfully validated against the Google allow list so we + // can return the result as the final validation result. + return googleAllowListResult + } + + // Check the community allow list if the Google allow list failed, and return the result as + // the final validation result. + return validatePrivilegedAppSignatureWithCommunityList(callingAppInfo) + } + + private suspend fun validatePrivilegedAppSignatureWithGoogleList( + callingAppInfo: CallingAppInfo, + ): Fido2ValidateOriginResult = + validatePrivilegedAppSignatureWithAllowList( + callingAppInfo = callingAppInfo, + fileName = GOOGLE_ALLOW_LIST_FILE_NAME, + ) + + private suspend fun validatePrivilegedAppSignatureWithCommunityList( + callingAppInfo: CallingAppInfo, + ): Fido2ValidateOriginResult = + validatePrivilegedAppSignatureWithAllowList( + callingAppInfo = callingAppInfo, + fileName = COMMUNITY_ALLOW_LIST_FILE_NAME, + ) + + private suspend fun validatePrivilegedAppSignatureWithAllowList( + callingAppInfo: CallingAppInfo, + fileName: String, ): Fido2ValidateOriginResult = assetManager - .readAsset(ALLOW_LIST_FILE_NAME) + .readAsset(fileName) .map { allowList -> callingAppInfo.validatePrivilegedApp( allowList = allowList, diff --git a/app/src/test/java/com/x8bit/bitwarden/data/autofill/fido2/manager/Fido2CredentialManagerTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/autofill/fido2/manager/Fido2CredentialManagerTest.kt index 19f9526680b..b9ae80f1e4a 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/autofill/fido2/manager/Fido2CredentialManagerTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/autofill/fido2/manager/Fido2CredentialManagerTest.kt @@ -34,6 +34,7 @@ import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockPublicKeyAt import com.x8bit.bitwarden.data.vault.datasource.sdk.util.toAndroidFido2PublicKeyCredential import com.x8bit.bitwarden.ui.vault.feature.addedit.util.createMockPasskeyAssertionOptions import com.x8bit.bitwarden.ui.vault.feature.addedit.util.createMockPasskeyAttestationOptions +import io.mockk.Ordering import io.mockk.coEvery import io.mockk.coVerify import io.mockk.every @@ -139,11 +140,43 @@ class Fido2CredentialManagerTest { coVerify(exactly = 1) { assetManager.readAsset( - fileName = DEFAULT_ALLOW_LIST_FILENAME, + fileName = GOOGLE_ALLOW_LIST_FILENAME, ) } } + @Suppress("MaxLineLength") + @Test + fun `validateOrigin should validate with community allow list when google allow list validation fails`() = + runTest { + coEvery { + assetManager.readAsset(GOOGLE_ALLOW_LIST_FILENAME) + } returns MISSING_PACKAGE_ALLOW_LIST.asSuccess() + every { + mockPrivilegedCallingAppInfo.getOrigin( + privilegedAllowlist = MISSING_PACKAGE_ALLOW_LIST, + ) + } throws IllegalStateException() + coEvery { + assetManager.readAsset(COMMUNITY_ALLOW_LIST_FILENAME) + } returns DEFAULT_ALLOW_LIST.asSuccess() + every { + mockPrivilegedCallingAppInfo.getOrigin( + privilegedAllowlist = DEFAULT_ALLOW_LIST, + ) + } returns DEFAULT_PACKAGE_NAME + + fido2CredentialManager.validateOrigin( + mockPrivilegedAppRequest.callingAppInfo, + mockPrivilegedAppRequest.requestJson, + ) + + coVerify(ordering = Ordering.ORDERED) { + assetManager.readAsset(GOOGLE_ALLOW_LIST_FILENAME) + assetManager.readAsset(COMMUNITY_ALLOW_LIST_FILENAME) + } + } + @Test fun `validateOrigin should return Success when privileged app is allowed`() = runTest { @@ -934,7 +967,7 @@ class Fido2CredentialManagerTest { ) coVerify { - assetManager.readAsset(DEFAULT_ALLOW_LIST_FILENAME) + assetManager.readAsset(GOOGLE_ALLOW_LIST_FILENAME) mockVaultSdkSource.authenticateFido2Credential( request = any(), fido2CredentialStore = any(), @@ -1033,7 +1066,8 @@ private val DEFAULT_STATEMENT = DigitalAssetLinkResponseJson( ), ), ) -private const val DEFAULT_ALLOW_LIST_FILENAME = "fido2_privileged_allow_list.json" +private const val GOOGLE_ALLOW_LIST_FILENAME = "fido2_privileged_google.json" +private const val COMMUNITY_ALLOW_LIST_FILENAME = "fido2_privileged_community.json" private val DEFAULT_STATEMENT_LIST = listOf(DEFAULT_STATEMENT) private const val DEFAULT_ALLOW_LIST = """ {