Re: [TLS] Eric Rescorla's No Objection on draft-ietf-tls-tls13-vectors-06: (with COMMENT)

[Martin Thomson <[e-mail]>]

Thanks for checking on this.  I had a couple of bugs with
deduplication that made some of the values unnecessarily repetitious.
There are a few values around Finished calculation that I added (the
values were there already, but you had to extract them from other
values).

A preview is available (though that uses the draft version numbers and
it doesn't have the Finished values because those changes haven't made
it into NSS yet):
https://tlswg.github.io/draft-ietf-tls-tls13-vectors/draft-ietf-tls-tls13-vectors.html

The diff isn't particularly enlightening because the handshakes are
completely different each time, but you should be able to find the
specific changes if you look for them.

On Wed, Aug 1, 2018 at 9:14 AM Eric Rescorla <[e-mail]> wrote:
> Has anyone checked these besides MT?

I wasn't aware that openssl was using them, but Kazu provided a ton of
feedback when building this.

> COMMENTS
> >         salt:  (absent)
>
> ARen't we using the convention 0?

Yes, but "salt: 0" is hard to distinguish from "salt (1 octet): 00",
so I decided to be more explicit.  I built special code for this, so
it could say anything; what would you prefer it to say?   "(zero)"?  I
tend to think that this is OK, but I don't care what it says.

> S 3.
> >      {server}  extract secret "handshake":
> >
> >         salt (32 octets):  6f 26 15 a1 08 c7 02 c5 67 8f 54 fc 9d ba b6 97
> >            16 c0 76 18 9c 48 25 0c eb ea c3 57 6c 36 11 ba
> >
> >         IKM (32 octets):  81 51 d1 46 4c 1b 55 53 36 23 b9 c2 24 6a 6a 0e
>
> You should specify Z above with the DH.

The IKM is Z.  I didn't see much point in repeating it.  Besides,
because this is driven from logging, it would require a whole bunch of
new logging to support recording of the key exchange.

> S 3.
> >      {server}  send a Finished handshake message
>
> Maybe include more of the finished computaitons.

Yeah, the computed HMAC wasn't logged, so I missed it.  If you are
willing to review the change to NSS to log the result
(https://phabricator.services.mozilla.com/D2589), I can include that
in the block explicitly, which is better.

> S 3.
> >         key output (16 octets):  26 79 a4 3e 1d 76 78 40 34 ea 17 97 d5 ad
> >            26 49
> >
> >         iv info (12 octets):  00 0c 08 74 6c 73 31 33 20 69 76 00
> >
> >         iv output (12 octets):  54 82 40 52 90 dd 0d 2f 81 c0 d9 42
>
> This is kind of an odd order.

I don't know how else to order this.

> S 3.
> >
> >         IKM (32 octets):  81 51 d1 46 4c 1b 55 53 36 23 b9 c2 24 6a 6a 0e
> >            6e 7e 18 50 63 e1 4a fd af f0 b6 e1 c6 1a 86 42
> >
> >         secret (32 octets):  5b 4f 96 5d f0 3c 68 2c 46 e6 ee 86 c3 11 63
> >            66 15 a1 d2 bb b2 43 45 c2 52 05 95 3c 87 9e 8d 06
>
> Aren't these the same as the server too?

Yeah, I had some bugs in the deduplication code that prevented some of
these duplicates from being picked up properly.

> S 3.
> >         key output (16 octets):  c6 6c b1 ae c5 19 df 44 c9 1e 10 99 55 11
> >            ac 8b
> >
> >         iv info (12 octets):  00 0c 08 74 6c 73 31 33 20 69 76 00
> >
> >         iv output (12 octets):  f7 f6 88 4c 49 81 71 6c 2d 0d 29 a4
>
> This is the same as the server write side, right?

Same here.

> S 3.
> >         server read traffic keys)
> >
> >      {client}  derive read traffic keys for application data (same as
> >         server write traffic keys)
> >
> >      {client}  calculate finished "tls13 finished":
>
> This isn't calculating the finished but rather the finished keys.

The fix mentioned above should address that.

> S 4.
> >         secret (32 octets):  04 8b 40 aa 09 ff d4 c6 76 9c 54 1a 2f 46 e2
> >            84 66 06 f7 0d 62 a6 15 97 77 29 c5 b2 81 c7 e7 15
> >
> >      {client}  send a ClientHello handshake message
> >
> >      {client}  calculate finished "tls13 finished":
>
> You should label this as the binder.

Done.  I've added the other inputs to this as well.

> S 4.
> >         output (32 octets):  a8 19 28 e3 08 5c 3a 85 63 ed 82 2d a9 af 7a
> >            b7 1a c5 43 2a 5f 9d 1e 6f 71 32 f1 8b 36 e2 c7 05
> >
> >      {client}  send handshake record:
> >
> >         payload (512 octets):  01 00 01 fc 03 03 88 09 d2 a3 9b f9 ae b3
>
> You should explain why this is 512

I don't think that it's appropriate to document all the design choices
here; that this is padded is one from many.  (It's also very hard to
pick this sort of exceptional condition out for special treatment.)

> S 4.
> >            36 db da 6a 62 6f 02 70 e2 0e eb c7 3d 6f ca e2 b1 a0 da 12 2e
> >            e9 04 2f 76 be 56 eb f4 1a a4 69 c3 d2 c9 da 91 97 d8 2f d3 99
> >            32 00 21 20 3c e6 69 de de c4 4e 5e 75 53 8f cc ab 3d b0 45 fb
> >            5d 21 01 19 99 e1 45 12 ee 3a b3 5f 2a f4 e9
> >
> >         ciphertext (517 octets):  16 03 01 02 00 01 00 01 fc 03 03 88 09
>
> I should have noted this earlier, but it's not really ciphertext.

Fixed. -> "complete record"

> S 5.
> >            f5 71 06 36 c0 5b 88 ab a0 35 38 0c 00 2b 00 03 02 03 04 00 0d
> >            00 20 00 1e 04 03 05 03 06 03 02 03 08 04 08 05 08 06 04 01 05
> >            01 06 01 02 01 04 02 05 02 06 02 02 02 00 2d 00 02 01 01 00 1c
> >            00 02 40 01
> >
> >      {server}  send a ServerHello handshake message
>
> Maybe note that this is a HRR

The tool that produces this doesn't know this, but I will add a note
above the example.