Commit f1b46d7a authored by Martin Kreichgauer's avatar Martin Kreichgauer Committed by Commit Bot

fido/win: work around a bug in Windows' handling of CTAP1 devices

The Windows WebAuthNGetAssertion API call allows setting the allow list
parameter via two separate fields/types: `WEBAUTHN_CREDENTIALS CredentialList`
and `PWEBAUTHN_CREDENTIAL_LIST pAllowCredentialList`. The latter is newer and
allows setting transport restrictions on each credential descriptor. However,
using it appears to prevent GetAssertion from falling back to the CTAP1
device protocol in cases where (a) the authenticator does not speak CTAP2, or
(b) it speaks CTAP1 and CTAP2 but the credential was created via CTAP1.

This change works around the issue by using the older field instead.
WebAuthNMakeCredential does not seem to suffer from the same issue and reliably
sticks to U2F if the authenticator is CTAP1-only or dwAuthenticatorAttachment
is WEBAUTHN_AUTHENTICATOR_ATTACHMENT_CROSS_PLATFORM_U2F_V2. Hence, nothing
changes for it.

Bug: 898718
Change-Id: I5e06cd48a3dd424b4763753d8e4d41d8c6680c68
Reviewed-on: https://chromium-review.googlesource.com/c/1357621
Commit-Queue: Martin Kreichgauer <martinkr@chromium.org>
Reviewed-by: default avatarAdam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613248}
parent 05fed45d
...@@ -133,18 +133,38 @@ static uint32_t ToWinTransportsMask( ...@@ -133,18 +133,38 @@ static uint32_t ToWinTransportsMask(
return result; return result;
} }
std::vector<WEBAUTHN_CREDENTIAL_EX> ToWinCredentialExVector( std::vector<WEBAUTHN_CREDENTIAL> ToWinCredentialVector(
const base::Optional<std::vector<PublicKeyCredentialDescriptor>>& const base::Optional<std::vector<PublicKeyCredentialDescriptor>>&
credentials) { credentials) {
std::vector<WEBAUTHN_CREDENTIAL_EX> result;
if (!credentials) { if (!credentials) {
return {}; return {};
} }
std::vector<WEBAUTHN_CREDENTIAL> result;
for (const auto& credential : *credentials) { for (const auto& credential : *credentials) {
if (credential.credential_type() != CredentialType::kPublicKey) { if (credential.credential_type() != CredentialType::kPublicKey) {
continue; continue;
} }
result.push_back(WEBAUTHN_CREDENTIAL{
WEBAUTHN_CREDENTIAL_CURRENT_VERSION,
credential.id().size(),
const_cast<unsigned char*>(credential.id().data()),
WEBAUTHN_CREDENTIAL_TYPE_PUBLIC_KEY,
});
}
return result;
}
std::vector<WEBAUTHN_CREDENTIAL_EX> ToWinCredentialExVector(
const base::Optional<std::vector<PublicKeyCredentialDescriptor>>&
credentials) {
if (!credentials) {
return {};
}
std::vector<WEBAUTHN_CREDENTIAL_EX> result;
for (const auto& credential : *credentials) {
if (credential.credential_type() != CredentialType::kPublicKey) {
continue;
}
result.push_back(WEBAUTHN_CREDENTIAL_EX{ result.push_back(WEBAUTHN_CREDENTIAL_EX{
WEBAUTHN_CREDENTIAL_EX_CURRENT_VERSION, credential.id().size(), WEBAUTHN_CREDENTIAL_EX_CURRENT_VERSION, credential.id().size(),
const_cast<unsigned char*>(credential.id().data()), const_cast<unsigned char*>(credential.id().data()),
......
...@@ -36,7 +36,12 @@ uint32_t ToWinAuthenticatorAttachment( ...@@ -36,7 +36,12 @@ uint32_t ToWinAuthenticatorAttachment(
AuthenticatorAttachment authenticator_attachment); AuthenticatorAttachment authenticator_attachment);
COMPONENT_EXPORT(DEVICE_FIDO) COMPONENT_EXPORT(DEVICE_FIDO)
std::vector<_WEBAUTHN_CREDENTIAL_EX> ToWinCredentialExVector( std::vector<WEBAUTHN_CREDENTIAL> ToWinCredentialVector(
const base::Optional<std::vector<PublicKeyCredentialDescriptor>>&
credentials);
COMPONENT_EXPORT(DEVICE_FIDO)
std::vector<WEBAUTHN_CREDENTIAL_EX> ToWinCredentialExVector(
const base::Optional<std::vector<PublicKeyCredentialDescriptor>>& const base::Optional<std::vector<PublicKeyCredentialDescriptor>>&
credentials); credentials);
......
...@@ -237,11 +237,15 @@ class WinWebAuthnApiImpl : public WinWebAuthnApi { ...@@ -237,11 +237,15 @@ class WinWebAuthnApiImpl : public WinWebAuthnApi {
} }
options.pCancellationId = &cancellation_id; options.pCancellationId = &cancellation_id;
auto allow_list_credentials = ToWinCredentialExVector(allow_list); // As of Nov 2018, the WebAuthNAuthenticatorGetAssertion method will fail
auto* allow_list_ptr = allow_list_credentials.data(); // to challenge credentials via CTAP1 if the allowList is passed in the
WEBAUTHN_CREDENTIAL_LIST credential_list{allow_list_credentials.size(), // extended form in WEBAUTHN_AUTHENTICATOR_GET_ASSERTION_OPTIONS (i.e.
&allow_list_ptr}; // pAllowCredentialList instead of CredentialList). The older field we
options.pAllowCredentialList = &credential_list; // use here as a work-around does not allow setting transport
// restrictions on the credential descriptor.
auto credentials = ToWinCredentialVector(allow_list);
options.CredentialList =
WEBAUTHN_CREDENTIALS{credentials.size(), credentials.data()};
WEBAUTHN_CLIENT_DATA client_data{ WEBAUTHN_CLIENT_DATA client_data{
WEBAUTHN_CLIENT_DATA_CURRENT_VERSION, client_data_json.size(), WEBAUTHN_CLIENT_DATA_CURRENT_VERSION, client_data_json.size(),
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment