Subject: Re: [Unbearable] comments on draft-ietf-tokbind-negotiation-02

From: Andrei Popov <[email-masked]>
Date: Fri, 1 Apr 2016 18:44:18 +0000

The editorial aspects would probably be easier handled by PR; I would not mind merging most of the suggestions.

> Though, one possible bug is that step 1 in section 4 refers to 'client' when it should be referring to 'server', yes?  see detailed inline comments on this and other stuff below.

No, the text is as intended: the client receiving the "token_binding" extension MUST terminate the handshake if the client did not include the "token_binding" extension in the client hello.

Cheers,

Andrei

-----Original Message-----
From: Unbearable [mailto:[email-masked]] On Behalf Of =JeffH
Sent: Thursday, March 31, 2016 8:44 AM
To: IETF Tokbind WG <[email-masked]>
Subject: [Unbearable] comments on draft-ietf-tokbind-negotiation-02

I basically have only editorial comments on -tokbind-negotiation-02 aka TBNEGO.

Though, one possible bug is that step 1 in section 4 refers to 'client' when it should be referring to 'server', yes?  see detailed inline comments on this and other stuff below.

An overall editorial comment is:

RFC5246 defines extensions generically as "Hello Extensions"
<https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2ftools.ietf.org%2fhtml%2frfc5246%23section-7.4.1.4&data=01%7c01%7cAndrei.Popov%40microsoft.com%7c11e4050ac38d484df3cb08d3597b6197%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=e2SJrXELfeldPvrOslqZXavfMbFkj0OkAL4NLhOCS8E%3d>, rather than as "client hello extension" or "server hello extension"..

  7.4.1.4.  Hello Extensions

     The extension format is:

        struct {
            ExtensionType extension_type;
            opaque extension_data<0..2^16-1>;
        } Extension;

        enum {
            signature_algorithms(13), (65535)
        } ExtensionType;

      Here:

      -  "extension_type" identifies the particular extension type.

      -  "extension_data" contains information specific to the particular
        extension type.

     The initial set of extensions is defined in a companion document [...].

     An extension type MUST NOT appear in the ServerHello unless the same
     extension type appeared in the corresponding ClientHello.  If a
     client receives an extension type in ServerHello that it did not
     request in the associated ClientHello, it MUST abort the handshake
     with an unsupported_extension fatal alert.

     Nonetheless, "server-oriented" extensions may be provided in the
     future within this framework. [...]

..thus I suggest re-working the TBNEGO sections to define a "TB Nego hello extension" with subsequent sections defining the client-side and server-side processing of the extension.

further editorial comments inline below:

 ><snip>
 >  Transport Layer Security (TLS) Extension for Token Binding Protocol
 >                              Negotiation
 >                   draft-ietf-tokbind-negotiation-02
 >
 >Abstract
 >
 >   This document specifies a Transport Layer Security (TLS) [RFC5246]
 >   extension for the negotiation of Token Binding protocol
 >   [I-D.ietf-tokbind-protocol] version and key parameters.
 >
 >Status of This Memo
 >
 >   <snip/>
 >
 >Table of Contents
 >
 >   1.  Introduction  . . . . . . . . . . . . . . . . . . . . . . . .   2
 >     1.1.  Requirements Language . . . . . . . . . . . . . . . . . .   2
 >   2.  Token Binding Negotiation Client Hello Extension  . . . . . .   2
 >   3.  Token Binding Negotiation Server Hello Extension  . . . . . .   3
 >   4.  Negotiating Token Binding Protocol Version and Key Parameters   4
 >   5.  IANA Considerations . . . . . . . . . . . . . . . . . . . . .   5
 >   6.  Security Considerations . . . . . . . . . . . . . . . . . . .   6
 >     6.1.  Downgrade Attacks . . . . . . . . . . . . . . . . . . . .   6
 >     6.2.  Triple Handshake Vulnerability in TLS 1.2 and Older TLS
 >           Versions  . . . . . . . . . . . . . . . . . . . . . . . .   6
 >   7.  Acknowledgements  . . . . . . . . . . . . . . . . . . . . . .   6
 >   8.  References  . . . . . . . . . . . . . . . . . . . . . . . . .   7
 >     8.1.  Normative References  . . . . . . . . . . . . . . . . . .   7
 >     8.2.  Informative References  . . . . . . . . . . . . . . . . .   7
 >   Authors' Addresses  . . . . . . . . . . . . . . . . . . . . . . .   8
 >
 >1.  Introduction
 >
 >   In order to use the Token Binding protocol
 >   [I-D.ietf-tokbind-protocol], the client and server need to agree on
 >   the Token Binding protocol version and the parameters (signature
 >   algorithm, length) of the Token Binding key.  This document specifies
 >   a new TLS extension to accomplish this negotiation without
 >   introducing additional network round-trips.
 >
 >1.1.  Requirements Language
 >
 >   The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
 >   "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this
 >   document are to be interpreted as described in [RFC2119].
 >
 >2.  Token Binding Negotiation Client Hello Extension

recast as "Token Binding Negotiation Hello Extension"

move client-side processing rules to

 >
 >   The client uses the "token_binding" TLS extension to indicate the
 >   highest supported Token Binding protocol version and key parameters.
 >
 >   enum {
 >       token_binding(TBD), (65535)
 >   } ExtensionType;
 >
 >   The "extension_data" field of this extension contains a
 >   "TokenBindingParameters" value.

OLD:
The "extension_data" field of this extension contains a "TokenBindingParameters" value.

NEW:

The token_binding extention's "extension_data" field, which is a component of the Extension struct as defined in Section 7.4.1.4 of [RFC5246], contains a "TokenBindingParameters" value, as defined below.

 >   struct {
 >       uint8 major;
 >       uint8 minor;
 >   } ProtocolVersion;

nit: rename this TBProtocolVersion ?

 >
 >   enum {
 >       rsa2048_pkcs1.5(0), rsa2048_pss(1), ecdsap256(2), (255)
 >   } TokenBindingKeyParameters;
 >
 >   struct {
 >       ProtocolVersion token_binding_version;
 >       TokenBindingKeyParameters key_parameters_list<1..2^8-1>
 >   } TokenBindingParameters;
 >
 >   "token_binding_version" indicates the version of the Token Binding
 >   protocol the client wishes to use during this connection.  This
 >   SHOULD be the latest (highest valued) version supported by the
 >   client.  [I-D.ietf-tokbind-protocol] describes version {1, 0} of the
 >   protocol.

add:

    "RFC Editor - please remove for RFC publication:
      Note to implementers - "

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

new parag..

 >   "key_parameters_list" contains the list of identifiers of the Token
 >   Binding key parameters supported by the client, in descending order
 >   of preference.
 >
 >3.  Token Binding Negotiation Server Hello Extension

 >
 >   The server uses the "token_binding" TLS extension to indicate support
 >   for the Token Binding protocol and to select the protocol version and
 >   key parameters.
 >
 >   The server that supports Token Binding and receives a client hello
 >   message containing the "token_binding" extension, will include the
 >   "token_binding" extension in the server hello if all of the following
 >   conditions are satisfied:
 >
 >   1.  The server supports the Token Binding protocol version offered by
 >       the client or a lower version.
 >
 >   2.  The server finds acceptable Token Binding key parameters on the
 >       client's list.
 >
 >   3.  The server is also negotiating Extended Master Secret [RFC7627]
 >       and Renegotiation Indication [RFC5746] TLS extensions.  This
 >       requirement only applies when TLS 1.2 or an older TLS version is
 >       used (see security considerations section below for more
 >       details).
 >
 >
 >   The server will ignore any key parameters that it does not recognize.
 >   The "extension_data" field of the "token_binding" extension is
 >   structured the same as described above for the client
 >   "extension_data".
 >
 >   "token_binding_version" contains the lower of the Token Binding
 >   protocol version offered by the client in the "token_binding"
 >   extension and the highest version supported by the server.
 >
 >   "key_parameters_list" contains exactly one Token Binding key
 >   parameters identifier selected by the server from the client's list.
 >
 >4.  Negotiating Token Binding Protocol Version and Key Parameters  >
 >   It is expected that a server will have a list of Token Binding key
 >   parameters identifiers that it supports, in preference order.  The
 >   server MUST only select an identifier that the client offered.  The
 >   server SHOULD select the most highly preferred key parameters
 >   identifier it supports which is also advertised by the client.  In
 >   the event that the server supports none of the key parameters that
 >   the client advertises, then the server MUST NOT include
 >   "token_binding" extension in the server hello.
 >
 >   The client receiving the "token_binding" extension MUST terminate the
 >   handshake with a fatal "unsupported_extension" alert if any of the
 >   following conditions are true:
 >
 >   1.  The client did not include the "token_binding" extension in the
 >       client hello.

s/client/server/ and s/client hello/ServerHello/ ?  plus some other cleanups, e.g.:

       1.  The server did not include the "token_binding" extension in its
          ServerHello message.

 >   2.  "token_binding_version" is higher than the Token Binding protocol
 >       version advertised by the client.

append:

..in its ClientHello message.

 >
 >   3.  "key_parameters_list" includes more than one Token Binding key
 >       parameters identifier.
 >
 >   4.  "key_parameters_list" includes an identifier that was not
 >       advertised by the client.
 >
 >   5.  TLS 1.2 or an older TLS version is used, but Extended Master
 >       Secret [RFC7627] and Renegotiation Indication [RFC5746] TLS
 >       extensions are not negotiated (see security considerations
 >       section below for more details).
 >
 >   If the "token_binding" extension is included in the server hello and
 >   the client supports the Token Binding protocol version selected by
 >   the server, it means that the version and key parameters have been
 >   negotiated between the client and the server and SHALL be definitive
 >   for the TLS connection.

move 3d chunk below up to here as a "NOTE:"

 >                             In this case, the client MUST use the
 >   negotiated key parameters in the "provided_token_binding" as
 >   described in [I-D.ietf-tokbind-protocol].

Does this stipulation really belong in this spec?  ISTM more appropriate to stipulate this in -tokbind-protocol.

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

there are security considerations here, yes?
how does the app layer know that TB was not negotiated?

 >   Please note that the Token Binding protocol version and key
 >   parameters are negotiated for each TLS connection, which means that
 >   the client and server include their "token_binding" extensions both
 >   in the full TLS handshake that establishes a new TLS session and in
 >   the subsequent abbreviated TLS handshakes that resume the TLS
 >   session.

Move this parag up above as a "NOTE:"

 >
 >5.  IANA Considerations
 >
 >   This document defines a new TLS extension "token_binding", which
 >   needs to be added to the IANA "Transport Layer Security (TLS)
 >   Extensions" registry.
 >
 >   This document establishes a registry for identifiers of Token Binding
 >   key parameters entitled "Token Binding Key Parameters" under the
 >   "Token Binding Protocol" heading.
 >
 >   Entries in this registry require the following fields:
 >
 >   o  Value: The octet value that identifies a set of Token Binding key
 >      parameters (0-255).
 >
 >   o  Description: The description of the Token Binding key parameters.
 >
 >   o  Specification: A reference to a specification that defines the
 >      Token Binding key parameters.
 >
 >   This registry operates under the "Expert Review" policy as defined in
 >   [RFC5226].  The designated expert is advised to encourage the
 >   inclusion of a reference to a permanent and readily available
 >   specification that enables the creation of interoperable
 >   implementations using the identified set of Token Binding key
 >   parameters.
 >
 >   An initial set of registrations for this registry follows:
 >
 >      Value: 0
 >
 >      Description: rsa2048_pkcs1.5
 >
 >      Specification: this document
 >
 >      Value: 1
 >
 >      Description: rsa2048_pss
 >
 >      Specification: this document
 >
 >      Value: 2
 >
 >      Description: ecdsap256
 >
 >      Specification: this document

 >
 >6.  Security Considerations
 >
 >6.1.  Downgrade Attacks
 >
 >   The Token Binding protocol version and key parameters are negotiated
 >   via "token_binding" extension within the TLS handshake.  TLS prevents
 >   active attackers from modifying the messages of the TLS handshake,
 >   therefore it is not possible for the attacker to remove or modify the
 >   "token_binding" extension.  The signature algorithm and key length
 >   used in the TokenBinding of type "provided_token_binding" MUST match
 >   the parameters negotiated via "token_binding" extension.

suggestion for 2nd sentence:
                                                             TLS prevents
    active attackers from modifying the messages of the TLS handshake,
    therefore it is not possible for the attacker to remove or modify the
    "token_binding" extension without detection by the TLS layer, as long
    as the following stipulation regarding the Triple Handshake
    Vulnerability is adhered to.

the last sentence..
                               The signature algorithm and key length
    used in the TokenBinding of type "provided_token_binding" MUST match
    the parameters negotiated via "token_binding" extension.

..seems unnecessary in this spec -- it is more properly addressed in -tokbind-protocol and -tokbind-https.

in any case, rather than saying "signature algorithm and key length", it should say "key parameters"

 >
 >6.2.  Triple Handshake Vulnerability in TLS 1.2 and Older TLS Versions  >
 >   The Token Binding protocol relies on the TLS Exporters [RFC5705] to
 >   associate a TLS connection with a Token Binding.  The triple
 >   handshake attack [TRIPLE-HS] is a known TLS protocol vulnerability
 >   allowing the attacker to synchronize exported keying material between
 >   TLS connections.  The attacker can then successfully replay bound
 >   tokens.  For this reason, the Token Binding protocol MUST NOT be
 >   negotiated with these TLS versions, unless the Extended Master Secret
 >   [RFC7627] and Renegotiation Indication [RFC5746] TLS extensions have
 >   also been negotiated.
 >
 >7.  Acknowledgements
 >
 >   This document incorporates comments and suggestions offered by Eric
 >   Rescorla, Gabriel Montenegro, Martin Thomson, Vinod Anupam, Bill Cox,
 >   Nick Harper and others.
 >
 >8.  References

<snip>

_______________________________________________
Unbearable mailing list
[email-masked]
https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fwww.ietf.org%2fmailman%2flistinfo%2funbearable&data=01%7c01%7cAndrei.Popov%40microsoft.com%7c11e4050ac38d484df3cb08d3597b6197%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=Hlg56W5gcUcHbLHGXxrEqDii%2bKgNCmZelb82R2tmCW8%3d