CTAP - WebAuthn navigator.credentials.get missing id and rawId fields when allowCredentials has exactly one element
Reported by
thomas.d...@gmail.com,
Oct 31
|
|||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:63.0) Gecko/20100101 Firefox/63.0 Steps to reproduce the problem: 1. Register exactly one CTAP2 authenticator to an origin 2. Call navigator.credentials.get with a PublicKeyCredentialRequestOptions with an allowList containing only the registered credential CrendetialID 3. Perform user gesture to allow credential to be released What is the expected behavior? navigator.credentials.get returns a PublicKeyCredential with id and rawId containing the CredentialID of the authenticator given in the allowList. What went wrong? The authenticatorGetAssertion_Response credential field (0x01) which contains the PublicKeyCredentialDescriptor is optional when the allowList has exactly one PublicKeyCredentialDescriptor (see [1]). When the authenticatorGetAssertion_Response credential field is absent, the id and rawId fields of the PublicKeyCredential returned by navigator.credentials.get must be set to the id of the unique PublicKeyCredential in the allowList (see [2]). Instead, the id and rawId fields of the PublicKeyCredential returned by navigator.credentials.get are set to the empty string. The authentication fails because the PublicKeyCredential returned to the RP is invalid. [1] https://fidoalliance.org/specs/fido-v2.0-id-20180227/fido-client-to-authenticator-protocol-v2.0-id-20180227.html#authenticatorGetAssertion [2] https://www.w3.org/TR/webauthn/#assertioncreationdata-credentialidresult Did this work before? N/A Chrome version: Version 70.0.3538.77 (Official Build) (64-bit) Channel: stable OS Version: 6.1 (Windows 7, Windows Server 2008 R2) Flash Version:
,
Oct 31
,
Oct 31
Hey Jun, could you take a look?
,
Nov 1
I accidentally triggered this bug and found the root cause: In ReadCtapGetAssertionResponse [0] we correctly respect that the CBOR response from the device may not have a credential set. If that's the case, we don't call AuthenticatorGetAssertionResponse::SetCredential. Later in CreateGetAssertionResponse [1], we simply call GetId/GetRawId on the response. Those are inherited from the ResponseData base class and, because they have never been initialized, return their default initialized values, empty string and empty vector. [0] https://cs.chromium.org/chromium/src/device/fido/device_response_converter.cc?gsn=AuthenticatorGetAssertionResponse&g=0&l=119 [1] https://cs.chromium.org/chromium/src/content/browser/webauth/authenticator_impl.cc?q=CreateGetAssertionResponse&sq=package:chromium&g=0&l=418
,
Nov 6
,
Nov 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b9499af054cb1410e602652f7b83fd1ce8139321 commit b9499af054cb1410e602652f7b83fd1ce8139321 Author: Jun Choi <hongjunchoi@chromium.org> Date: Wed Nov 07 07:43:51 2018 Manually set credential for GetAssertion response with empty credentials According to the CTAP spec, it is valid for external authenticators to return response to GetAssertion request with an empty credential when only one credential is listed in the allowed list section of the corresponding GetAssertion request. In this case, manually set credential id before returning the response to the relying party. Bug: 900577 Change-Id: I9ea65eea1c1dc83d6f5312839c0d5db81b0febed Reviewed-on: https://chromium-review.googlesource.com/c/1318973 Reviewed-by: Kim Paulhamus <kpaulhamus@chromium.org> Commit-Queue: Jun Choi <hongjunchoi@chromium.org> Cr-Commit-Position: refs/heads/master@{#605986} [modify] https://crrev.com/b9499af054cb1410e602652f7b83fd1ce8139321/device/fido/fido_test_data.h [modify] https://crrev.com/b9499af054cb1410e602652f7b83fd1ce8139321/device/fido/get_assertion_handler_unittest.cc [modify] https://crrev.com/b9499af054cb1410e602652f7b83fd1ce8139321/device/fido/get_assertion_request_handler.cc
,
Nov 7
Requesting merge of https://chromium-review.googlesource.com/c/chromium/src/+/1318973 to M71. Reasons for requesting merge is as follows: 1) This CL is purely regression. It fixes the error in current WebAuthentication API implementation which causes the browser to return to GetAssertion API with empty credential. 2) This bug is isolated. It only occurs when external connected authenticators returns a response with empty credential. This is an edge case. However, as WebAuthentication API is user facing and the authenticator causing this issue is out int the market, this bug could cause confusion to the relying parties using Chrome's WebAuthN API. 3) This CL is minor fix and the entire feature is guarded by 2 flags : kWebAuthenticationAPI UI flag flag and kWebAuthenticationCtap2 UI flag.
,
Nov 8
This bug requires manual review: M71 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Nov 8
Approving merge to M71 branch 3578 based on comment #7. Please merge ASAP.
,
Nov 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1325a42211b0d51dc3a13964e0fb2aa69f95c639 Commit: 1325a42211b0d51dc3a13964e0fb2aa69f95c639 Author: hongjunchoi@chromium.org Commiter: hongjunchoi@chromium.org Date: 2018-11-08 15:38:22 +0000 UTC Manually set credential for GetAssertion response with empty credentials According to the CTAP spec, it is valid for external authenticators to return response to GetAssertion request with an empty credential when only one credential is listed in the allowed list section of the corresponding GetAssertion request. In this case, manually set credential id before returning the response to the relying party. Bug: 900577 Change-Id: I9ea65eea1c1dc83d6f5312839c0d5db81b0febed Reviewed-on: https://chromium-review.googlesource.com/c/1318973 Reviewed-by: Kim Paulhamus <kpaulhamus@chromium.org> Commit-Queue: Jun Choi <hongjunchoi@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#605986}(cherry picked from commit b9499af054cb1410e602652f7b83fd1ce8139321) Reviewed-on: https://chromium-review.googlesource.com/c/1326495 Reviewed-by: Jun Choi <hongjunchoi@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#579} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
,
Nov 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1325a42211b0d51dc3a13964e0fb2aa69f95c639 commit 1325a42211b0d51dc3a13964e0fb2aa69f95c639 Author: Jun Choi <hongjunchoi@chromium.org> Date: Thu Nov 08 15:38:22 2018 Manually set credential for GetAssertion response with empty credentials According to the CTAP spec, it is valid for external authenticators to return response to GetAssertion request with an empty credential when only one credential is listed in the allowed list section of the corresponding GetAssertion request. In this case, manually set credential id before returning the response to the relying party. Bug: 900577 Change-Id: I9ea65eea1c1dc83d6f5312839c0d5db81b0febed Reviewed-on: https://chromium-review.googlesource.com/c/1318973 Reviewed-by: Kim Paulhamus <kpaulhamus@chromium.org> Commit-Queue: Jun Choi <hongjunchoi@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#605986}(cherry picked from commit b9499af054cb1410e602652f7b83fd1ce8139321) Reviewed-on: https://chromium-review.googlesource.com/c/1326495 Reviewed-by: Jun Choi <hongjunchoi@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#579} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/1325a42211b0d51dc3a13964e0fb2aa69f95c639/device/fido/fido_test_data.h [modify] https://crrev.com/1325a42211b0d51dc3a13964e0fb2aa69f95c639/device/fido/get_assertion_handler_unittest.cc [modify] https://crrev.com/1325a42211b0d51dc3a13964e0fb2aa69f95c639/device/fido/get_assertion_request_handler.cc
,
Nov 15
|
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by dtapu...@chromium.org
, Oct 31