Skip to content

Commit 0820061

Browse files
SaintPatrckSaintPatrck
authored
[PM-11886] Update handling of unprivileged apps and improve error messaging (#4694)
1 parent 2aa371a commit 0820061

26 files changed

+569
-340
lines changed

‎app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/manager/Fido2CredentialManagerImpl.kt

Lines changed: 82 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import com.bitwarden.fido.Origin
66
import com.bitwarden.fido.UnverifiedAssetLink
77
import com.bitwarden.sdk.Fido2CredentialStore
88
import com.bitwarden.vault.CipherView
9+
import com.x8bit.bitwarden.R
910
import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2CreateCredentialRequest
1011
import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2CredentialAssertionRequest
1112
import com.x8bit.bitwarden.data.autofill.fido2.model.Fido2CredentialAssertionResult
@@ -22,9 +23,11 @@ import com.x8bit.bitwarden.data.vault.datasource.sdk.model.AuthenticateFido2Cred
2223
import com.x8bit.bitwarden.data.vault.datasource.sdk.model.RegisterFido2CredentialRequest
2324
import com.x8bit.bitwarden.data.vault.datasource.sdk.util.toAndroidAttestationResponse
2425
import com.x8bit.bitwarden.data.vault.datasource.sdk.util.toAndroidFido2PublicKeyCredential
25-
import com.x8bit.bitwarden.ui.platform.base.util.toHostOrPathOrNull
26+
import com.x8bit.bitwarden.ui.platform.base.util.asText
27+
import com.x8bit.bitwarden.ui.platform.base.util.prefixHttpsIfNecessaryOrNull
2628
import kotlinx.serialization.SerializationException
2729
import kotlinx.serialization.json.Json
30+
import timber.log.Timber
2831

2932
/**
3033
* Primary implementation of [Fido2CredentialManager].
@@ -47,41 +50,45 @@ class Fido2CredentialManagerImpl(
4750
fido2CreateCredentialRequest: Fido2CreateCredentialRequest,
4851
selectedCipherView: CipherView,
4952
): Fido2RegisterCredentialResult {
50-
val clientData = if (fido2CreateCredentialRequest.callingAppInfo.isOriginPopulated()) {
51-
fido2CreateCredentialRequest
52-
.callingAppInfo
53+
val callingAppInfo = fido2CreateCredentialRequest.callingAppInfo
54+
val clientData = if (fido2CreateCredentialRequest.origin.isNullOrEmpty()) {
55+
ClientData.DefaultWithExtraData(androidPackageName = callingAppInfo.packageName)
56+
} else {
57+
callingAppInfo
5358
.getAppSigningSignatureFingerprint()
5459
?.let { ClientData.DefaultWithCustomHash(hash = it) }
55-
?: return Fido2RegisterCredentialResult.Error
56-
} else {
57-
ClientData.DefaultWithExtraData(
58-
androidPackageName = fido2CreateCredentialRequest
59-
.callingAppInfo
60-
.packageName,
60+
?: return Fido2RegisterCredentialResult.Error(
61+
R.string.passkey_operation_failed_because_app_is_signed_incorrectly.asText(),
62+
)
63+
}
64+
val sdkOrigin = if (fido2CreateCredentialRequest.origin.isNullOrEmpty()) {
65+
val host = getOriginUrlFromAttestationOptionsOrNull(
66+
requestJson = fido2CreateCredentialRequest.requestJson,
67+
)
68+
?: return Fido2RegisterCredentialResult.Error(
69+
R.string.passkey_operation_failed_because_host_url_is_not_present_in_request
70+
.asText(),
71+
)
72+
Origin.Android(
73+
UnverifiedAssetLink(
74+
packageName = callingAppInfo.packageName,
75+
sha256CertFingerprint = callingAppInfo.getSignatureFingerprintAsHexString()
76+
?: return Fido2RegisterCredentialResult.Error(
77+
R.string.passkey_operation_failed_because_app_signature_is_invalid
78+
.asText(),
79+
),
80+
host = host,
81+
assetLinkUrl = host,
82+
),
6183
)
84+
} else {
85+
Origin.Web(fido2CreateCredentialRequest.origin)
6286
}
63-
val assetLinkUrl = fido2CreateCredentialRequest
64-
.origin
65-
?: getOriginUrlFromAttestationOptionsOrNull(fido2CreateCredentialRequest.requestJson)
66-
?: return Fido2RegisterCredentialResult.Error
67-
68-
val origin = Origin.Android(
69-
UnverifiedAssetLink(
70-
packageName = fido2CreateCredentialRequest.packageName,
71-
sha256CertFingerprint = fido2CreateCredentialRequest
72-
.callingAppInfo
73-
.getSignatureFingerprintAsHexString()
74-
?: return Fido2RegisterCredentialResult.Error,
75-
host = assetLinkUrl.toHostOrPathOrNull()
76-
?: return Fido2RegisterCredentialResult.Error,
77-
assetLinkUrl = assetLinkUrl,
78-
),
79-
)
8087
return vaultSdkSource
8188
.registerFido2Credential(
8289
request = RegisterFido2CredentialRequest(
8390
userId = userId,
84-
origin = origin,
91+
origin = sdkOrigin,
8592
requestJson = """{"publicKey": ${fido2CreateCredentialRequest.requestJson}}""",
8693
clientData = clientData,
8794
selectedCipherView = selectedCipherView,
@@ -95,7 +102,11 @@ class Fido2CredentialManagerImpl(
95102
.mapCatching { json.encodeToString(it) }
96103
.fold(
97104
onSuccess = { Fido2RegisterCredentialResult.Success(it) },
98-
onFailure = { Fido2RegisterCredentialResult.Error },
105+
onFailure = {
106+
Fido2RegisterCredentialResult.Error(
107+
R.string.passkey_registration_failed_due_to_an_internal_error.asText(),
108+
)
109+
},
99110
)
100111
}
101112

@@ -114,8 +125,10 @@ class Fido2CredentialManagerImpl(
114125
try {
115126
json.decodeFromString<PasskeyAttestationOptions>(requestJson)
116127
} catch (e: SerializationException) {
128+
Timber.e(e, "Failed to decode passkey attestation options.")
117129
null
118130
} catch (e: IllegalArgumentException) {
131+
Timber.e(e, "Failed to decode passkey attestation options.")
119132
null
120133
}
121134

@@ -125,11 +138,14 @@ class Fido2CredentialManagerImpl(
125138
try {
126139
json.decodeFromString<PasskeyAssertionOptions>(requestJson)
127140
} catch (e: SerializationException) {
141+
Timber.e(e, "Failed to decode passkey assertion options: $e")
128142
null
129143
} catch (e: IllegalArgumentException) {
144+
Timber.e(e, "Failed to decode passkey assertion options: $e")
130145
null
131146
}
132147

148+
@Suppress("LongMethod")
133149
override suspend fun authenticateFido2Credential(
134150
userId: String,
135151
request: Fido2CredentialAssertionRequest,
@@ -139,39 +155,52 @@ class Fido2CredentialManagerImpl(
139155
val clientData = request.clientDataHash
140156
?.let { ClientData.DefaultWithCustomHash(hash = it) }
141157
?: ClientData.DefaultWithExtraData(androidPackageName = callingAppInfo.getAppOrigin())
142-
val origin = callingAppInfo.origin
143-
?: getOriginUrlFromAssertionOptionsOrNull(request.requestJson)
144-
?: return Fido2CredentialAssertionResult.Error
145158
val relyingPartyId = json
146159
.decodeFromStringOrNull<PasskeyAssertionOptions>(request.requestJson)
147160
?.relyingPartyId
148-
?: return Fido2CredentialAssertionResult.Error
161+
?: return Fido2CredentialAssertionResult.Error(
162+
R.string.passkey_operation_failed_because_relying_party_cannot_be_identified
163+
.asText(),
164+
)
149165

150166
val validateOriginResult = validateOrigin(
151167
callingAppInfo = callingAppInfo,
152168
relyingPartyId = relyingPartyId,
153169
)
154170

171+
val sdkOrigin = if (!request.origin.isNullOrEmpty()) {
172+
Origin.Web(request.origin)
173+
} else {
174+
val hostUrl = getOriginUrlFromAssertionOptionsOrNull(request.requestJson)
175+
?: return Fido2CredentialAssertionResult.Error(
176+
R.string.passkey_operation_failed_because_host_url_is_not_present_in_request
177+
.asText(),
178+
)
179+
Origin.Android(
180+
UnverifiedAssetLink(
181+
packageName = callingAppInfo.packageName,
182+
sha256CertFingerprint = callingAppInfo.getSignatureFingerprintAsHexString()
183+
?: return Fido2CredentialAssertionResult.Error(
184+
R.string.passkey_operation_failed_because_app_signature_is_invalid
185+
.asText(),
186+
),
187+
host = hostUrl,
188+
assetLinkUrl = hostUrl,
189+
),
190+
)
191+
}
192+
155193
return when (validateOriginResult) {
156194
is Fido2ValidateOriginResult.Error -> {
157-
Fido2CredentialAssertionResult.Error
195+
Fido2CredentialAssertionResult.Error(validateOriginResult.messageResId.asText())
158196
}
159197

160198
is Fido2ValidateOriginResult.Success -> {
161199
vaultSdkSource
162200
.authenticateFido2Credential(
163201
request = AuthenticateFido2CredentialRequest(
164202
userId = userId,
165-
origin = Origin.Android(
166-
UnverifiedAssetLink(
167-
callingAppInfo.packageName,
168-
callingAppInfo.getSignatureFingerprintAsHexString()
169-
?: return Fido2CredentialAssertionResult.Error,
170-
origin.toHostOrPathOrNull()
171-
?: return Fido2CredentialAssertionResult.Error,
172-
origin,
173-
),
174-
),
203+
origin = sdkOrigin,
175204
requestJson = """{"publicKey": ${request.requestJson}}""",
176205
clientData = clientData,
177206
selectedCipherView = selectedCipherView,
@@ -183,7 +212,13 @@ class Fido2CredentialManagerImpl(
183212
.mapCatching { json.encodeToString(it) }
184213
.fold(
185214
onSuccess = { Fido2CredentialAssertionResult.Success(it) },
186-
onFailure = { Fido2CredentialAssertionResult.Error },
215+
onFailure = {
216+
Timber.e(it, "Failed to authenticate FIDO2 credential.")
217+
Fido2CredentialAssertionResult.Error(
218+
R.string.passkey_authentication_failed_due_to_an_internal_error
219+
.asText(),
220+
)
221+
},
187222
)
188223
}
189224
}
@@ -195,13 +230,13 @@ class Fido2CredentialManagerImpl(
195230
private fun getOriginUrlFromAssertionOptionsOrNull(requestJson: String) =
196231
getPasskeyAssertionOptionsOrNull(requestJson)
197232
?.relyingPartyId
198-
?.let { "$HTTPS$it" }
233+
?.prefixHttpsIfNecessaryOrNull()
199234

200235
private fun getOriginUrlFromAttestationOptionsOrNull(requestJson: String) =
201236
getPasskeyAttestationOptionsOrNull(requestJson)
202237
?.relyingParty
203238
?.id
204-
?.let { "$HTTPS$it" }
239+
?.prefixHttpsIfNecessaryOrNull()
205240
}
206241

207242
private const val MAX_AUTHENTICATION_ATTEMPTS = 5

‎app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/manager/Fido2OriginManager.kt

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,4 @@ interface Fido2OriginManager {
2020
callingAppInfo: CallingAppInfo,
2121
relyingPartyId: String,
2222
): Fido2ValidateOriginResult
23-
24-
/**
25-
* Returns the privileged app origin, or null if the calling app is not allowed.
26-
*
27-
* @param callingAppInfo The calling app info.
28-
*
29-
* @return The privileged app origin, or null.
30-
*/
31-
suspend fun getPrivilegedAppOriginOrNull(callingAppInfo: CallingAppInfo): String?
3223
}

‎app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/manager/Fido2OriginManagerImpl.kt

Lines changed: 4 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,6 @@ class Fido2OriginManagerImpl(
3232
}
3333
}
3434

35-
override suspend fun getPrivilegedAppOriginOrNull(callingAppInfo: CallingAppInfo): String? {
36-
if (!callingAppInfo.isOriginPopulated()) return null
37-
return callingAppInfo.getOrigin(getGoogleAllowListOrNull().orEmpty())
38-
?: callingAppInfo.getOrigin(getCommunityAllowListOrNull().orEmpty())
39-
?.takeUnless { !callingAppInfo.isOriginPopulated() }
40-
}
41-
4235
private suspend fun validateCallingApplicationAssetLinks(
4336
callingAppInfo: CallingAppInfo,
4437
relyingPartyId: String,
@@ -123,7 +116,10 @@ class Fido2OriginManagerImpl(
123116
}
124117
.fold(
125118
onSuccess = { it },
126-
onFailure = { Fido2ValidateOriginResult.Error.Unknown },
119+
onFailure = {
120+
Timber.e(it, "Failed to validate privileged app: ${callingAppInfo.packageName}")
121+
Fido2ValidateOriginResult.Error.Unknown
122+
},
127123
)
128124

129125
/**
@@ -157,16 +153,4 @@ class Fido2OriginManagerImpl(
157153
?: false
158154
}
159155
.takeUnless { it.isEmpty() }
160-
161-
private suspend fun getGoogleAllowListOrNull(): String? =
162-
assetManager
163-
.readAsset(GOOGLE_ALLOW_LIST_FILE_NAME)
164-
.onFailure { Timber.e(it, "Failed to read Google allow list.") }
165-
.getOrNull()
166-
167-
private suspend fun getCommunityAllowListOrNull(): String? =
168-
assetManager
169-
.readAsset(COMMUNITY_ALLOW_LIST_FILE_NAME)
170-
.onFailure { Timber.e(it, "Failed to read Community allow list.") }
171-
.getOrNull()
172156
}

‎app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/model/Fido2CredentialAssertionResult.kt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package com.x8bit.bitwarden.data.autofill.fido2.model
22

3+
import com.x8bit.bitwarden.ui.platform.base.util.Text
4+
35
/**
46
* Represents possible outcomes of a FIDO 2 credential assertion request.
57
*/
@@ -13,5 +15,5 @@ sealed class Fido2CredentialAssertionResult {
1315
/**
1416
* Indicates there was an error and the assertion was not successful.
1517
*/
16-
data object Error : Fido2CredentialAssertionResult()
18+
data class Error(val message: Text) : Fido2CredentialAssertionResult()
1719
}

‎app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/model/Fido2GetCredentialsResult.kt

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package com.x8bit.bitwarden.data.autofill.fido2.model
22

33
import androidx.credentials.provider.BeginGetPublicKeyCredentialOption
44
import com.bitwarden.fido.Fido2CredentialAutofillView
5+
import com.x8bit.bitwarden.ui.platform.base.util.Text
56

67
/**
78
* Represents the result of a FIDO 2 Get Credentials request.
@@ -24,5 +25,7 @@ sealed class Fido2GetCredentialsResult {
2425
/**
2526
* Indicates an error was encountered when querying for matching credentials.
2627
*/
27-
data object Error : Fido2GetCredentialsResult()
28+
data class Error(
29+
val message: Text,
30+
) : Fido2GetCredentialsResult()
2831
}

‎app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/model/Fido2RegisterCredentialResult.kt

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package com.x8bit.bitwarden.data.autofill.fido2.model
22

3+
import com.x8bit.bitwarden.ui.platform.base.util.Text
4+
35
/**
46
* Models the data returned from creating a FIDO 2 credential.
57
*/
@@ -9,13 +11,13 @@ sealed class Fido2RegisterCredentialResult {
911
* Indicates the credential has been successfully registered.
1012
*/
1113
data class Success(
12-
val registrationResponse: String,
14+
val responseJson: String,
1315
) : Fido2RegisterCredentialResult()
1416

1517
/**
1618
* Indicates there was an error and the credential was not registered.
1719
*/
18-
data object Error : Fido2RegisterCredentialResult()
20+
data class Error(val message: Text) : Fido2RegisterCredentialResult()
1921

2022
/**
2123
* Indicates the user cancelled the request.

‎app/src/main/java/com/x8bit/bitwarden/data/autofill/fido2/model/PublicKeyCredentialDescriptor.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,5 @@ data class PublicKeyCredentialDescriptor(
1313
@SerialName("id")
1414
val id: String,
1515
@SerialName("transports")
16-
val transports: List<String>,
16+
val transports: List<String>?,
1717
)

0 commit comments

Comments
 (0)