CTAP2 device on Windows doesn't process properly if authenticatorMakeCredential response is too long |
||||||||
Issue descriptionLink to original bug filed with webauthn demo: https://github.com/google/webauthndemo/issues/41#issuecomment-403703429 In summary: CTAP2, USB authenticator, Chrome 67+ on Windows. If 3-tier attestation certificates are more than 0x75F bytes, the response doesn't make it back to the browser. Webauthndemo, for example, always shows "Waiting for user touch". Same authenticator and Chrome on a Mac works fine. See the original bug for response data. "You can see in the file that USB device (30.3 USB Input Device) has returned the data to Windows, but Windows driver (31 HID-compliant device ) does not return the data to chrome or Chrome does not read from Windows because of some errors( the first HID package of response data for 31 is lost )." Jun validated the response with our CBOR parser tests, which were able to process the data without issue.
,
Aug 8
,
Aug 8
I have a fix in progress.
,
Aug 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/acf579dae9cdc50d885dfdcd01aedaed74e5c678 commit acf579dae9cdc50d885dfdcd01aedaed74e5c678 Author: Reilly Grant <reillyg@google.com> Date: Fri Aug 10 01:06:43 2018 Queue HID input reports on Windows This change unifies the HID input report handling on Windows with the existing logic on Linux and macOS where input reports are continuously read from the device and queued until a request is made to read them. This is necessary on Windows as well because the OS driver maintains a limited ring buffer of input reports and so if they are not read in a timely manner they will be discarded. This is a problem for protocols such as CTAP2 which break a larger message into multiple reports. Bug: 862207 Change-Id: I222e1cb2df8633e0bec77d9a64a9cf91760ba417 Reviewed-on: https://chromium-review.googlesource.com/1168153 Commit-Queue: Reilly Grant <reillyg@chromium.org> Reviewed-by: Matt Reynolds <mattreynolds@chromium.org> Cr-Commit-Position: refs/heads/master@{#581994} [modify] https://crrev.com/acf579dae9cdc50d885dfdcd01aedaed74e5c678/services/device/hid/hid_connection.cc [modify] https://crrev.com/acf579dae9cdc50d885dfdcd01aedaed74e5c678/services/device/hid/hid_connection.h [modify] https://crrev.com/acf579dae9cdc50d885dfdcd01aedaed74e5c678/services/device/hid/hid_connection_linux.cc [modify] https://crrev.com/acf579dae9cdc50d885dfdcd01aedaed74e5c678/services/device/hid/hid_connection_linux.h [modify] https://crrev.com/acf579dae9cdc50d885dfdcd01aedaed74e5c678/services/device/hid/hid_connection_mac.cc [modify] https://crrev.com/acf579dae9cdc50d885dfdcd01aedaed74e5c678/services/device/hid/hid_connection_mac.h [modify] https://crrev.com/acf579dae9cdc50d885dfdcd01aedaed74e5c678/services/device/hid/hid_connection_win.cc [modify] https://crrev.com/acf579dae9cdc50d885dfdcd01aedaed74e5c678/services/device/hid/hid_connection_win.h [modify] https://crrev.com/acf579dae9cdc50d885dfdcd01aedaed74e5c678/services/device/hid/hid_service_win.cc [modify] https://crrev.com/acf579dae9cdc50d885dfdcd01aedaed74e5c678/services/device/hid/mock_hid_connection.cc [modify] https://crrev.com/acf579dae9cdc50d885dfdcd01aedaed74e5c678/services/device/hid/mock_hid_connection.h [modify] https://crrev.com/acf579dae9cdc50d885dfdcd01aedaed74e5c678/services/device/hid/mock_hid_service.cc
,
Aug 10
,
Aug 10
Reilly, given that CTAP2-by-default is a lunching with M69, do we want to merge this back?
,
Aug 10
Given how much this changes the behavior of the HID stack on Windows I'm not comfortable merging it back until it has been verified on dev-channel.
,
Aug 14
Thanks for addressing this, Reilly! We should know in a couple of days then, right?
,
Aug 22
Requesting merge to m69 due to devices entering the market that will run into the problem before m70 is stable on Oct 16. This has been verified on dev.
,
Aug 22
This bug requires manual review: We are only 12 days from stable. Please contact the milestone owner if you have questions. Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 22
How safe is the change to merge to M69 this late in release cycle?
,
Aug 23
This is a significant change to the behavior on Windows but makes it match up with behavior on other platforms so I find that less concerning. It has been baking on canary/dev for over a week and I have not seen any reports of problems.
,
Aug 23
Thank you reillyg@. Is it truly critical to merge this change to M69 this late in release cycle?
,
Aug 23
We think so. There are going to be devices on the market prior to m70 that will effectively not work on Windows without this fix. When the WebAuthN API is used, the devices will blink and request user consent but the request will never make it back to the server, so users won't be able to register these devices as second factors for their accounts.
,
Aug 23
Approving merge to M69 branch 3497 based on comment #12 and #14. Please merge ASAP. Thank you.
,
Aug 24
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/189d094e63bdcaf6f5ef90f0d6ad17096e982945 commit 189d094e63bdcaf6f5ef90f0d6ad17096e982945 Author: Reilly Grant <reillyg@google.com> Date: Fri Aug 24 14:24:04 2018 [M-69] Queue HID input reports on Windows This change unifies the HID input report handling on Windows with the existing logic on Linux and macOS where input reports are continuously read from the device and queued until a request is made to read them. This is necessary on Windows as well because the OS driver maintains a limited ring buffer of input reports and so if they are not read in a timely manner they will be discarded. This is a problem for protocols such as CTAP2 which break a larger message into multiple reports. Bug: 862207 Change-Id: I222e1cb2df8633e0bec77d9a64a9cf91760ba417 Reviewed-on: https://chromium-review.googlesource.com/1168153 Commit-Queue: Reilly Grant <reillyg@chromium.org> Reviewed-by: Matt Reynolds <mattreynolds@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#581994}(cherry picked from commit acf579dae9cdc50d885dfdcd01aedaed74e5c678) Reviewed-on: https://chromium-review.googlesource.com/1188642 Reviewed-by: Reilly Grant <reillyg@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#799} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/189d094e63bdcaf6f5ef90f0d6ad17096e982945/services/device/hid/hid_connection.cc [modify] https://crrev.com/189d094e63bdcaf6f5ef90f0d6ad17096e982945/services/device/hid/hid_connection.h [modify] https://crrev.com/189d094e63bdcaf6f5ef90f0d6ad17096e982945/services/device/hid/hid_connection_linux.cc [modify] https://crrev.com/189d094e63bdcaf6f5ef90f0d6ad17096e982945/services/device/hid/hid_connection_linux.h [modify] https://crrev.com/189d094e63bdcaf6f5ef90f0d6ad17096e982945/services/device/hid/hid_connection_mac.cc [modify] https://crrev.com/189d094e63bdcaf6f5ef90f0d6ad17096e982945/services/device/hid/hid_connection_mac.h [modify] https://crrev.com/189d094e63bdcaf6f5ef90f0d6ad17096e982945/services/device/hid/hid_connection_win.cc [modify] https://crrev.com/189d094e63bdcaf6f5ef90f0d6ad17096e982945/services/device/hid/hid_connection_win.h [modify] https://crrev.com/189d094e63bdcaf6f5ef90f0d6ad17096e982945/services/device/hid/hid_service_win.cc [modify] https://crrev.com/189d094e63bdcaf6f5ef90f0d6ad17096e982945/services/device/hid/mock_hid_connection.cc [modify] https://crrev.com/189d094e63bdcaf6f5ef90f0d6ad17096e982945/services/device/hid/mock_hid_connection.h [modify] https://crrev.com/189d094e63bdcaf6f5ef90f0d6ad17096e982945/services/device/hid/mock_hid_service.cc |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by skybird...@gmail.com
, Aug 8