[Unbearable] Adam Roach's Yes on draft-ietf-tokbind-https-14: (with COMMENT)

[Adam Roach <[e-mail]>]

Adam Roach has entered the following ballot position for
draft-ietf-tokbind-https-14: Yes

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-tokbind-https/



----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Thanks to everyone who worked on this document. I am balloting "Yes", but
still have a handful of comments, including several that I believe are
rather important.


---------------------------------------------------------------------------

§2:

>  Once a client and server have negotiated the Token Binding Protocol
>  with HTTP/1.1 or HTTP/2 (see [I-D.ietf-tokbind-protocol] and
>  [I-D.ietf-tokbind-negotiation])

Presuming this document is intended to cover use of TLS 1.3, I believe this
list needs to also include [I-D.ietf-tokbind-tls13].

---------------------------------------------------------------------------

§5.3:

>  When a client receives the Include-Referred-Token-Binding-ID header,
>  it includes the referred token binding even if both the Token
>  Provider and the Token Consumer fall under the same eTLD+1 and the
>  provided and referred token binding IDs are the same.  Note that the
>  referred token binding is sent only on the request resulting from the
>  redirect and not on any subsequent requests to the Token Provider.

I think this needs some clarification about handling of multiple redirections of
the transaction. E.g.: Token Consumer sends a 3xx to redirect the user to a
Token Provider (using, perhaps, an endpoint that is in the process of being
migrated), and then the Token Provider sends an additional 3xx to get the
client to the correct server.  Presumably, the inclusion of the referred token
binding should survive both redirections, but this text might be read as
preventing that.

---------------------------------------------------------------------------

§5.3:

>  This header field has only meaning if the HTTP status code is 301,
>  302, 303, 307 or 308, and MUST be ignored by the client for any other
>  status codes.

I'm somewhat less sanguine about this than Alexey is: we've had 3xx-class
response codes registered as recently as three years ago, and I see no reason to
believe that the future won't hold additional development work on HTTP overall.

While I understand that 305 and 306 are deprecated, and the use of the header
field is nonsensical in 304 and, to a lesser degree, in 300, it seems that there
is no harm that results in any of these cases if *this* document doesn't
prohibit them.

Taken one at a time -- In the case of 300, the presence of the header field
would indicate that whichever option was followed by the user agent would
receive a copy of the token binding, which is as sensible a thing for a server
to ask for as 300 is in the first place.

In the case of 304, there is no server to receive the token binding, so no harm
could possible be induced.

For 305 and 306, to the extent that these are used any more (and they're not),
the request will arrive back at the same origin that sent the response; which,
again, causes no information to be divulged that should not be.

I would strongly recommend changing this to cover all codes in the 300-399
range, for the purpose of forward-compatibility with new redirection codes.

---------------------------------------------------------------------------

§5.4:

>  The TLS Extension for Token Binding Protocol Negotiation
>  [I-D.ietf-tokbind-negotiation]

Same comment as above regarding [I-D.ietf-tokbind-tls13].

---------------------------------------------------------------------------

§5.5:

>  | {EKMn}Ksm: | EKM for server "n", signed by private key of TBID    |
>  |            | "m", where "n" must represent server receiving the   |
>  |            | ETBMSG, if a conveyed TB's type is                   |
>  |            | provided_token_binding, then m = n, else if TB's     |
>  |            | type is referred_token_binding, then m != n. E.g.,   |
>  |            | see step 1b in diagram below.                        |


I was able, with some effort, to muddle through these words and (I think) figure
out the intention, but the construction is very difficult to follow. I think you
want to swap the comma after "ETBMSG" for a period.


---------------------------------------------------------------------------

§11.1:

>  [fetch-spec]
>             WhatWG, "Fetch", Living Standard ,
>             <https://fetch.spec.whatwg.org/>.

I share Alexey's concern about a normative reference to a living document. I
would like to suggest referencing a specific snapshot (e.g., commit hash), but
the specific referenced document makes this infeasible by means of an
aggressive red-box warning that effectively precludes doing so. I agree that
understanding the document is not a prerequisite to understanding or
implementing this one, and so agree that (for the time being) moving the
document to the informative reference section is advisable.