Implement complete FIDO AppID and Facet algorithm

RESOLVED FIXED in Firefox 58

Status

()

P5
enhancement
RESOLVED FIXED
3 years ago
9 months ago

People

(Reporter: jcj, Assigned: jcj)

Tracking

(Blocks: 2 bugs)

unspecified
mozilla58
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox58 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
Bug 1231681 did not implement the complete "FIDO AppID and Facet Specification" (https://fidoalliance.org/specs/fido-u2f-v1.0-nfc-bt-amendment-20150514/fido-appid-and-facets.html), particularly omitting the remote fetches of the TrustedFacets resources. This bug is to complete that implementation.
(Assignee)

Updated

3 years ago
Assignee: nobody → jjones
Blocks: 1065729
Depends on: 1231681
Component: DOM: Security → DOM: Device Interfaces
(Assignee)

Comment 1

3 years ago
One can use WebIDL to parse out the JSON in a simpler fashion than was originally expected. See JSON parsing in SubtleCrypto:

https://dxr.mozilla.org/mozilla-central/source/obj-x86_64-pc-linux-gnu/dom/bindings/SubtleCryptoBinding.cpp#3268
(Assignee)

Updated

2 years ago
Blocks: 1296138
(Assignee)

Updated

a year ago
Assignee: jjones → nobody
(Assignee)

Comment 2

a year ago
This algorithm is 1) not needed for most use cases of using U2F, and 2) not simple to implement, so downgrading to P5. Chances are we'll WONTFIX this unless we find a use case in the wild.
Severity: normal → enhancement
Priority: -- → P5
Comment hidden (advocacy)
(Assignee)

Comment 4

a year ago
Patches are certainly welcome!

Re: #1, one is conversations among FIDO developers, and also https://github.com/prefiks/u2f4moz/issues/59#issuecomment-326103302 where I asked that of the u2f4moz developer.

Re: #2, this is true; My focus is supporting U2F hardware tokens for Web Authentication, not the FIDO U2F JS API. FIDO U2F API is a best-effort spare-time activity for me. But I'm happy to help others move it along!
Comment hidden (advocacy)
Comment hidden (advocacy)
(Assignee)

Comment 7

a year ago
(In reply to My1 from comment #5)
> well jcj there are usecases in the wild.
> as of some comments on https://bugzilla.mozilla.org/show_bug.cgi?id=1065729
> facebook and github, 2 very big sires apparently use multifacet.
> 
> also the solution we have now is just half-baked, multifacet has many uses,
> like subdomains or apps, so the facets should be properly supported.

Fair. That said, Github's U2F works fine for me as-is -- used it just now, and Facebook has previously worked alright before their user-agent detection. While they might use the facet descriptors for some federation login aspects, it's not needed for basic login flows.

In the good news though, I don't personally have any plans to ship FIDO U2F API support, so we don't need to worry about shipping a halfway implementation. I expect this to remain an experimental feature - the purpose in getting it here was to help along W3C Web Authentication.

Other good news: Since this initial implementation, a C++ API for DOM Fetch now exists [1], so it's now just somewhat painful to fetch and decode the Facet metadata -- when I wrote this code last year, it was going to be much harder.


[1] http://searchfox.org/mozilla-central/source/dom/fetch/Fetch.h

Comment 8

a year ago
Hi.  I was a co-editor / author of the FIDO Alliance facet spec (https://fidoalliance.org/specs/fido-u2f-v1.0-ps-20141009/fido-appid-and-facets-ps-20141009.html) with Dirk Balfanz.  In the years since we developed that standard, we've discussed it several times, and both agree that the application to Web Origins was generally too restrictive and perhaps a mistake entirely.  I was originally concerned that a generic eTLD+1 scope would create too broad a scope for phishing using compromised hosts that are otherwise low-value and weakly secured but share the same general DNS scope.  In hindsight, I've come to feel that the concrete benefit of this is much outweighed by the additional complexity for web use cases and the latency introduced in fetching and parsing the facet list.

The Web Authentication WG at the W3C has decided that scoping credentials to the scope of single organizational control, as reflected by the Public Suffix List, is sufficiently secure and meets most use cases.

I would not be sad if Firefox chose to implement U2F using the Web Authentication scoping rules.  However, I would seriously consider not enabling U2F features of Facebook for Firefox if it implements an exact origin match only, since this is likely to leave people stuck in circumstances they cannot remedy.  We root our AppID at https://www.facebook.com but use many other domains, e.g. https://business.facebook.com, https://m.facebook.com, https://www.alpha.facebook.com, https://m.alpha.facebook.com, etc.  We use U2F in more places than just login, e.g. to do in-context re-authentication or "step up" auth on these domains. Introducing additional redirects to a canonical single U2F host would be a big engineering effort and the user experience far from ideal.

Scoping to the eTLD+1 would work fine for us. Algorithmically, this could look like taking both the AppID domain and domain issuing the challenge, descending towards the least-specific label until a PSL boundary is hit, and seeing if the eTLD+1 is a match for both.

These are the rules we'll have to live with anyway in the Web Authentication world, so I see little harm in applying them now, especially if the alternative is the simple match deployed today, which will just break existing scenarios to the point we (Facebook) likely can't enable it.
Comment hidden (advocacy)

Comment 10

a year ago
(In reply to My1 from comment #9)
> well I think it IS a bit too restrictive. it would have been more awesome if
> you could allow any domain or whatever in the app id json file and not just
> subdomains.
> 
> especially for services like amazon or so which run the same accounts over a
> ton of different domains this would have made it easier for those to
> implement auth.

The point is to make it so that no user decision is necessary to avoid phishing attacks.  It is the responsibility of the user agent to verify the authenticity of the remote site issuing the challenge and whether it is a valid scope for signing.  If any origin could sign with any key, forwarding attacks would be possible.  

This would also allow U2F to be used as a sort of super-cookie.  

It is important that U2F not regress to the situation we have today where we are expecting end-users to make trust decisions about what domain they see in the address bar, nor to introduce tracking and non-consensual authentication issues which might make the security benefits of U2F unacceptable to some due to privacy risks.

Restricting to a single organizational scope as defined by the PSL meets these goals while being congruent with other features of the web platform, like cookies and domain lowering, which already allow similar levels information sharing.
Comment hidden (advocacy)

Comment 12

a year ago
(In reply to My1 from comment #11)
> well if site B would want to be able to get users from site A in, site A
> would obviously put Site B into the json file, and the json file as far as I
> know is the only way to get multiple places for one authentication scope
> anyway.
> 
> meaning no phisher can try to get a U2F recognize a challenge for for
> example facebook.com if they dont have access to facebook's appid file.

This bug is about the Firefox implementation.  Nobody is going to update the 3 year old FIDO spec at this point to support web facets outside the eTLD+1, and if Firefox chose to do this, no site would support it because it wouldn't work on any other user agent.  I'm sorry you aren't happy with the spec, My1, but the reasonable choices for resolving this bug seem to be to:

1) stick with current strict match implementation (and break many existing service deployments of U2F)
2) implement the full Facet spec
3) implement the WebAuthN scoping, which is a superset of the Facet scoping and will work with all existing deployments but be somewhat less restrictive
Comment hidden (advocacy)
Comment hidden (advocacy)

Comment 15

a year ago
(In reply to g.rantila from comment #13)

> Alternative 3, going with eTLD+1, is a FIDO U2F API breaker - it's against
> how sites *today* do it, and it's a breaking change to Chrome's
> implementation. That would basically mean one would need to User Agent
> detect the browser and create per-browser Registration/Sign requests.
> Developers wouldn't really love it. If this would, like 1, break existing
> deployments, I would block Firefox. Sounds more like a whole new version of
> the spec.
> 
>

I don't think #3 will break any existing service deployments.  The choice to allow or not a signing request based on facet scoping is internal to the user agent and the Web Authentication scoping is a superset of the FIDO 1.0 scoping.
Comment hidden (advocacy)
Our guidelines ask you don't argue about priority setting on bugs. Triage leads and Mozillians working on the bug set the priority. See: https://bugzilla.mozilla.org/page.cgi?id=etiquette.html

Extended, off-topic comments on a bug add noise to the signal. Per, https://bugzilla.mozilla.org/page.cgi?id=restrict_comments_guidelines.html, we've restricted comments on this bug to Mozillians with EDITBUGS permissions.
Restrict Comments: true
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 20

a year ago
mozreview-review
Comment on attachment 8913563 [details]
Bug 1244959 - Use IsRegistrableDomainSuffixOfOrEqualTo for U2F Facets

https://reviewboard.mozilla.org/r/184944/#review191414

I think this is a good compromise, but also a great idea because (as you wrote) folks can focus on a single security model that won't break with WebAuthn in the future. Let's do this.

::: commit-message-6dea0:7
(Diff revision 2)
> +
> +In Comment 8 of Bug 1244959 [1], Brad Hill argues that instead of leaving our
> +U2F Facet support completely half-way, that we could use the Public Suffix logic
> +introduced into HTML for W3C Web Authentication (the method named
> +IsRegistrableDomainSuffixOfOrEqualTo) to scope the FIDO AppID to an eTLD+1
> +heirarchy. This is a deviation from the FIDO specification, but doesn't break

nit: hierarchy

::: dom/u2f/U2F.cpp:149
(Diff revision 2)
> +  nsHTMLDocument* html = document->AsHTMLDocument();
> +  if (NS_WARN_IF(!html)) {
> +    return ErrorCode::BAD_REQUEST;
> +  }
> +
> +  // Use the base domain as the facet for evalution. This lets this algorithm

nit: evaluation

::: dom/u2f/U2F.cpp:171
(Diff revision 2)
> +  MOZ_LOG(gU2FLog, LogLevel::Debug,
> +          ("AppId %s Facet %s", appIdHost.get(), lowestFacetHost.get()));
> +
> +  if (html->IsRegistrableDomainSuffixOfOrEqualTo(NS_ConvertUTF8toUTF16(lowestFacetHost),
> +                                                 appIdHost)) {
>      return ErrorCode::OK;

(I love the fact that `ErrorCode::OK` exists.)
Attachment #8913563 - Flags: review?(ttaubert) → review+
(Assignee)

Comment 21

a year ago
mozreview-review-reply
Comment on attachment 8913563 [details]
Bug 1244959 - Use IsRegistrableDomainSuffixOfOrEqualTo for U2F Facets

https://reviewboard.mozilla.org/r/184944/#review191414

Thanks!
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Assignee: nobody → jjones
Status: NEW → ASSIGNED
Keywords: checkin-needed

Comment 23

a year ago
mozreview-review
Comment on attachment 8913563 [details]
Bug 1244959 - Use IsRegistrableDomainSuffixOfOrEqualTo for U2F Facets

https://reviewboard.mozilla.org/r/184944/#review191430


C/C++ static analysis didn't find any defects in this patch. Hooray!

Comment 24

a year ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f2d25c30aaed
Use IsRegistrableDomainSuffixOfOrEqualTo for U2F Facets r=ttaubert
Keywords: checkin-needed

Comment 25

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f2d25c30aaed
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox58: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
(Assignee)

Updated

a year ago
Duplicate of this bug: 1419298
You need to log in before you can comment on or make changes to this bug.