Registration requests from background tabs should time out / be disallowed |
||||||
Issue descriptionTo avoid ambiguity on which origin is issuing a registration request, such a request from background tabs should either silently time out, be suspended, or fail with NotAllowedError. We should implement and have browsertest / ui_test coverage for this behavior.
,
Apr 2 2018
,
Apr 2 2018
,
Apr 4 2018
,
Apr 12 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/441a5eae1f3c959eb0732ecf2b2e93698ae8f458 commit 441a5eae1f3c959eb0732ecf2b2e93698ae8f458 Author: Adam Langley <agl@chromium.org> Date: Thu Apr 12 14:58:47 2018 webauthn: require that requests come from a frame in a focused window. The spec suggests aborting an operation when focus[1] is lost, but that's advice for site developers. I can't find anything in the spec about preventing background tabs from triggering operations. The cryptotoken extension refused to start a registration request, or to send a registration response to, anything but the active tab in the focused window. But background tabs could complete an authentication request. This change does something similar: it rejects both authentication and registration requests unless the requesting frame is in a focused window. It also performs that check before returning responses. This is slightly different from the cryptotoken behaviour because cryptotoken could only use what the extensions API exposed. For example, if the omnibox was focused, cryptotoken would complete a registration from the foreground tab but this code will reject it. I think this behaviour is better, and it's certainly far more inline with the content / browser separation. This change has been split from its tests, which will come in a future CL. [1] https://w3c.github.io/webauthn/#abortoperation Bug: 827266 Change-Id: If6e97dd6526e175f40718724eda21e3efd434f7f Reviewed-on: https://chromium-review.googlesource.com/991073 Commit-Queue: Adam Langley <agl@chromium.org> Reviewed-by: Nasko Oskov <nasko@chromium.org> Reviewed-by: Robert Sesek <rsesek@chromium.org> Reviewed-by: Balazs Engedy <engedy@chromium.org> Cr-Commit-Position: refs/heads/master@{#550195} [modify] https://crrev.com/441a5eae1f3c959eb0732ecf2b2e93698ae8f458/chrome/browser/chrome_content_browser_client.cc [modify] https://crrev.com/441a5eae1f3c959eb0732ecf2b2e93698ae8f458/chrome/browser/chrome_content_browser_client.h [modify] https://crrev.com/441a5eae1f3c959eb0732ecf2b2e93698ae8f458/components/password_manager/content/common/credential_manager_mojom_traits.cc [modify] https://crrev.com/441a5eae1f3c959eb0732ecf2b2e93698ae8f458/content/browser/webauth/authenticator_impl.cc [modify] https://crrev.com/441a5eae1f3c959eb0732ecf2b2e93698ae8f458/content/public/browser/content_browser_client.cc [modify] https://crrev.com/441a5eae1f3c959eb0732ecf2b2e93698ae8f458/content/public/browser/content_browser_client.h [modify] https://crrev.com/441a5eae1f3c959eb0732ecf2b2e93698ae8f458/third_party/blink/public/platform/modules/credentialmanager/credential_manager.mojom [modify] https://crrev.com/441a5eae1f3c959eb0732ecf2b2e93698ae8f458/third_party/blink/public/platform/modules/webauth/authenticator.mojom [modify] https://crrev.com/441a5eae1f3c959eb0732ecf2b2e93698ae8f458/third_party/blink/renderer/modules/credentialmanager/credential_manager_type_converters.cc [modify] https://crrev.com/441a5eae1f3c959eb0732ecf2b2e93698ae8f458/third_party/blink/renderer/modules/credentialmanager/credentials_container.cc
,
Apr 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/441a5eae1f3c959eb0732ecf2b2e93698ae8f458 commit 441a5eae1f3c959eb0732ecf2b2e93698ae8f458 Author: Adam Langley <agl@chromium.org> Date: Thu Apr 12 14:58:47 2018 webauthn: require that requests come from a frame in a focused window. The spec suggests aborting an operation when focus[1] is lost, but that's advice for site developers. I can't find anything in the spec about preventing background tabs from triggering operations. The cryptotoken extension refused to start a registration request, or to send a registration response to, anything but the active tab in the focused window. But background tabs could complete an authentication request. This change does something similar: it rejects both authentication and registration requests unless the requesting frame is in a focused window. It also performs that check before returning responses. This is slightly different from the cryptotoken behaviour because cryptotoken could only use what the extensions API exposed. For example, if the omnibox was focused, cryptotoken would complete a registration from the foreground tab but this code will reject it. I think this behaviour is better, and it's certainly far more inline with the content / browser separation. This change has been split from its tests, which will come in a future CL. [1] https://w3c.github.io/webauthn/#abortoperation Bug: 827266 Change-Id: If6e97dd6526e175f40718724eda21e3efd434f7f Reviewed-on: https://chromium-review.googlesource.com/991073 Commit-Queue: Adam Langley <agl@chromium.org> Reviewed-by: Nasko Oskov <nasko@chromium.org> Reviewed-by: Robert Sesek <rsesek@chromium.org> Reviewed-by: Balazs Engedy <engedy@chromium.org> Cr-Commit-Position: refs/heads/master@{#550195} [modify] https://crrev.com/441a5eae1f3c959eb0732ecf2b2e93698ae8f458/chrome/browser/chrome_content_browser_client.cc [modify] https://crrev.com/441a5eae1f3c959eb0732ecf2b2e93698ae8f458/chrome/browser/chrome_content_browser_client.h [modify] https://crrev.com/441a5eae1f3c959eb0732ecf2b2e93698ae8f458/components/password_manager/content/common/credential_manager_mojom_traits.cc [modify] https://crrev.com/441a5eae1f3c959eb0732ecf2b2e93698ae8f458/content/browser/webauth/authenticator_impl.cc [modify] https://crrev.com/441a5eae1f3c959eb0732ecf2b2e93698ae8f458/content/public/browser/content_browser_client.cc [modify] https://crrev.com/441a5eae1f3c959eb0732ecf2b2e93698ae8f458/content/public/browser/content_browser_client.h [modify] https://crrev.com/441a5eae1f3c959eb0732ecf2b2e93698ae8f458/third_party/blink/public/platform/modules/credentialmanager/credential_manager.mojom [modify] https://crrev.com/441a5eae1f3c959eb0732ecf2b2e93698ae8f458/third_party/blink/public/platform/modules/webauth/authenticator.mojom [modify] https://crrev.com/441a5eae1f3c959eb0732ecf2b2e93698ae8f458/third_party/blink/renderer/modules/credentialmanager/credential_manager_type_converters.cc [modify] https://crrev.com/441a5eae1f3c959eb0732ecf2b2e93698ae8f458/third_party/blink/renderer/modules/credentialmanager/credentials_container.cc
,
Apr 18 2018
The side effect of this change is that the developer tools console can't be open when making a request. Is this intentional?
,
Apr 18 2018
*not that it can't be open, it just can't be used to make or debug a request.
,
Apr 19 2018
> The side effect of this change is that the developer tools console can't be open when making a request. Is this intentional? Not really. The main intent of the change was to ensure that we didn't preclude the possibility of launching with M67. I'm still catching up from vacation and have some follow-up changes to land. I'll certainly look to see whether we can support having dev tools focused.
,
Apr 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3a01c2259dfad218493b628483fe8e1d9cb2acbc commit 3a01c2259dfad218493b628483fe8e1d9cb2acbc Author: Adam Langley <agl@chromium.org> Date: Thu Apr 26 00:14:55 2018 webauthn: require focused tab, rather than focused frame. Kim points out in the bug that the current logic (see https://chromium-review.googlesource.com/c/chromium/src/+/991073) doesn't allow dev tools to be focused, so it's not possible to trigger webauthn requests from the dev tools console. Nasko thought that the addition of a ContentBrowserClient API was too large a hammer too. I've not been able to come up with any cleaner solution however, but this change perhaps makes the hammer more reasonable: In order to address the devtools issue, this change switches the focus check to requiring that the tab be active in the focused window. (This also matches what the cryptotoken extension does.) This means that it has to chrome/ logic, rather than content/ logic, but we already had to add an API to ContentBrowserClient for this and we can reuse that to solve this problem too. Additionally, this change adds an interactive_ui_test to ensure that we don't regress. Bug: 827266 ,836203 Change-Id: I62226fc0366a83dc69d9cb32016108bdbe590d29 Reviewed-on: https://chromium-review.googlesource.com/1007637 Commit-Queue: Adam Langley <agl@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> Reviewed-by: Nasko Oskov <nasko@chromium.org> Reviewed-by: Balazs Engedy <engedy@chromium.org> Cr-Commit-Position: refs/heads/master@{#553838} [modify] https://crrev.com/3a01c2259dfad218493b628483fe8e1d9cb2acbc/chrome/browser/DEPS [modify] https://crrev.com/3a01c2259dfad218493b628483fe8e1d9cb2acbc/chrome/browser/chrome_content_browser_client.cc [modify] https://crrev.com/3a01c2259dfad218493b628483fe8e1d9cb2acbc/chrome/browser/chrome_content_browser_client.h [modify] https://crrev.com/3a01c2259dfad218493b628483fe8e1d9cb2acbc/chrome/browser/devtools/BUILD.gn [add] https://crrev.com/3a01c2259dfad218493b628483fe8e1d9cb2acbc/chrome/browser/webauth_interactive_uitest.cc [modify] https://crrev.com/3a01c2259dfad218493b628483fe8e1d9cb2acbc/chrome/test/BUILD.gn [modify] https://crrev.com/3a01c2259dfad218493b628483fe8e1d9cb2acbc/content/browser/webauth/authenticator_impl.cc [modify] https://crrev.com/3a01c2259dfad218493b628483fe8e1d9cb2acbc/content/browser/webauth/authenticator_impl_unittest.cc [modify] https://crrev.com/3a01c2259dfad218493b628483fe8e1d9cb2acbc/content/public/browser/content_browser_client.cc [modify] https://crrev.com/3a01c2259dfad218493b628483fe8e1d9cb2acbc/content/public/browser/content_browser_client.h [modify] https://crrev.com/3a01c2259dfad218493b628483fe8e1d9cb2acbc/device/fido/virtual_fido_device.h [modify] https://crrev.com/3a01c2259dfad218493b628483fe8e1d9cb2acbc/device/fido/virtual_u2f_device.cc
,
Apr 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c3b8f3551053d5271f7eb5c8967a189aadc144bb commit c3b8f3551053d5271f7eb5c8967a189aadc144bb Author: Max Morin <maxmorin@chromium.org> Date: Thu Apr 26 09:53:30 2018 Revert "webauthn: require focused tab, rather than focused frame." This reverts commit 3a01c2259dfad218493b628483fe8e1d9cb2acbc. Reason for revert: New test is flaky: crbug.com/837124. Original change's description: > webauthn: require focused tab, rather than focused frame. > > Kim points out in the bug that the current logic (see > https://chromium-review.googlesource.com/c/chromium/src/+/991073) > doesn't allow dev tools to be focused, so it's not possible to trigger > webauthn requests from the dev tools console. > > Nasko thought that the addition of a ContentBrowserClient API was too > large a hammer too. I've not been able to come up with any cleaner > solution however, but this change perhaps makes the hammer more > reasonable: > > In order to address the devtools issue, this change switches the focus > check to requiring that the tab be active in the focused window. (This > also matches what the cryptotoken extension does.) This means that it > has to chrome/ logic, rather than content/ logic, but we already had to > add an API to ContentBrowserClient for this and we can reuse that to > solve this problem too. > > Additionally, this change adds an interactive_ui_test to ensure that we > don't regress. > > Bug: 827266 ,836203 > Change-Id: I62226fc0366a83dc69d9cb32016108bdbe590d29 > Reviewed-on: https://chromium-review.googlesource.com/1007637 > Commit-Queue: Adam Langley <agl@chromium.org> > Reviewed-by: Lei Zhang <thestig@chromium.org> > Reviewed-by: Nasko Oskov <nasko@chromium.org> > Reviewed-by: Balazs Engedy <engedy@chromium.org> > Cr-Commit-Position: refs/heads/master@{#553838} TBR=nasko@chromium.org,thestig@chromium.org,agl@chromium.org,engedy@chromium.org No-Try: true Bug: 827266 , 836203, 837124 Change-Id: I9a243c93eac8d27eb33fae22c703f1faaeb7b83e Reviewed-on: https://chromium-review.googlesource.com/1029970 Commit-Queue: Max Morin <maxmorin@chromium.org> Reviewed-by: Max Morin <maxmorin@chromium.org> Cr-Commit-Position: refs/heads/master@{#553968} [modify] https://crrev.com/c3b8f3551053d5271f7eb5c8967a189aadc144bb/chrome/browser/DEPS [modify] https://crrev.com/c3b8f3551053d5271f7eb5c8967a189aadc144bb/chrome/browser/chrome_content_browser_client.cc [modify] https://crrev.com/c3b8f3551053d5271f7eb5c8967a189aadc144bb/chrome/browser/chrome_content_browser_client.h [modify] https://crrev.com/c3b8f3551053d5271f7eb5c8967a189aadc144bb/chrome/browser/devtools/BUILD.gn [delete] https://crrev.com/a62ef37c0f828a8f85db123feefbc49f373b8c41/chrome/browser/webauth_interactive_uitest.cc [modify] https://crrev.com/c3b8f3551053d5271f7eb5c8967a189aadc144bb/chrome/test/BUILD.gn [modify] https://crrev.com/c3b8f3551053d5271f7eb5c8967a189aadc144bb/content/browser/webauth/authenticator_impl.cc [modify] https://crrev.com/c3b8f3551053d5271f7eb5c8967a189aadc144bb/content/browser/webauth/authenticator_impl_unittest.cc [modify] https://crrev.com/c3b8f3551053d5271f7eb5c8967a189aadc144bb/content/public/browser/content_browser_client.cc [modify] https://crrev.com/c3b8f3551053d5271f7eb5c8967a189aadc144bb/content/public/browser/content_browser_client.h [modify] https://crrev.com/c3b8f3551053d5271f7eb5c8967a189aadc144bb/device/fido/virtual_fido_device.h [modify] https://crrev.com/c3b8f3551053d5271f7eb5c8967a189aadc144bb/device/fido/virtual_u2f_device.cc
,
Apr 28 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/573d3aca9826b8735f3f3a51016ade1dd32c31d5 commit 573d3aca9826b8735f3f3a51016ade1dd32c31d5 Author: Adam Langley <agl@chromium.org> Date: Sat Apr 28 00:32:13 2018 webauthn: require focused tab, rather than focused frame. (This is a reland of 3a01c2259dfad218493b628483fe8e1d9cb2acbc.) Kim points out in the bug that the current logic (see https://chromium-review.googlesource.com/c/chromium/src/+/991073) doesn't allow dev tools to be focused, so it's not possible to trigger webauthn requests from the dev tools console. Nasko thought that the addition of a ContentBrowserClient API was too large a hammer too. I've not been able to come up with any cleaner solution however, but this change perhaps makes the hammer more reasonable: In order to address the devtools issue, this change switches the focus check to requiring that the tab be active in the focused window. (This also matches what the cryptotoken extension does.) This means that it has to chrome/ logic, rather than content/ logic, but we already had to add an API to ContentBrowserClient for this and we can reuse that to solve this problem too. This change also moves the final focus check when registering to the point after the press, but before any permissions bubble. This is because it turns out that we have five different UI focus behaviours across four platforms and doing the focus check after trying to close the bubble was a mistake. Additionally, this change adds an interactive_ui_test to ensure that we don't regress. TBR=thestig,nasko Bug: 827266 ,836203 Change-Id: I954164d79e8dd375060fcc780a800904fe547c12 Reviewed-on: https://chromium-review.googlesource.com/1033336 Commit-Queue: Adam Langley <agl@chromium.org> Reviewed-by: Balazs Engedy <engedy@chromium.org> Cr-Commit-Position: refs/heads/master@{#554580} [modify] https://crrev.com/573d3aca9826b8735f3f3a51016ade1dd32c31d5/chrome/browser/DEPS [modify] https://crrev.com/573d3aca9826b8735f3f3a51016ade1dd32c31d5/chrome/browser/chrome_content_browser_client.cc [modify] https://crrev.com/573d3aca9826b8735f3f3a51016ade1dd32c31d5/chrome/browser/chrome_content_browser_client.h [modify] https://crrev.com/573d3aca9826b8735f3f3a51016ade1dd32c31d5/chrome/browser/devtools/BUILD.gn [add] https://crrev.com/573d3aca9826b8735f3f3a51016ade1dd32c31d5/chrome/browser/webauth_interactive_uitest.cc [modify] https://crrev.com/573d3aca9826b8735f3f3a51016ade1dd32c31d5/chrome/test/BUILD.gn [modify] https://crrev.com/573d3aca9826b8735f3f3a51016ade1dd32c31d5/content/browser/webauth/authenticator_impl.cc [modify] https://crrev.com/573d3aca9826b8735f3f3a51016ade1dd32c31d5/content/browser/webauth/authenticator_impl.h [modify] https://crrev.com/573d3aca9826b8735f3f3a51016ade1dd32c31d5/content/browser/webauth/authenticator_impl_unittest.cc [modify] https://crrev.com/573d3aca9826b8735f3f3a51016ade1dd32c31d5/content/public/browser/content_browser_client.cc [modify] https://crrev.com/573d3aca9826b8735f3f3a51016ade1dd32c31d5/content/public/browser/content_browser_client.h [modify] https://crrev.com/573d3aca9826b8735f3f3a51016ade1dd32c31d5/device/fido/virtual_fido_device.h [modify] https://crrev.com/573d3aca9826b8735f3f3a51016ade1dd32c31d5/device/fido/virtual_u2f_device.cc
,
Apr 30 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6bad787998a38a82978395e9bca614003a9f9e6a commit 6bad787998a38a82978395e9bca614003a9f9e6a Author: Adam Langley <agl@chromium.org> Date: Mon Apr 30 18:15:28 2018 webauthn: require focused tab, rather than focused frame. Merge to M67. (This is a reland of 3a01c2259dfad218493b628483fe8e1d9cb2acbc.) Kim points out in the bug that the current logic (see https://chromium-review.googlesource.com/c/chromium/src/+/991073) doesn't allow dev tools to be focused, so it's not possible to trigger webauthn requests from the dev tools console. Nasko thought that the addition of a ContentBrowserClient API was too large a hammer too. I've not been able to come up with any cleaner solution however, but this change perhaps makes the hammer more reasonable: In order to address the devtools issue, this change switches the focus check to requiring that the tab be active in the focused window. (This also matches what the cryptotoken extension does.) This means that it has to chrome/ logic, rather than content/ logic, but we already had to add an API to ContentBrowserClient for this and we can reuse that to solve this problem too. This change also moves the final focus check when registering to the point after the press, but before any permissions bubble. This is because it turns out that we have five different UI focus behaviours across four platforms and doing the focus check after trying to close the bubble was a mistake. Additionally, this change adds an interactive_ui_test to ensure that we don't regress. TBR=thestig,nasko (cherry picked from commit 573d3aca9826b8735f3f3a51016ade1dd32c31d5) Bug: 827266 ,836203 Change-Id: I954164d79e8dd375060fcc780a800904fe547c12 Reviewed-on: https://chromium-review.googlesource.com/1033336 Commit-Queue: Adam Langley <agl@chromium.org> Reviewed-by: Balazs Engedy <engedy@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#554580} Reviewed-on: https://chromium-review.googlesource.com/1035206 Reviewed-by: Adam Langley <agl@chromium.org> Cr-Commit-Position: refs/branch-heads/3396@{#385} Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428} [modify] https://crrev.com/6bad787998a38a82978395e9bca614003a9f9e6a/chrome/browser/DEPS [modify] https://crrev.com/6bad787998a38a82978395e9bca614003a9f9e6a/chrome/browser/chrome_content_browser_client.cc [modify] https://crrev.com/6bad787998a38a82978395e9bca614003a9f9e6a/chrome/browser/chrome_content_browser_client.h [modify] https://crrev.com/6bad787998a38a82978395e9bca614003a9f9e6a/chrome/browser/devtools/BUILD.gn [add] https://crrev.com/6bad787998a38a82978395e9bca614003a9f9e6a/chrome/browser/webauth_interactive_uitest.cc [modify] https://crrev.com/6bad787998a38a82978395e9bca614003a9f9e6a/chrome/test/BUILD.gn [modify] https://crrev.com/6bad787998a38a82978395e9bca614003a9f9e6a/content/browser/webauth/authenticator_impl.cc [modify] https://crrev.com/6bad787998a38a82978395e9bca614003a9f9e6a/content/browser/webauth/authenticator_impl.h [modify] https://crrev.com/6bad787998a38a82978395e9bca614003a9f9e6a/content/browser/webauth/authenticator_impl_unittest.cc [modify] https://crrev.com/6bad787998a38a82978395e9bca614003a9f9e6a/content/public/browser/content_browser_client.cc [modify] https://crrev.com/6bad787998a38a82978395e9bca614003a9f9e6a/content/public/browser/content_browser_client.h [modify] https://crrev.com/6bad787998a38a82978395e9bca614003a9f9e6a/device/fido/virtual_fido_device.cc [modify] https://crrev.com/6bad787998a38a82978395e9bca614003a9f9e6a/device/fido/virtual_fido_device.h
,
Jun 5 2018
Can we close this bug out?
,
Jun 5 2018
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by engedy@chromium.org
, Mar 31 2018