- From: Mike West <notifications@github.com>
- Date: Mon, 14 Nov 2016 05:46:39 -0800
- To: whatwg/fetch <fetch@noreply.github.com>
- Message-ID: <whatwg/fetch/pull/325/review/8384184@github.com>
mikewest requested changes on this pull request.
Hey @vanupam, thanks for pinging me about this (again). At a high level, I think this patch could be refactored a bit to make it a little clearer what's going on. I've left comments inline.
At a lower level, the document's been completely restructured as a result of changing the build tool from Anolis to Bikeshed. You'll need to move your changes to `fetch.bs`, which hopefully won't be too onerous.
Thanks!
> @@ -202,6 +202,34 @@ <h2 class="short">Conformance</h2>
<span>process request body</span> for <var>request</var>.
<!-- XXX xref "read", "payload body" -->
+<h3>Token Binding</h3>
Nit: I'd suggest moving this section down into section 4, as it's describing an extension to HTTP rather than core infrastructure of Fetch.
> @@ -202,6 +202,34 @@ <h2 class="short">Conformance</h2>
<span>process request body</span> for <var>request</var>.
<!-- XXX xref "read", "payload body" -->
+<h3>Token Binding</h3>
+
+<p>The technique of Token Binding allows user agents to work with servers to protect credentials
+like HTTP cookies.
+User agents and servers mutually agree to use Token Binding by negotiating while setting up
+TLS connections, as described in the
+<a href="https://tools.ietf.org/html/draft-ietf-tokbind-negotiation">Token Binding Negotiation</a>
+spec.
In the new `fetch.bs`, you'll see a `<pre class="biblio">` at the top where you should add references to the drafts this patch relies on. Something like:
```
"TOKBIND-NEGOTIATION": {
"authors": [ "Andrei Popov", "Magnus Nystroem", "Dirk Balfanz", "Adam Langley" ],
"title": "Transport Layer Security (TLS) Extension for Token Binding Protocol Negotiation",
"publisher": "IETF",
"href": "https://tools.ietf.org/html/draft-ietf-tokbind-negotiation"
},
"TOKBIND-PROTOCOL": {
"authors": [ "Andrei Popov", "Magnus Nystroem", "Dirk Balfanz", "Adam Langley", "Jeff Hodges" ],
"title": "The Token Binding Protocol Version 1.0",
"publisher": "IETF",
"href": "https://tools.ietf.org/html/draft-ietf-tokbind-protocol"
},
"TOKBIND-HTTPS": {
"authors": [ "Andrei Popov", "Magnus Nystroem", "Dirk Balfanz", "Adam Langley", "Jeff Hodges" ],
"title": "Token Binding over HTTP",
"publisher": "IETF",
"href": "https://tools.ietf.org/html/draft-ietf-tokbind-https"
},
```
you can then link to them via something like `[[TOKBIND-NEGOTIATION]]` for informative references, and `[[!TOKBIND-NEGOTIATION]]` for normative references.
> @@ -202,6 +202,34 @@ <h2 class="short">Conformance</h2>
<span>process request body</span> for <var>request</var>.
<!-- XXX xref "read", "payload body" -->
+<h3>Token Binding</h3>
+
+<p>The technique of Token Binding allows user agents to work with servers to protect credentials
I'd suggest rewording this section a bit. Perhaps something like:
> ```
> In order to protect security tokens like <a>credentials</a> and OAuth tokens, user agents
> and servers can use <dfn>token binding</dfn> to cryptographically associate a given
> security token with a particular TLS connection, thereby making it more difficult for
> attackers to abuse the capabilities the security token grants. This technique is described in
> detail in [[TOKBIND-NEGOTIATION]], [[TOKBIND-PROTOCOL]], and [[TOKBIND-HTTPS]].
>
> A <dfn>token binding ID</dfn> is the representation of a token binding, as described in
> <a href="https://tools.ietf.org/html/draft-ietf-tokbind-protocol#section-3.2">section 3.2</a>
> of [[TOKBIND-PROTOCOL]].
> ```
I don't think you need to define "token binding message", as you only use it when constructing the `Sec-Token-Binding` header that we'll talk about in a minute.
> +<a href="https://tools.ietf.org/html/draft-ietf-tokbind-protocol#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/html/draft-ietf-tokbind-protocol#section-3.2">section 3.2</a>
+of the Token Binding Protocol spec.
+
+<p>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/html/draft-ietf-tokbind-protocol#section-3">section 3</a>
+of the Token Binding Protocol spec.
+A server uses the <span title=concept-token-binding-message>Token Binding Message</span> received
+from a user agent to bind credentials that it issues to that user agent,
+and to verify that credentials presented by the user agent are correctly bound.
I'd suggest defining the `Sec-Token-Binding` header here instead of defining "token binding message". Something like:
> ```
> The `<dfn http-header><code>Sec-Token-Binding</code></dfn>` header is used to transmit
> token binding metadata from the user agent to the server. Its <a for="header">value</a> is a
> base64url-encoded string [[!RFC4648]]:
>
> <pre>
> Sec-Token-Binding = 1*( ALPHA / DIGIT / "-" / "_" ) *2( "=" )
> </pre>
I'd also suggest moving the value generation up from inside HTTP-network-or-cache fetch, and into this section. Something like:
> ```
> To <dfn>add token binding metadata to <var>request</var></dfn>, run the following steps:
>
> 1. If ...
> ```
We'll get to the `...` bit down below where the text currently lives.
> @@ -1363,6 +1419,16 @@ <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 by adding a Token Binding Negotiation extension in the TLS Client Hello as
+ described in
+ <a href="https://tools.ietf.org/html/draft-ietf-tokbind-negotiation#section-2">section 2</a>
+ of the Token Binding Negotiation spec.
+
+ <p class="note no-backref">If Token Binding Negotiation succeeds, the user agent can use
+ <span title=concept-token-binding>Token Binding</span> with the server.
I'd suggest adding an explicit property to the connection that describes the process in a little more detail. That is, during negotiation, both parties establish a `token_binding_version` and `key_parameters_list`, etc. @annevk will likely have opinions here about what level of detail is appropriate, but I'd suggest abstracting all that into something like `<dfn for="connection">toking binding metadata</dfn>` which is either `null` or an opaque object (as far as Fetch is concerned) that you can pass into the `Sec-Token-Binding` header generation algorithm.
> + <li><p>If the user agent supports <span title=concept-token-binding>Token Binding</span>,
+ build a <code title="">Sec-Token-Binding</code> <span title=concept-header>header</span>
+ by running these substeps:
+ <p class="note no-backref">A <code title="">Sec-Token-Binding</code>
+ <span title=concept-header>header</span> allows a server
+ to bind cookies that it issues to a <span title=concept-token-binding-id>Token Binding Id</span>,
+ and to verify that cookies presented by the user agent are bound to the correct
+ <span title=concept-token-binding-id>Token Binding Id</span>.
+
+ <ol>
+ <li><p>Set the <var>httpRequest</var>'s
+ <span title=concept-request-use-token-binding>use-token-binding flag</span>.
+ <li><p>Initialize <var>tokenBindingMessage</var> to null.
+ <li><p>Let <var>providedTokenBindingId</var> be the result of looking up the
+ <span title=concept-token-binding-id>Token Binding Id</span> for the <var>httpRequest</var>'s
+ <span title=concept-request-current-url>current url</span>.
It's probably worth moving this step and the following note out of this algorithm, and into a separate bit of the "Token Binding" section, saying something like "To <dfn>generate a token binding ID for a |url|</dfn>, user agents MUST follow the steps defined in [insert reasonable reference here... it's not clear which draft defines how user agents generate this value. https://tools.ietf.org/html/draft-ietf-tokbind-https-06#section-2 and 2.1 seem relevant, but doesn't really spell it out clearly...]".
> @@ -3210,6 +3323,73 @@ <h4 id="should-response-to-request-be-blocked-due-to-nosniff?"><dfn title="shoul
<li>If <var>cookies</var> is not the empty string, append
`<code title=http-cookies>Cookie</code>`/<var>cookies</var> to <var>httpRequest</var>'s
<span title=concept-request-header-list>header list</span>.
+
+ <li><p>If the user agent supports <span title=concept-token-binding>Token Binding</span>,
+ build a <code title="">Sec-Token-Binding</code> <span title=concept-header>header</span>
+ by running these substeps:
+ <p class="note no-backref">A <code title="">Sec-Token-Binding</code>
+ <span title=concept-header>header</span> allows a server
+ to bind cookies that it issues to a <span title=concept-token-binding-id>Token Binding Id</span>,
+ and to verify that cookies presented by the user agent are bound to the correct
+ <span title=concept-token-binding-id>Token Binding Id</span>.
+
+ <ol>
+ <li><p>Set the <var>httpRequest</var>'s
+ <span title=concept-request-use-token-binding>use-token-binding flag</span>.
Isn't this a property of the connection, not the request? It seems so, and also seems like you'd be able to replace most of the "If the user agent supports token binding..." bits with "If |connection|'s <a>token binding metadata</a> is not `null`..."
> @@ -3210,6 +3323,73 @@ <h4 id="should-response-to-request-be-blocked-due-to-nosniff?"><dfn title="shoul
<li>If <var>cookies</var> is not the empty string, append
`<code title=http-cookies>Cookie</code>`/<var>cookies</var> to <var>httpRequest</var>'s
<span title=concept-request-header-list>header list</span>.
+
+ <li><p>If the user agent supports <span title=concept-token-binding>Token Binding</span>,
+ build a <code title="">Sec-Token-Binding</code> <span title=concept-header>header</span>
+ by running these substeps:
+ <p class="note no-backref">A <code title="">Sec-Token-Binding</code>
+ <span title=concept-header>header</span> allows a server
+ to bind cookies that it issues to a <span title=concept-token-binding-id>Token Binding Id</span>,
+ and to verify that cookies presented by the user agent are bound to the correct
+ <span title=concept-token-binding-id>Token Binding Id</span>.
+
+ <ol>
+ <li><p>Set the <var>httpRequest</var>'s
+ <span title=concept-request-use-token-binding>use-token-binding flag</span>.
+ <li><p>Initialize <var>tokenBindingMessage</var> to null.
What is `|tokenBindingMessage|`? I think we probably want to treat it as a simple dictionary as far as Fetch is concerned, which means that `null` might not be the best representation of an empty thing. Especially since step 4 below adds values to it.
> + <span title=concept-request-current-url>current url</span>.
+
+ <p class="note no-backref">A user agent negotiates the use of Token Binding with a server
+ when it sets up a TLS connection to that server, creating a
+ <span title=concept-token-binding-id>Token Binding Id</span> at the end
+ of the negotiation. The <span title=concept-token-binding-id>Token Binding Id</span>
+ is saved and can then simply be looked up whenever needed.
+ If the user agent is unable to negotiate Token Binding with a server, it will not create
+ a <span title=concept-token-binding-id>Token Binding Id</span>.
+ If the <span title=concept-token-binding-id>Token Binding Id</span> for a server is
+ deleted by a user agent (e.g., as a result of some user action), the user agent will
+ need to re-establish a TLS connection to negotiate Token Binding and create a new
+ <span title=concept-token-binding-id>Token Binding Id</span>.
+
+ <li><p>If <var>providedTokenBindingId</var> is not null, then add a
+ <code title="">provided_token_binding</code> to <var>tokenBindingMessage</var>.
What's the value of `provided_token_binding`? `providedTokenBindingId`?
> + <span title=concept-token-binding-id>Token Binding Id</span>
+ for the <var>httpRequest</var>'s
+ <span title=concept-request-referred-token-binding-origin>referred-token-binding-origin</span>.
+ <p class="note no-backref">A server indicates that it wants the user agent to use a
+ Referred Token Binding when doing a redirect to another server,
+ or when code from that server's origin executes in the user agent.
+ The user agent would have negotiated the use of
+ Token Binding with that server when it set up a TLS connection to that server,
+ creating a <span title=concept-token-binding-id>Token Binding Id</span>
+ at the end of the negotiation.
+ This <span title=concept-token-binding-id>Token Binding Id</span> is saved and can
+ then simply be looked up whenever needed. If the
+ <span title=concept-token-binding-id>Token Binding Id</span> for the server is
+ deleted by a user agent (e.g., as a result of some user action), the user agent will
+ need to re-establish a TLS connection to negotiate Token Binding and create a new
+ <span title=concept-token-binding-id>Token Binding Id</span>.
This note needs to move as well.
> + <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 <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 flag</span>
+ to be set for the subsequent <span title=concept-request>request</span> via the
+ <code title>Include-Referred-Token-Binding-ID</code> <span title=concept-header>header</span>
+ in its <span title=concept-response>response</span>, the referring
+ <span title=concept-request-origin>origin</span> tells the user agent to disclose to the target
+ server 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>.
It's probably worth moving all of this (notes included) out into an algorithm in the token binding section to simplify HTTP-redirect fetch, which is already complicated enough. :)
> @@ -3026,6 +3092,46 @@ <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
This header should be defined explicitly above in the "Token Binding" section.
--
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-8384184
Received on Monday, 14 November 2016 13:47:16 UTC