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

[Benjamin Kaduk <[e-mail]>]

On Mon, Oct 26, 2020 at 05:38:33PM -0700, Eric Rescorla wrote:
> On Fri, Oct 23, 2020 at 7:13 PM Benjamin Kaduk <[e-mail]> wrote:
> 
> > Hi Ekr,
> >
> > Thanks for chiming in.
> >
> > On Thu, Oct 15, 2020 at 08:59:43AM -0700, Eric Rescorla wrote:
> > >
> > > - I agree with Ben that the current construction has some awkward
> > > properties and that prefixing the length field would remedy that. It's
> > > been quite some time since we had this discussion but as I recall the
> > > rationale was to protect the bits on the wire as-is rather than some
> > > reconstructed version of them (see a number of long discussions on
> > > this topic), so just prefixing the CID length is also not ideal.
> >
> > I think the current scheme is unfortunately using fields put together in a
> > rather different order than the actual bits on the wire (more below).
> >
> 
> Right. I forgot that we also preserved the TLS order of the seqnum.
> 
> 
> > >
> > > This is a little goofy but it has (I think) the properties that (1) the
> > > bytes appear
> > > in the MAC in the order they appear on the wire (2) fixed-length metadata
> > > appears in
> > > the front (the seq_num already does) (3) the duplicated tls12_cid in the
> > > front avoids confusion with MAC input for other records.
> >
> > I like (1) and (2) and agree with (3), though I'm having a little trouble
> > lining up your figure with the DTLSCiphertext structure, which I'll repeat
> > here so I'm not flipping back and forth as much:
> >
> >         struct {
> >             ContentType special_type = tls12_cid;
> >             ProtocolVersion version;
> >             uint16 epoch;
> >             uint48 sequence_number;
> >             opaque cid[cid_length];               // New field
> >             uint16 length;
> >             opaque enc_content[DTLSCiphertext.length];
> >         } DTLSCiphertext;
> >
> > Ah, your proposal looks more natural if I compare it to the current AEAD
> > "additional_data" from the -07:
> >
> > #      additional_data = seq_num +
> > #                        tls12_cid +
> > #                        DTLSCipherText.version +
> > #                        cid +
> > #                        cid_length +
> > #                        length_of_DTLSInnerPlaintext;
> >
> > If I could try to synthesize your key points (as I understand them), hewing
> > more strictly to the "bits on the wire" philosophy would suggest having us
> > use:
> >
> > additional_data:
> > 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 length_of_DTLSInnerPlaintext;
> > };
> >
> >
> Mostly.
> 
> I would expect the length here to not be DTLSInnerPlaintext but rather the
> length field that appears on the wire, as in TLS 1.3. Do you agree with
> that? If not, let's discuss. If so, we can talk about the others.

Good catch, the length on the wire makes more sense.  (I presume I just
cribbed from the -07 here and didn't think hard enough.)

-Ben