Closed Bug 1436078 Opened 2 years ago Closed 2 years ago

Web Authentication - Support already-enrolled U2F devices with Google Accounts

Categories

(Core :: DOM: Device Interfaces, enhancement, P1, major)

60 Branch
enhancement

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: jcj, Assigned: jcj)

References

(Blocks 2 open bugs, )

Details

(Whiteboard: [webauthn] [webauthn-interop][u2f])

Attachments

(1 file)

To paraphrase the Intent-to-Ship [1]:

Web Authentication is on-track to ship in Firefox 60, and contains
within it support for already-deployed USB-connected FIDO U2F devices, and
we intend to ship with a spec extension feature implemented to support
devices that were already-enrolled using the older U2F Javascript API .

This bug will hard-code support in Gecko to permit Google Accounts’ cross-origin U2F behavior, the same way as Chrome has. This is proposed for a period of 5 years, until 2023 The code should be a small search similar to Chrome [2].

[1] https://groups.google.com/d/msg/mozilla.dev.platform/Uiu3fwnA2xw/201ynAiPAQAJ
[2] https://chromium.googlesource.com/chromium/src.git/+/master/chrome/browser/extensions/api/cryptotoken_private/cryptotoken_private_api.cc#161
Blocks: 1436085
Comment on attachment 8948852 [details]
Bug 1436078 - Hard-code U2F permissions for Google Accounts

https://reviewboard.mozilla.org/r/218234/#review224096

Let's do this.
Attachment #8948852 - Flags: review?(ttaubert) → review+
Assignee: nobody → jjones
Status: NEW → ASSIGNED
Comment on attachment 8948852 [details]
Bug 1436078 - Hard-code U2F permissions for Google Accounts

https://reviewboard.mozilla.org/r/218234/#review224158

::: dom/u2f/U2F.cpp:217
(Diff revision 1)
> +  // Bug #1436078 - Permit Google Accounts. Remove in Bug #1436085 in Jan 2023.
> +  if (lowestFacetHost.EqualsLiteral("google.com") &&
> +      (aAppId.Equals(kGoogleAccountsAppId1) ||
> +       aAppId.Equals(kGoogleAccountsAppId2))) {
> +    MOZ_LOG(gU2FLog, LogLevel::Debug,
> +            ("U2F permitted for Google Accounts via Bug #1436085"));
> +    return ErrorCode::OK;
> +  }

On second thought... we should only allow this for sign operations, not for registration, right? The function currently isn't aware of that... maybe we need another parameter?
Attachment #8948852 - Flags: review+ → review-
Comment on attachment 8948852 [details]
Bug 1436078 - Hard-code U2F permissions for Google Accounts

https://reviewboard.mozilla.org/r/218234/#review224158

> On second thought... we should only allow this for sign operations, not for registration, right? The function currently isn't aware of that... maybe we need another parameter?

Good idea!
Comment on attachment 8948852 [details]
Bug 1436078 - Hard-code U2F permissions for Google Accounts

https://reviewboard.mozilla.org/r/218234/#review224210

Awesome, thank you.
Attachment #8948852 - Flags: review?(ttaubert) → review+
hg error in cmd: hg push -r tip ssh://hg.mozilla.org/integration/autoland: pushing to ssh://hg.mozilla.org/integration/autoland
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 1 changes to 1 files
remote: 
remote: 
remote: ************************** ERROR ****************************
remote: Rev ff1f162dd874 needs "Bug N" or "No bug" in the commit message.
remote: J.C. Jones <jjones@mozilla.com>
remote: Bug #1436078 - Hard-code U2F permissions for Google Accounts r=ttaubert
remote: 
remote: This patch support already-enrolled U2F devices at Google Accounts by adding a
remote: hard-coded "OK" into the U2F EvaluateAppID method, per the intent-to-ship [1].
remote: 
remote: This adds no tests, as this is not testable in our infrastructure. It will
remote: require cooporation with Google Accounts to validate.
remote: 
remote: [1] https://groups.google.com/d/msg/mozilla.dev.platform/Uiu3fwnA2xw/201ynAiPAQAJ
remote: 
remote: MozReview-Commit-ID: 1YLd5sfeTKv
remote: *************************************************************
remote: 
remote: 
remote: transaction abort!
remote: rollback completed
remote: pretxnchangegroup.c_commitmessage hook failed
abort: push failed on remote
Pushed by ttaubert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/89ac5a28c228
Hard-code U2F permissions for Google Accounts r=ttaubert
Blast! No static enums for me. I've a new try build running without that modifier: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ca9f726ea9d1
Flags: needinfo?(jjones)
Did you mean "enum class" instead of "static enum" ?
(In reply to Tom Schuster [:evilpie] from comment #13)
> Did you mean "enum class" instead of "static enum" ?

This is a lesson in C++ to me. :) Yeah, that's what I wanted, to avoid polluting the name scope. Thanks, Tom.
Pushed by ttaubert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/62646c1718b2
Hard-code U2F permissions for Google Accounts r=ttaubert
https://hg.mozilla.org/mozilla-central/rev/62646c1718b2
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
See Also: → 1539541
You need to log in before you can comment on or make changes to this bug.