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

nharper commented on this pull request.



> +    <a href="https://tools.ietf.org/html/draft-ietf-tokbind-negotiation#section-2">section 2</a>
+    of the Token Binding Negotiation spec [[!TOKBIND-NEGOTIATION]].
+    If Token Binding negotiation succeeds, set
+    <a for=/>token-binding protocol version</a> and
+    <a for=/>token-binding key parameters</a>
+    in <var>connection</var>'s <a for=connection>TLS connection metadata</a>
+    to the negotiated values,
+    and <a for=/>token-binding Exported Keying Material</a> to the computed value.
+
+    <p class="note no-backref">
+    The user agent will use <a for=/>Token Binding</a> for any
+    <a for=/>request</a> sent over a TLS connection for which Token Binding
+    negotiation was successful.
+    <a for=/>Token Binding</a> is only proposed when <var>credentials</var>
+    is true, so connections which successfully negotiate a binding will never be
+    pooled with connections that do not include credentials.

I don't think that we should require in this spec that negotiation of Token Binding for the whole connection be segmented based on the credentials boolean (although an implementation may certainly choose to do so). I think it's reasonable for an implementation to always negotiate Token Binding, and only send the header when credentials is true.

> +    <a href="https://tools.ietf.org/html/draft-ietf-tokbind-negotiation#section-2">section 2</a>
+    of the Token Binding Negotiation spec [[!TOKBIND-NEGOTIATION]].
+    If Token Binding negotiation succeeds, set
+    <a for=/>token-binding protocol version</a> and
+    <a for=/>token-binding key parameters</a>
+    in <var>connection</var>'s <a for=connection>TLS connection metadata</a>
+    to the negotiated values,
+    and <a for=/>token-binding Exported Keying Material</a> to the computed value.
+
+    <p class="note no-backref">
+    The user agent will use <a for=/>Token Binding</a> for any
+    <a for=/>request</a> sent over a TLS connection for which Token Binding
+    negotiation was successful.
+    <a for=/>Token Binding</a> is only proposed when <var>credentials</var>
+    is true, so connections which successfully negotiate a binding will never be
+    pooled with connections that do not include credentials.

A UA would have a different key for `google.com` and `youtube.com` (to use that example). Since Token Binding messages are per-request, it's fine for those origins to be coalesced on the same H2 connections - different Token Binding messages using different keys will be sent for the requests on the different origins. So, there are different bindings for `google.com` and `youtube.com`.

The part where things get tricky with coalescing is that the use of Token Binding for requests on that connection is negotiated once for the connection. The practical effect of this is that all origins that can transitively be coalesced should support the same Token Binding protocol version and key parameter (and anything else that might get negotiated in a future version of Token Binding).

> +of [[TOKBIND-PROTOCOL]].
+
+<p>At a very high level, a user agent negotiates the use of Token Binding with
+a server when it sets up a TLS connection to the server.
+It sends its highest supported
+<dfn export id=concept-token-binding-protocol-version>token-binding protocol version</dfn>
+and supported cryptographic algorithms and parameters (the
+<dfn export id=concept-token-binding-key-parameters>token-binding key parameters</dfn>)
+in a <code>token_binding</code> Client Hello Extension, 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 [[!TOKBIND-NEGOTIATION]].
+The user agent saves the values of the <a for=/>token-binding protocol version</a> and
+<a for=/>token-binding key parameters</a> that were negotiated for the
+TLS connection in metadata for that connection.
+
+<p>The user agent uses a `<dfn http-header><code>Sec-Token-Binding</code></dfn>`

Nit on the suggested wording: the proof-of-possession is connection-specific, not request-specific.

> @@ -3237,6 +3566,31 @@ in addition to <a>HTTP fetch</a> above.
   nullity has already been checked. The <a lt=extract for=BodyInit>extracting</a> operation cannot
   throw as it was called for the same <a for=body>source</a> before.
 
+ <li><p>Let <var>useReferredTokenBinding</var> be the result of
+ <a for=header>parsing</a> `<a http-header><code>Include-Referred-Token-Binding-ID</code></a>`
+ in <var>actualResponse</var>'s <a for=response>header list</a>.
+
+ <li><p>Set the <var>request</var>'s <a for=request>referred-token-binding origin</a> to null.
+
+ <li><p>If <var>useReferredTokenBinding</var> is `<code>true</code>`

The current behavior in Chromium is to let headers coalesce, and then compare to the string "true", meaning that if the header is sent multiple times we treat it as if the header was set to "false" (or any other value, or not present). I'm not sure that's the behavior we actually want.

If we want to reject the response (instead of ignoring) if the header has a bad value, another thing to consider is whether the header was sent but Token Binding wasn't negotiated for the connection (or a Token Binding wasn't sent on the request). Right now Chromium will ignore the header if Token Binding wasn't negotiated.

> @@ -3237,6 +3566,31 @@ in addition to <a>HTTP fetch</a> above.
   nullity has already been checked. The <a lt=extract for=BodyInit>extracting</a> operation cannot
   throw as it was called for the same <a for=body>source</a> before.
 
+ <li><p>Let <var>useReferredTokenBinding</var> be the result of
+ <a for=header>parsing</a> `<a http-header><code>Include-Referred-Token-Binding-ID</code></a>`
+ in <var>actualResponse</var>'s <a for=response>header list</a>.
+
+ <li><p>Set the <var>request</var>'s <a for=request>referred-token-binding origin</a> to null.
+
+ <li><p>If <var>useReferredTokenBinding</var> is `<code>true</code>`
+ and the <var>request</var>'s <a for=request>use-token-binding flag</a> is set,
+ then set the <var>request</var>'s <a for=request>referred-token-binding origin</a> to
+  <var>request</var>'s <a for=request>origin</a>.

I think it would be really annoying or difficult to implement if we don't just ignore `Content-Security-Policy: sandbox`. From an implementer's perspective, I think it's reasonable to follow the `Include-Referred-Token-Binding-ID` header on the redirect and ignore the CSP.

-- 
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-102517907