Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Review Web Authentication spec #97

Closed
travisleithead opened this issue Dec 2, 2015 · 24 comments
Closed

Review Web Authentication spec #97

travisleithead opened this issue Dec 2, 2015 · 24 comments
Assignees

Comments

@travisleithead
Copy link
Contributor

@plinss
Copy link
Member

plinss commented Dec 2, 2015

Version 1.0 here: https://fidoalliance.org/specifications/download/

@dbaron
Copy link
Member

dbaron commented Dec 2, 2015

There are also 3 FIDO specs in the submission:

FIDO 2.0 Web API
FIDO 2.0 Key Attestation Format
FIDO 2.0 Signature Format

My understanding is the JS API in 2.0 has been substantially modernized in terms of following best practices (promises, string vs. numeric constants) relative to 1.0. It's not yet clear whether we'll be able to avoid being stuck with 1.0.

@travisleithead
Copy link
Contributor Author

Reviewed this in Melbourne (raw minutes)

Points to review with FIDO folks/WG when it get started below:
TODOs:
- clarification on mismatch between WebCrypto, FIDO Signature Format, JWA nomenclature on hash algorithm naming
- clarification on some data structures
- Credential interface, e.g.:

  • type
  • id
  • imageUri (align with manifest syntax for multiple sizes?) in User Account Information
    • reasons for eTLD+1 rather than origin (and unclear wording in section about how the origin and RP ID are used in authenticator operations)
    • feedback on the API design
      • options object instead of optional params?
      • clarify the Credential interface between Credential Management draft and FIDO drafts
    • specs need more examples & explainer doc, e.g. end-to-end code for Credentials + FIDO?
    • specs need more links of terms to their definitions
    • request time to meet at a TAG call

We also compared it to previous requirements for keygen replacement (here), and found that it meets the requirements. However, it does not provide a way to generate credentials that are not origin-specific. Though this was not a specific requirement, some folks were interested in having that capability for identity purposes. We note that FIDO is not trying to provide identity, only authentication.

@dbaron
Copy link
Member

dbaron commented Jan 14, 2016

/cc @bifurcation @vijaybh @balfanz @arnar @jmhodges so that some folks not on the TAG, and actually involved with FIDO, are cc:ed here (based on the github usernames I could find)

At some point (maybe after the WG starts?) it's probably worth setting up some discussion time with the TAG

@domenic
Copy link
Member

domenic commented Jan 14, 2016

@jmhodges
Copy link

Wrong Jeff Hodges!

@vijaybh
Copy link

vijaybh commented Jan 14, 2016

/cc @equalsJeffH to pull in the right Jeff Hodges, and also @leshi and @rlin1 who owned the signature and attestation docs respectively.

We'll definitely set up time with TAG once the WG gets under way, but let me answer a couple things here so you can have an idea of our direction.

Regarding no way to generate non-origin-specific credentials, this is by design. We were looking to do a simple thing well, rather than do a complicated thing badly. Even the desirability of a global cross-origin identity is controversial due to the privacy implications and we did not want to poke that hornet's nest. We did get a lot of feedback that we should not produce something that tilts the playing field towards mega-identity-providers, so we took some pains to ensure that the whole system would be usable even for small websites creating their own identities.

Regarding eTLD+1, this was a compromise. We got feedback that origin can be too strict for some RPs - for example login.foo.com may want to use the identities created by identity.foo.com. At some point FIDO created its own mechanism using facet lists in a text file at a particular well-known path under the origin that created the credential; this would say what other "facets" (i.e. origins or App IDs) should be considered equivalent to this origin for purposes of same-origin policy enforcement. This seemed really solid, but it also seemed like an extra complication that was not really part of our core mission as FIDO. So the eTLD+1 mechanism allows anyone with the same eTLD+1 as the credential creating origin to use the credential, but it also puts the actual origin of the caller in the signature so the replying party can choose whether it wants to accept this use of the credential or not. Ideally if we wanted to strengthen this mechanism, we would reuse something existing such as manifests and not invent yet another SOP - any guidance here would be welcome.

@domenic
Copy link
Member

domenic commented Jan 14, 2016

This might be naive, since I don't have too much depth into this problem space. But what about using origin instead of eTLD+1, and then letting websites use the existing cross-origin communication mechanisms (CORS, iframes-with-postMessage, server-side communication) to smuggle the credentials over to another origin?

@vijaybh
Copy link

vijaybh commented Jan 14, 2016

That was my initial reaction as well. The feedback we got was that the added complexity and performance hit from this was too much for people.

@annevk
Copy link
Member

annevk commented Jan 14, 2016

Do you have some pointers to the prior discussion? It's a little hard to judge the information as-is.

@vijaybh
Copy link

vijaybh commented Jan 14, 2016

Unfortunately this was not captured anyplace where I can share it. If there is interest we could put together a short description of the issue.

@annevk
Copy link
Member

annevk commented Jan 14, 2016

I think that would be great, since anything new that relies on public suffix is kind of a red flag. We have managed to keep it contained to cookies and document.domain for quite a while now.

@mnot mnot added this to the tag-telcon-2016-02-10 milestone Jan 14, 2016
@equalsJeffH
Copy link

@bifurcation is Richard Barnes of mozilla, yes?

@mnot
Copy link
Member

mnot commented Jan 17, 2016

Da.

@balfanz
Copy link

balfanz commented Jan 27, 2016

People were asking for a rationalization of the rule that an Authenticator should scope keys by eTLD+1:

I agree that from a “purity” point of view, it makes sense to scope keys by origin. In practice, we (Google) have found however, that that doesn’t quite work.

Here are all the origins at Google that need access to a user’s FIDO key, for example:

[
  "https://accounts.google.com",
  "https://accounts.sandbox.google.com",
  "https://myaccount.google.com",
  "https://ac-autopush-myaccount.corp.google.com",
  "https://ac-daily-0-myaccount.corp.google.com",
  "https://ac-daily-1-myaccount.corp.google.com",
  "https://ac-daily-2-myaccount.corp.google.com",
  "https://ac-daily-3-myaccount.corp.google.com",
  "https://ac-daily-4-myaccount.corp.google.com",
  "https://ac-daily-5-myaccount.corp.google.com",
  "https://ac-daily-6-myaccount.corp.google.com",
  "https://security.google.com",
  "https://ac-autopush-security.corp.google.com",
  "https://ac-daily-0-security.corp.google.com",
  "https://ac-daily-1-security.corp.google.com",
  "https://ac-daily-2-security.corp.google.com",
  "https://ac-daily-3-security.corp.google.com",
  "https://ac-daily-4-security.corp.google.com",
  "https://ac-daily-5-security.corp.google.com",
  "https://ac-daily-6-security.corp.google.com"
]

There are two reasons that we need to access a user’s key from multiple origins. First, many of them are origins used by our services as they “soak” during a staged rollout process, and where we test functionality before it is released more widely. The second reason is related to site security architecture. We use security.google.com (Security) for users’ account management, including coaching users through the process of creating FIDO credentials. This site hosts a lot of material aimed at helping users, including tutorials and so on. User login is performed by accounts.google.com (Accounts). Accounts is at the top of our security hierarchy; it does not load pages from any other origin and it is intentionally architected to have the smallest possible attack surface.
We want the FIDO authenticator (which may be a physically separate device from the client running the browser) to help protect user privacy, by making any one site’s credentials invisible to other sites. This is what we are currently using the rpId eTLD+1 mechanism for. In addition, the actual web origin of the caller is incorporated into the signature that the authenticator produces, so the site can make fine-grained decisions (similar to CORS credentialed requests) on whether to accept or reject it.

From the site perspective, login is a very common and performance-sensitive operation, so we would like to do it as efficiently as possible. The suggested alternatives seem to be slower and complex to implement. For instance, the login page on Accounts could load Security in an iframe and make calls to it through postMessage, but this would be undesirable for performance and security reasons.

@wseltzer
Copy link

The https://www.w3.org/Webauthn group will meet F2F on March 4. I've brought this issue to the chairs' attention.

@torgo
Copy link
Member

torgo commented Mar 30, 2016

Discussed at London f2f on 30-March-2016 with @wseltzer.

@slightlyoff
Copy link
Member

I'd like to dig into the <iframe> perf and security concerns.

X-Frame-Options allows e.g. the Security page to Accounts page to decide what parent origins to allow embedding from. The postMessage dance does suck, but in terms of performance, a Service Worker should allow the Security page to appear near-instantly in every case.

Is the complexity concern that allowing iframing at all is opening pandora's box?

Also, doesn't the eTLD+1 reliance on the public suffix list open this feature up to potential subversion? Obviously everything else would suffer from a suffix list hack, but perhaps not to the same extent?

@travisleithead
Copy link
Contributor Author

Note, some additional comments in the Credentials management API #49.

@travisleithead travisleithead changed the title Review FIDO spec Review Web Authentication spec Jul 30, 2016
@dbaron
Copy link
Member

dbaron commented Nov 2, 2016

The working group's document is:
Web Authentication: An API for accessing Scoped Credentials

This appears to be based on material from all 3 of the documents in the above member submission, so I think it's the only document we should be looking at in this review.

@slightlyoff
Copy link
Member

Discussed again at the F2F in Tokyo.

If we're reading the current spec correctly, it appears that the eTLD+1 logic is still in place, but using a new form of it (the document.domain relaxation).

It seems this is now opt-in based on setting the RP ID directly. In discussion, we've come around to the use of eTLD+1 here (given that it is in an explicit opt-in form). We continue to prefer that eTLD+1 not be used in other specs in the future, acknowledging that the operational burden in this case is unique.

As an editorial note, the examples that are currently in Section 10 would be best at the top of the document or in a separate Explainer-style document.

It's odd that timeoutSeconds is specified in seconds. Most of the rest of the platform uses milliseconds. Is there a reason not to do so here?

Lastly we note that the goog-android2 Attestation Format name is...um....really ugly.

In general these are nits. Thanks for taking the time to engage with us and for being responsive to feedback.

@domenic
Copy link
Member

domenic commented Nov 2, 2016

It's odd that timeoutSeconds is specified in seconds. Most of the rest of the platform uses milliseconds. Is there a reason not to do so here?

We in particular have https://w3ctag.github.io/design-principles/#milliseconds on this subject :)

@leshi
Copy link

leshi commented Nov 2, 2016

Thank you all for the review! 👍

We'll take a look at the feedback and will take action.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Mar 21, 2017
…lTo r=smaug

This commit refactors the SetDomain method in a Document to call a new function
IsRegistrableDomainSuffixOfOrEqualTo(), defined in HTML [1]. This commit tries
not to rename anything except input variables, so as to remain as clear as
possible. It likely should have various variables renamed, but given the
author's unfamiliarity with this module, review seems a good time to do that.
It's also duplicating comments a little bit; let me know which one(s) you'd like
to keep!

Note: Commentary on the HTML change is available in the PR [2], and the
rationale for this behavior in Web Auentication, where this algorithm will be
used, is also recorded [3].

Updates: Refactored two new protected methods to avoid code duplication.

[1] https://html.spec.whatwg.org/multipage/browsers.html#is-a-registrable-domain-suffix-of-or-is-equal-to
[2] whatwg/html#2365
[3] w3ctag/design-reviews#97 (comment)

MozReview-Commit-ID: 4Dr8yOMdhez

--HG--
extra : rebase_source : 5d042c4e97b8866027c81ea0f1c544ce1721b7c4
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Mar 24, 2017
…lTo r=smaug

This commit refactors the SetDomain method in a Document to call a new function
IsRegistrableDomainSuffixOfOrEqualTo(), defined in HTML [1]. This commit tries
not to rename anything except input variables, so as to remain as clear as
possible. It likely should have various variables renamed, but given the
author's unfamiliarity with this module, review seems a good time to do that.
It's also duplicating comments a little bit; let me know which one(s) you'd like
to keep!

Note: Commentary on the HTML change is available in the PR [2], and the
rationale for this behavior in Web Auentication, where this algorithm will be
used, is also recorded [3].

Update 1: Refactored two new protected methods to avoid code duplication.
Update 2: Bugfix: Be sure to use CreateInheritingURIForHost for the
provided domain so as to catch internationalized domains.
Update 3: Nit-fix and rebase

[1] https://html.spec.whatwg.org/multipage/browsers.html#is-a-registrable-domain-suffix-of-or-is-equal-to
[2] whatwg/html#2365
[3] w3ctag/design-reviews#97 (comment)

MozReview-Commit-ID: 4Dr8yOMdhez

--HG--
extra : rebase_source : 634bd3c7b60c7ad996ee38ab7314071b426e3f6f
Manishearth pushed a commit to Manishearth/gecko-dev that referenced this issue Mar 28, 2017
…lTo r=smaug

This commit refactors the SetDomain method in a Document to call a new function
IsRegistrableDomainSuffixOfOrEqualTo(), defined in HTML [1]. This commit tries
not to rename anything except input variables, so as to remain as clear as
possible. It likely should have various variables renamed, but given the
author's unfamiliarity with this module, review seems a good time to do that.
It's also duplicating comments a little bit; let me know which one(s) you'd like
to keep!

Note: Commentary on the HTML change is available in the PR [2], and the
rationale for this behavior in Web Auentication, where this algorithm will be
used, is also recorded [3].

Update 1: Refactored two new protected methods to avoid code duplication.
Update 2: Bugfix: Be sure to use CreateInheritingURIForHost for the
provided domain so as to catch internationalized domains.
Update 3: Nit-fix and rebase

[1] https://html.spec.whatwg.org/multipage/browsers.html#is-a-registrable-domain-suffix-of-or-is-equal-to
[2] whatwg/html#2365
[3] w3ctag/design-reviews#97 (comment)

MozReview-Commit-ID: 4Dr8yOMdhez
Manishearth pushed a commit to Manishearth/gecko-dev that referenced this issue Mar 28, 2017
…lTo r=smaug

This commit refactors the SetDomain method in a Document to call a new function
IsRegistrableDomainSuffixOfOrEqualTo(), defined in HTML [1]. This commit tries
not to rename anything except input variables, so as to remain as clear as
possible. It likely should have various variables renamed, but given the
author's unfamiliarity with this module, review seems a good time to do that.
It's also duplicating comments a little bit; let me know which one(s) you'd like
to keep!

Note: Commentary on the HTML change is available in the PR [2], and the
rationale for this behavior in Web Auentication, where this algorithm will be
used, is also recorded [3].

Updates: Refactored two new protected methods to avoid code duplication.

[1] https://html.spec.whatwg.org/multipage/browsers.html#is-a-registrable-domain-suffix-of-or-is-equal-to
[2] whatwg/html#2365
[3] w3ctag/design-reviews#97 (comment)

MozReview-Commit-ID: 4Dr8yOMdhez
@equalsJeffH
Copy link

webauthn and credential management drafts were harmonized upon (essentially):

w3c/webauthn#384
and
w3c/webappsec-credential-management#73
and
w3c/webappsec-credential-management#72

gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 1, 2019
…lTo r=smaug

This commit refactors the SetDomain method in a Document to call a new function
IsRegistrableDomainSuffixOfOrEqualTo(), defined in HTML [1]. This commit tries
not to rename anything except input variables, so as to remain as clear as
possible. It likely should have various variables renamed, but given the
author's unfamiliarity with this module, review seems a good time to do that.
It's also duplicating comments a little bit; let me know which one(s) you'd like
to keep!

Note: Commentary on the HTML change is available in the PR [2], and the
rationale for this behavior in Web Auentication, where this algorithm will be
used, is also recorded [3].

Updates: Refactored two new protected methods to avoid code duplication.

[1] https://html.spec.whatwg.org/multipage/browsers.html#is-a-registrable-domain-suffix-of-or-is-equal-to
[2] whatwg/html#2365
[3] w3ctag/design-reviews#97 (comment)

MozReview-Commit-ID: 4Dr8yOMdhez

UltraBlame original commit: ffa2f50d49ce0356bbf02ea0940060283dcc007c
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 1, 2019
…lTo r=smaug

This commit refactors the SetDomain method in a Document to call a new function
IsRegistrableDomainSuffixOfOrEqualTo(), defined in HTML [1]. This commit tries
not to rename anything except input variables, so as to remain as clear as
possible. It likely should have various variables renamed, but given the
author's unfamiliarity with this module, review seems a good time to do that.
It's also duplicating comments a little bit; let me know which one(s) you'd like
to keep!

Note: Commentary on the HTML change is available in the PR [2], and the
rationale for this behavior in Web Auentication, where this algorithm will be
used, is also recorded [3].

Update 1: Refactored two new protected methods to avoid code duplication.
Update 2: Bugfix: Be sure to use CreateInheritingURIForHost for the
provided domain so as to catch internationalized domains.
Update 3: Nit-fix and rebase

[1] https://html.spec.whatwg.org/multipage/browsers.html#is-a-registrable-domain-suffix-of-or-is-equal-to
[2] whatwg/html#2365
[3] w3ctag/design-reviews#97 (comment)

MozReview-Commit-ID: 4Dr8yOMdhez

UltraBlame original commit: 1147dd18f1d91034757ea50a01281fae6e43eda6
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 1, 2019
…lTo r=smaug

This commit refactors the SetDomain method in a Document to call a new function
IsRegistrableDomainSuffixOfOrEqualTo(), defined in HTML [1]. This commit tries
not to rename anything except input variables, so as to remain as clear as
possible. It likely should have various variables renamed, but given the
author's unfamiliarity with this module, review seems a good time to do that.
It's also duplicating comments a little bit; let me know which one(s) you'd like
to keep!

Note: Commentary on the HTML change is available in the PR [2], and the
rationale for this behavior in Web Auentication, where this algorithm will be
used, is also recorded [3].

Updates: Refactored two new protected methods to avoid code duplication.

[1] https://html.spec.whatwg.org/multipage/browsers.html#is-a-registrable-domain-suffix-of-or-is-equal-to
[2] whatwg/html#2365
[3] w3ctag/design-reviews#97 (comment)

MozReview-Commit-ID: 4Dr8yOMdhez

UltraBlame original commit: ffa2f50d49ce0356bbf02ea0940060283dcc007c
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 1, 2019
…lTo r=smaug

This commit refactors the SetDomain method in a Document to call a new function
IsRegistrableDomainSuffixOfOrEqualTo(), defined in HTML [1]. This commit tries
not to rename anything except input variables, so as to remain as clear as
possible. It likely should have various variables renamed, but given the
author's unfamiliarity with this module, review seems a good time to do that.
It's also duplicating comments a little bit; let me know which one(s) you'd like
to keep!

Note: Commentary on the HTML change is available in the PR [2], and the
rationale for this behavior in Web Auentication, where this algorithm will be
used, is also recorded [3].

Update 1: Refactored two new protected methods to avoid code duplication.
Update 2: Bugfix: Be sure to use CreateInheritingURIForHost for the
provided domain so as to catch internationalized domains.
Update 3: Nit-fix and rebase

[1] https://html.spec.whatwg.org/multipage/browsers.html#is-a-registrable-domain-suffix-of-or-is-equal-to
[2] whatwg/html#2365
[3] w3ctag/design-reviews#97 (comment)

MozReview-Commit-ID: 4Dr8yOMdhez

UltraBlame original commit: 1147dd18f1d91034757ea50a01281fae6e43eda6
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 1, 2019
…lTo r=smaug

This commit refactors the SetDomain method in a Document to call a new function
IsRegistrableDomainSuffixOfOrEqualTo(), defined in HTML [1]. This commit tries
not to rename anything except input variables, so as to remain as clear as
possible. It likely should have various variables renamed, but given the
author's unfamiliarity with this module, review seems a good time to do that.
It's also duplicating comments a little bit; let me know which one(s) you'd like
to keep!

Note: Commentary on the HTML change is available in the PR [2], and the
rationale for this behavior in Web Auentication, where this algorithm will be
used, is also recorded [3].

Updates: Refactored two new protected methods to avoid code duplication.

[1] https://html.spec.whatwg.org/multipage/browsers.html#is-a-registrable-domain-suffix-of-or-is-equal-to
[2] whatwg/html#2365
[3] w3ctag/design-reviews#97 (comment)

MozReview-Commit-ID: 4Dr8yOMdhez

UltraBlame original commit: ffa2f50d49ce0356bbf02ea0940060283dcc007c
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 1, 2019
…lTo r=smaug

This commit refactors the SetDomain method in a Document to call a new function
IsRegistrableDomainSuffixOfOrEqualTo(), defined in HTML [1]. This commit tries
not to rename anything except input variables, so as to remain as clear as
possible. It likely should have various variables renamed, but given the
author's unfamiliarity with this module, review seems a good time to do that.
It's also duplicating comments a little bit; let me know which one(s) you'd like
to keep!

Note: Commentary on the HTML change is available in the PR [2], and the
rationale for this behavior in Web Auentication, where this algorithm will be
used, is also recorded [3].

Update 1: Refactored two new protected methods to avoid code duplication.
Update 2: Bugfix: Be sure to use CreateInheritingURIForHost for the
provided domain so as to catch internationalized domains.
Update 3: Nit-fix and rebase

[1] https://html.spec.whatwg.org/multipage/browsers.html#is-a-registrable-domain-suffix-of-or-is-equal-to
[2] whatwg/html#2365
[3] w3ctag/design-reviews#97 (comment)

MozReview-Commit-ID: 4Dr8yOMdhez

UltraBlame original commit: 1147dd18f1d91034757ea50a01281fae6e43eda6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests