New issue
Advanced search Search tips

Issue 898718 link

Starred by 8 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Feature



Sign in to add a comment

Integrate with Windows WebAuthn for talking to FIDO authenticators

Project Member Reported by martinkr@google.com, Oct 25

Issue description

Tracking bug for webauthn.dll integration of //device/fido for USB devices.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 31

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/90c62574f47315a1a1b8343871616ea27e54be1b

commit 90c62574f47315a1a1b8343871616ea27e54be1b
Author: Martin Kreichgauer <martinkr@google.com>
Date: Wed Oct 31 04:24:15 2018

fido: move the full clientDataJSON string into request classes

CtapMakeCredentialRequest and CtapGetAssertionRequest previously only
stored the hashed client data. This moves hashing out of
AuthenticatorImpl and into these request classes so that
FidoAuthenticator instances have access to the unhashed string if they
require (e.g. in the Windows WebAuthn API).

Bug: 898718
Change-Id: I4a08987a0be5d6ca07318d37f8c0bcc6c8612a06
Reviewed-on: https://chromium-review.googlesource.com/c/1300677
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Jun Choi <hongjunchoi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604148}
[modify] https://crrev.com/90c62574f47315a1a1b8343871616ea27e54be1b/content/browser/webauth/authenticator_impl.cc
[modify] https://crrev.com/90c62574f47315a1a1b8343871616ea27e54be1b/device/fido/ctap_get_assertion_request.cc
[modify] https://crrev.com/90c62574f47315a1a1b8343871616ea27e54be1b/device/fido/ctap_get_assertion_request.h
[modify] https://crrev.com/90c62574f47315a1a1b8343871616ea27e54be1b/device/fido/ctap_make_credential_request.cc
[modify] https://crrev.com/90c62574f47315a1a1b8343871616ea27e54be1b/device/fido/ctap_make_credential_request.h
[modify] https://crrev.com/90c62574f47315a1a1b8343871616ea27e54be1b/device/fido/ctap_request_unittest.cc
[modify] https://crrev.com/90c62574f47315a1a1b8343871616ea27e54be1b/device/fido/fido_test_data.h
[modify] https://crrev.com/90c62574f47315a1a1b8343871616ea27e54be1b/device/fido/get_assertion_handler_unittest.cc
[modify] https://crrev.com/90c62574f47315a1a1b8343871616ea27e54be1b/device/fido/get_assertion_task_unittest.cc
[modify] https://crrev.com/90c62574f47315a1a1b8343871616ea27e54be1b/device/fido/mac/browsing_data_deletion_unittest.mm
[modify] https://crrev.com/90c62574f47315a1a1b8343871616ea27e54be1b/device/fido/mac/get_assertion_operation_unittest_mac.mm
[modify] https://crrev.com/90c62574f47315a1a1b8343871616ea27e54be1b/device/fido/mac/make_credential_operation_unittest_mac.mm
[modify] https://crrev.com/90c62574f47315a1a1b8343871616ea27e54be1b/device/fido/make_credential_handler_unittest.cc
[modify] https://crrev.com/90c62574f47315a1a1b8343871616ea27e54be1b/device/fido/make_credential_task_unittest.cc
[modify] https://crrev.com/90c62574f47315a1a1b8343871616ea27e54be1b/device/fido/u2f_command_constructor_unittest.cc
[modify] https://crrev.com/90c62574f47315a1a1b8343871616ea27e54be1b/device/fido/u2f_register_operation_unittest.cc
[modify] https://crrev.com/90c62574f47315a1a1b8343871616ea27e54be1b/device/fido/u2f_sign_operation_unittest.cc
[modify] https://crrev.com/90c62574f47315a1a1b8343871616ea27e54be1b/device/fido/virtual_ctap2_device.cc
[modify] https://crrev.com/90c62574f47315a1a1b8343871616ea27e54be1b/device/fido/virtual_ctap2_device.h

Project Member

Comment 2 by bugdroid1@chromium.org, Oct 31

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7fc042a486e073e1df6453e0d40f416e6c28c8e1

commit 7fc042a486e073e1df6453e0d40f416e6c28c8e1
Author: Martin Kreichgauer <martinkr@chromium.org>
Date: Wed Oct 31 22:09:48 2018

fido: add initial code for integrating with the Windows WebAuthn API

WinWebAuthnApi is a wrapper around the native WebAuthn library. The
corresponding headers have not made it into the Windows SDK used by Chromium
yet, therefore none of this code is currently included in the build.

WinNativeCrossPlatformAuthenticator is a FidoAuthenticator implementation that
forwards requests to the native library.

Bug: 898718
Change-Id: I1c144bdeabe763debaadadad2c4937f9778c1d96
Reviewed-on: https://chromium-review.googlesource.com/c/1298734
Commit-Queue: Martin Kreichgauer <martinkr@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604405}
[modify] https://crrev.com/7fc042a486e073e1df6453e0d40f416e6c28c8e1/device/fido/BUILD.gn
[add] https://crrev.com/7fc042a486e073e1df6453e0d40f416e6c28c8e1/device/fido/buildflags.gni
[modify] https://crrev.com/7fc042a486e073e1df6453e0d40f416e6c28c8e1/device/fido/fido_constants.h
[add] https://crrev.com/7fc042a486e073e1df6453e0d40f416e6c28c8e1/device/fido/win/authenticator.cc
[add] https://crrev.com/7fc042a486e073e1df6453e0d40f416e6c28c8e1/device/fido/win/authenticator.h
[add] https://crrev.com/7fc042a486e073e1df6453e0d40f416e6c28c8e1/device/fido/win/type_conversions.cc
[add] https://crrev.com/7fc042a486e073e1df6453e0d40f416e6c28c8e1/device/fido/win/type_conversions.h
[add] https://crrev.com/7fc042a486e073e1df6453e0d40f416e6c28c8e1/device/fido/win/webauthn_api.cc
[add] https://crrev.com/7fc042a486e073e1df6453e0d40f416e6c28c8e1/device/fido/win/webauthn_api.h

Project Member

Comment 3 by bugdroid1@chromium.org, Nov 2

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6c763866d7e1479277be50b8404549b12d652858

commit 6c763866d7e1479277be50b8404549b12d652858
Author: Martin Kreichgauer <martinkr@chromium.org>
Date: Fri Nov 02 01:43:15 2018

fido: factor out a FidoDiscoveryFactory class

This moves the static factory methods for instantiating subclasses of
FidoDeviceDiscovery out of FidoDeviceDiscovery and into a new class called
FidoDiscoveryFactory. The ScopedFidoDeviceDiscoveryFactory is moved along with
it.

This will simplify changes that let us instantiate non-device authenticators
(Windows, Touch ID) through the existing factory methods. Also, instantiating
different kinds of FidoDiscovery is becoming complex enough that centralizing
this code in a single place, separate from any of the FidoDiscovery
implementations seems sensible.

Bug: 898718
Change-Id: I6c114c5aa9e8db4538a003108c4e7acec7100fbd
Reviewed-on: https://chromium-review.googlesource.com/c/1311841
Commit-Queue: Martin Kreichgauer <martinkr@chromium.org>
Reviewed-by: Kim Paulhamus <kpaulhamus@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604806}
[modify] https://crrev.com/6c763866d7e1479277be50b8404549b12d652858/content/browser/webauth/scoped_virtual_authenticator_environment.cc
[modify] https://crrev.com/6c763866d7e1479277be50b8404549b12d652858/content/browser/webauth/scoped_virtual_authenticator_environment.h
[modify] https://crrev.com/6c763866d7e1479277be50b8404549b12d652858/content/browser/webauth/webauth_browsertest.cc
[modify] https://crrev.com/6c763866d7e1479277be50b8404549b12d652858/content/test/BUILD.gn
[modify] https://crrev.com/6c763866d7e1479277be50b8404549b12d652858/device/fido/BUILD.gn
[modify] https://crrev.com/6c763866d7e1479277be50b8404549b12d652858/device/fido/fake_fido_discovery.cc
[modify] https://crrev.com/6c763866d7e1479277be50b8404549b12d652858/device/fido/fake_fido_discovery.h
[modify] https://crrev.com/6c763866d7e1479277be50b8404549b12d652858/device/fido/fake_fido_discovery_unittest.cc
[modify] https://crrev.com/6c763866d7e1479277be50b8404549b12d652858/device/fido/fido_device_discovery.cc
[modify] https://crrev.com/6c763866d7e1479277be50b8404549b12d652858/device/fido/fido_device_discovery.h
[add] https://crrev.com/6c763866d7e1479277be50b8404549b12d652858/device/fido/fido_discovery_factory.cc
[add] https://crrev.com/6c763866d7e1479277be50b8404549b12d652858/device/fido/fido_discovery_factory.h
[modify] https://crrev.com/6c763866d7e1479277be50b8404549b12d652858/device/fido/fido_request_handler_base.cc
[modify] https://crrev.com/6c763866d7e1479277be50b8404549b12d652858/device/fido/fido_request_handler_base.h
[modify] https://crrev.com/6c763866d7e1479277be50b8404549b12d652858/device/fido/get_assertion_request_handler.cc
[modify] https://crrev.com/6c763866d7e1479277be50b8404549b12d652858/device/fido/scoped_virtual_fido_device.cc
[modify] https://crrev.com/6c763866d7e1479277be50b8404549b12d652858/device/fido/scoped_virtual_fido_device.h

Project Member

Comment 4 by bugdroid1@chromium.org, Nov 2

Project Member

Comment 5 by bugdroid1@chromium.org, Nov 6

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f8d1900f4307d912b2501836c04d9966cbd03a5b

commit f8d1900f4307d912b2501836c04d9966cbd03a5b
Author: Martin Kreichgauer <martinkr@chromium.org>
Date: Tue Nov 06 02:02:50 2018

fido: use WinNativeCrossPlatformAuthenticator when flag enabled

This adds a FidoDiscoveryBase class to instantiate the Windows
authenticator. Instantiatiation of the new Discovery at request time is
conditioned on a newly added feature flag and availability of the
Windows WebAuthn API (which acts as our proxy signal for whether USB
devices are blocked). For requests where the discovery is enabled, the
existing FidoHidDiscovery is disabled.

Bug: 898718
Change-Id: I2048e179bd3987ed3703b3818870253fe2a50e2c
Reviewed-on: https://chromium-review.googlesource.com/c/1313733
Commit-Queue: Martin Kreichgauer <martinkr@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Adam Langley <agl@chromium.org>
Reviewed-by: Kim Paulhamus <kpaulhamus@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605567}
[modify] https://crrev.com/f8d1900f4307d912b2501836c04d9966cbd03a5b/device/fido/BUILD.gn
[add] https://crrev.com/f8d1900f4307d912b2501836c04d9966cbd03a5b/device/fido/features.cc
[add] https://crrev.com/f8d1900f4307d912b2501836c04d9966cbd03a5b/device/fido/features.h
[modify] https://crrev.com/f8d1900f4307d912b2501836c04d9966cbd03a5b/device/fido/fido_discovery_factory.cc
[add] https://crrev.com/f8d1900f4307d912b2501836c04d9966cbd03a5b/device/fido/win/discovery.cc
[add] https://crrev.com/f8d1900f4307d912b2501836c04d9966cbd03a5b/device/fido/win/discovery.h

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/df06312dadd11254c469eef00c4b4aaa27d1e877

commit df06312dadd11254c469eef00c4b4aaa27d1e877
Author: Martin Kreichgauer <martinkr@chromium.org>
Date: Thu Nov 08 00:43:36 2018

fido: initial tests for Windows WebAuthn API integration

This adds a fake implementation of WinWebAuthnApi along with a scoped
override of WinWebAuthnApi::GetDefault, as well as a test to verify USB
authenticator instantiation depending on API availability and feature
flag state.

Bug: 898718
Change-Id: Iff3a317b4540986775a40fc872754ba0571ae88a
Reviewed-on: https://chromium-review.googlesource.com/c/1316839
Commit-Queue: Martin Kreichgauer <martinkr@chromium.org>
Reviewed-by: Jun Choi <hongjunchoi@chromium.org>
Reviewed-by: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606254}
[modify] https://crrev.com/df06312dadd11254c469eef00c4b4aaa27d1e877/device/fido/BUILD.gn
[modify] https://crrev.com/df06312dadd11254c469eef00c4b4aaa27d1e877/device/fido/fido_request_handler_base.h
[modify] https://crrev.com/df06312dadd11254c469eef00c4b4aaa27d1e877/device/fido/fido_request_handler_unittest.cc
[modify] https://crrev.com/df06312dadd11254c469eef00c4b4aaa27d1e877/device/fido/get_assertion_handler_unittest.cc
[modify] https://crrev.com/df06312dadd11254c469eef00c4b4aaa27d1e877/device/fido/hid/fake_hid_impl_for_testing.cc
[modify] https://crrev.com/df06312dadd11254c469eef00c4b4aaa27d1e877/device/fido/hid/fake_hid_impl_for_testing.h
[modify] https://crrev.com/df06312dadd11254c469eef00c4b4aaa27d1e877/device/fido/hid/fido_hid_discovery_unittest.cc
[modify] https://crrev.com/df06312dadd11254c469eef00c4b4aaa27d1e877/device/fido/win/discovery.cc
[modify] https://crrev.com/df06312dadd11254c469eef00c4b4aaa27d1e877/device/fido/win/discovery.h
[add] https://crrev.com/df06312dadd11254c469eef00c4b4aaa27d1e877/device/fido/win/fake_webauthn_api.cc
[add] https://crrev.com/df06312dadd11254c469eef00c4b4aaa27d1e877/device/fido/win/fake_webauthn_api.h
[modify] https://crrev.com/df06312dadd11254c469eef00c4b4aaa27d1e877/device/fido/win/webauthn_api.cc
[modify] https://crrev.com/df06312dadd11254c469eef00c4b4aaa27d1e877/device/fido/win/webauthn_api.h

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d0392055d87abcb3b311cf8b4d6a8fe9b1dc0578

commit d0392055d87abcb3b311cf8b4d6a8fe9b1dc0578
Author: Martin Kreichgauer <martinkr@chromium.org>
Date: Fri Nov 09 01:15:57 2018

fido/win: build Windows WebAuthn API integration unconditionally

This makes fido/win include webauthn.h from
//third_party/microsoft_webauthn and drops the USE_WIN_WEBAUTHN_API
build flag we previously added due to the missing header file.

The implementation remains behind a default-off feature flag.

Bug: 898718
Change-Id: I45e100f8b10cc32ea7e32d2842a63c950b5965bb
Reviewed-on: https://chromium-review.googlesource.com/c/1325074
Commit-Queue: Martin Kreichgauer <martinkr@chromium.org>
Reviewed-by: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606686}
[modify] https://crrev.com/d0392055d87abcb3b311cf8b4d6a8fe9b1dc0578/device/fido/BUILD.gn
[modify] https://crrev.com/d0392055d87abcb3b311cf8b4d6a8fe9b1dc0578/device/fido/DEPS
[delete] https://crrev.com/2740836b762251b1b8b21983fe46e6f468403b53/device/fido/buildflags.gni
[modify] https://crrev.com/d0392055d87abcb3b311cf8b4d6a8fe9b1dc0578/device/fido/features.cc
[modify] https://crrev.com/d0392055d87abcb3b311cf8b4d6a8fe9b1dc0578/device/fido/features.h
[modify] https://crrev.com/d0392055d87abcb3b311cf8b4d6a8fe9b1dc0578/device/fido/fido_discovery_factory.cc
[modify] https://crrev.com/d0392055d87abcb3b311cf8b4d6a8fe9b1dc0578/device/fido/get_assertion_handler_unittest.cc
[modify] https://crrev.com/d0392055d87abcb3b311cf8b4d6a8fe9b1dc0578/device/fido/win/authenticator.cc
[modify] https://crrev.com/d0392055d87abcb3b311cf8b4d6a8fe9b1dc0578/device/fido/win/type_conversions.cc
[modify] https://crrev.com/d0392055d87abcb3b311cf8b4d6a8fe9b1dc0578/device/fido/win/type_conversions.h
[modify] https://crrev.com/d0392055d87abcb3b311cf8b4d6a8fe9b1dc0578/device/fido/win/webauthn_api.h
[add] https://crrev.com/d0392055d87abcb3b311cf8b4d6a8fe9b1dc0578/third_party/microsoft_webauthn/BUILD.gn
[modify] https://crrev.com/d0392055d87abcb3b311cf8b4d6a8fe9b1dc0578/third_party/microsoft_webauthn/README.chromium
[modify] https://crrev.com/d0392055d87abcb3b311cf8b4d6a8fe9b1dc0578/third_party/microsoft_webauthn/webauthn.h

Project Member

Comment 9 by bugdroid1@chromium.org, Nov 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bb253c29e6be82162bf0026930c44776ce15bb26

commit bb253c29e6be82162bf0026930c44776ce15bb26
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Fri Nov 09 01:55:03 2018

Revert "fido/win: build Windows WebAuthn API integration unconditionally"

This reverts commit d0392055d87abcb3b311cf8b4d6a8fe9b1dc0578.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 606686 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtL2QwMzkyMDU1ZDg3YWJjYjNiMzExY2Y4YjRkNmE4ZmU5YjFkYzA1NzgM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium/linux-rel/9945

Sample Failed Step: compile

Original change's description:
> fido/win: build Windows WebAuthn API integration unconditionally
> 
> This makes fido/win include webauthn.h from
> //third_party/microsoft_webauthn and drops the USE_WIN_WEBAUTHN_API
> build flag we previously added due to the missing header file.
> 
> The implementation remains behind a default-off feature flag.
> 
> Bug: 898718
> Change-Id: I45e100f8b10cc32ea7e32d2842a63c950b5965bb
> Reviewed-on: https://chromium-review.googlesource.com/c/1325074
> Commit-Queue: Martin Kreichgauer <martinkr@chromium.org>
> Reviewed-by: Adam Langley <agl@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#606686}

Change-Id: I53414fbe41280dbcb7a753599c946f5ef5ebd4ee
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 898718
Reviewed-on: https://chromium-review.googlesource.com/c/1328223
Cr-Commit-Position: refs/heads/master@{#606693}
[modify] https://crrev.com/bb253c29e6be82162bf0026930c44776ce15bb26/device/fido/BUILD.gn
[modify] https://crrev.com/bb253c29e6be82162bf0026930c44776ce15bb26/device/fido/DEPS
[add] https://crrev.com/bb253c29e6be82162bf0026930c44776ce15bb26/device/fido/buildflags.gni
[modify] https://crrev.com/bb253c29e6be82162bf0026930c44776ce15bb26/device/fido/features.cc
[modify] https://crrev.com/bb253c29e6be82162bf0026930c44776ce15bb26/device/fido/features.h
[modify] https://crrev.com/bb253c29e6be82162bf0026930c44776ce15bb26/device/fido/fido_discovery_factory.cc
[modify] https://crrev.com/bb253c29e6be82162bf0026930c44776ce15bb26/device/fido/get_assertion_handler_unittest.cc
[modify] https://crrev.com/bb253c29e6be82162bf0026930c44776ce15bb26/device/fido/win/authenticator.cc
[modify] https://crrev.com/bb253c29e6be82162bf0026930c44776ce15bb26/device/fido/win/type_conversions.cc
[modify] https://crrev.com/bb253c29e6be82162bf0026930c44776ce15bb26/device/fido/win/type_conversions.h
[modify] https://crrev.com/bb253c29e6be82162bf0026930c44776ce15bb26/device/fido/win/webauthn_api.h
[delete] https://crrev.com/3b5865d078208c5828d9506da427d4c9288d821b/third_party/microsoft_webauthn/BUILD.gn
[modify] https://crrev.com/bb253c29e6be82162bf0026930c44776ce15bb26/third_party/microsoft_webauthn/README.chromium
[modify] https://crrev.com/bb253c29e6be82162bf0026930c44776ce15bb26/third_party/microsoft_webauthn/webauthn.h

Project Member

Comment 10 by bugdroid1@chromium.org, Nov 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/203f7c767e6cfd6246939cb327da03e1f004d60f

commit 203f7c767e6cfd6246939cb327da03e1f004d60f
Author: Martin Kreichgauer <martinkr@chromium.org>
Date: Fri Nov 09 03:02:37 2018

third_party: update microsoft_webauthn to 248f7abb10e427622253612d39ccb2fbfa5c7cfe

Bug: 898718
Change-Id: Ib0ff64a97b2fb012a967ec6344c2eccacf876436
Reviewed-on: https://chromium-review.googlesource.com/c/1327533
Commit-Queue: Martin Kreichgauer <martinkr@chromium.org>
Reviewed-by: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606719}
[modify] https://crrev.com/203f7c767e6cfd6246939cb327da03e1f004d60f/third_party/microsoft_webauthn/README.chromium
[modify] https://crrev.com/203f7c767e6cfd6246939cb327da03e1f004d60f/third_party/microsoft_webauthn/webauthn.h

Project Member

Comment 11 by bugdroid1@chromium.org, Nov 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7537781ba1b6539f556b2c08139abd9d82c0734c

commit 7537781ba1b6539f556b2c08139abd9d82c0734c
Author: Martin Kreichgauer <martinkr@chromium.org>
Date: Fri Nov 09 18:58:13 2018

Revert "Revert "fido/win: build Windows WebAuthn API integration unconditionally""

This reverts commit bb253c29e6be82162bf0026930c44776ce15bb26.

This is a reland of the original breaking commit,
d0392055d87abcb3b311cf8b4d6a8fe9b1dc0578, with a fix for the build
failure.

Bug: 898718
Change-Id: I0e14e97861d060bb6378dc6ad43ca52806812f1e
Reviewed-on: https://chromium-review.googlesource.com/c/1328232
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606922}
[modify] https://crrev.com/7537781ba1b6539f556b2c08139abd9d82c0734c/device/fido/BUILD.gn
[modify] https://crrev.com/7537781ba1b6539f556b2c08139abd9d82c0734c/device/fido/DEPS
[delete] https://crrev.com/dfa0f14fb34433bf3e722e64b0ba5619de236cf0/device/fido/buildflags.gni
[modify] https://crrev.com/7537781ba1b6539f556b2c08139abd9d82c0734c/device/fido/features.cc
[modify] https://crrev.com/7537781ba1b6539f556b2c08139abd9d82c0734c/device/fido/features.h
[modify] https://crrev.com/7537781ba1b6539f556b2c08139abd9d82c0734c/device/fido/fido_discovery_factory.cc
[modify] https://crrev.com/7537781ba1b6539f556b2c08139abd9d82c0734c/device/fido/fido_request_handler_unittest.cc
[modify] https://crrev.com/7537781ba1b6539f556b2c08139abd9d82c0734c/device/fido/get_assertion_handler_unittest.cc
[modify] https://crrev.com/7537781ba1b6539f556b2c08139abd9d82c0734c/device/fido/win/authenticator.cc
[modify] https://crrev.com/7537781ba1b6539f556b2c08139abd9d82c0734c/device/fido/win/type_conversions.cc
[modify] https://crrev.com/7537781ba1b6539f556b2c08139abd9d82c0734c/device/fido/win/type_conversions.h
[modify] https://crrev.com/7537781ba1b6539f556b2c08139abd9d82c0734c/device/fido/win/webauthn_api.h
[add] https://crrev.com/7537781ba1b6539f556b2c08139abd9d82c0734c/third_party/microsoft_webauthn/BUILD.gn
[modify] https://crrev.com/7537781ba1b6539f556b2c08139abd9d82c0734c/third_party/microsoft_webauthn/README.chromium
[modify] https://crrev.com/7537781ba1b6539f556b2c08139abd9d82c0734c/third_party/microsoft_webauthn/webauthn.h

Project Member

Comment 12 by bugdroid1@chromium.org, Nov 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fffbb7998ab8228899f0bcdfc0bce963cbce43ed

commit fffbb7998ab8228899f0bcdfc0bce963cbce43ed
Author: Martin Kreichgauer <martinkr@chromium.org>
Date: Fri Nov 09 21:39:47 2018

fido: make FidoAuthenticator::Options optional.

This changes the return type of FidoAuthenticator::Options() from
AuthenticatorSupportedOptions to
base::Optional<AuthenticatorSupportedOptions>.

The FidoAuthenticator subclass for the Windows WebAuthn API can
potentially talk to a number of different authenticators with
different capabilities and hence cannot return a sensible value for
this method. It previously returned a "maximum possible" Options
value; but that lead to a bug where GetAssertionHandler assumed it
could set the user verification requirement of a request from
"preferred" to "required" because the Windows authenticator reported
itself as having a PIN set up (while the actual physical device might
not even support PINs).

By returning an Optional, we make the Windows case more explicit.

The aforementioned bug is fixed by moving the "effective uv
requirement" computation for GetAssertion into
FidoDeviceAuthenticator.

For MakeCredential, we have a similar bug: Because
CtapMakeCredentialRequest only stores the UV requirement as a bool
rather than the full enum, we currently default to uv=false for
authenticators that *do* support UV (even in the non-Windows case),
which is wrong. I'm going to address this in an immediate follow up CL.

Bug: 898718
Change-Id: I3e40296578929c75585e820aa5764bd623e1fe79
Reviewed-on: https://chromium-review.googlesource.com/c/1321852
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Kim Paulhamus <kpaulhamus@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606973}
[modify] https://crrev.com/fffbb7998ab8228899f0bcdfc0bce963cbce43ed/content/browser/webauth/authenticator_impl.cc
[modify] https://crrev.com/fffbb7998ab8228899f0bcdfc0bce963cbce43ed/device/fido/authenticator_supported_options.cc
[modify] https://crrev.com/fffbb7998ab8228899f0bcdfc0bce963cbce43ed/device/fido/authenticator_supported_options.h
[modify] https://crrev.com/fffbb7998ab8228899f0bcdfc0bce963cbce43ed/device/fido/fido_authenticator.h
[modify] https://crrev.com/fffbb7998ab8228899f0bcdfc0bce963cbce43ed/device/fido/fido_device_authenticator.cc
[modify] https://crrev.com/fffbb7998ab8228899f0bcdfc0bce963cbce43ed/device/fido/fido_device_authenticator.h
[modify] https://crrev.com/fffbb7998ab8228899f0bcdfc0bce963cbce43ed/device/fido/get_assertion_request_handler.cc
[modify] https://crrev.com/fffbb7998ab8228899f0bcdfc0bce963cbce43ed/device/fido/mac/authenticator.h
[modify] https://crrev.com/fffbb7998ab8228899f0bcdfc0bce963cbce43ed/device/fido/mac/authenticator.mm
[modify] https://crrev.com/fffbb7998ab8228899f0bcdfc0bce963cbce43ed/device/fido/make_credential_request_handler.cc
[modify] https://crrev.com/fffbb7998ab8228899f0bcdfc0bce963cbce43ed/device/fido/win/authenticator.cc
[modify] https://crrev.com/fffbb7998ab8228899f0bcdfc0bce963cbce43ed/device/fido/win/authenticator.h

Project Member

Comment 13 by bugdroid1@chromium.org, Nov 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dfbcc8a568f50d23733ab3a73c2a958aa22129c0

commit dfbcc8a568f50d23733ab3a73c2a958aa22129c0
Author: Martin Kreichgauer <martinkr@chromium.org>
Date: Fri Nov 09 22:31:00 2018

fido/win: gate Windows WebAuthn API availability on a minimum version

There are versions of webauthn.dll that ship without BLE support. On
platforms where that is the case, direct access to FIDO token is not
blocked by the OS and so we don't want to integrate with webauthn.dll
there. This changes WinWebAuthnApi to call WebAuthNGetApiVersion on
initialization and mark the API as unavailable if the version number is
 not at least the current version (1).

The check may be overridden by a flag while we wait for
WebAuthNGetApiVersion function to ship.

Bug: 898718
Change-Id: Ic20e55a416d858214b9f444031f4700567933eb4
Reviewed-on: https://chromium-review.googlesource.com/c/1327885
Commit-Queue: Martin Kreichgauer <martinkr@chromium.org>
Reviewed-by: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607005}
[modify] https://crrev.com/dfbcc8a568f50d23733ab3a73c2a958aa22129c0/device/fido/features.cc
[modify] https://crrev.com/dfbcc8a568f50d23733ab3a73c2a958aa22129c0/device/fido/features.h
[modify] https://crrev.com/dfbcc8a568f50d23733ab3a73c2a958aa22129c0/device/fido/win/webauthn_api.cc

Project Member

Comment 14 by bugdroid1@chromium.org, Nov 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b1d355cbfe64cfdbc9107c70021b34fbb17bedc7

commit b1d355cbfe64cfdbc9107c70021b34fbb17bedc7
Author: Martin Kreichgauer <martinkr@chromium.org>
Date: Sat Nov 10 02:11:47 2018

fido: make CtapMakeCredentialRequest carry the full UserVerificationRequirement

This adds a UserVerificationRequirement enum field to
CtapMakeCredentialRequest. The existing "effective" user verification
bool is removed and the computation of the effective value (required vs
discouraged) is moved into FidoDeviceAuthenticator, in parallel to how
this is handled for GetAssertion requests.

This change makes the full uv requirement available to the Windows
authenticators, and at the same time fixes a bug for device
authenticators where requests with uv="preferred" would result in
CTAP device requests with uv=false even for authenticators with UV
support.

Also rename CtapMakeCredentialRequest::resident_key_supported() to
resident_key_required().

Bug: 898718
Change-Id: I80ba3f052f871dce711c15ed8659812ad7cab10b
Reviewed-on: https://chromium-review.googlesource.com/c/1324379
Commit-Queue: Martin Kreichgauer <martinkr@chromium.org>
Reviewed-by: Kim Paulhamus <kpaulhamus@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607074}
[modify] https://crrev.com/b1d355cbfe64cfdbc9107c70021b34fbb17bedc7/content/browser/webauth/authenticator_impl.cc
[modify] https://crrev.com/b1d355cbfe64cfdbc9107c70021b34fbb17bedc7/device/fido/ctap_make_credential_request.cc
[modify] https://crrev.com/b1d355cbfe64cfdbc9107c70021b34fbb17bedc7/device/fido/ctap_make_credential_request.h
[modify] https://crrev.com/b1d355cbfe64cfdbc9107c70021b34fbb17bedc7/device/fido/ctap_request_unittest.cc
[modify] https://crrev.com/b1d355cbfe64cfdbc9107c70021b34fbb17bedc7/device/fido/fido_device_authenticator.cc
[modify] https://crrev.com/b1d355cbfe64cfdbc9107c70021b34fbb17bedc7/device/fido/make_credential_request_handler.cc
[modify] https://crrev.com/b1d355cbfe64cfdbc9107c70021b34fbb17bedc7/device/fido/make_credential_task.cc
[modify] https://crrev.com/b1d355cbfe64cfdbc9107c70021b34fbb17bedc7/device/fido/make_credential_task_unittest.cc
[modify] https://crrev.com/b1d355cbfe64cfdbc9107c70021b34fbb17bedc7/device/fido/u2f_command_constructor.cc
[modify] https://crrev.com/b1d355cbfe64cfdbc9107c70021b34fbb17bedc7/device/fido/u2f_command_constructor_unittest.cc
[modify] https://crrev.com/b1d355cbfe64cfdbc9107c70021b34fbb17bedc7/device/fido/virtual_ctap2_device.cc
[modify] https://crrev.com/b1d355cbfe64cfdbc9107c70021b34fbb17bedc7/device/fido/win/authenticator.cc

Project Member

Comment 15 by bugdroid1@chromium.org, Nov 13

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c6de3bcea5342586895fe09952f3132eaf707dbd

commit c6de3bcea5342586895fe09952f3132eaf707dbd
Author: Martin Kreichgauer <martinkr@chromium.org>
Date: Tue Nov 13 03:14:00 2018

fido: make transport fields optional

This changes the return values of
FidoAuthenticator::AuthenticatorTransport() and
AuthenticatorMakeCredentialResponse::transport_used() to
base::Optional<FidoTransportProtocol>.

This accommodates the Windows API integration use case where (a) the
FidoAuthenticator subclass cannot guarantee any particular transport
type and (b) the response we receive from the API contains no hints
about which transports were used.

Bug: 898718
Change-Id: Iadb83310f03d5f54ad50babb73e0963bdd21e3c2
Reviewed-on: https://chromium-review.googlesource.com/c/1325220
Reviewed-by: Jun Choi <hongjunchoi@chromium.org>
Commit-Queue: Martin Kreichgauer <martinkr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607463}
[modify] https://crrev.com/c6de3bcea5342586895fe09952f3132eaf707dbd/chrome/browser/webauthn/authenticator_request_dialog_model.cc
[modify] https://crrev.com/c6de3bcea5342586895fe09952f3132eaf707dbd/chrome/browser/webauthn/chrome_authenticator_request_delegate.cc
[modify] https://crrev.com/c6de3bcea5342586895fe09952f3132eaf707dbd/content/browser/webauth/authenticator_impl.cc
[modify] https://crrev.com/c6de3bcea5342586895fe09952f3132eaf707dbd/content/browser/webauth/authenticator_impl.h
[modify] https://crrev.com/c6de3bcea5342586895fe09952f3132eaf707dbd/device/fido/authenticator_make_credential_response.cc
[modify] https://crrev.com/c6de3bcea5342586895fe09952f3132eaf707dbd/device/fido/authenticator_make_credential_response.h
[modify] https://crrev.com/c6de3bcea5342586895fe09952f3132eaf707dbd/device/fido/fido_authenticator.h
[modify] https://crrev.com/c6de3bcea5342586895fe09952f3132eaf707dbd/device/fido/fido_device_authenticator.cc
[modify] https://crrev.com/c6de3bcea5342586895fe09952f3132eaf707dbd/device/fido/fido_device_authenticator.h
[modify] https://crrev.com/c6de3bcea5342586895fe09952f3132eaf707dbd/device/fido/fido_request_handler.h
[modify] https://crrev.com/c6de3bcea5342586895fe09952f3132eaf707dbd/device/fido/fido_request_handler_base.cc
[modify] https://crrev.com/c6de3bcea5342586895fe09952f3132eaf707dbd/device/fido/fido_request_handler_unittest.cc
[modify] https://crrev.com/c6de3bcea5342586895fe09952f3132eaf707dbd/device/fido/get_assertion_handler_unittest.cc
[modify] https://crrev.com/c6de3bcea5342586895fe09952f3132eaf707dbd/device/fido/get_assertion_request_handler.cc
[modify] https://crrev.com/c6de3bcea5342586895fe09952f3132eaf707dbd/device/fido/get_assertion_request_handler.h
[modify] https://crrev.com/c6de3bcea5342586895fe09952f3132eaf707dbd/device/fido/mac/authenticator.h
[modify] https://crrev.com/c6de3bcea5342586895fe09952f3132eaf707dbd/device/fido/mac/authenticator.mm
[modify] https://crrev.com/c6de3bcea5342586895fe09952f3132eaf707dbd/device/fido/make_credential_handler_unittest.cc
[modify] https://crrev.com/c6de3bcea5342586895fe09952f3132eaf707dbd/device/fido/make_credential_request_handler.cc
[modify] https://crrev.com/c6de3bcea5342586895fe09952f3132eaf707dbd/device/fido/make_credential_request_handler.h
[modify] https://crrev.com/c6de3bcea5342586895fe09952f3132eaf707dbd/device/fido/win/authenticator.cc
[modify] https://crrev.com/c6de3bcea5342586895fe09952f3132eaf707dbd/device/fido/win/authenticator.h
[modify] https://crrev.com/c6de3bcea5342586895fe09952f3132eaf707dbd/device/fido/win/type_conversions.cc

Project Member

Comment 16 by bugdroid1@chromium.org, Nov 14

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b129ae569a92f457b78f20011c9d728c86f953ef

commit b129ae569a92f457b78f20011c9d728c86f953ef
Author: Martin Kreichgauer <martinkr@chromium.org>
Date: Wed Nov 14 21:26:33 2018

fido: invoke Windows API authenticator for all transports

Rather than only forwarding USB requests to the Windows WebAuthn API,
we are going to forward all requests. This consequently renames the
FidoAuthenticator subclass to WinWebAuthnApiAuthenticator. The
corresponding FidoDiscovery is instantiated for all requests where the
WebAuthn API is available (i.e., the feature is flag enabled, and the
DLL can be loaded and is at least version 1). When it is indeed
available, all other discoveries are disabled and the UI is suppressed
via a newly introduced option in the TransportAvailabilityInfo struct
relayed to embedders via the TransportAvailabilityObserver interface.

Note this temporarily breaks caBLE whenever the Windows API is
flag-enabled and available. (But the OS would presumably block the
device communication in that case anyway.)

Bug: 898718
Change-Id: If2bddac4bac519d1aa5aa9cb5d8fdc326297de73
Reviewed-on: https://chromium-review.googlesource.com/c/1330897
Commit-Queue: Martin Kreichgauer <martinkr@chromium.org>
Reviewed-by: Jun Choi <hongjunchoi@chromium.org>
Reviewed-by: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608127}
[modify] https://crrev.com/b129ae569a92f457b78f20011c9d728c86f953ef/chrome/browser/webauthn/authenticator_request_dialog_model.cc
[modify] https://crrev.com/b129ae569a92f457b78f20011c9d728c86f953ef/chrome/browser/webauthn/chrome_authenticator_request_delegate.cc
[modify] https://crrev.com/b129ae569a92f457b78f20011c9d728c86f953ef/chrome/browser/webauthn/chrome_authenticator_request_delegate.h
[modify] https://crrev.com/b129ae569a92f457b78f20011c9d728c86f953ef/device/fido/fido_discovery_factory.cc
[modify] https://crrev.com/b129ae569a92f457b78f20011c9d728c86f953ef/device/fido/fido_discovery_factory.h
[modify] https://crrev.com/b129ae569a92f457b78f20011c9d728c86f953ef/device/fido/fido_request_handler_base.cc
[modify] https://crrev.com/b129ae569a92f457b78f20011c9d728c86f953ef/device/fido/fido_request_handler_base.h
[modify] https://crrev.com/b129ae569a92f457b78f20011c9d728c86f953ef/device/fido/get_assertion_handler_unittest.cc
[modify] https://crrev.com/b129ae569a92f457b78f20011c9d728c86f953ef/device/fido/win/authenticator.cc
[modify] https://crrev.com/b129ae569a92f457b78f20011c9d728c86f953ef/device/fido/win/authenticator.h
[modify] https://crrev.com/b129ae569a92f457b78f20011c9d728c86f953ef/device/fido/win/discovery.cc
[modify] https://crrev.com/b129ae569a92f457b78f20011c9d728c86f953ef/device/fido/win/discovery.h

Project Member

Comment 17 by bugdroid1@chromium.org, Nov 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8cd3170aec761acf1bc6203dfbfbf952e2e4f48d

commit 8cd3170aec761acf1bc6203dfbfbf952e2e4f48d
Author: Martin Kreichgauer <martinkr@chromium.org>
Date: Fri Nov 16 02:10:43 2018

fido: plumb AuthenticatorAttachment into CtapMakeCredentialRequest

This adds an AuthenticatorAttachment field to
CtapMakeCredentialRequest. The field is initialized by
MakeCredentialRequestHandler from the equivalent field on its
AuthenticatorSelectionCriteria parameter, and is consumed by
WinNativeApiAuthenticator to initialize the equivalent parameter passed
to the Windows WebAuthn API.

Also moves the AuthenticatorAttachment enum out of
AuthenticatorSelectionCriteria and into fido_constants.h, and fixes a
common misspelling of attachment throughout the code.

Also migrates the allowList and excludeList parameters of the Windows
WebAuthn API to an updated version that respects the
PublicKeyCredentialDescriptor's transports field.

Bug: 898718
Change-Id: I4dbc80929fcb6c4ff534822acd4b80410c14ae7f
Reviewed-on: https://chromium-review.googlesource.com/c/1335785
Commit-Queue: Martin Kreichgauer <martinkr@chromium.org>
Reviewed-by: Kim Paulhamus <kpaulhamus@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608631}
[modify] https://crrev.com/8cd3170aec761acf1bc6203dfbfbf952e2e4f48d/content/browser/webauth/authenticator_type_converters.cc
[modify] https://crrev.com/8cd3170aec761acf1bc6203dfbfbf952e2e4f48d/content/browser/webauth/authenticator_type_converters.h
[modify] https://crrev.com/8cd3170aec761acf1bc6203dfbfbf952e2e4f48d/device/fido/authenticator_selection_criteria.cc
[modify] https://crrev.com/8cd3170aec761acf1bc6203dfbfbf952e2e4f48d/device/fido/authenticator_selection_criteria.h
[modify] https://crrev.com/8cd3170aec761acf1bc6203dfbfbf952e2e4f48d/device/fido/ctap_make_credential_request.cc
[modify] https://crrev.com/8cd3170aec761acf1bc6203dfbfbf952e2e4f48d/device/fido/ctap_make_credential_request.h
[modify] https://crrev.com/8cd3170aec761acf1bc6203dfbfbf952e2e4f48d/device/fido/fido_constants.h
[modify] https://crrev.com/8cd3170aec761acf1bc6203dfbfbf952e2e4f48d/device/fido/make_credential_handler_unittest.cc
[modify] https://crrev.com/8cd3170aec761acf1bc6203dfbfbf952e2e4f48d/device/fido/make_credential_request_handler.cc
[modify] https://crrev.com/8cd3170aec761acf1bc6203dfbfbf952e2e4f48d/device/fido/win/authenticator.cc
[modify] https://crrev.com/8cd3170aec761acf1bc6203dfbfbf952e2e4f48d/device/fido/win/type_conversions.cc
[modify] https://crrev.com/8cd3170aec761acf1bc6203dfbfbf952e2e4f48d/device/fido/win/type_conversions.h

Project Member

Comment 18 by bugdroid1@chromium.org, Nov 20

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/57d5d1dc0fc221b00d04ced5734e670cf602386c

commit 57d5d1dc0fc221b00d04ced5734e670cf602386c
Author: Martin Kreichgauer <martinkr@chromium.org>
Date: Tue Nov 20 07:11:55 2018

fido: add U2F-only mode for cryptotoken requests

This adds an |is_u2f_only_| field to CtapMakeCredentialRequest to make
authenticators aware whether the current request originates from the
cryptotoken (U2F) extensions. If it does, request handling changes as follows:
 - For Windows API requests, U2F-only mode is enabled.
 - FidoDeviceAuthenticators will only communicate via U2F, not CTAP.

This is done to force the authenticator return a U2F-format attestation
statement, rather than one in an (incompatible) CTAP2 format such as
"packed".

GetAssertionTask already implements a U2F fallback for all requests with an App
ID extension (and CTAP need not be strictly avoided as is the case with
MakeCredential); thus no separate boolean field is required for
CtapGetAssertionRequest.

Bug: 898718
Change-Id: I8f17655d8df530546b62e2237c73aaa997483060
Reviewed-on: https://chromium-review.googlesource.com/c/1338952
Commit-Queue: Kim Paulhamus <kpaulhamus@chromium.org>
Reviewed-by: Adam Langley <agl@chromium.org>
Reviewed-by: Kim Paulhamus <kpaulhamus@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609618}
[modify] https://crrev.com/57d5d1dc0fc221b00d04ced5734e670cf602386c/content/browser/webauth/authenticator_impl.cc
[modify] https://crrev.com/57d5d1dc0fc221b00d04ced5734e670cf602386c/content/browser/webauth/authenticator_impl_unittest.cc
[modify] https://crrev.com/57d5d1dc0fc221b00d04ced5734e670cf602386c/device/fido/ctap_make_credential_request.h
[modify] https://crrev.com/57d5d1dc0fc221b00d04ced5734e670cf602386c/device/fido/make_credential_task.cc
[modify] https://crrev.com/57d5d1dc0fc221b00d04ced5734e670cf602386c/device/fido/make_credential_task_unittest.cc
[modify] https://crrev.com/57d5d1dc0fc221b00d04ced5734e670cf602386c/device/fido/win/authenticator.cc
[modify] https://crrev.com/57d5d1dc0fc221b00d04ced5734e670cf602386c/device/fido/win/authenticator.h

Project Member

Comment 19 by bugdroid1@chromium.org, Nov 21

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6d6c29c7fdcd584f5c8c86c6ab0a860f00a170ab

commit 6d6c29c7fdcd584f5c8c86c6ab0a860f00a170ab
Author: Martin Kreichgauer <martinkr@chromium.org>
Date: Wed Nov 21 08:10:12 2018

fido: disable attestation consent prompt for cryptotoken requests

The ContentBrowserClient in WebAuthn requests originating from the cryptotoken
extension is not associated with any tab and therefore trying to show dialogs,
such as the one for collecting attestation consent, will fail. cryptotoken
has its own attestation consent dialog already, so we can safely disable
attestation consent prompts if the caller origin is cryptotoken.

Bug: 898718
Change-Id: Ic2d530665494cfa10ac4c3a7812e8284091e50fb
Reviewed-on: https://chromium-review.googlesource.com/c/1340586
Commit-Queue: Kim Paulhamus <kpaulhamus@chromium.org>
Commit-Queue: Adam Langley <agl@chromium.org>
Reviewed-by: Adam Langley <agl@chromium.org>
Reviewed-by: Kim Paulhamus <kpaulhamus@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609947}
[modify] https://crrev.com/6d6c29c7fdcd584f5c8c86c6ab0a860f00a170ab/content/browser/webauth/authenticator_impl.cc
[modify] https://crrev.com/6d6c29c7fdcd584f5c8c86c6ab0a860f00a170ab/content/browser/webauth/authenticator_impl.h
[modify] https://crrev.com/6d6c29c7fdcd584f5c8c86c6ab0a860f00a170ab/content/browser/webauth/authenticator_impl_unittest.cc

Components: Blink>WebAuthentication
Labels: M-72 OS-Windows
Summary: Integrate with Windows WebAuthn for talking to FIDO authenticators (was: Integrate with Windows WebAuthn for talking to USB devices)
Project Member

Comment 21 by bugdroid1@chromium.org, Nov 29

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/977c0047cdf283e64d76b2ee0024743cbc8f1997

commit 977c0047cdf283e64d76b2ee0024743cbc8f1997
Author: Martin Kreichgauer <martinkr@chromium.org>
Date: Thu Nov 29 00:09:04 2018

fido: implement IsUVPAA for Windows

This wires the IsUserVerifyingPlatformAuthenticator API call up to its
implementation in the native Windows API, where available.

Bug: 898718
Change-Id: I40307ff39f8dc8e02197debc48041aad62d253ac
Reviewed-on: https://chromium-review.googlesource.com/c/1351900
Commit-Queue: Adam Langley <agl@chromium.org>
Reviewed-by: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611948}
[modify] https://crrev.com/977c0047cdf283e64d76b2ee0024743cbc8f1997/content/browser/webauth/authenticator_impl.cc
[modify] https://crrev.com/977c0047cdf283e64d76b2ee0024743cbc8f1997/content/browser/webauth/authenticator_impl_unittest.cc
[modify] https://crrev.com/977c0047cdf283e64d76b2ee0024743cbc8f1997/device/fido/ctap_get_assertion_request.h
[modify] https://crrev.com/977c0047cdf283e64d76b2ee0024743cbc8f1997/device/fido/ctap_make_credential_request.h
[modify] https://crrev.com/977c0047cdf283e64d76b2ee0024743cbc8f1997/device/fido/win/authenticator.cc
[modify] https://crrev.com/977c0047cdf283e64d76b2ee0024743cbc8f1997/device/fido/win/authenticator.h
[modify] https://crrev.com/977c0047cdf283e64d76b2ee0024743cbc8f1997/device/fido/win/fake_webauthn_api.cc
[modify] https://crrev.com/977c0047cdf283e64d76b2ee0024743cbc8f1997/device/fido/win/fake_webauthn_api.h

Project Member

Comment 22 by bugdroid1@chromium.org, Nov 29

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2542c2aeef86fd8bdee6c6f16902ca39496ceb2a

commit 2542c2aeef86fd8bdee6c6f16902ca39496ceb2a
Author: Martin Kreichgauer <martinkr@chromium.org>
Date: Thu Nov 29 04:16:16 2018

fido/win: make WinWebAuthnApiAuthenticator dtor not block

In debug mode, Windows WebAuthn requests would previously crash because the
WinWebAuthnApiAuthenticator destructor invoked the destructor of its
base::Thread member, which is blocking, on a task runner that was non-blocking.

This works around the issue by moving the thread for doing the blocking Windows
API calls into the WinWebAuthnApi wrapper class. Unfortunately, this moves the
inputs to the API call out of scope with the actual blocking call, which is a
problem since they get passed as pointers. Therefore we now have to stash all
of this data temporarily in the FidoAuthenticator instance while the API call
is pending.

Bug: 898718
Change-Id: Ic36c4f9119e1fcc452cc789182f98c3c59f51610
Reviewed-on: https://chromium-review.googlesource.com/c/1354559
Commit-Queue: Martin Kreichgauer <martinkr@chromium.org>
Reviewed-by: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612034}
[modify] https://crrev.com/2542c2aeef86fd8bdee6c6f16902ca39496ceb2a/content/browser/webauth/authenticator_impl_unittest.cc
[modify] https://crrev.com/2542c2aeef86fd8bdee6c6f16902ca39496ceb2a/device/fido/win/authenticator.cc
[modify] https://crrev.com/2542c2aeef86fd8bdee6c6f16902ca39496ceb2a/device/fido/win/authenticator.h
[modify] https://crrev.com/2542c2aeef86fd8bdee6c6f16902ca39496ceb2a/device/fido/win/fake_webauthn_api.cc
[modify] https://crrev.com/2542c2aeef86fd8bdee6c6f16902ca39496ceb2a/device/fido/win/fake_webauthn_api.h
[modify] https://crrev.com/2542c2aeef86fd8bdee6c6f16902ca39496ceb2a/device/fido/win/webauthn_api.cc
[modify] https://crrev.com/2542c2aeef86fd8bdee6c6f16902ca39496ceb2a/device/fido/win/webauthn_api.h

Project Member

Comment 23 by bugdroid1@chromium.org, Nov 29

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9f832ef137dabcd06769d0671672cf15c12eb39c

commit 9f832ef137dabcd06769d0671672cf15c12eb39c
Author: Martin Kreichgauer <martinkr@chromium.org>
Date: Thu Nov 29 20:58:27 2018

fido: add the unhashed AppID to CtapGetAssertionRequest

This changes CtapGetAssertionRequest to carry the unhashed AppID rather only
its hashed form, the "Alternative Application Parameter". The unhashed value
is passed to the Windows WebAuthn API for U2F sign requests.

Bug: 898718
Change-Id: I2d9e6b80463859adba9a786e0c478c9d63e4164b
Reviewed-on: https://chromium-review.googlesource.com/c/1354689
Commit-Queue: Martin Kreichgauer <martinkr@chromium.org>
Reviewed-by: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612349}
[modify] https://crrev.com/9f832ef137dabcd06769d0671672cf15c12eb39c/content/browser/webauth/authenticator_impl.cc
[modify] https://crrev.com/9f832ef137dabcd06769d0671672cf15c12eb39c/content/browser/webauth/authenticator_impl.h
[modify] https://crrev.com/9f832ef137dabcd06769d0671672cf15c12eb39c/device/fido/ctap_get_assertion_request.cc
[modify] https://crrev.com/9f832ef137dabcd06769d0671672cf15c12eb39c/device/fido/ctap_get_assertion_request.h
[modify] https://crrev.com/9f832ef137dabcd06769d0671672cf15c12eb39c/device/fido/fido_test_data.h
[modify] https://crrev.com/9f832ef137dabcd06769d0671672cf15c12eb39c/device/fido/get_assertion_task_unittest.cc
[modify] https://crrev.com/9f832ef137dabcd06769d0671672cf15c12eb39c/device/fido/u2f_sign_operation_unittest.cc
[modify] https://crrev.com/9f832ef137dabcd06769d0671672cf15c12eb39c/device/fido/win/authenticator.cc

Project Member

Comment 24 by bugdroid1@chromium.org, Nov 30

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/afad373b647aa7be8ccf7f478977cb9a27b42a6e

commit afad373b647aa7be8ccf7f478977cb9a27b42a6e
Author: Martin Kreichgauer <martinkr@chromium.org>
Date: Fri Nov 30 20:19:34 2018

fido/win: make WebAuthnApi own its arguments

In http://crrev.com/c/1354559, the thread waiting for the blocking
Windows WebAuthn API calls to return was moved into the WinWebAuthnApi
singleton. Unfortunately, this introduced a potential use-after-free, because
arguments to the blocking API calls get passed via pointer and the owner of those
arguments, WinHelloApiAuthenticator, wasn't generally guaranteed to outlive the
duration of the request.

This change fixes the issue by moving all data passed to the Windows API calls
into WinWebAuthnApi by value.

Bug: 898718
Change-Id: Ib4f2e86f4ef04cbac712682d4cff386709e735eb
Reviewed-on: https://chromium-review.googlesource.com/c/1356334
Commit-Queue: Martin Kreichgauer <martinkr@chromium.org>
Reviewed-by: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612753}
[modify] https://crrev.com/afad373b647aa7be8ccf7f478977cb9a27b42a6e/device/fido/win/authenticator.cc
[modify] https://crrev.com/afad373b647aa7be8ccf7f478977cb9a27b42a6e/device/fido/win/authenticator.h
[modify] https://crrev.com/afad373b647aa7be8ccf7f478977cb9a27b42a6e/device/fido/win/fake_webauthn_api.cc
[modify] https://crrev.com/afad373b647aa7be8ccf7f478977cb9a27b42a6e/device/fido/win/fake_webauthn_api.h
[modify] https://crrev.com/afad373b647aa7be8ccf7f478977cb9a27b42a6e/device/fido/win/type_conversions.cc
[modify] https://crrev.com/afad373b647aa7be8ccf7f478977cb9a27b42a6e/device/fido/win/webauthn_api.cc
[modify] https://crrev.com/afad373b647aa7be8ccf7f478977cb9a27b42a6e/device/fido/win/webauthn_api.h

Project Member

Comment 25 by bugdroid1@chromium.org, Nov 30

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/11f9ef9e0667da718e57f4eef5ee8a74e66d99bc

commit 11f9ef9e0667da718e57f4eef5ee8a74e66d99bc
Author: Martin Kreichgauer <martinkr@chromium.org>
Date: Fri Nov 30 21:42:23 2018

fido/win: don't fail if Windows doesn't echo the HmacSecret extension

Windows generally doesn't seem to echo the HmacSecret extension in their
response to AuthenticatorMakeCredential. However, AuthenticatorImpl already
parses the hmacSecret response value out of the authenticator data, so we
should not fail the request over this.

Bug: 898718
Change-Id: I1e732ab189f240df2be09ec89135af143634238f
Reviewed-on: https://chromium-review.googlesource.com/c/1357460
Commit-Queue: Martin Kreichgauer <martinkr@chromium.org>
Commit-Queue: Adam Langley <agl@chromium.org>
Reviewed-by: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612798}
[modify] https://crrev.com/11f9ef9e0667da718e57f4eef5ee8a74e66d99bc/device/fido/win/authenticator.cc

Project Member

Comment 26 by bugdroid1@chromium.org, Dec 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f1b46d7a7279787041b7371e5b5acb4a60f5db1e

commit f1b46d7a7279787041b7371e5b5acb4a60f5db1e
Author: Martin Kreichgauer <martinkr@chromium.org>
Date: Mon Dec 03 21:20:30 2018

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: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613248}
[modify] https://crrev.com/f1b46d7a7279787041b7371e5b5acb4a60f5db1e/device/fido/win/type_conversions.cc
[modify] https://crrev.com/f1b46d7a7279787041b7371e5b5acb4a60f5db1e/device/fido/win/type_conversions.h
[modify] https://crrev.com/f1b46d7a7279787041b7371e5b5acb4a60f5db1e/device/fido/win/webauthn_api.cc

Labels: Merge-Request-72
Requesting merge to M72 of the following commits:

918351a05ebbed5deb6d0ce8207fc5aee2caf02f
7cfa011c627035d1dcf550c5b75d9225459d0e23
afad373b647aa7be8ccf7f478977cb9a27b42a6e
11f9ef9e0667da718e57f4eef5ee8a74e66d99bc
f1b46d7a7279787041b7371e5b5acb4a60f5db1e

These commits are required to keep the U2F and WebAuthentication APIs working in future versions of Windows 10. All new code is flag controlled, and even if enabled, it currently only affects a pre-release version of Windows 10. WebAuthentication on other platforms is unaffected.
Cc: agl@chromium.org
Project Member

Comment 29 by sheriffbot@chromium.org, Dec 4

Labels: -Merge-Request-72 Merge-Review-72 Hotlist-Merge-Review
This bug requires manual review: DEPS changes referenced in bugdroid comments.
Please contact the milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-72 Merge-Approved-72
As discussed over email, approving #27 for M72. However, let's be extra careful about these changes and ensure that the enterprise test team is able to test this change thoroughly, as this can impact enterprise logins. 

M72 branch:3626
Project Member

Comment 31 by bugdroid1@chromium.org, Dec 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5cadda883eac95a742541d7e454e150bba70dd5a

commit 5cadda883eac95a742541d7e454e150bba70dd5a
Author: Martin Kreichgauer <martinkr@chromium.org>
Date: Wed Dec 05 02:56:44 2018

fido/win: re-add transport restrictions in allowList; populate transport_used

This is a follow-up to the Windows issue worked around in
f1b46d7a7279787041b7371e5b5acb4a60f5db1e. Based on feedback from
Microsoft, we can set both the CredentialList and pAllowCredentialList
fields of the WEBAUTHN_AUTHENTICATOR_GET_ASSERTION_OPTIONS struct to
avoid the issue while retaining support for transport restrictions in
the allow list.

Also fix wrong initialization of WEBAUTHN_CREDENTIAL_LIST structs.

Also bump the third_party webauthn header version so we can fill in the
transport_used field of AuthenticatorMakeCredentialResponse.

Bug: 898718
Change-Id: I8a7668967aafdf5107f0da74923ab3c2f8c129ac
Reviewed-on: https://chromium-review.googlesource.com/c/1359490
Commit-Queue: Adam Langley <agl@chromium.org>
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Adam Langley <agl@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613834}
[modify] https://crrev.com/5cadda883eac95a742541d7e454e150bba70dd5a/device/fido/win/type_conversions.cc
[modify] https://crrev.com/5cadda883eac95a742541d7e454e150bba70dd5a/device/fido/win/webauthn_api.cc
[modify] https://crrev.com/5cadda883eac95a742541d7e454e150bba70dd5a/third_party/microsoft_webauthn/README.chromium
[modify] https://crrev.com/5cadda883eac95a742541d7e454e150bba70dd5a/third_party/microsoft_webauthn/webauthn.h

Re #30-

Thanks Abdul! I'd like to extend my request to also cover 5cadda883eac95a742541d7e454e150bba70dd5a. Same justification as #27.
@martinkr@ looks like this just landed in canary today, Let us make sure canary is good  and based on the result , we can approve the merge tomorrow.

abdulsyed@ feel free to suggest alternate action if you disagree
Project Member

Comment 34 by bugdroid1@chromium.org, Dec 5

Labels: -merge-approved-72 merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/11d1bb70d8ea37a7593b00903862702c866712e2

commit 11d1bb70d8ea37a7593b00903862702c866712e2
Author: Martin Kreichgauer <martinkr@google.com>
Date: Wed Dec 05 23:12:06 2018

[M72] fido/win: make WebAuthnApi own its arguments

In http://crrev.com/c/1354559, the thread waiting for the blocking
Windows WebAuthn API calls to return was moved into the WinWebAuthnApi
singleton. Unfortunately, this introduced a potential use-after-free, because
arguments to the blocking API calls get passed via pointer and the owner of those
arguments, WinHelloApiAuthenticator, wasn't generally guaranteed to outlive the
duration of the request.

This change fixes the issue by moving all data passed to the Windows API calls
into WinWebAuthnApi by value.

(cherry picked from commit afad373b647aa7be8ccf7f478977cb9a27b42a6e)

Bug: 898718
Change-Id: Id61d131451dd57b6acbb2d69664775cc549b7df5
Reviewed-on: https://chromium-review.googlesource.com/c/1356334
Commit-Queue: Martin Kreichgauer <martinkr@chromium.org>
Reviewed-by: Adam Langley <agl@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#612753}
Reviewed-on: https://chromium-review.googlesource.com/c/1364231
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Cr-Commit-Position: refs/branch-heads/3626@{#89}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/11d1bb70d8ea37a7593b00903862702c866712e2/device/fido/win/authenticator.cc
[modify] https://crrev.com/11d1bb70d8ea37a7593b00903862702c866712e2/device/fido/win/authenticator.h
[modify] https://crrev.com/11d1bb70d8ea37a7593b00903862702c866712e2/device/fido/win/fake_webauthn_api.cc
[modify] https://crrev.com/11d1bb70d8ea37a7593b00903862702c866712e2/device/fido/win/fake_webauthn_api.h
[modify] https://crrev.com/11d1bb70d8ea37a7593b00903862702c866712e2/device/fido/win/type_conversions.cc
[modify] https://crrev.com/11d1bb70d8ea37a7593b00903862702c866712e2/device/fido/win/webauthn_api.cc
[modify] https://crrev.com/11d1bb70d8ea37a7593b00903862702c866712e2/device/fido/win/webauthn_api.h

Project Member

Comment 35 by bugdroid1@chromium.org, Dec 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9f4007a7ea27c5bdc79b6c8e325dbfb8c90db76d

commit 9f4007a7ea27c5bdc79b6c8e325dbfb8c90db76d
Author: Martin Kreichgauer <martinkr@google.com>
Date: Wed Dec 05 23:22:27 2018

[M72] fido/win: don't fail if Windows doesn't echo the HmacSecret extension

Windows generally doesn't seem to echo the HmacSecret extension in their
response to AuthenticatorMakeCredential. However, AuthenticatorImpl already
parses the hmacSecret response value out of the authenticator data, so we
should not fail the request over this.

(cherry picked from commit 11f9ef9e0667da718e57f4eef5ee8a74e66d99bc)

Bug: 898718
Change-Id: I1e732ab189f240df2be09ec89135af143634238f
Reviewed-on: https://chromium-review.googlesource.com/c/1357460
Commit-Queue: Martin Kreichgauer <martinkr@chromium.org>
Commit-Queue: Adam Langley <agl@chromium.org>
Reviewed-by: Adam Langley <agl@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#612798}
Reviewed-on: https://chromium-review.googlesource.com/c/1364215
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Cr-Commit-Position: refs/branch-heads/3626@{#90}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/9f4007a7ea27c5bdc79b6c8e325dbfb8c90db76d/device/fido/win/authenticator.cc

Merged all five CLs from https://bugs.chromium.org/p/chromium/issues/detail?id=898718#c27 to M72 (refs/branch-heads/3626).
Project Member

Comment 37 by bugdroid1@chromium.org, Dec 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c8e09127f981d140acfcc6700ed77741c275b38b

commit c8e09127f981d140acfcc6700ed77741c275b38b
Author: Martin Kreichgauer <martinkr@google.com>
Date: Wed Dec 05 23:26:57 2018

[M72] 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.

(cherry picked from commit f1b46d7a7279787041b7371e5b5acb4a60f5db1e)

Bug: 898718
Change-Id: I5e06cd48a3dd424b4763753d8e4d41d8c6680c68
Reviewed-on: https://chromium-review.googlesource.com/c/1357621
Commit-Queue: Martin Kreichgauer <martinkr@chromium.org>
Reviewed-by: Adam Langley <agl@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#613248}
Reviewed-on: https://chromium-review.googlesource.com/c/1364161
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Cr-Commit-Position: refs/branch-heads/3626@{#91}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/c8e09127f981d140acfcc6700ed77741c275b38b/device/fido/win/type_conversions.cc
[modify] https://crrev.com/c8e09127f981d140acfcc6700ed77741c275b38b/device/fido/win/type_conversions.h
[modify] https://crrev.com/c8e09127f981d140acfcc6700ed77741c275b38b/device/fido/win/webauthn_api.cc

Labels: Merge-Request-72
(Forgot to set the Merge Requeted label in #32).
Project Member

Comment 39 by sheriffbot@chromium.org, Dec 7

Labels: -Merge-Request-72 Merge-Review-72
This bug requires manual review: DEPS changes referenced in bugdroid comments.
Please contact the milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-72 Merge-Approved-72
Project Member

Comment 41 by bugdroid1@chromium.org, Dec 7

Labels: -merge-approved-72
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/91f2729c151fb5fb47bf90dfdf7967203e3eba9d

commit 91f2729c151fb5fb47bf90dfdf7967203e3eba9d
Author: Martin Kreichgauer <martinkr@google.com>
Date: Fri Dec 07 01:45:37 2018

[M72] fido/win: re-add transport restrictions in allowList; populate transport_used

This is a follow-up to the Windows issue worked around in
f1b46d7a7279787041b7371e5b5acb4a60f5db1e. Based on feedback from
Microsoft, we can set both the CredentialList and pAllowCredentialList
fields of the WEBAUTHN_AUTHENTICATOR_GET_ASSERTION_OPTIONS struct to
avoid the issue while retaining support for transport restrictions in
the allow list.

Also fix wrong initialization of WEBAUTHN_CREDENTIAL_LIST structs.

Also bump the third_party webauthn header version so we can fill in the
transport_used field of AuthenticatorMakeCredentialResponse.

(cherry picked from commit 5cadda883eac95a742541d7e454e150bba70dd5a)

Bug: 898718
Change-Id: I8a7668967aafdf5107f0da74923ab3c2f8c129ac
Reviewed-on: https://chromium-review.googlesource.com/c/1359490
Commit-Queue: Adam Langley <agl@chromium.org>
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Adam Langley <agl@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#613834}
Reviewed-on: https://chromium-review.googlesource.com/c/1366921
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Cr-Commit-Position: refs/branch-heads/3626@{#130}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/91f2729c151fb5fb47bf90dfdf7967203e3eba9d/device/fido/win/type_conversions.cc
[modify] https://crrev.com/91f2729c151fb5fb47bf90dfdf7967203e3eba9d/device/fido/win/webauthn_api.cc
[modify] https://crrev.com/91f2729c151fb5fb47bf90dfdf7967203e3eba9d/third_party/microsoft_webauthn/README.chromium
[modify] https://crrev.com/91f2729c151fb5fb47bf90dfdf7967203e3eba9d/third_party/microsoft_webauthn/webauthn.h

Project Member

Comment 42 by bugdroid1@chromium.org, Dec 12

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/adbcb1d0763e773988ce3c0df5b6e906a2cf9bf9

commit adbcb1d0763e773988ce3c0df5b6e906a2cf9bf9
Author: Martin Kreichgauer <martinkr@chromium.org>
Date: Wed Dec 12 21:08:02 2018

fido/win: fix a crash when exclude_list is set in MakeCredential requests

When initializing pExcludeCredentialList parameter of a
WEBAUTHN_MAKE_CREDENTIAL_OPTIONS struct, the code calls std::transform
with a wrong value for the output iterator argument. As a result, the
code crashes if an exclude_list was set in the request. This fixes the
issue by passing the correct output iterator. (The initialization of
the equivalent pAllowCredentialList parameter during GetAssertion was
already correct.)

Bug: 898718
Change-Id: Id0611348f6b4e208d9176d714b10da95b474bd2e
Reviewed-on: https://chromium-review.googlesource.com/c/1374401
Reviewed-by: Adam Langley <agl@chromium.org>
Commit-Queue: Martin Kreichgauer <martinkr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616035}
[modify] https://crrev.com/adbcb1d0763e773988ce3c0df5b6e906a2cf9bf9/device/fido/win/webauthn_api.cc

Requesting another merge for adbcb1d0763e773988ce3c0df5b6e906a2cf9bf9. This is a small, isolated fix a recent regression in the Windows WebAuthn integration. I verified this fix manually.
Labels: Merge-Request-72
(see #43)
let's let it bake in canary first for at least 24 hours. 
Project Member

Comment 46 by sheriffbot@chromium.org, Dec 13

Labels: -Merge-Request-72 Merge-Review-72
This bug requires manual review: DEPS changes referenced in bugdroid comments.
Please contact the milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Re #45: I successfully verified this again in Canary 73.0.3639.0.
Labels: -Merge-Review-72 Merge-Approved-72
approved for Comment #43 for M72, Branch: 3626
Project Member

Comment 50 by bugdroid1@chromium.org, Dec 14

Labels: -merge-approved-72
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bed77499540eaa79eda78afdc0f82df8b06439c7

commit bed77499540eaa79eda78afdc0f82df8b06439c7
Author: Martin Kreichgauer <martinkr@google.com>
Date: Fri Dec 14 23:51:53 2018

[M72] fido/win: fix a crash when exclude_list is set in MakeCredential requests

When initializing pExcludeCredentialList parameter of a
WEBAUTHN_MAKE_CREDENTIAL_OPTIONS struct, the code calls std::transform
with a wrong value for the output iterator argument. As a result, the
code crashes if an exclude_list was set in the request. This fixes the
issue by passing the correct output iterator. (The initialization of
the equivalent pAllowCredentialList parameter during GetAssertion was
already correct.)

(cherry picked from commit adbcb1d0763e773988ce3c0df5b6e906a2cf9bf9)

Bug: 898718
Change-Id: Id0611348f6b4e208d9176d714b10da95b474bd2e
Reviewed-on: https://chromium-review.googlesource.com/c/1374401
Reviewed-by: Adam Langley <agl@chromium.org>
Commit-Queue: Martin Kreichgauer <martinkr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#616035}
Reviewed-on: https://chromium-review.googlesource.com/c/1379221
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Cr-Commit-Position: refs/branch-heads/3626@{#377}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/bed77499540eaa79eda78afdc0f82df8b06439c7/device/fido/win/webauthn_api.cc

Labels: Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/11d1bb70d8ea37a7593b00903862702c866712e2

Commit: 11d1bb70d8ea37a7593b00903862702c866712e2
Author: martinkr@google.com
Commiter: martinkr@google.com
Date: 2018-12-05 23:12:06 +0000 UTC

[M72] fido/win: make WebAuthnApi own its arguments

In http://crrev.com/c/1354559, the thread waiting for the blocking
Windows WebAuthn API calls to return was moved into the WinWebAuthnApi
singleton. Unfortunately, this introduced a potential use-after-free, because
arguments to the blocking API calls get passed via pointer and the owner of those
arguments, WinHelloApiAuthenticator, wasn't generally guaranteed to outlive the
duration of the request.

This change fixes the issue by moving all data passed to the Windows API calls
into WinWebAuthnApi by value.

(cherry picked from commit afad373b647aa7be8ccf7f478977cb9a27b42a6e)

Bug: 898718
Change-Id: Id61d131451dd57b6acbb2d69664775cc549b7df5
Reviewed-on: https://chromium-review.googlesource.com/c/1356334
Commit-Queue: Martin Kreichgauer <martinkr@chromium.org>
Reviewed-by: Adam Langley <agl@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#612753}
Reviewed-on: https://chromium-review.googlesource.com/c/1364231
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Cr-Commit-Position: refs/branch-heads/3626@{#89}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/91f2729c151fb5fb47bf90dfdf7967203e3eba9d

Commit: 91f2729c151fb5fb47bf90dfdf7967203e3eba9d
Author: martinkr@google.com
Commiter: martinkr@google.com
Date: 2018-12-07 01:45:37 +0000 UTC

[M72] fido/win: re-add transport restrictions in allowList; populate transport_used

This is a follow-up to the Windows issue worked around in
f1b46d7a7279787041b7371e5b5acb4a60f5db1e. Based on feedback from
Microsoft, we can set both the CredentialList and pAllowCredentialList
fields of the WEBAUTHN_AUTHENTICATOR_GET_ASSERTION_OPTIONS struct to
avoid the issue while retaining support for transport restrictions in
the allow list.

Also fix wrong initialization of WEBAUTHN_CREDENTIAL_LIST structs.

Also bump the third_party webauthn header version so we can fill in the
transport_used field of AuthenticatorMakeCredentialResponse.

(cherry picked from commit 5cadda883eac95a742541d7e454e150bba70dd5a)

Bug: 898718
Change-Id: I8a7668967aafdf5107f0da74923ab3c2f8c129ac
Reviewed-on: https://chromium-review.googlesource.com/c/1359490
Commit-Queue: Adam Langley <agl@chromium.org>
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Adam Langley <agl@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#613834}
Reviewed-on: https://chromium-review.googlesource.com/c/1366921
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Cr-Commit-Position: refs/branch-heads/3626@{#130}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/9f4007a7ea27c5bdc79b6c8e325dbfb8c90db76d

Commit: 9f4007a7ea27c5bdc79b6c8e325dbfb8c90db76d
Author: martinkr@google.com
Commiter: martinkr@google.com
Date: 2018-12-05 23:22:27 +0000 UTC

[M72] fido/win: don't fail if Windows doesn't echo the HmacSecret extension

Windows generally doesn't seem to echo the HmacSecret extension in their
response to AuthenticatorMakeCredential. However, AuthenticatorImpl already
parses the hmacSecret response value out of the authenticator data, so we
should not fail the request over this.

(cherry picked from commit 11f9ef9e0667da718e57f4eef5ee8a74e66d99bc)

Bug: 898718
Change-Id: I1e732ab189f240df2be09ec89135af143634238f
Reviewed-on: https://chromium-review.googlesource.com/c/1357460
Commit-Queue: Martin Kreichgauer <martinkr@chromium.org>
Commit-Queue: Adam Langley <agl@chromium.org>
Reviewed-by: Adam Langley <agl@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#612798}
Reviewed-on: https://chromium-review.googlesource.com/c/1364215
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Cr-Commit-Position: refs/branch-heads/3626@{#90}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/bed77499540eaa79eda78afdc0f82df8b06439c7

Commit: bed77499540eaa79eda78afdc0f82df8b06439c7
Author: martinkr@google.com
Commiter: martinkr@google.com
Date: 2018-12-14 23:51:53 +0000 UTC

[M72] fido/win: fix a crash when exclude_list is set in MakeCredential requests

When initializing pExcludeCredentialList parameter of a
WEBAUTHN_MAKE_CREDENTIAL_OPTIONS struct, the code calls std::transform
with a wrong value for the output iterator argument. As a result, the
code crashes if an exclude_list was set in the request. This fixes the
issue by passing the correct output iterator. (The initialization of
the equivalent pAllowCredentialList parameter during GetAssertion was
already correct.)

(cherry picked from commit adbcb1d0763e773988ce3c0df5b6e906a2cf9bf9)

Bug: 898718
Change-Id: Id0611348f6b4e208d9176d714b10da95b474bd2e
Reviewed-on: https://chromium-review.googlesource.com/c/1374401
Reviewed-by: Adam Langley <agl@chromium.org>
Commit-Queue: Martin Kreichgauer <martinkr@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#616035}
Reviewed-on: https://chromium-review.googlesource.com/c/1379221
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Cr-Commit-Position: refs/branch-heads/3626@{#377}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/c8e09127f981d140acfcc6700ed77741c275b38b

Commit: c8e09127f981d140acfcc6700ed77741c275b38b
Author: martinkr@google.com
Commiter: martinkr@google.com
Date: 2018-12-05 23:26:57 +0000 UTC

[M72] 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.

(cherry picked from commit f1b46d7a7279787041b7371e5b5acb4a60f5db1e)

Bug: 898718
Change-Id: I5e06cd48a3dd424b4763753d8e4d41d8c6680c68
Reviewed-on: https://chromium-review.googlesource.com/c/1357621
Commit-Queue: Martin Kreichgauer <martinkr@chromium.org>
Reviewed-by: Adam Langley <agl@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#613248}
Reviewed-on: https://chromium-review.googlesource.com/c/1364161
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Cr-Commit-Position: refs/branch-heads/3626@{#91}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
Should this new Windows implementation allow for resident keys to be created and the 'allowCredentials' list to be empty? This was fully disabled with https://bugs.chromium.org/p/chromium/issues/detail?id=896404 but it would seem to me that with Windows handling the credential and it supporting resident keys this block should be removed. 

Tested Chrome Canary 73.0.3671.0 on Windows 10 18312 with https://webauthnsample.azurewebsites.net 
We intend to lift this restriction eventually, but not in M72.

Sign in to add a comment