fido webauthn appid extension not working for devices registered with u2f
Reported by
br...@dropbox.com,
Aug 31
|
|||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:61.0) Gecko/20100101 Firefox/61.0 Steps to reproduce the problem: 1. Register an authenticator using U2F API 2. Try to get a WebAuthn assertion using the authenticator To run the attached test case, it should be on a domain with https with a Yubikey/other authenticator plugged in. I used a self-signed cert and redirected "testlocal.com" to localhost in /etc/hosts. When the test page loads, open the developer console and watch the console logs while tapping the Yubikey twice. The first tap is for registration and always works, while the second tap will print a success message in working versions but in Chrome Beta doesn't do anything This issue can also be tested on dropbox.com by registering a key using U2F (disable WebAuthn in Firefox/Chrome), then attempting to sign in using Chrome Beta (with WebAuthn enabled) What is the expected behavior? When requesting an assertion, user is prompted to use authenticator (ex: Yubikey starts blinking), and when the user selects the previously registered authenticator (ex: by tapping Yubikey), a valid assertion is returned What went wrong? When user selects previously registered (with U2F) authenticator, it stops blinking, but a valid assertion is not returned. Instead, the request eventually times out. But if user selects a device registered using WebAuthn (not U2F), a valid assertion is returned, suggesting the issue is with the appid extension. Did this work before? Yes 68.0.3440.106 Does this work in other browsers? N/A Chrome version: 69.0.3497.72 (Official Build) beta (64-bit) Channel: n/a OS Version: OS X 10.12 Flash Version: 30.0.0.154 This also works in Chrome Dev channel, so it only seems to be broken in Chrome Beta.
,
Sep 3
,
Sep 3
bradg@Thank for filling the issue... As per comment #0 to test this issue, it requires Yubikey/other authenticator plugged in which is not available with the team here. Hence routing to in-house team to help in further triaging of this issue and adding TE-NeedsTriageFromHYD label to it. Thanks..!
,
Sep 3
,
Sep 3
Thanks for the report, Brad! This indeed broke in M69 (crrev.com/c/1070707) and was subsequently fixed (crrev.com/c/1159522), but only after the M69 branch was cut, and looks like the fix wasn't merged back. It would be sad to have the appid extension in this broken state for the entirety of M69. We should discuss this ASAP and talk to the release engineers about options. Will tentatively mark this as stable-blocker for now.
,
Sep 3
We already cut M69 stable RC for Destkop for release tomorrow. Can we consider this fix for next M69 stable respin (if any)? or this is indeed a blocker and we need to merge immediately for release tomorrow? If yes, how safe is the change to merge to M69 this late in release cycle?
,
Sep 3
The fix is not quite a trivial cherry-pick, but the merge isn't complex. Candidate in https://chromium-review.googlesource.com/c/chromium/src/+/1202682/. Safety: this was expected to be a safe change: the merge contains a test for the fix and other test coverage is decent. It's been on canary/dev for a month without issues. The unknown was how it would merge and that doesn't appear to be a problem. I recommend to merging once Balazs has completed manual testing.
,
Sep 3
Thank you agl@, pls request a merge to M69. I will approve merge to M69 after engedy@ (Balazs) completes manual testing.
,
Sep 3
,
Sep 3
This bug requires manual review: We are only 0 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
,
Sep 3
Verified crrev.com/c/1202682 locally to be good.
,
Sep 3
Approving merge for crrev.com/c/1202682 to M69 branch 3497 based on comments #5, #7, #11 and per internal group chat/email thread. Pls merge. Thank you.
,
Sep 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c6c955cd49ca8a3d6676614d5fbd9a65d0221796 commit c6c955cd49ca8a3d6676614d5fbd9a65d0221796 Author: Adam Langley <agl@chromium.org> Date: Mon Sep 03 16:55:54 2018 webauthn: fix appid extension. It broke because of the test that the response RP ID hash was actually the hash of the RP ID. (Which isn't true when using an AppID hash.) Obviously we were missing test coverage here, so add that now that it's easy to do with VirtualFidoDevice. (Confirmed that the test failed before the rest of these changes and passes now.) Bug: 879765 Change-Id: Iccc988a88e57c8f1b6624b34ea75437cfaf72f8c Cr-Original-Original-Commit-Position: refs/heads/master@{#580197}(cherry picked from commit 5a5d5af32d7d328d1788ec54007a0e0a975db930) Reviewed-on: https://chromium-review.googlesource.com/1202682 Reviewed-by: Balazs Engedy <engedy@chromium.org> Reviewed-by: Adam Langley <agl@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#868} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/content/browser/webauth/authenticator_impl_unittest.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.h [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/get_assertion_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/make_credential_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.h
,
Sep 3
,
Sep 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c6c955cd49ca8a3d6676614d5fbd9a65d0221796 commit c6c955cd49ca8a3d6676614d5fbd9a65d0221796 Author: Adam Langley <agl@chromium.org> Date: Mon Sep 03 16:55:54 2018 webauthn: fix appid extension. It broke because of the test that the response RP ID hash was actually the hash of the RP ID. (Which isn't true when using an AppID hash.) Obviously we were missing test coverage here, so add that now that it's easy to do with VirtualFidoDevice. (Confirmed that the test failed before the rest of these changes and passes now.) Bug: 879765 Change-Id: Iccc988a88e57c8f1b6624b34ea75437cfaf72f8c Cr-Original-Original-Commit-Position: refs/heads/master@{#580197}(cherry picked from commit 5a5d5af32d7d328d1788ec54007a0e0a975db930) Reviewed-on: https://chromium-review.googlesource.com/1202682 Reviewed-by: Balazs Engedy <engedy@chromium.org> Reviewed-by: Adam Langley <agl@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#868} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/content/browser/webauth/authenticator_impl_unittest.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.h [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/get_assertion_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/make_credential_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.h
,
Sep 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c6c955cd49ca8a3d6676614d5fbd9a65d0221796 commit c6c955cd49ca8a3d6676614d5fbd9a65d0221796 Author: Adam Langley <agl@chromium.org> Date: Mon Sep 03 16:55:54 2018 webauthn: fix appid extension. It broke because of the test that the response RP ID hash was actually the hash of the RP ID. (Which isn't true when using an AppID hash.) Obviously we were missing test coverage here, so add that now that it's easy to do with VirtualFidoDevice. (Confirmed that the test failed before the rest of these changes and passes now.) Bug: 879765 Change-Id: Iccc988a88e57c8f1b6624b34ea75437cfaf72f8c Cr-Original-Original-Commit-Position: refs/heads/master@{#580197}(cherry picked from commit 5a5d5af32d7d328d1788ec54007a0e0a975db930) Reviewed-on: https://chromium-review.googlesource.com/1202682 Reviewed-by: Balazs Engedy <engedy@chromium.org> Reviewed-by: Adam Langley <agl@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#868} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/content/browser/webauth/authenticator_impl_unittest.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.h [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/get_assertion_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/make_credential_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.h
,
Sep 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c6c955cd49ca8a3d6676614d5fbd9a65d0221796 commit c6c955cd49ca8a3d6676614d5fbd9a65d0221796 Author: Adam Langley <agl@chromium.org> Date: Mon Sep 03 16:55:54 2018 webauthn: fix appid extension. It broke because of the test that the response RP ID hash was actually the hash of the RP ID. (Which isn't true when using an AppID hash.) Obviously we were missing test coverage here, so add that now that it's easy to do with VirtualFidoDevice. (Confirmed that the test failed before the rest of these changes and passes now.) Bug: 879765 Change-Id: Iccc988a88e57c8f1b6624b34ea75437cfaf72f8c Cr-Original-Original-Commit-Position: refs/heads/master@{#580197}(cherry picked from commit 5a5d5af32d7d328d1788ec54007a0e0a975db930) Reviewed-on: https://chromium-review.googlesource.com/1202682 Reviewed-by: Balazs Engedy <engedy@chromium.org> Reviewed-by: Adam Langley <agl@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#868} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/content/browser/webauth/authenticator_impl_unittest.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.h [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/get_assertion_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/make_credential_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.h
,
Sep 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c6c955cd49ca8a3d6676614d5fbd9a65d0221796 commit c6c955cd49ca8a3d6676614d5fbd9a65d0221796 Author: Adam Langley <agl@chromium.org> Date: Mon Sep 03 16:55:54 2018 webauthn: fix appid extension. It broke because of the test that the response RP ID hash was actually the hash of the RP ID. (Which isn't true when using an AppID hash.) Obviously we were missing test coverage here, so add that now that it's easy to do with VirtualFidoDevice. (Confirmed that the test failed before the rest of these changes and passes now.) Bug: 879765 Change-Id: Iccc988a88e57c8f1b6624b34ea75437cfaf72f8c Cr-Original-Original-Commit-Position: refs/heads/master@{#580197}(cherry picked from commit 5a5d5af32d7d328d1788ec54007a0e0a975db930) Reviewed-on: https://chromium-review.googlesource.com/1202682 Reviewed-by: Balazs Engedy <engedy@chromium.org> Reviewed-by: Adam Langley <agl@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#868} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/content/browser/webauth/authenticator_impl_unittest.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.h [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/get_assertion_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/make_credential_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.h
,
Sep 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c6c955cd49ca8a3d6676614d5fbd9a65d0221796 commit c6c955cd49ca8a3d6676614d5fbd9a65d0221796 Author: Adam Langley <agl@chromium.org> Date: Mon Sep 03 16:55:54 2018 webauthn: fix appid extension. It broke because of the test that the response RP ID hash was actually the hash of the RP ID. (Which isn't true when using an AppID hash.) Obviously we were missing test coverage here, so add that now that it's easy to do with VirtualFidoDevice. (Confirmed that the test failed before the rest of these changes and passes now.) Bug: 879765 Change-Id: Iccc988a88e57c8f1b6624b34ea75437cfaf72f8c Cr-Original-Original-Commit-Position: refs/heads/master@{#580197}(cherry picked from commit 5a5d5af32d7d328d1788ec54007a0e0a975db930) Reviewed-on: https://chromium-review.googlesource.com/1202682 Reviewed-by: Balazs Engedy <engedy@chromium.org> Reviewed-by: Adam Langley <agl@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#868} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/content/browser/webauth/authenticator_impl_unittest.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.h [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/get_assertion_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/make_credential_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.h
,
Sep 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c6c955cd49ca8a3d6676614d5fbd9a65d0221796 commit c6c955cd49ca8a3d6676614d5fbd9a65d0221796 Author: Adam Langley <agl@chromium.org> Date: Mon Sep 03 16:55:54 2018 webauthn: fix appid extension. It broke because of the test that the response RP ID hash was actually the hash of the RP ID. (Which isn't true when using an AppID hash.) Obviously we were missing test coverage here, so add that now that it's easy to do with VirtualFidoDevice. (Confirmed that the test failed before the rest of these changes and passes now.) Bug: 879765 Change-Id: Iccc988a88e57c8f1b6624b34ea75437cfaf72f8c Cr-Original-Original-Commit-Position: refs/heads/master@{#580197}(cherry picked from commit 5a5d5af32d7d328d1788ec54007a0e0a975db930) Reviewed-on: https://chromium-review.googlesource.com/1202682 Reviewed-by: Balazs Engedy <engedy@chromium.org> Reviewed-by: Adam Langley <agl@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#868} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/content/browser/webauth/authenticator_impl_unittest.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.h [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/get_assertion_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/make_credential_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.h
,
Sep 3
bugdroid appears to be renotifying about the merged change every ten minutes. Locking down bug in the hopes of stopping it.
,
Sep 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c6c955cd49ca8a3d6676614d5fbd9a65d0221796 commit c6c955cd49ca8a3d6676614d5fbd9a65d0221796 Author: Adam Langley <agl@chromium.org> Date: Mon Sep 03 16:55:54 2018 webauthn: fix appid extension. It broke because of the test that the response RP ID hash was actually the hash of the RP ID. (Which isn't true when using an AppID hash.) Obviously we were missing test coverage here, so add that now that it's easy to do with VirtualFidoDevice. (Confirmed that the test failed before the rest of these changes and passes now.) Bug: 879765 Change-Id: Iccc988a88e57c8f1b6624b34ea75437cfaf72f8c Cr-Original-Original-Commit-Position: refs/heads/master@{#580197}(cherry picked from commit 5a5d5af32d7d328d1788ec54007a0e0a975db930) Reviewed-on: https://chromium-review.googlesource.com/1202682 Reviewed-by: Balazs Engedy <engedy@chromium.org> Reviewed-by: Adam Langley <agl@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#868} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/content/browser/webauth/authenticator_impl_unittest.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.h [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/get_assertion_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/make_credential_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.h
,
Sep 3
Infra bug filed here: https://bugs.chromium.org/p/chromium/issues/detail?id=880099. That label didn't stop bugdroid, trying another.
,
Sep 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c6c955cd49ca8a3d6676614d5fbd9a65d0221796 commit c6c955cd49ca8a3d6676614d5fbd9a65d0221796 Author: Adam Langley <agl@chromium.org> Date: Mon Sep 03 16:55:54 2018 webauthn: fix appid extension. It broke because of the test that the response RP ID hash was actually the hash of the RP ID. (Which isn't true when using an AppID hash.) Obviously we were missing test coverage here, so add that now that it's easy to do with VirtualFidoDevice. (Confirmed that the test failed before the rest of these changes and passes now.) Bug: 879765 Change-Id: Iccc988a88e57c8f1b6624b34ea75437cfaf72f8c Cr-Original-Original-Commit-Position: refs/heads/master@{#580197}(cherry picked from commit 5a5d5af32d7d328d1788ec54007a0e0a975db930) Reviewed-on: https://chromium-review.googlesource.com/1202682 Reviewed-by: Balazs Engedy <engedy@chromium.org> Reviewed-by: Adam Langley <agl@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#868} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/content/browser/webauth/authenticator_impl_unittest.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.h [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/get_assertion_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/make_credential_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.h
,
Sep 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c6c955cd49ca8a3d6676614d5fbd9a65d0221796 commit c6c955cd49ca8a3d6676614d5fbd9a65d0221796 Author: Adam Langley <agl@chromium.org> Date: Mon Sep 03 16:55:54 2018 webauthn: fix appid extension. It broke because of the test that the response RP ID hash was actually the hash of the RP ID. (Which isn't true when using an AppID hash.) Obviously we were missing test coverage here, so add that now that it's easy to do with VirtualFidoDevice. (Confirmed that the test failed before the rest of these changes and passes now.) Bug: 879765 Change-Id: Iccc988a88e57c8f1b6624b34ea75437cfaf72f8c Cr-Original-Original-Commit-Position: refs/heads/master@{#580197}(cherry picked from commit 5a5d5af32d7d328d1788ec54007a0e0a975db930) Reviewed-on: https://chromium-review.googlesource.com/1202682 Reviewed-by: Balazs Engedy <engedy@chromium.org> Reviewed-by: Adam Langley <agl@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#868} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/content/browser/webauth/authenticator_impl_unittest.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.h [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/get_assertion_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/make_credential_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.h
,
Sep 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c6c955cd49ca8a3d6676614d5fbd9a65d0221796 commit c6c955cd49ca8a3d6676614d5fbd9a65d0221796 Author: Adam Langley <agl@chromium.org> Date: Mon Sep 03 16:55:54 2018 webauthn: fix appid extension. It broke because of the test that the response RP ID hash was actually the hash of the RP ID. (Which isn't true when using an AppID hash.) Obviously we were missing test coverage here, so add that now that it's easy to do with VirtualFidoDevice. (Confirmed that the test failed before the rest of these changes and passes now.) Bug: 879765 Change-Id: Iccc988a88e57c8f1b6624b34ea75437cfaf72f8c Cr-Original-Original-Commit-Position: refs/heads/master@{#580197}(cherry picked from commit 5a5d5af32d7d328d1788ec54007a0e0a975db930) Reviewed-on: https://chromium-review.googlesource.com/1202682 Reviewed-by: Balazs Engedy <engedy@chromium.org> Reviewed-by: Adam Langley <agl@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#868} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/content/browser/webauth/authenticator_impl_unittest.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.h [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/get_assertion_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/make_credential_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.h
,
Sep 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c6c955cd49ca8a3d6676614d5fbd9a65d0221796 commit c6c955cd49ca8a3d6676614d5fbd9a65d0221796 Author: Adam Langley <agl@chromium.org> Date: Mon Sep 03 16:55:54 2018 webauthn: fix appid extension. It broke because of the test that the response RP ID hash was actually the hash of the RP ID. (Which isn't true when using an AppID hash.) Obviously we were missing test coverage here, so add that now that it's easy to do with VirtualFidoDevice. (Confirmed that the test failed before the rest of these changes and passes now.) Bug: 879765 Change-Id: Iccc988a88e57c8f1b6624b34ea75437cfaf72f8c Cr-Original-Original-Commit-Position: refs/heads/master@{#580197}(cherry picked from commit 5a5d5af32d7d328d1788ec54007a0e0a975db930) Reviewed-on: https://chromium-review.googlesource.com/1202682 Reviewed-by: Balazs Engedy <engedy@chromium.org> Reviewed-by: Adam Langley <agl@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#868} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/content/browser/webauth/authenticator_impl_unittest.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.h [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/get_assertion_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/make_credential_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.h
,
Sep 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c6c955cd49ca8a3d6676614d5fbd9a65d0221796 commit c6c955cd49ca8a3d6676614d5fbd9a65d0221796 Author: Adam Langley <agl@chromium.org> Date: Mon Sep 03 16:55:54 2018 webauthn: fix appid extension. It broke because of the test that the response RP ID hash was actually the hash of the RP ID. (Which isn't true when using an AppID hash.) Obviously we were missing test coverage here, so add that now that it's easy to do with VirtualFidoDevice. (Confirmed that the test failed before the rest of these changes and passes now.) Bug: 879765 Change-Id: Iccc988a88e57c8f1b6624b34ea75437cfaf72f8c Cr-Original-Original-Commit-Position: refs/heads/master@{#580197}(cherry picked from commit 5a5d5af32d7d328d1788ec54007a0e0a975db930) Reviewed-on: https://chromium-review.googlesource.com/1202682 Reviewed-by: Balazs Engedy <engedy@chromium.org> Reviewed-by: Adam Langley <agl@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#868} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/content/browser/webauth/authenticator_impl_unittest.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.h [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/get_assertion_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/make_credential_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.h
,
Sep 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c6c955cd49ca8a3d6676614d5fbd9a65d0221796 commit c6c955cd49ca8a3d6676614d5fbd9a65d0221796 Author: Adam Langley <agl@chromium.org> Date: Mon Sep 03 16:55:54 2018 webauthn: fix appid extension. It broke because of the test that the response RP ID hash was actually the hash of the RP ID. (Which isn't true when using an AppID hash.) Obviously we were missing test coverage here, so add that now that it's easy to do with VirtualFidoDevice. (Confirmed that the test failed before the rest of these changes and passes now.) Bug: 879765 Change-Id: Iccc988a88e57c8f1b6624b34ea75437cfaf72f8c Cr-Original-Original-Commit-Position: refs/heads/master@{#580197}(cherry picked from commit 5a5d5af32d7d328d1788ec54007a0e0a975db930) Reviewed-on: https://chromium-review.googlesource.com/1202682 Reviewed-by: Balazs Engedy <engedy@chromium.org> Reviewed-by: Adam Langley <agl@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#868} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/content/browser/webauth/authenticator_impl_unittest.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.h [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/get_assertion_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/make_credential_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.h
,
Sep 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c6c955cd49ca8a3d6676614d5fbd9a65d0221796 commit c6c955cd49ca8a3d6676614d5fbd9a65d0221796 Author: Adam Langley <agl@chromium.org> Date: Mon Sep 03 16:55:54 2018 webauthn: fix appid extension. It broke because of the test that the response RP ID hash was actually the hash of the RP ID. (Which isn't true when using an AppID hash.) Obviously we were missing test coverage here, so add that now that it's easy to do with VirtualFidoDevice. (Confirmed that the test failed before the rest of these changes and passes now.) Bug: 879765 Change-Id: Iccc988a88e57c8f1b6624b34ea75437cfaf72f8c Cr-Original-Original-Commit-Position: refs/heads/master@{#580197}(cherry picked from commit 5a5d5af32d7d328d1788ec54007a0e0a975db930) Reviewed-on: https://chromium-review.googlesource.com/1202682 Reviewed-by: Balazs Engedy <engedy@chromium.org> Reviewed-by: Adam Langley <agl@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#868} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/content/browser/webauth/authenticator_impl_unittest.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.h [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/get_assertion_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/make_credential_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.h
,
Sep 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c6c955cd49ca8a3d6676614d5fbd9a65d0221796 commit c6c955cd49ca8a3d6676614d5fbd9a65d0221796 Author: Adam Langley <agl@chromium.org> Date: Mon Sep 03 16:55:54 2018 webauthn: fix appid extension. It broke because of the test that the response RP ID hash was actually the hash of the RP ID. (Which isn't true when using an AppID hash.) Obviously we were missing test coverage here, so add that now that it's easy to do with VirtualFidoDevice. (Confirmed that the test failed before the rest of these changes and passes now.) Bug: 879765 Change-Id: Iccc988a88e57c8f1b6624b34ea75437cfaf72f8c Cr-Original-Original-Commit-Position: refs/heads/master@{#580197}(cherry picked from commit 5a5d5af32d7d328d1788ec54007a0e0a975db930) Reviewed-on: https://chromium-review.googlesource.com/1202682 Reviewed-by: Balazs Engedy <engedy@chromium.org> Reviewed-by: Adam Langley <agl@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#868} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/content/browser/webauth/authenticator_impl_unittest.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.h [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/get_assertion_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/make_credential_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.h
,
Sep 3
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c6c955cd49ca8a3d6676614d5fbd9a65d0221796 commit c6c955cd49ca8a3d6676614d5fbd9a65d0221796 Author: Adam Langley <agl@chromium.org> Date: Mon Sep 03 16:55:54 2018 webauthn: fix appid extension. It broke because of the test that the response RP ID hash was actually the hash of the RP ID. (Which isn't true when using an AppID hash.) Obviously we were missing test coverage here, so add that now that it's easy to do with VirtualFidoDevice. (Confirmed that the test failed before the rest of these changes and passes now.) Bug: 879765 Change-Id: Iccc988a88e57c8f1b6624b34ea75437cfaf72f8c Cr-Original-Original-Commit-Position: refs/heads/master@{#580197}(cherry picked from commit 5a5d5af32d7d328d1788ec54007a0e0a975db930) Reviewed-on: https://chromium-review.googlesource.com/1202682 Reviewed-by: Balazs Engedy <engedy@chromium.org> Reviewed-by: Adam Langley <agl@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#868} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/content/browser/webauth/authenticator_impl_unittest.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/ctap_get_assertion_request.h [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/get_assertion_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/make_credential_task.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.cc [modify] https://crrev.com/c6c955cd49ca8a3d6676614d5fbd9a65d0221796/device/fido/response_data.h
,
Sep 3
,
Sep 4
This fix was verified by agl@ on M69 stable RC version 69.0.3497.81. Thank you. |
|||||||||||||
►
Sign in to add a comment |
|||||||||||||
Comment 1 by viswa.karala@chromium.org
, Sep 2