[Gen-art] Genart last call review of draft-ietf-kitten-tls-channel-bindings-for-tls13-09

[Dale Worley via Datatracker <[e-mail]>]

Reviewer: Dale Worley
Review result: Ready with Nits

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

Document: draft-ietf-kitten-tls-channel-bindings-for-tls13-09
Reviewer: Dale R. Worley
Review Date: 2021-10-14
IETF LC End Date: 2021-10-15
IESG Telechat date: (not known)

Summary:

    This draft is basically ready for publication, but has nits that
    should be fixed before publication.

Nits/editorial comments:

1.  Introduction

   The "unique" channel binding types defined in [RFC5929] were found to

Given that this is the introduction and RFC 5929 is referenced without
its title, it would help the naive reader to change "channel binding
types" to "channel binding types for TLS".

The use of "unique" seems to refer to both (1) the names of the
channel binding types contain the word "unique" and (2) they are
"unique" channel bindings in the terminology of RFC 5056.  It seems
like this sentence should be expanded to make both references clear,
or at least to point to RFC 5056 as well.

   Furthermore, this document updates [RFC5929] by adding an additional
   unique channel binding type that replaces some usage of "tls-unique".

It seems worthwhile to provide the name of the new binding type here.
(And notice that it is "unique" in the sense of RFC 5056 but does not
contain "unique" in its name.  I'm a little surprised you didn't name
it "tls-unique-exporter" to maintain the parallelism.)

2.  The 'tls-exporter' Channel Binding Type

   Context value:  Empty context value.

Since RFC 5705 allows the context value to be absent, and also to be a
byte string of arbitrary length including 0, it might be less
ambiguous to say "Zero-length string".

   SCRAM [RFC5802] and GSS-API over SASL [RFC5801] define "tls-unique"
   as the default channel binding to use over TLS.  As "tls-unique" is
   not defined for TLS 1.3 (and greater), this document updates
   [RFC5801] and [RFC5802] to use "tls-exporter" as the default channel
   binding over TLS 1.3 (and greater).

It seems like this paragraph should be promoted to be its own section
with an extremely descriptive title like "TLS 1.3 with SCRAM and with
GSS-API over SASL".

3.  Security Considerations

   Implementations MUST NOT use the channel binding to
   protect secret information.

It's not clear to me that channel binding has any purpose other than
"to protect secret information".  I suspect that you mean something
like "the channel binding data MUST NOT be used as a key to protect
secret information".  And that statement is essentially the same as
the last sentence of section 3.1; indeed perhaps that sentence should
be moved to section 3 to replace the existing sentence.

3.1.  Use with Legacy TLS

   extra precaution must be taken to ensure that the
   chosen cipher suites always result in unique master secrets.

The appearance of this paragraph in this section suggests (but does
not assert) that in TLS 1.3, the cipher negotiation always results in
unique master secrets.  Indeed, it would be extremely convenient if
(standard-conformant) use of TLS 1.3 always did so, and if so, it would
be convenient to inform the user by asserting that at the end of
section 2 (after moving the current last paragraph to a different
section).

[END]