Subject: [kitten] Comments on draft-tls-channel-bindings-for-tls13

From: Robbie Harwood <[email-masked]>
Date: Wed, 18 Nov 2020 14:49:02 -0500

(This is a review of
https://datatracker.ietf.org/doc/draft-ietf-kitten-tls-channel-bindings-for-tls13/
version -00.)

First: thanks for putting this together.  I have a couple comments, but
nothing about the document seems immediately wrongheaded to me.

In "1. Introduction":

> After problems were found with the channel binding types defined in
> [RFC5929] they were not defined for TLS 1.3 (see [RFC8446] section
> 7.5).

This document doesn't seem to address what the problems with the
previous channel binding types were.  It also doesn't appear in the
referenced section (i.e., RFC 8446 section 7.5 doesn't seem to have this
information; moreover, that section doesn't reference RFC 5929).

RFC 8446 section C.5 does call out cbs as a protection mechanism, but
merely notes that they're not implemented for 1.3, so it's probably not
a case of wrong section.

I think it would be good to explain what the problems were by way of
background in the introduction.

In "2. The 'tls-exporter' Channel Binding Type":

> IANA will register the 'tls-exporter' channel binding type to match the
> description below.

This feels redundant with the formal IANA considerations section later
in the document.  I think it would be more helpful to orient it toward
explaining the cb type as pertains to RFC 5705 and RFC 8446.

> When TLS renegotiation is enabled channel binding using the "tls-exporter"
> type is not define and MUST NOT be supported.

Hmm, this text is a bit awkward right now.  I suggest something like:

    When TLS renegotiation is enabled, implementations MUST NOT support
    channel binding using the "tls-exporter" type as it is not defined.

In "3. Security Considerations":

> While it is possible to use this channel binding mechanism with TLS versions
> below 1.3

Curious - is there a practical reason to want this CB type with TLS <=
1.2?

Please also see the idnits here:
https://www6.ietf.org/tools/idnits?url=https://www.ietf.org/archive/id/draft-ietf-kitten-tls-channel-bindings-for-tls13-00.txt

For completeness, they are:

> ** The abstract seems to contain references ([RFC5056], [RFC8446]),
> which it shouldn't.  Please replace those with straight textual
> mentions of the documents in question.
>
> == The 'Updates: ' line in the draft header should list only the
> _numbers_ of the RFCs which will be updated by this document (if
> approved); it should not include the word 'RFC' in the list.
>
> -- The document date (December 2020) is 27 days in the future.  Is
> this intentional?

Thanks,
--Robbie

---

From: "Sam Whited" <[email-masked]>
Date: Wed, 18 Nov 2020 21:41:32 -0500

Responses inline:

On Wed, Nov 18, 2020, at 14:49, Robbie Harwood wrote:
> previous channel binding types were.

I'll add some text here. The TL;DR is that without the extended master
secret fix from RFC7627 tls-unique is not actually unique. I also recall
someone suggesting that the unique data really wasn't long enough, which
seems likely, but I don't have a citation for that off the top of my
head so I'll stick to mentioning the triple handshake vulnerability and
the extended master secret extension and leave it at that.

> It also doesn't appear in the referenced section (i.e., RFC 8446
> section 7.5 doesn't seem to have this information; moreover, that
> section doesn't reference RFC 5929).
>
> merely notes that they're not implemented for 1.3, so it's probably
> not a case of wrong section.

That was indeed supposed to be a reference to C.5. That's the only
reference I'm aware of for why the two CB mechanisms from 5929 aren't
defined in 1.3. This is a very confusing off-the-cuff reference in the
TLS 1.3 spec, but it's what I'm basing the entire need for a new CB
mechanism on.

> In "2. The 'tls-exporter' Channel Binding Type": … This feels
> redundant with the formal IANA considerations section later in the
> document.  I think it would be more helpful to orient it toward

I did it this way because RFC5929 did it this way, but I agree it's
awkward. I'll rewrite this to be descriptive and give a bit of
background on EKM and TLS 1.3 instead.

>     When TLS renegotiation is enabled, implementations MUST NOT
>     support channel binding using the "tls-exporter" type as it is not
>     defined.

Sounds good to me; thanks.

> Curious - is there a practical reason to want this CB type with TLS <=

Not that I can think of. I suppose it would be nice to have a single
channel binding type that works regardless of the TLS version being
used, but it seems simpler to just let TLS 1.2 die and not worry about
it since channel binding isn't widely supported right now anyways.

> Please also see the idnits here:

Will do, thanks.

I'll have a new version up soon.

—Sam

-- 
Sam Whited

---

From: Benjamin Kaduk <[email-masked]>
Date: Wed, 25 Nov 2020 18:21:34 -0800

> TLS 1.3 spec, but it's what I'm basing the entire need for a new CB

I would have to go trawl through the archives to be sure, but in TLS 1.3
the first Finished is sent by the server, and does not cover the client
Certificate.  So the RFC 5929 tls-unique is not fit for purpose for
purposes of client authentication.
(And the exporter interface has better cryptographic hygeine in general,
though that's not exactly a compellling argument to disallow its usage
entirely.)

> Sounds good to me; thanks.

It may (or may not) be worth a reminder that any renegotiation is
pre-TLS-1.3.

> it since channel binding isn't widely supported right now anyways.

Indeed, and I've been attempting to promote this unified usage.
(Though the main case I can remember doing so, draft-ietf-nfsv4-rpc-tls,
has since gone 1.3-only, so it doesn't matter there anymore.)

Thanks,

Ben

> _______________________________________________
> Kitten mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/kitten

---

From: "Sam Whited" <[email-masked]>
Date: Thu, 26 Nov 2020 13:43:55 +0000

Responses inline:

On Thu, Nov 26, 2020, at 02:21, Benjamin Kaduk wrote:
> I would have to go trawl through the archives to be sure, but in TLS
> 1.3 the first Finished is sent by the server, and does not cover the
> client Certificate.  So the RFC 5929 tls-unique is not fit …

Not to mention that in some situations it may not even exist (ie. when
0-RTT is enabled). If you find a citation for this I'm game to expand
this section a bit to cover why it wasn't great for TLS 1.3
specifically.

> It may (or may not) be worth a reminder that any renegotiation is

Probably a good idea; I'll ear-mark that for the next version. I don't
have it in front of me, but I vaguely remember mentioning that it
wasn't defined when renegotiation was in use, but since we don't define
it for TLS 1.2 anyways that's probably redundant. I'll try to clear
that up anyways.

> Indeed, and I've been attempting to promote this unified usage.

If you can think of a way to do it safely I'm not against defining this
for TLS 1.2, I just don't think I personally have the expertise to do so
since the key exchange mechanisms may or may not be safe for channel
binding and it feels bad to have a CB mechanisms that has to have a big
allow list of ciphers and lots of warnings like "Don't do this if you're
using RSA key exchange, it won't actually be a valid binding", etc.

—Sam