Ballot for draft-ietf-tokbind-protocol
Yes
No Objection
Note: This ballot was opened for revision 16 and is now closed.
Please also see Victor’s OpsDir review (if not already done). You may want RFC 8174 instead of RFC 2119. I’m slightly confused by “This message MUST be sent if the client and server successfully negotiated the use of the Token Binding protocol” - is this *always* true? What if I’ve cleared my token bindings? Likely just confused...
Thanks for a well-written document. I have only one very small nit to incorporate if a new version of the document is produced prior to RFC editor hand-off: §1: > A Token Binding is established by a user agent generating a private- > public key pair (possibly, within a secure hardware module, such as > TPM) Nit: remove comma after "possibly"
Thank you for completing this work! Nit: I think SHA256 needs a reference on first use.
Thanks for this document. I am balloting "yes", but I have a few minor comments: Minor Comments: §1.1: Please consider using the boilerplate from 8174 across this cluster. There are a few instances of lower case keywords across the set. §3.1: This section does not seem to sufficiently define the difference between provided_token_binding and referred_token_binding, other than as a mention of the use case from the HTTPS doc. It would be nice if other application protocol bindings did not have to refer to the HTTPS doc to fully understand the basic protocol. (Or is it assumed that only HTTPS will use referred_token_binding)? §3.2, last paragraph: Why is this a SHOULD? Do you envision scenarios where it would make sense to behave differently? For example, if an application made Token Binding IDs available as structured data, what would be the consequences? §3.3, last bullet: Is the EKM from the TLS connection at the time the binding is established (rather than when it might later be used)? §4.1, 2nd paragraph: Why only a SHOULD? It seems like intentionally storing keys in an insecure manner makes the entire protocol pointless. §6.1 (and others): I'm not sure what it means for the expert to be "advised to encourage". Is there something more concrete that could be said? Does this give the advisor grounds to reject a registration? Editorial Comments: §1: Please expand TPM on first mention.
Updated to note that using referred_token_bindings inherently involves exposing a client's identifier (token binding ID) used with the Relying Party to the Identity Provider. Though this is in some sense obvious, it's probably still worth mentioning in the Privacy Considerations (along with the mitigating factor that the IdP is pretty inherently trusted). Original ballot follows. Section 2 When a server supporting the Token Binding protocol receives a bound token, the server compares the Token Binding ID in the token with the Token Binding ID established with the client. If the bound token came from a TLS connection without a Token Binding, or if the Token Binding IDs do not match, the token is rejected. "came from" is perhaps a bit ambiguous; I'd suggest "is received on" instead. Section 3.3 The need for synchronization between application+token-binding and the TLS stack (around renegotiation) is real, but is also potentially really hard. Can we get some guidance on how to not screw it up? Section 3.4 An implementation MUST ignore any unknown Token Binding types. This is the section on extensions; do we mean to say that unknown extension types are to be ignored? (If we do, it would seem to duplicate a line in Section 4.2.) Section 4.1 The client MUST include at least one TokenBinding structure in the Token Binding message. The key parameters used in the provided_token_binding MUST match those negotiated with the server (e.g., via [I-D.ietf-tokbind-negotiation] or a different mechanism). This seems to imply but not specifically state that the mandatory TokenBinding is of type provided_token_binding. Changing "the" to "this" in the second line would subtly sneak this in, but it's probably better to also explicitly say "of type provided_token_binding" in the first sentence. Section 4.2 I would suggest adding a pargaraph break between the sentences in If the use of the Token Binding protocol was not negotiated, but the client sends the Token Binding message, the server MUST reject any contained bindings. If the Token Binding type is "provided_token_binding", the server MUST verify that the signature algorithm (including elliptic curve in the case of ECDSA) and key length in the Token Binding message match those negotiated with this client (e.g., via [I-D.ietf-tokbind-negotiation] or a different mechanism). If a Token Binding is rejected, any associated bound tokens MUST also be rejected by the server. [...] Is "associated" scoped to "presented on the current TLS connection" or a more global property? (I understand that the association could be either via direct embedding of ID in token or via an external database mapping.) Section 6.1 This policy sounds more like "Specification Required" than "Expert Review" (the former still includes expert review).
A clearly written document. Thank you. A small nit: the document states that it specifies version 1.0 of protocol, but the actual version value is defined in tokbind-negotiation document, there is no mention of version encoding in the protocol document itself. Is this intentional?
Sorry for spamming but one more update (after reading draft-ietf-tokbind-https): In sec 3: "This message MUST be sent in the client's first application protocol message." Why is that a generic requirement for all uses of token binding and not just for HTTPS? Update (after reading draft-ietf-tokbind-negotiation): Given different negotiation mechanisms could be used, maybe it would make sense to say slightly more about version handling in this doc as well, e.g. at least explaining/requiring that version negotiation is done by the negotiation protocol... Maybe I'm just missing something but given the TokenBindingType and the TB_ExtensionType share the same number space, how do I know if there is another TokenBinding or and an TB_Extension following after the signature? Nit: Please spell out TPM (in sec 1).