Subject: Re: [TLS] Fwd: Re: AD review of draft-ietf-tls-dtls-connection-id-07

From: Benjamin Kaduk <[email-masked]>
Date: Mon, 16 Nov 2020 22:07:45 -0800

On Fri, Oct 30, 2020 at 01:28:12PM +0100, Achim Kraus wrote:
> Hi Ekr,
> 
> > As for EtM
> >
> > Encrypt-then-MAC:
> > struct {
> >    uint8 marker = tls12_cid;
> >    uint8 cid_len;
> >    uint8 content_type = tls12_cid;      \
> >    uint16 DTLSCiphertext.version;       |  appears on wire
> >    uint64 seq_num; // includes epoch    |
> >    opaque cid[cid_len];                 /
> >    uint16 iv_length;
> >    opaque IV[iv_length];
> >    uint16 enc_content_length;
> >    opaque enc_content[enc_content_length];
> > };
> >
> 
> I failed to understand the reasons behind this proposal.
> 
> 1. Why should the "marker" be added, if the "content_type" is already in
> the MAC, and this special MAC is only applied for tls12_cid records.
> What is the intended benefit of that?

This is another general hygiene item; we are preserving the invariant that
the first byte of the MAC input is the content type -- this is at present
(IIRC) invariant across all TLS versions and MtE/EtM, and not something to
change lightly.

It appears twice so that we keep the entire "bits on wire" together, and
there's not (IMO) much incentive to try to justify deduplicating the two
locations of tls12_cid, since the incremental cost of MACing another byte
is pretty small.

> 2. Why should a "uint16 iv_length" be added?
> 2.a If it should be added, why as "uint16" instead of "uint8"
> 2.b If it should be added, why in the middle? It's not on the wire and
> so I would assume, if at all, to have that at the begin.

(Peter answered the "why have it at all" question.)

I think you are right that putting it before the "appears on wire" bits
makes a bit more sense.

I think it was uint16 out of chance/laziness, as I split the combined
"length of (IV + DTLSCiphertext.enc_content)" and just used the same width
of length field for both IV and enc_content.  Making it uint8 is probably
an improvement.

Thanks for taking a careful look,

Ben

---

From: Achim Kraus <[email-masked]>
Date: Tue, 17 Nov 2020 10:09:34 +0100

Hi Ben,

> (IIRC) invariant across all TLS versions and MtE/EtM, and not something to

"All TLS versions"?

TLS 1.2, RFC 5246

https://tools.ietf.org/html/rfc5246#section-6.2.3.1

The MAC is generated as:

       MAC(MAC_write_key, seq_num +
                             TLSCompressed.type +
                             TLSCompressed.version +
                             TLSCompressed.length +
                             TLSCompressed.fragment);

https://tools.ietf.org/html/rfc5246#section-6.2.3.3

The additional authenticated data, which we denote as
    additional_data, is defined as follows:

       additional_data = seq_num + TLSCompressed.type +
                         TLSCompressed.version + TLSCompressed.length;

So, as far as I see, at least the TLS 1.2 variant doesn't do that.
RFC6347 doesn't change that, so DTLS 1.2 also doesn't do that.

Maybe you precise the "all" to a set of versions to make it easier to
check it?

best regards
Achim

---

From: Benjamin Kaduk <[email-masked]>
Date: Tue, 17 Nov 2020 01:16:00 -0800

> > (IIRC) invariant across all TLS versions and MtE/EtM, and not something to

Apparently I did not remember correctly.

Sorry about that.

-Ben

---

From: Eric Rescorla <[email-masked]>
Date: Tue, 17 Nov 2020 01:46:07 -0800

Argh, Good catch. I'll revise the proposal in light of this point.

-Ekr