Allow callers to explicitly specify RP ID #198

vijaybh · 2016-09-14T06:25:04Z

Alternative approach to #162 suggested by @balfanz. This change also gets rid of references to PSL and eTLD+1, replacing them with references to the HTML spec.

…plicit-rpid

@balfanz

Alternative to #162 as suggested by @balfanz. This change also gets rid of references to PSL and eTLD+1, replacing them with references to the HTML specification instead.

equalsJeffH

overall this looks fine, tho there are some details to fix up in terms of how it interfaces with the HTML spec.

equalsJeffH · 2016-09-16T00:14:56Z

index.bs

@@ -130,8 +130,8 @@ or a combination of both.

 This specification relies on several other underlying specifications.

-: HTML5
-:: The concept of <dfn for='web'>origin</dfn> and the <dfn>Navigator</dfn> interface are defined in [[!HTML5]].
+: HTML 5.1


I would s/HTML 5.1/HTML/ here since the citation in the next line is clearly to HTML51.

equalsJeffH · 2016-09-16T00:15:36Z

index.bs

@@ -222,7 +218,8 @@ NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED",  "MAY", and

 : <dfn>Relying Party Identifier</dfn>
 : <dfn>RP ID</dfn>
-:: A Relying Party Identifier is derived from a <a>[RP]</a>'s web origin's hostname by computing the hostname's <a>eTLD+1</a>. 
+:: A Relying Party Identifier defines the scope of a given credential, i.e. the set of web origins that the client will permit


s/web origins/origins/

equalsJeffH · 2016-09-16T00:18:28Z

index.bs

-    "public suffix + 1" or "PS+1" (which is also referred to as the "Effective Top-Level Domain plus One" or "<a>eTLD+1</a>")
-    part of |callerOrigin| [[PSL]]. Let |rpId| be the lowercase form of this RP ID. Set |rpIdHash| to the SHA-256 hash of the
-    UTF-8 encoding of |rpId|.
+3. Set |callerOrigin| to the <a link-for='web'>origin</a> of the caller. If {{CredentialOptions/rpId}} is not specified, then


fyi, the fix I'm working on for #171 & #172 alters the first sentence, but not the remainder of this step, i.e., it ought to mesh ok (I hope).

equalsJeffH · 2016-09-16T00:26:26Z

index.bs

-    part of |callerOrigin| [[PSL]]. Let |rpId| be the lowercase form of this RP ID. Set |rpIdHash| to the SHA-256 hash of the
-    UTF-8 encoding of |rpId|.
+3. Set |callerOrigin| to the <a link-for='web'>origin</a> of the caller. If {{CredentialOptions/rpId}} is not specified, then
+    set |rpId| to the Unicode serialization of |callerOrigin| as specified in [[!HTML51-20160621]] section 6.4. If


I do not think we want/need to invoke the unicode serialization here. The |rpId| will already be in asciiDomain format and we shouldn't alter. Anne confirmed in the comments on #178 that origin.host and origin.domain are asciiDomains, by default, if non-null.

equalsJeffH · 2016-09-16T00:53:51Z

index.bs

+    `domain` by running the algorithm in [[!HTML51-20160621]] section 6.4.1 (but do not change the current document's `domain`).
+    If it is not permissible, reject |promise| with a <a>DOMException</a> whose name is "SecurityError", and terminate this
+    algorithm. If it is permissible, then set |rpId| to the Unicode serialization of {{CredentialOptions/rpId}}. Set |rpIdHash|
+    to the SHA-256 hash of the UTF-8 encoding of |rpId|.


I think we want to write the above set of lines more like so:

If {{CredentialOptions/rpId}} is specified, then invoke the relaxing the same-origin restriction "setting" algorithm using {{CredentialOptions/rpId}} as the given value without setting origin's |domain|. If no errors are thrown, then set |rpId| to the value of |host| as yielded by the algorithm. Set |rpIdHash| to the SHA-256 hash of |rpId|.

given that the internal default representation of origin.host and origin.domain is asciiDomain ( U-labels have been converted to A-labels (see https://tools.ietf.org/html/rfc5890#section-2.3.2 and foregoing figure 1 on pg 10 ) ) format, we should not be stipulating UTF-8 encoding of them, AFAICT.

The jeffh-origin-et-al branch will soon have the <pre class="anchors"> anchors set up to properly ref these things in HTML51 and so once both branches are eventually merged we should be good.

equalsJeffH · 2016-09-16T00:57:48Z

index.bs

@@ -564,6 +568,9 @@ authorizing an authenticator with which to complete the operation.

    - The <dfn>timeoutSeconds</dfn> parameter specifies a time, in seconds, that the caller is willing to wait for the call to
        complete. This is treated as a hint, and may be overridden by the platform.
+
+    - The <dfn>rpId</dfn> parameter explicitly specifies the RP ID that the credential should be associated with. If it is
+        omitted, the RP ID will be set to the caller's origin.


i suspect that the term "caller's origin" will not pass muster here -- will try to figure out more precise term for where we say "caller" or "caller's".

fyi/fwiw, i did nose around in the HTML spec(s) and they also use the term "caller" (in terms of the caller of an algorithm/operation), so I think/hope we're ok with using it in a similar fashion.

equalsJeffH · 2016-09-16T00:58:49Z

index.bs

@@ -620,6 +628,9 @@ user consent to a specific transaction. The structure of these signatures is def

    - The optional <dfn>timeoutSeconds</dfn> parameter specifies a time, in seconds, that the caller is willing to wait for the
        call to complete. This is treated as a hint, and may be overridden by the platform.
+
+    - The optional <dfn>rpId</dfn> parameter specifies the rpId claimed by the caller. If it is omitted, it will be assumed to
+        be equal to the caller's origin.


see comment above wrt "caller" et al.

Also fixed.

Since rpId is now equal to origin except when explicitly specified by the caller, and it is folded into the authenticatorData, having it in ClientData adds no value. Fixes #189

…plicit-rpid

equalsJeffH · 2016-09-16T22:08:17Z

index.bs

+    `domain` by running the algorithm in [[!HTML51-20160621]] section 6.4.1 (but do not change the current document's `domain`).
+    If it is not permissible, reject |promise| with a <a>DOMException</a> whose name is "SecurityError", and terminate this
+    algorithm. If it is permissible, then set |rpId| to the Unicode serialization of {{CredentialOptions/rpId}}. Set |rpIdHash|
+    to the SHA-256 hash of the UTF-8 encoding of |rpId|.


similar comments to the above chunk of text as I had on the foregoing.

Tweaked the text a little bit to hopefully make it better.

equalsJeffH · 2016-09-17T00:39:23Z

index.bs

@@ -222,7 +218,8 @@ NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED",  "MAY", and

 : <dfn>Relying Party Identifier</dfn>
 : <dfn>RP ID</dfn>
-:: A Relying Party Identifier is derived from a <a>[RP]</a>'s web origin's hostname by computing the hostname's <a>eTLD+1</a>. 
+:: A Relying Party Identifier defines the scope of a given credential, i.e. the set of web origins that the client will permit
+    to access that credential. It is derived from a <a>[RP]</a>'s web origin's hostname or directly specified by the [RP]. 


s/access/wield/

suggest revise last sentence to be:

It a [RP]'s origin's |host| component, or is explicitly specified by the [RP] to be either its origin's |host| component or the |host| component with subdomains removed such that it continues to pass the checks in relaxing the same-origin restriction algorithm. See step 3 of both {{#makeCredential}} and {{#getAssertion}}.

I rewrote the definition - let me know if this is better.

equalsJeffH · 2016-09-17T00:43:28Z

index.bs

+3. Set |callerOrigin| to the <a link-for='web'>origin</a> of the caller. If {{CredentialOptions/rpId}} is not specified, then
+    set |rpId| to the Unicode serialization of |callerOrigin| as specified in [[!HTML51-20160621]] section 6.4. If
+    {{CredentialOptions/rpId}} is specified, then check if its value would be an acceptable setting for the current document's
+    `domain` by running the algorithm in [[!HTML51-20160621]] section 6.4.1 (but do not change the current document's `domain`).


fyi/fwiw, the linking style used in PR #202 allows direct linking to things like that algorithm such that we do not need to mention section #s. also, i suggest citing HTML51 as simply [[!HTML51]].

balfanz · 2016-09-20T08:18:42Z

index.bs

@@ -553,6 +554,7 @@ authorizing an authenticator with which to complete the operation.
 <pre class="idl">
    dictionary CredentialOptions {
        unsigned long                       timeoutSeconds;
+        USVString                           rpId;


Why USVString here? See http://heycam.github.io/webidl/#idl-USVString

This RP ID will be sent to the authenticator "over the wire" when authenticatorMakeCredential is invoked. That seems to fit the USVString use case in the above guidance, though I would welcome expert guidance here.

balfanz · 2016-09-20T08:19:05Z

index.bs

@@ -610,6 +615,7 @@ user consent to a specific transaction. The structure of these signatures is def
 <pre class="idl">
    dictionary AssertionOptions {
        unsigned long                      timeoutSeconds;
+        USVString                          rpId;


Why USVString here? See http://heycam.github.io/webidl/#idl-USVString

See above comment regarding same issue in CredentialOptions.

balfanz · 2016-09-20T08:20:04Z

index.bs

@@ -691,7 +700,6 @@ string-valued keys. Values may be any type that has a valid encoding in JSON. It
    dictionary ClientData {
        required DOMString           challenge;
        required DOMString           origin;
-        required DOMString           rpId;


How come you're removing it from the ClientData?

See #189. The rpId is being passed in the options so it's not clear why it should also be in the client data. Also the RP ID now gets hashed into the signature as well so it seems redundant to have this here.

equalsJeffH · 2016-09-26T22:07:16Z

I've reviewed portions of the latest commits and see some issues that ought to be cleaned up. I may be able to comment more tonight, and if not, I will tomorrow morning.

equalsJeffH · 2016-09-27T19:55:13Z

index.bs

+    <a>relaxing the same-origin restriction</a> by setting the `document.domain` attribute, using
+    {{ScopedCredentialOptions/rpId}} as the given value but without changing the current document's `domain`. If any errors are
+    thrown, reject |promise| with a <a>DOMException</a> whose name is "SecurityError", and terminate this algorithm. If no
+    errors are thrown, set |rpId| to the value of `host` as computed by the algorithm. Set |rpIdHash| to the SHA-256 hash of the


s/the algorithm/the procedure/

Thanks. I also rearranged the sentences so it reads a bit better (avoid switching back and forth between "this algorithm" and "that procedure".

equalsJeffH · 2016-09-27T19:56:23Z

index.bs

+    <a>relaxing the same-origin restriction</a> by setting the `document.domain` attribute, using
+    {{ScopedCredentialOptions/rpId}} as the given value but without changing the current document's `domain`. If any errors are
+    thrown, reject |promise| with a <a>DOMException</a> whose name is "SecurityError", and terminate this algorithm. If no
+    errors are thrown, set |rpId| to the value of `host` as computed by the algorithm. Set |rpIdHash| to the SHA-256 hash of the
    UTF-8 encoding of |rpId|.


AFAIU, we do not need to UTF-8 encode rpId. It will be in "asciiDomain" format and we ought to simply hash that, unless there's something I"m missing....

You're right, but I wonder if we should be Unicode serializing the RP ID here since it will go to the authenticator which may want to display it. However we can discuss that separately.

equalsJeffH · 2016-09-27T19:58:09Z

index.bs

+    <a>relaxing the same-origin restriction</a> by setting the `document.domain` attribute, using
+    {{ScopedCredentialOptions/rpId}} as the given value but without changing the current document's `domain`. If any errors are
+    thrown, reject |promise| with a <a>DOMException</a> whose name is "SecurityError", and terminate this algorithm. If no
+    errors are thrown, set |rpId| to the value of `host` as computed by the algorithm. Set |rpIdHash| to the SHA-256 hash of the


s/the algorithm/the procedure/

equalsJeffH · 2016-09-27T19:58:22Z

index.bs

+    <a>relaxing the same-origin restriction</a> by setting the `document.domain` attribute, using
+    {{ScopedCredentialOptions/rpId}} as the given value but without changing the current document's `domain`. If any errors are
+    thrown, reject |promise| with a <a>DOMException</a> whose name is "SecurityError", and terminate this algorithm. If no
+    errors are thrown, set |rpId| to the value of `host` as computed by the algorithm. Set |rpIdHash| to the SHA-256 hash of the
    UTF-8 encoding of |rpId|.


AFAIU, we do not need to UTF-8 encode rpId. It will be in "asciiDomain" format and we ought to simply hash that, unless there's something I"m missing....

equalsJeffH · 2016-09-27T20:11:38Z

index.bs

@@ -564,6 +568,9 @@ authorizing an authenticator with which to complete the operation.

    - The <dfn>timeoutSeconds</dfn> parameter specifies a time, in seconds, that the caller is willing to wait for the call to
        complete. This is treated as a hint, and may be overridden by the platform.
+
+    - The <dfn>rpId</dfn> parameter explicitly specifies the RP ID that the credential should be associated with. If it is
+        omitted, the RP ID will be set to the caller's origin.


fyi/fwiw, i did nose around in the HTML spec(s) and they also use the term "caller" (in terms of the caller of an algorithm/operation), so I think/hope we're ok with using it in a similar fashion.

equalsJeffH

Looks good to me modulo these detail-level comments. The comments wrt not UTF-8 encoding prior to hashing rpId are the material comments.

Also, we should include "fixes #178" in a commit msg here prior to merge.

equalsJeffH · 2016-09-27T20:13:22Z

index.bs

@@ -1287,16 +1288,15 @@ Upon receiving an attestation statement in the form of a {{WebAuthnAttestation}}

 2. Verify that the {{ClientData/challenge}} in the {{ClientData}} matches the challenge that was sent to the authenticator.

-3. Verify that the {{ClientData/origin}} and {{ClientData/rpId}} in the {{ClientData}} match the origin and RP ID used by the
-    RP.
+3. Verify that the {{ClientData/origin}} in the {{ClientData}} matches the origin used by the RP.


s/origin used by the RP/[RP]'s origin/ ?

equalsJeffH · 2016-09-27T23:14:06Z

I did "approve changes" ahead of time so @vijaybh is otherwise unimpeded in finishing this up and merging.

equalsJeffH · 2016-09-27T23:29:31Z

Also, the current state of these changes (updated per my latest comments) may address @annevk's concerns expressed in #218 "relaxing the same-origin restriction", though we have not answered his query "why are you using this?"

Note this PR also fixes #178 by removing all use of the term lowercase

…#198)

vijaybh added 3 commits September 12, 2016 23:20

Merge remote-tracking branch 'refs/remotes/origin/master' into vgb-ex…

6a2458e

…plicit-rpid

Merge branch 'master' into vgb-explicit-rpid

6651690

Allow caller to explicitly specify its claimed RP ID

443482e

Alternative to #162 as suggested by @balfanz. This change also gets rid of references to PSL and eTLD+1, replacing them with references to the HTML specification instead.

vijaybh added this to the WD-02 milestone Sep 14, 2016

equalsJeffH suggested changes Sep 16, 2016

View reviewed changes

vijaybh added 2 commits September 16, 2016 02:04

Remove rpId from ClientData

a9c6b38

Since rpId is now equal to origin except when explicitly specified by the caller, and it is folded into the authenticatorData, having it in ClientData adds no value. Fixes #189

Merge remote-tracking branch 'refs/remotes/origin/master' into vgb-ex…

7cbf57a

…plicit-rpid

This was referenced Sep 16, 2016

Allow caller to pick between strict and eTLD+1 matching #162

Closed

Proper origin terminology, references, and setting of callerOrigin #202

Merged

equalsJeffH reviewed Sep 16, 2016

View reviewed changes

Incorporate feedback from @equalsJeffH

7943865

equalsJeffH reviewed Sep 17, 2016

View reviewed changes

balfanz reviewed Sep 20, 2016

View reviewed changes

equalsJeffH assigned vijaybh Sep 20, 2016

vijaybh added 2 commits September 20, 2016 07:03

Merge from master

782a6d5

Merge branch 'master' into vgb-explicit-rpid

a79f880

leshi added the type:technical label Sep 23, 2016

More feedback from @equalsJeffH

f44ba9e

equalsJeffH mentioned this pull request Sep 27, 2016

lowercase form of this RP ID #178

Closed

equalsJeffH reviewed Sep 27, 2016

View reviewed changes

equalsJeffH suggested changes Sep 27, 2016

View reviewed changes

equalsJeffH approved these changes Sep 27, 2016

View reviewed changes

Incorporate feedback from @equalsJeffH

f03e4e5

Note this PR also fixes #178 by removing all use of the term lowercase

vijaybh merged commit ca1f0ee into master Sep 28, 2016

vijaybh deleted the vgb-explicit-rpid branch September 28, 2016 06:37

WebAuthnBot pushed a commit that referenced this pull request Sep 28, 2016

Built by Travis-CI: ca1f0ee Allow callers to explicitly specify RP ID (…

3b00cdf

…#198)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow callers to explicitly specify RP ID #198

Allow callers to explicitly specify RP ID #198

vijaybh commented Sep 14, 2016

equalsJeffH left a comment

equalsJeffH Sep 16, 2016

equalsJeffH Sep 16, 2016 •

edited

Loading

equalsJeffH Sep 16, 2016

equalsJeffH Sep 16, 2016

equalsJeffH Sep 16, 2016

equalsJeffH Sep 16, 2016 •

edited

Loading

vijaybh Sep 23, 2016

equalsJeffH Sep 27, 2016

equalsJeffH Sep 16, 2016

vijaybh Sep 23, 2016

equalsJeffH Sep 16, 2016

vijaybh Sep 23, 2016

equalsJeffH Sep 17, 2016

vijaybh Sep 23, 2016

equalsJeffH Sep 17, 2016

balfanz Sep 20, 2016

vijaybh Sep 23, 2016

balfanz Sep 20, 2016

vijaybh Sep 23, 2016

balfanz Sep 20, 2016

vijaybh Sep 23, 2016

equalsJeffH commented Sep 26, 2016

equalsJeffH Sep 27, 2016

vijaybh Sep 28, 2016

equalsJeffH Sep 27, 2016

vijaybh Sep 28, 2016

equalsJeffH Sep 27, 2016

equalsJeffH Sep 27, 2016

equalsJeffH Sep 27, 2016

equalsJeffH left a comment

equalsJeffH Sep 27, 2016

equalsJeffH commented Sep 27, 2016

equalsJeffH commented Sep 27, 2016

Allow callers to explicitly specify RP ID #198

Allow callers to explicitly specify RP ID #198

Conversation

vijaybh commented Sep 14, 2016

equalsJeffH left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

equalsJeffH Sep 16, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

equalsJeffH Sep 16, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

equalsJeffH commented Sep 26, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

equalsJeffH left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

equalsJeffH commented Sep 27, 2016

equalsJeffH commented Sep 27, 2016

equalsJeffH Sep 16, 2016 •

edited

Loading

equalsJeffH Sep 16, 2016 •

edited

Loading