Re: [Unbearable] WGLC 3 on core documents

    This isn't going to be a short review.

General

(I have raised these in the past, I don't expect things to change, I
just wanted to get them on the record.)

These documents go to great lengths to create a generic solution that
can be used in multiple protocols.  However, the test set - those
protocols that have been validated to work with the framework - is
size 1.  That makes me very nervous about the ability of this protocol
to be applied in contexts outside of HTTP.  (The extensions field is
even less well justified, it has zero uses.)

The use of a TLS extension is unnecessary complexity.  Ostensibly,
this is to save on latency.  Any interactions with a server that might
result in the creation of a token that requires this level of
protection necessarily take multiple round trips.  Thus, negotiating
the use of this extension in the application protocol is perfectly
feasible.

I wish that eTLD+1 weren't baked into this.  It's unnecessary if you
accept a one-time cost for each origin.  Only performance degrades.
If we didn't bake in eTLD+1, we would create better incentives.  Yes,
I realize that this is a disincentive to deploy token binding.

I don't believe that referred token bindings need a signature.  Adding
a signature only creates busy work for clients.  My rationale is this:
a token provider might not support the signature scheme that is
negotiated with a token consumer.  Therefore, the token provider might
be unable to verify the signature on the referred token binding.  If a
security proof were to depend on the token provider verifying that
signature, we have an entirely different set of constraints on the
design (and a three-way negotiation to somehow build).  The attack in
Section 7.4 of the HTTP doc is thwarted simply by having the client
send its TBID to the Token Provider.

draft-ietf-tokbind-protocol-12

This doesn't really talk much about certificate-based client
authentication and its relationship to that.  A server that uses

Section 1

Terminology: Intuitively, I read "Token Binding" as referring to the
mechanism, or perhaps the signature that is carried, or perhaps the
binding of the bearer token to the key pair.  But it apparently means
the key pair that is minted for each eTLD+1.  I found that confusing.

   When issuing a security token to a client that supports Token
   Binding, a server includes the client's Token Binding ID in the
   token.

Inclusion in the token is not necessary.  The token could reference
server-side state and THAT could include the public key.

   In order to successfully export and replay a bound security token, an
   attacker needs to also be able to export the client's private key, [...]

The attacker only needs to be able to *use* the client's private key.

Section 3

   The Token Binding message is sent by the client to prove possession
   of one or more private keys held by the client.  This message MUST be
   sent if the client and server successfully negotiated the use of the
   Token Binding protocol via [I-D.ietf-tokbind-negotiation], and MUST
   NOT be sent otherwise.  This message MUST be sent in the client's
   first application protocol message.

The first MUST here is impossible to create a test for because this
document doesn't define how the message is carried.  I would suggest
moving any requirement to include a token binding into the using
portion (the HTTP doc).

The MUST NOT forbids negotiating use of token binding in other ways.
Other than the fact that you *can't* send a token binding in the
clear, I see no reason to include this clause.

Section 3.1

Suggested edit:

   o  referred_token_binding - used when requesting tokens [that are
intended ]to be
      presented to a different server.

Section 3.2

I observe that it is strange to omit leading zeros in one case and
require them in another.

Section 3.3

It's odd that the signature doesn't cover the entire blob.  More odd
that it doesn't cover extensions (which makes me feel further
vindicated in asking for their removal).

I'd be happier if you just forbid renegotiation.  The current text
makes it sound really, really difficult to get this right.  That it
probably isn't that hard in practice is only because of the limited
ways in which renegotiation is used in the real world.  Note that even
if messages themselves don't cross boundaries, it is possible for the
processing of a message to occur on the other side of a renegotiation
from when it was sent.  So you can go through a lot of effort to
generate a valid message and still have it fail to validate.

   o  Context value: NULL

In TLS 1.3, the context of NULL is the same as an empty sequence.
That might be cause for some confusion.

Section 4.1

   In order to prevent cooperating
   servers from linking user identities, different keys SHOULD be used
   by the client for connections to different servers

Why only SHOULD?

Section 4.2

  Token Bindings of type
   "referred_token_binding" may use different key parameters than those
   negotiated with this client.

I think that this is true, but misleading.  They MUST use whatever key
parameters the client negotiated with the other server.

Section 5

"the cookies" - ?

"the server MUST discard the token" - this is wrong, because it
implies that the server might ignore the token.  In the HTTP case at
least the server is required to reject the request.  "the server MUST
treat the token as invalid" might be better, but that leaves a lot to
the application protocol.

Section 6.1, 6.2

Tables would be easier to read, I think.

Section 7

Since the token binding signature is a secret, you should recommend
constant time validation.  I am not aware of any implementation that
does signature validation in constant time, but it's possible that
they exist.

Section 7.1

"by [the] malware"

The idea that token binding keys are especially high value isn't borne
out by deployment experience.  I believe that no implementation is
willing to apply additional protection for these keys due to the
performance cost that imposes.  Also, the keys are not appreciably
worse to lose than some of the long-lived authentication tokens that
are routinely held in browser cookie jars.  If the intent is to
improve the security situation, then it's true that these need to be
better protected than a cookie jar, but this could say that directly
instead.  I really don't know what to do about the fact that they
aren't that well protected in practice.

Section 7.2

This appears to be identical to the text in the negotiation draft.  I
would remove it from this doc and instead include a reference to the
negotiation security considerations section.  The same would seem to
apply to Section 7.5.

Section 7.3

"Token Binding IDs are never transmitted in clear text" - do you
actually require that?

Section 7.5

The important observation here is that there are attacks that allow an
attacker to force two connections to have the same exporter secret
(the master section in TLS 1.2 or earlier).  That's important to say.
(But, see above).

draft-ietf-tokbind-negotiation-07

I've already established my position: I don't think that this is
necessary.  I've also already shared my view on the version field in
here.  That's going to rust closed, if it isn't already.

Section 2

Does the working group intend to remove the following?  Or are those
version numbers burned permanently?  I note that there is no IANA
registry for the version field, and no other text reserving these
values.

   Prototype implementations of Token Binding drafts can
   indicate support of a specific draft version, e.g. {0, 1} or {0, 2}.

Section 3

   1.  The server supports the Token Binding protocol version offered by
       the client or a lower version.

This implies a very precise set of rules for handling changes in
versions that isn't very well described in the document.  It means
that the value of the extension in any version MUST be valid in all
earlier versions, though the earlier versions might only use a prefix
of the encoded value.  It also means that servers MUST ignore trailing
octets if the version number is higher than one they support.  It also
implies that there can be no gaps in version support and that client
MUST support a contiguous range of all versions from {0, 0} up to the
version they advertise, otherwise the server might pick a version they
don't know.  All of that needs to be written down.

Of course, it seems like you can successfully negotiate Token Binding,
but then not use it:

   If the client does not support the Token Binding protocol version
   selected by the server, then the connection proceeds without Token
   Binding.

Given that a client MUST send a Token Binding message when it is
negotiated (see comment about that above), this creates a problem when
the first message arrives at the server without a Token Binding
message - the server and client now disagree about whether they have
enabled the feature.

draft-ietf-tokbind-https

Abstract

The abstract here is quite long.  I believe that you can't include
references in abstracts either (it's a style guide thing).

"bind security tokens [...] to TLS [...] connections" - is that really
the case?  I guess that it's a transitive binding: the token is bound
to the public key, the public key is then bound to the connection.
However, if you read on, the server binds the tokens it issues, but it
doesn't say to what, which could be misinterpreted.

Section 1

"inoculating" is an odd choice of word to use.  Why not just say that
it "prevents the use of the token on connections unless the client is
also able to prove possession of the key pair".

"TokenBindingMessages" isn't used in the main doc, it uses "Token
Binding messages" in full.

   This means that Token Binding over HTTP is only
   defined when the HTTP protocol is layered on top of TLS (commonly
   referred to as HTTPS).

This assumes something that is no longer correct (that use of TLS ==
use of https://).  See
https://datatracker.ietf.org/doc/draft-ietf-httpbis-http2-encryption/

Section 2

You can refer to RFC7515 for the definition of base64url rather than
defining it again yourself.  You already use the name (without
defining it).

The ABNF is incomplete, you need to define the grammar for
EncodedTokenBindingMessage.

The example should probably be an actual example.  As it is, it's not
much better than the ABNF.

This section includes some rules regarding use that I think are
generally unnecessary.  PUT changes the resource using the enclosed
"representation", not the header fields of the request.  Vary can
reasonably list any request header field it chooses.  And the
prohibition on including a token in the trailers is unnecessary, since
it has to be possible to ignore trailers and presumably this is
important in some way.

Did you know that it is more efficient to allow multiple
Sec-Token-Binding header fields than it is to have the current
length-prefixed structure?  You could comma separate encoded
TokenBinding values and save a few bytes.

I don't think that you need a SHOULD regarding header compression.
Mention that it is possible and that will suffice.

Section 2.1

The lack of any rules for scoping outside of the web browsing case
makes me nervous.  Either embrace eTLD+1 and let everyone do the same
bad things, or say that if you are using cookies, use eTLD+1,
otherwise use Origin.  Or you could say use the same damned token
across all requests if tracking isn't a concern - it's a concern that
is fairly unique to browsers after all.  In all cases, you need to
have clear, mandatory rules.

Section 3

Does this mean that a client MUST NOT pipeline requests if Token
Binding is negotiated?

The server also MUST process any TokenBindingMessages (or Token
Binding messages) BEFORE initiating renegotiation.

Section 5.2

This uses "may" and "must" a bit which makes it unclear whether there
are any interoperability requirements.

The fourth bullet point seems to be more than just a step in the
process.  Maybe this doesn't make sense to format as a bullet list.

Section 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.

Why?  Because two signatures are better somehow?  Seems like busy work to me.

   The TokenBindingMessage SHOULD contain a TokenBinding with
   TokenBindingType referred_token_binding.

Under what circumstances would a client NOT include a referred Token Binding?

The redirect-chain approach makes my head hurt.  Not because it's
especially complex, but it's really hard to understand as written.  It
doesn't establish how TP1, despite being unable to generate a security
token in the first place, is suddenly able to generate a security
token later in the sequence.  Isn't it easier if TP1 and TP2 just pass
the ID for TC to each other over an authenticated channel (which might
be the URL) so that TP2 can bind to TC directly?

Section 5.5

Some of the table entries include 'n' - a number, but not all.

"contain"/"containing" - use "bound to" instead

This would be easier if the tokens were shown using a notation like
Token2<TBID1> or something like that.  The sequence diagram is very
hard to follow with its mix of odd notations and prose.

1a doesn't need to mention that the first server can bind to TBID1.
Focus on the goal, which is describing the federated case.

Section 7.3

Doesn't this belong in the main doc?

   For example, the client's Token Binding private key must be kept
   secret by the client.

Why is this a "for example" and not a MUST?

   A client should not be tricked into sending a Sec-
   Token-Binding header field to a server that contains Token Binding
   messages about key pairs that the client does not control.

What contains the messages?  The server?  Doesn't the header field
carry these messages?

I think that the integrity protection requirement is more simply
stated: A client MUST NOT send a Token Binding message to a server
unless it actually controls the corresponding key.  This implies that
an attacker MUST NOT be allowed to insert or modify a Token Binding
message.  Clients who do not understand that the Token Binding
protocol exists are protected by virtue of not negotiating the use of
the protocol. [[ It is also somewhat guaranteed by the use of the Sec-
prefix, though this is a weaker assurance.  ]]  This might allow an
attacker to either steal tokens and have them bound to their own keys,
or to deny clients the ability to use the tokens they acquire
legitimately.

Section 7.4

See meta-level comment about the necessity of the signature.

This analysis assumes that the MitM isn't able to convince the Token
Consumer that the client simply doesn't support Token Binding: a much
easier way to avoid having to attack the protocol.  Of course, this is
only possible if they can remain in-the-middle indefinitely.  Please
say something about this.

Section 8.3

"must" or "should" and "may" all seem to be in need to capitalization
or replacement with equivalent and less confusing words.

On 17 February 2017 at 10:49, Leif Johansson <leifj@sunet.se> wrote:
>
> This starts a 3 week WGLC on the core documents:
>
> https://tools.ietf.org/html/draft-ietf-tokbind-protocol-12
> https://tools.ietf.org/html/draft-ietf-tokbind-negotiation-07
> https://tools.ietf.org/html/draft-ietf-tokbind-https-08
>
> Any comments should be in hand by the 10th (any TZ). The recently
> raised security considerations issue will be treated as a WGLC
> comment.
>
>         BestR
>         Leif & John
>
> _______________________________________________
> Unbearable mailing list
> Unbearable@ietf.org
> https://www.ietf.org/mailman/listinfo/unbearable