Bug 196046

Summary: [WebAuthN] Add a quirk for google.com when processing AppID extension
Product: WebKit Reporter: Alexei Czeskis <aczeskis>
Component: PlatformAssignee: Jiewen Tan <jiewen_tan>
Status: RESOLVED FIXED    
Severity: Normal CC: alex.gaynor, bfulgham, commit-queue, jiewen_tan, simon.fraser, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 181943    
Attachments:
Description Flags
Patch
bfulgham: review+, bfulgham: commit-queue-
Patch for landing none

Description Alexei Czeskis 2019-03-20 17:36:30 PDT
For historical reasons (being the first U2F implementor) Google uses a non-standard (cross-origin) AppID.  The App ID is “www.gstatic.com” for logins to “google.com” and its subdomains.

This bug requests an exception on the cross-origin check for valid AppIds in the case of google.com and gstatic.com.

Both Chrome and Firefox already make this exception.

Firefox tracking bug and implementation:
  https://bugzilla.mozilla.org/show_bug.cgi?id=1436078

Chrome's implementation:
  https://cs.chromium.org/chromium/src/content/browser/webauth/authenticator_common.cc?l=252&rcl=4d674f923c5a1f03b2262132cb621a3db78f7562
Comment 1 Radar WebKit Bug Importer 2019-03-20 18:13:04 PDT
<rdar://problem/49088479>
Comment 2 Jiewen Tan 2019-05-01 21:58:36 PDT
Created attachment 368761 [details]
Patch
Comment 3 Brent Fulgham 2019-05-02 10:00:55 PDT
Comment on attachment 368761 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=368761&action=review

> Source/WebCore/Modules/webauthn/AuthenticatorCoordinator.cpp:84
> +{

Please add a comment:

"
FIXME(BUG #): Remove this quirk in 2023
As an early adopter of U2F features, Google has a large number of existing device registrations that authenticate 'google.com' against 'gstatic.com'.
Firefox and other browsers have agreed to grant an exception to the AppId rules for a limited time period (5 years from January, 2018) to allow existing
Google users to seamlessly transition to proper WebAuthN behavior.
"

Then please file a bug to remove this quirk in 2023.
Comment 4 Jiewen Tan 2019-05-02 11:34:27 PDT
Comment on attachment 368761 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=368761&action=review

>> Source/WebCore/Modules/webauthn/AuthenticatorCoordinator.cpp:84
>> +{
> 
> Please add a comment:
> 
> "
> FIXME(BUG #): Remove this quirk in 2023
> As an early adopter of U2F features, Google has a large number of existing device registrations that authenticate 'google.com' against 'gstatic.com'.
> Firefox and other browsers have agreed to grant an exception to the AppId rules for a limited time period (5 years from January, 2018) to allow existing
> Google users to seamlessly transition to proper WebAuthN behavior.
> "
> 
> Then please file a bug to remove this quirk in 2023.

Added.
Comment 5 Jiewen Tan 2019-05-02 11:34:49 PDT
(In reply to Brent Fulgham from comment #3)
> Comment on attachment 368761 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=368761&action=review
> 
> > Source/WebCore/Modules/webauthn/AuthenticatorCoordinator.cpp:84
> > +{
> 
> Please add a comment:
> 
> "
> FIXME(BUG #): Remove this quirk in 2023
> As an early adopter of U2F features, Google has a large number of existing
> device registrations that authenticate 'google.com' against 'gstatic.com'.
> Firefox and other browsers have agreed to grant an exception to the AppId
> rules for a limited time period (5 years from January, 2018) to allow
> existing
> Google users to seamlessly transition to proper WebAuthN behavior.
> "
> 
> Then please file a bug to remove this quirk in 2023.

Thanks Brent for r+ this patch.
Comment 6 Jiewen Tan 2019-05-02 11:36:34 PDT
Created attachment 368798 [details]
Patch for landing
Comment 7 WebKit Commit Bot 2019-05-02 12:15:13 PDT
Comment on attachment 368798 [details]
Patch for landing

Clearing flags on attachment: 368798

Committed r244879: <https://trac.webkit.org/changeset/244879>
Comment 8 Simon Fraser (smfr) 2019-05-02 12:57:44 PDT
Comment on attachment 368761 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=368761&action=review

>>>> Source/WebCore/Modules/webauthn/AuthenticatorCoordinator.cpp:84
>>>> +{
>>> 
>>> Please add a comment:
>>> 
>>> "
>>> FIXME(BUG #): Remove this quirk in 2023
>>> As an early adopter of U2F features, Google has a large number of existing device registrations that authenticate 'google.com' against 'gstatic.com'.
>>> Firefox and other browsers have agreed to grant an exception to the AppId rules for a limited time period (5 years from January, 2018) to allow existing
>>> Google users to seamlessly transition to proper WebAuthN behavior.
>>> "
>>> 
>>> Then please file a bug to remove this quirk in 2023.
>> 
>> Added.
> 
> Thanks Brent for r+ this patch.

This needs to go through the Quirks class so that the Develop menu switch can turn it off.
Comment 9 Jiewen Tan 2019-05-02 14:04:59 PDT
(In reply to Simon Fraser (smfr) from comment #8)
> Comment on attachment 368761 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=368761&action=review
> 
> >>>> Source/WebCore/Modules/webauthn/AuthenticatorCoordinator.cpp:84
> >>>> +{
> >>> 
> >>> Please add a comment:
> >>> 
> >>> "
> >>> FIXME(BUG #): Remove this quirk in 2023
> >>> As an early adopter of U2F features, Google has a large number of existing device registrations that authenticate 'google.com' against 'gstatic.com'.
> >>> Firefox and other browsers have agreed to grant an exception to the AppId rules for a limited time period (5 years from January, 2018) to allow existing
> >>> Google users to seamlessly transition to proper WebAuthN behavior.
> >>> "
> >>> 
> >>> Then please file a bug to remove this quirk in 2023.
> >> 
> >> Added.
> > 
> > Thanks Brent for r+ this patch.
> 
> This needs to go through the Quirks class so that the Develop menu switch
> can turn it off.

I would argue it is not meaningful to turn Quirks off. Basically, the whole WebAuthentication feature will not work in Google.com if this is off.
Comment 10 Wenson Hsieh 2019-05-02 14:31:04 PDT
Comment on attachment 368761 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=368761&action=review

>>>>>> Source/WebCore/Modules/webauthn/AuthenticatorCoordinator.cpp:84
>>>>>> +{
>>>>> 
>>>>> Please add a comment:
>>>>> 
>>>>> "
>>>>> FIXME(BUG #): Remove this quirk in 2023
>>>>> As an early adopter of U2F features, Google has a large number of existing device registrations that authenticate 'google.com' against 'gstatic.com'.
>>>>> Firefox and other browsers have agreed to grant an exception to the AppId rules for a limited time period (5 years from January, 2018) to allow existing
>>>>> Google users to seamlessly transition to proper WebAuthN behavior.
>>>>> "
>>>>> 
>>>>> Then please file a bug to remove this quirk in 2023.
>>>> 
>>>> Added.
>>> 
>>> Thanks Brent for r+ this patch.
>> 
>> This needs to go through the Quirks class so that the Develop menu switch can turn it off.
> 
> I would argue it is not meaningful to turn Quirks off. Basically, the whole WebAuthentication feature will not work in Google.com if this is off.

I believe the utility in being able to turn off quirks is that web developers can easily test their content against the un-quirked browser engine, to make sure that their content will work when we finally remove the quirk.
Comment 11 Jiewen Tan 2019-05-02 14:37:42 PDT
(In reply to Wenson Hsieh from comment #10)
> Comment on attachment 368761 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=368761&action=review
> 
> >>>>>> Source/WebCore/Modules/webauthn/AuthenticatorCoordinator.cpp:84
> >>>>>> +{
> >>>>> 
> >>>>> Please add a comment:
> >>>>> 
> >>>>> "
> >>>>> FIXME(BUG #): Remove this quirk in 2023
> >>>>> As an early adopter of U2F features, Google has a large number of existing device registrations that authenticate 'google.com' against 'gstatic.com'.
> >>>>> Firefox and other browsers have agreed to grant an exception to the AppId rules for a limited time period (5 years from January, 2018) to allow existing
> >>>>> Google users to seamlessly transition to proper WebAuthN behavior.
> >>>>> "
> >>>>> 
> >>>>> Then please file a bug to remove this quirk in 2023.
> >>>> 
> >>>> Added.
> >>> 
> >>> Thanks Brent for r+ this patch.
> >> 
> >> This needs to go through the Quirks class so that the Develop menu switch can turn it off.
> > 
> > I would argue it is not meaningful to turn Quirks off. Basically, the whole WebAuthentication feature will not work in Google.com if this is off.
> 
> I believe the utility in being able to turn off quirks is that web
> developers can easily test their content against the un-quirked browser
> engine, to make sure that their content will work when we finally remove the
> quirk.

I don't think they would have any un-quirked version. I probably shouldn't name this as quirks.