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

From: Dale Worley via Datatracker <[email-masked]>
Date: Thu, 14 Oct 2021 17:20:58 -0700

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]

---

From: "Sam Whited" <[email-masked]>
Date: Fri, 15 Oct 2021 08:57:45 -0400

Thank you for the detailed review! I believe I have addressed your
feedback in a draft that I will upload shortly, but have a few
comments (inline).

> types" to "channel binding types for TLS".

Since this document only updates the general-purpose "tls-unique" and
does not specifically define a new version of "tls-unique-for-telnet" I
have just changed the use of "unique" here to "tls-unique" to refer to
the specific channel binding type. Hopefully this makes it clear that it
is a TLS channel binding without the redundant "tls-unique channel
binding for TLS", but I'm happy to work on this further.

> it "tls-unique-exporter" to maintain the parallelism.)

I'd be happy to change that if everyone wants, I have no preference.

> (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

This one I had a lot of trouble with. I tried to put in some new
language, but it feels out of place to me somehow. I'm not sure that
this document should make assertions about the correctness of TLS 1.3,
as well vetted as it has been, so I tried to phrase it in terms of "this
mechanism is useful so long as this property holds", which seems like it
might belong in security considerations, not the registration section?

—Sam

-- 
Sam Whited

---

From: [email-masked] (Dale R. Worley)
Date: Fri, 15 Oct 2021 23:25:01 -0400

> might belong in security considerations, not the registration section?

This is probably the only really significant point in my review ...  I
can understand your caution here.  It seems to me that the ideal
solution is for TLS 1.3 to have been explicitly designed so that there
are unique master secrets, and then you just reference that.  Now it
seems that everybody thinks TLS 1.3 has this property, so I'd expect
that was an explicit design goal, and it would be documented somewhere.
And then this document could just point to that.

Dale

---

From: Benjamin Kaduk <[email-masked]>
Date: Sun, 17 Oct 2021 21:50:08 -0700

> And then this document could just point to that.

The last paragraph of
https://datatracker.ietf.org/doc/html/rfc8446#appendix-D discusses how TLS
1.3 should be treated as always providing the RFC 7627 "extended master
secret" behavior; that RFC, in turn, discusses the (non-)uniqueness of the
master secret in the absence of the "extended" behavior.

https://datatracker.ietf.org/doc/html/rfc8446#appendix-D also discusses the
uniqueness of TLS 1.3 secrets/keys.

-Ben

---

From: "Sam Whited" <[email-masked]>
Date: Mon, 18 Oct 2021 06:43:59 -0400

Looks like the same link got included twice? But thank you, I'll try to
rework the language again to reference that.

—Sam

On Mon, Oct 18, 2021, at 00:50, Benjamin Kaduk wrote:
>> >> 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).
>> >
>> > language, but it feels out of place to me somehow. I'm not sure
>> > that this document should make assertions about the correctness of
>> > TLS 1.3, as well vetted as it has been, so I tried to phrase it in
>> > terms of "this mechanism is useful so long as this property holds",
>> > which seems like it might belong in security considerations, not
>> > the registration section?
>>
>> This is probably the only really significant point in my review ...
>> I can understand your caution here.  It seems to me that the ideal
>> solution is for TLS 1.3 to have been explicitly designed so that
>> there are unique master secrets, and then you just reference that.
>> Now it seems that everybody thinks TLS 1.3 has this property, so I'd
>> expect that was an explicit design goal, and it would be documented
>> somewhere. And then this document could just point to that.
>
> The last paragraph of
> 1.3 should be treated as always providing the RFC 7627 "extended
>   master secret" behavior; that RFC, in turn, discusses the (non-
>   )uniqueness of the master secret in the absence of the "extended"
>   behavior.
>
> https://datatracker.ietf.org/doc/html/rfc8446#appendix-D also
> discusses the uniqueness of TLS 1.3 secrets/keys.
>
> -Ben

-- 
Sam Whited