Re: [whatwg/fetch] Update Fetch to support Token Binding. (#325)

annevk commented on this pull request.

It seems like there's still quite a few things to sort out. @mikewest should probably review this too since it touches upon some cookie-related wording. (I also noticed that this introduces new headers that do not use the JSON syntax for their values...)

If you happen to be at TPAC next week let me know and I'd be happy to go through it in person if that makes things easier.

> +A <dfn title=concept-token-binding>Token Binding</dfn> is established by the user agent generating
+a public-private key-pair for the server and proving possession of the private key on every request
+to the server, as described in
+<a href="https://tools.ietf.org/id/draft-ietf-tokbind-protocol-10.xml#rfc.section.1">section 1</a>
+of the Token Binding Protocol spec.
+A <dfn title=concept-token-binding-id>Token Binding Id</dfn> is the representation of a
+<span title=concept-token-binding>Token Binding</span>, as described in
+<a href="https://tools.ietf.org/id/draft-ietf-tokbind-protocol-10.xml#rfc.section.3.2">section 3.2</a>
+of the Token Binding Protocol spec.
+
+A <dfn title=concept-token-binding-message>Token Binding Message</dfn> is transmitted by
+a user agent to a server to prove possession of
+<span title=concept-token-binding>Token Binding</span> keys.
+The value of a <span title=concept-token-binding-message>Token Binding Message</span> is specified
+in <a href="https://tools.ietf.org/id/draft-ietf-tokbind-protocol-10.xml#rfc.section.3">section 3</a>
+of the Token Binding Protocol spec.

The way I was thinking of this was that we could add a subsection to Infrastructure, just like "Client hints list" is a subsection.

> @@ -949,6 +971,34 @@ <h4 id=terminology-headers>Headers</h4>
 Unless stated otherwise, it is unset.
 
 <p>A <span title=concept-request>request</span> has an associated
+<dfn title=concept-request-use-token-binding>use-token-binding flag</dfn>.
+Unless stated otherwise, it is unset.
+
+<p class="note no-backref"> <span title=concept-request>Request</span>'s
+<span title=concept-request-use-token-binding>use-token-binding</span> flag indicates whether the

Flag should be inside the `</span>`. That seems to be frequently missed.

> @@ -949,6 +971,34 @@ <h4 id=terminology-headers>Headers</h4>
 Unless stated otherwise, it is unset.
 
 <p>A <span title=concept-request>request</span> has an associated
+<dfn title=concept-request-use-token-binding>use-token-binding flag</dfn>.
+Unless stated otherwise, it is unset.
+
+<p class="note no-backref"> <span title=concept-request>Request</span>'s

There should be no space after `<p>`.

> @@ -160,6 +160,28 @@ <h2 class="short">Conformance</h2>
 
 <hr>
 
+<p>
+The technique of Token Binding allows user agents to work with servers to protect credentials
+like HTTP cookies.
+A <dfn title=concept-token-binding>Token Binding</dfn> is established by the user agent generating
+a public-private key-pair for the server and proving possession of the private key on every request
+to the server, as described in
+<a href="https://tools.ietf.org/id/draft-ietf-tokbind-protocol-10.xml#rfc.section.1">section 1</a>
+of the Token Binding Protocol spec.
+A <dfn title=concept-token-binding-id>Token Binding Id</dfn> is the representation of a

Is this meant to be a new paragraph?

> @@ -160,6 +160,28 @@ <h2 class="short">Conformance</h2>
 
 <hr>
 
+<p>
+The technique of Token Binding allows user agents to work with servers to protect credentials
+like HTTP cookies.
+A <dfn title=concept-token-binding>Token Binding</dfn> is established by the user agent generating
+a public-private key-pair for the server and proving possession of the private key on every request
+to the server, as described in
+<a href="https://tools.ietf.org/id/draft-ietf-tokbind-protocol-10.xml#rfc.section.1">section 1</a>
+of the Token Binding Protocol spec.
+A <dfn title=concept-token-binding-id>Token Binding Id</dfn> is the representation of a
+<span title=concept-token-binding>Token Binding</span>, as described in
+<a href="https://tools.ietf.org/id/draft-ietf-tokbind-protocol-10.xml#rfc.section.3.2">section 3.2</a>
+of the Token Binding Protocol spec.
+
+A <dfn title=concept-token-binding-message>Token Binding Message</dfn> is transmitted by

A paragraph requires a `<p>`.

> @@ -160,6 +160,28 @@ <h2 class="short">Conformance</h2>
 
 <hr>
 
+<p>

There's no need for a newline here.

> @@ -160,6 +160,28 @@ <h2 class="short">Conformance</h2>
 
 <hr>
 
+<p>
+The technique of Token Binding allows user agents to work with servers to protect credentials
+like HTTP cookies.
+A <dfn title=concept-token-binding>Token Binding</dfn> is established by the user agent generating

Did you mean to start a new paragraph here?

> +<p>A <span title=concept-request>request</span> has an associated
+<dfn title=concept-request-use-referred-token-binding>use-referred-token-binding flag</dfn>.
+Unless stated otherwise, it is unset.
+
+<p class="note no-backref"> <span title=concept-request>Request</span>'s
+<span title=concept-request-use-referred-token-binding>use-referred-token-binding</span>
+flag controls whether the user agent will send the
+<span title=concept-token-binding-id>Token Binding Id</span>
+used by the user agent with the referring <span title=concept-request-origin>origin</span>,
+in addition to the <span title=concept-token-binding-id>Token Binding Id</span> used by the
+user agent with the <span title=concept-request-origin>origin</span>, when it transmits the
+<span title=concept-request>request</span> to the server.
+
+<p>A <span title=concept-request>request</span> has an associated
+<dfn title=concept-request-referred-token-binding-origin>referred-token-binding-origin</dfn>,
+which is an <span title=concept-request-origin>origin</span>. Unless stated otherwise, it is unset.

This should take either null or an origin with null as the default. Only flags can be unset.

> @@ -1363,6 +1413,10 @@ <h4 id=terminology-headers>Headers</h4>
   <p>Let <var>connection</var> be the result of establishing an HTTP connection to
   <var>origin</var>. <span data-anolis-ref>HTTP</span> <span data-anolis-ref>TLS</span>
 
+  <p>If <var>credentials</var> is true and if the user agent supports
+  <span title=concept-token-binding>Token Binding</span>, propose the use of Token Binding for the
+  TLS connection.

What does "propose" mean here?

> @@ -3026,6 +3080,45 @@ <h4 id="should-response-to-request-be-blocked-due-to-nosniff?"><dfn title="shoul
  to `<code title>GET</code>` and <var>request</var>'s <span title=concept-request-body>body</span>
  to null.
 
+ <li><p>Let <var>useReferredTokenBinding</var> be the result of

Since this `<li>` contains multiple block-level children it should be followed by a newline. https://github.com/whatwg/fetch/blob/master/README.md has formatting guidelines that should help.

> @@ -3026,6 +3080,45 @@ <h4 id="should-response-to-request-be-blocked-due-to-nosniff?"><dfn title="shoul
  to `<code title>GET</code>` and <var>request</var>'s <span title=concept-request-body>body</span>
  to null.
 
+ <li><p>Let <var>useReferredTokenBinding</var> be the result of
+   <span title=concept-header-parse>parsing</span>
+   `<code title>Include-Referred-Token-Binding-ID</code>` in <var>actualResponse</var>'s
+   <span title=concept-response-header-list>header list</span>.
+   The value of this <span title=concept-header>header</span> is specified in
+   <a href="https://tools.ietf.org/id/draft-ietf-tokbind-https-05.xml#rfc.section.3.3">section 3.3</a>
+   of the Token Binding over HTTP spec.

Is there some kind of latest draft link we could use or will you update Fetch each time a new draft is issued?

> + <li><p>Let <var>useReferredTokenBinding</var> be the result of
+   <span title=concept-header-parse>parsing</span>
+   `<code title>Include-Referred-Token-Binding-ID</code>` in <var>actualResponse</var>'s
+   <span title=concept-response-header-list>header list</span>.
+   The value of this <span title=concept-header>header</span> is specified in
+   <a href="https://tools.ietf.org/id/draft-ietf-tokbind-https-05.xml#rfc.section.3.3">section 3.3</a>
+   of the Token Binding over HTTP spec.
+   <p class="note no-backref">By setting the <code title>Include-Referred-Token-Binding-ID</code>
+   <span title=concept-header>header</span> to "<code title>true</code>",
+   the <span title=concept-request-origin>origin</span> that sends the
+   redirect <span title=concept-response>response</span> tells the user agent to disclose the
+   <span title=concept-token-binding-id>Token Binding Id</span> used by the user agent with that
+   <span title=concept-request-origin>origin</span> to the target origin.
+
+   <li><p>Unset the <var>request</var>'s
+     <span title=concept-request-use-referred-token-binding>use-referred-token-binding</span> flag.

flag should be before `</span>`.

> +   <span title=concept-header-parse>parsing</span>
+   `<code title>Include-Referred-Token-Binding-ID</code>` in <var>actualResponse</var>'s
+   <span title=concept-response-header-list>header list</span>.
+   The value of this <span title=concept-header>header</span> is specified in
+   <a href="https://tools.ietf.org/id/draft-ietf-tokbind-https-05.xml#rfc.section.3.3">section 3.3</a>
+   of the Token Binding over HTTP spec.
+   <p class="note no-backref">By setting the <code title>Include-Referred-Token-Binding-ID</code>
+   <span title=concept-header>header</span> to "<code title>true</code>",
+   the <span title=concept-request-origin>origin</span> that sends the
+   redirect <span title=concept-response>response</span> tells the user agent to disclose the
+   <span title=concept-token-binding-id>Token Binding Id</span> used by the user agent with that
+   <span title=concept-request-origin>origin</span> to the target origin.
+
+   <li><p>Unset the <var>request</var>'s
+     <span title=concept-request-use-referred-token-binding>use-referred-token-binding</span> flag.
+   <li><p>If <var>useReferredTokenBinding</var> is "<code title>true</code>",

Result of header value parsing cannot be a string. Perhaps you meant to use ` rather than "? Also, is this a case-sensitive match? Is that how the RFC defines it?

> +   <span title=concept-response-header-list>header list</span>.
+   The value of this <span title=concept-header>header</span> is specified in
+   <a href="https://tools.ietf.org/id/draft-ietf-tokbind-https-05.xml#rfc.section.3.3">section 3.3</a>
+   of the Token Binding over HTTP spec.
+   <p class="note no-backref">By setting the <code title>Include-Referred-Token-Binding-ID</code>
+   <span title=concept-header>header</span> to "<code title>true</code>",
+   the <span title=concept-request-origin>origin</span> that sends the
+   redirect <span title=concept-response>response</span> tells the user agent to disclose the
+   <span title=concept-token-binding-id>Token Binding Id</span> used by the user agent with that
+   <span title=concept-request-origin>origin</span> to the target origin.
+
+   <li><p>Unset the <var>request</var>'s
+     <span title=concept-request-use-referred-token-binding>use-referred-token-binding</span> flag.
+   <li><p>If <var>useReferredTokenBinding</var> is "<code title>true</code>",
+     and the <var>request</var>'s
+     <span title=concept-request-use-token-binding>use-token-binding</span> flag is set,

`... flag</span>`

> +   the <span title=concept-request-origin>origin</span> that sends the
+   redirect <span title=concept-response>response</span> tells the user agent to disclose the
+   <span title=concept-token-binding-id>Token Binding Id</span> used by the user agent with that
+   <span title=concept-request-origin>origin</span> to the target origin.
+
+   <li><p>Unset the <var>request</var>'s
+     <span title=concept-request-use-referred-token-binding>use-referred-token-binding</span> flag.
+   <li><p>If <var>useReferredTokenBinding</var> is "<code title>true</code>",
+     and the <var>request</var>'s
+     <span title=concept-request-use-token-binding>use-token-binding</span> flag is set,
+     then perform the following substeps:
+
+   <ol>
+     <li><p>Set the <var>request</var>'s
+       <span title=concept-request-use-referred-token-binding>use-referred-token-binding</span>
+       flag.

`... flag</span>`

> +   <span title=concept-request-origin>origin</span> to the target origin.
+
+   <li><p>Unset the <var>request</var>'s
+     <span title=concept-request-use-referred-token-binding>use-referred-token-binding</span> flag.
+   <li><p>If <var>useReferredTokenBinding</var> is "<code title>true</code>",
+     and the <var>request</var>'s
+     <span title=concept-request-use-token-binding>use-token-binding</span> flag is set,
+     then perform the following substeps:
+
+   <ol>
+     <li><p>Set the <var>request</var>'s
+       <span title=concept-request-use-referred-token-binding>use-referred-token-binding</span>
+       flag.
+       <li><p>Set the <var>request</var>'s
+         <span title=concept-request-referred-token-binding-origin>referred-token-binding-origin</span>
+         to the <var>request</var>'s <span title=concept-request-origin>origin</span>.

s/to the/to/

> +   <li><p>If <var>useReferredTokenBinding</var> is "<code title>true</code>",
+     and the <var>request</var>'s
+     <span title=concept-request-use-token-binding>use-token-binding</span> flag is set,
+     then perform the following substeps:
+
+   <ol>
+     <li><p>Set the <var>request</var>'s
+       <span title=concept-request-use-referred-token-binding>use-referred-token-binding</span>
+       flag.
+       <li><p>Set the <var>request</var>'s
+         <span title=concept-request-referred-token-binding-origin>referred-token-binding-origin</span>
+         to the <var>request</var>'s <span title=concept-request-origin>origin</span>.
+   </ol>
+
+   <p class="note no-backref">By causing the
+   <span title=concept-request-use-referred-token-binding>use-referred-token-binding</span> flag

`... flag</span>`

> @@ -4841,6 +4996,11 @@ <h4 id="should-response-to-request-be-blocked-due-to-nosniff?"><dfn title="shoul
 getter must return <span title=concept-Request-request>request</span>'s
 <span title=concept-request-credentials-mode>credentials mode</span>.
 
+<p>The <dfn title=dom-Request-use-referred-token-binding><code>useReferredTokenBinding</code></dfn>
+attribute's getter must return true if <span title=concept-Request-request>request</span>'s
+<span title=concept-request-use-referred-token-binding>use-referred-token-binding flag</span>
+is set, and false otherwise.

It seems like you did. I'd appreciate it if you could reply to the more substantial review comments so it's somewhat clear what's going on. That would help make reviewing easier, especially since you don't do updates all that often.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/fetch/pull/325#pullrequestreview-316949

Received on Friday, 16 September 2016 09:18:33 UTC