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

Please coordinate with the HTML spec to extract the relevant bits of the document.domain setter so you can call them #256

Closed
bzbarsky opened this issue Nov 4, 2016 · 9 comments

Comments

@bzbarsky
Copy link

bzbarsky commented Nov 4, 2016

Currently https://w3c.github.io/webauthn/#dom-webauthentication-makecredential step 3 says to do https://www.w3.org/TR/html51/browsers.html#relaxing-the-same-origin-restriction but not quite. Except it doesn't make it clear which document, and it's not clear whether you actually want the interaction with sandboxing that the document.domain setter has, whether you actually want the behavior to be affected by previous document.domain sets (as the actual document.domain setter is) and so forth.

All this stuff should be clearly defined, ideally by:

  1. Defining clearly which document you're working with.
  2. Working together with the HTML spec to extract the relevant algorithm for your use. If this proves impossible, because you actually want somewhat different behavior from the document.domain setter, then the algorithm needs to just be clearly defined.
@bzbarsky
Copy link
Author

bzbarsky commented Nov 4, 2016

This applies to https://w3c.github.io/webauthn/#getAssertion step 3 too.

@equalsJeffH equalsJeffH added this to the CR milestone Nov 6, 2016
@equalsJeffH
Copy link
Contributor

Yes, the present spec language for this is a stab-in-the-dark-with-the-hope-of-sparking-helpful-feedback, and it seems to have worked :)

In terms of (2), i.e. the relevant algorithm, it seems to me it is nominally steps 4 - 7 (inclusive) of the domain attribute setter defined hereabouts: https://html.spec.whatwg.org/#dom-document-domain

So, by "extract the algorithm" you mean to say "revise the HTML spec such that the those steps are defined as a distinct algorithm which we an then invoke from the webauthn spec (and which the document.domain setter also utilizes) ?

@bzbarsky
Copy link
Author

bzbarsky commented Nov 7, 2016

nominally steps 4 - 7 (inclusive) of the domain attribute setter defined hereabouts

OK. So if a page is loaded from "https://foo.bar.com" and then sets document.domain to "bar.com", should it be able to use "foo.bar.com" as its rpId? Sounds to me like you're saying it shouldn't be able to....

So, by "extract the algorithm" you mean to say

Yes. Again, assuming the desired behavior is similar enough that this makes sense.

jcjones added a commit to jcjones/webauthn that referenced this issue Dec 20, 2016
Fixes w3c#256, where the algorithm is incompletely specified via reference to
a procedure in HTML51 that isn't itself pulled out as an explicit algorithm.
jcjones added a commit to jcjones/webauthn that referenced this issue Dec 20, 2016
Fixes w3c#256, where the algorithm is incompletely specified via reference to
a procedure in HTML51 that isn't itself pulled out as an explicit algorithm.
jcjones added a commit to jcjones/webauthn that referenced this issue Dec 20, 2016
Issue w3c#256 notes that it is "not clear whether [we] actually want the
interaction with sandboxing that the document.domain setter has", nor
"whether [we] actually want the behavior to be affected by previous
`document.domain` sets".

This patch offers a way to fix that, by:

1) Extracting the procedure from HTML51 into a forked algorithm
2) Adjusting said algorithm to operate on a Document's "original Domain" so as
   to be independent of previous `document.domain` set operations.
3) Keeping the sandboxing interactions, though I'm not entirely versed in
   whether there are problematic corner cases here.

This spec necessarily adds normative reference to the PSL (which was
transitively referenced via normative reference from HTML51 before), and also
to the URL specification (also previously transitive from HTML51).
jcjones added a commit to jcjones/webauthn that referenced this issue Dec 20, 2016
Fixes w3c#256, where the algorithm is incompletely specified via reference to
a procedure in HTML51 that isn't itself pulled out as an explicit algorithm.
jcjones added a commit to jcjones/webauthn that referenced this issue Dec 20, 2016
Issue w3c#256 notes that it is "not clear whether [we] actually want the
interaction with sandboxing that the document.domain setter has", nor
"whether [we] actually want the behavior to be affected by previous
`document.domain` sets".

This patch offers a way to fix that, by:

1) Extracting the procedure from HTML51 into a forked algorithm
2) Adjusting said algorithm to operate on a Document's "original Domain" so as
   to be independent of previous `document.domain` set operations.
3) Keeping the sandboxing interactions, though I'm not entirely versed in
   whether there are problematic corner cases here.

This spec necessarily adds normative reference to the PSL (which was
transitively referenced via normative reference from HTML51 before), and also
to the URL specification (also previously transitive from HTML51).

- The original commit mistakenly excluded the change to specify operation on
the Document's "original Domain", so this is a fix-up to that commit.
@equalsJeffH equalsJeffH assigned jcjones and equalsJeffH and unassigned jcjones Dec 21, 2016
@equalsJeffH
Copy link
Contributor

equalsJeffH commented Dec 22, 2016

nominally steps 4 - 7 (inclusive) of the domain attribute setter defined hereabouts

OK. So if a page is loaded from "https://foo.bar.com" and then sets document.domain to "bar.com", should it be able to use "foo.bar.com" as its rpId? Sounds to me like you're saying it shouldn't be able to....

Our present intent (AIUI, and if it is correct) is that even if "https://foo.bar.com" is loaded and then sets document.domain to "bar.com", it would still be able to assert "foo.bar.com" as its rpId.

@bzbarsky
Copy link
Author

OK, then you need to somewhat modify the HTML algorithm, or extract parts of it, or something.

jcjones added a commit to jcjones/webauthn that referenced this issue Dec 27, 2016
Issue w3c#256 notes that it is "not clear whether [we] actually want the
interaction with sandboxing that the document.domain setter has", nor
"whether [we] actually want the behavior to be affected by previous
`document.domain` sets".

This patch offers a way to fix that, by:

1) Extracting the procedure from HTML51 into a forked algorithm
2) Adjusting said algorithm to operate on a Document's "original Domain" so as
   to be independent of previous `document.domain` set operations.
3) Keeping the sandboxing interactions, though I'm not entirely versed in
   whether there are problematic corner cases here.

This spec necessarily adds normative reference to the PSL (which was
transitively referenced via normative reference from HTML51 before), and also
to the URL specification (also previously transitive from HTML51).

- The original commit mistakenly excluded the change to specify operation on
the Document's "original Domain", so this is a fix-up to that commit.
jcjones added a commit to jcjones/webauthn that referenced this issue Dec 27, 2016
This commit addresses all the comments on Pull w3c#314 except that from JeffH[1]
about moving from section 5 to section 4.

I've attempted to reduce the number of anchors in use, per domenic, but
ended up having to add one for the WebIDL spec to link the "SecurityError"
DOMException.

[1] https://github.com/w3c/webauthn/pull/314/files#r93364196
jcjones added a commit to jcjones/webauthn that referenced this issue Dec 27, 2016
Per equalsJeffH's comment [1], this doesn't belong in Authenticator Model
but instead in the API section.

[1] https://github.com/w3c/webauthn/pull/314/files#r93364196
jcjones added a commit to jcjones/webauthn that referenced this issue Dec 27, 2016
- Replace <a>SecurityError</a> with {{SecurityError}}
- Remove unnecessary anchor
@equalsJeffH
Copy link
Contributor

equalsJeffH commented Dec 27, 2016

Ok, thx. We have a stab at such an extraction in this PR: #314. It has this crucial step:

  1. Let |originalHost| be this Document object’s origin’s host.

..which we hope is correct. See discussion with @domenic in PR #314.

jcjones added a commit to jcjones/webauthn that referenced this issue Dec 28, 2016
jcjones added a commit to jcjones/webauthn that referenced this issue Jan 3, 2017
Updates per bzbarsky's review at w3c#314 (review)

(Thanks again, bzbarsky!)
jcjones added a commit to jcjones/webauthn that referenced this issue Jan 6, 2017
…on" algorithm

This is an alternative to PR w3c#314 which inlines the "Relaxing the Same-Origin
Restriction" algorithm within our document. Instead, this clarifies:

1) That the setter should be called on the current Document
2) That the active sandboxing flag set should be empty when the algorithm runs
3) Thrown errors should be used in the promise, not occluded

Note: This version **permits** earlier set operations to `document.domain` to
affect the outcome of the algorithm, in contrast to PR w3c#314 (at this time). This
is mostly because the language is more natural here, but either PR could
be changed to honor earlier set operations -- or not.
@jwatt
Copy link

jwatt commented Jan 11, 2017

Why is document.domain being used in WebAuthn? To quote Bobby Holley in https://bugzilla.mozilla.org/show_bug.cgi?id=1329764#c2

We should absolutely not be building any support for document.domain (or any analogous machinery) into new specs. It mostly breaks the security model of the web, and vendors have gone to great lengths to reduce document.domain support to the bare minimum required for web-compat.

It seems like WebAuthn is one spec where we should have the APIs throw if document.domain has been set to push websites towards using pushMessage instead. That's particularly the case since it's the big sites like FB that we need to push away from document.domain in order to deprecate document.domain, and they're likely to want to have WebAuthn support.

@bzbarsky
Copy link
Author

This isn't about document.domain per se. It's about a similar "hey, pretend like I'm a super-domain of my actual domain" setup in a slightly different context. The idea is that foo.bar.com can generate auth tokens for bar.com, if I understand correctly.

jcjones added a commit to jcjones/webauthn that referenced this issue Feb 22, 2017
jcjones added a commit to jcjones/webauthn that referenced this issue Feb 22, 2017
…" algorithm

- Rebased on top of PR 336's linking changes
- Updates from review by jyasskin and equalsJeffH
- Catches a couple extra un-linked DOMExceptions
- Refers to the new "is a registrable domain suffix of or is equal to" algorithm
  in WHATWG HTML [1]
  - Note, there is still a linking error after a bikeshed update, but I'm
    guessing that change is so new that's not in the bikeshed metadata yet, so
    I'm going to leave it for now and hope it fixes itself.

[1] https://html.spec.whatwg.org/multipage/browsers.html#is-a-registrable-domain-suffix-of-or-is-equal-to
jcjones added a commit to jcjones/webauthn that referenced this issue Feb 22, 2017
…" algorithm

- Rebased on top of PR 336's linking changes
- Updates from review by jyasskin and equalsJeffH
- Catches a couple extra un-linked DOMExceptions
- Refers to the new "is a registrable domain suffix of or is equal to" algorithm
  in WHATWG HTML [1]
  - Note, there is still a linking error after a bikeshed update, but I'm
    guessing that change is so new that's not in the bikeshed metadata yet, so
    I'm going to leave it for now and hope it fixes itself.

[1] https://html.spec.whatwg.org/multipage/browsers.html#is-a-registrable-domain-suffix-of-or-is-equal-to
jcjones added a commit that referenced this issue Feb 23, 2017
…ithm

Bug #256 - Clarify call of the "Relaxing the Same-Origin Restriction" algorithm
@equalsJeffH
Copy link
Contributor

this was fixed by PR #319 (but not auto-closed ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants