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

From: Eric Rescorla <[email-masked]>
Date: Tue, 31 Jul 2018 16:14:32 -0700

Eric Rescorla has entered the following ballot position for
draft-ietf-tls-tls13-vectors-06: No Objection

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-tls-tls13-vectors/

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Rich version of this review at:
https://mozphab-ietf.devsvcdev.mozaws.net/D3562

Has anyone checked these besides MT?

COMMENTS
S 3.
>            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}  extract secret "early":
>   
>         salt:  (absent)

ARen't we using the convention 0?

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.

S 3.
>            64 00
>   
>         output (32 octets):  a8 0c b7 d1 5d b3 4a 17 ab b0 c2 37 65 be 68
>            c2 6d 3f 10 da 34 90 5b 09 99 47 e5 5e 37 db 17 b3
>   
>      {server}  send a Finished handshake message

Maybe include more of the finished computaitons.

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.

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?

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?

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.

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.

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

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.

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

---

From: "Salz, Rich" <[email-masked]>
Date: Tue, 31 Jul 2018 23:30:08 +0000

Yes, openssl:

commit d6ce9da49b131cad85da8c94c617febf6c8d9073
Author: Matt Caswell <[email-masked]>
Date:   Thu Jul 19 12:46:02 2018 +0100

    Update the TLSv1.3 test vectors
    
    Use the latest version of the test vectors available in:
    https://tools.ietf.org/html/draft-ietf-tls-tls13-vectors-06
    
    Reviewed-by: Rich Salz <[email-masked]>
    (Merged from https://github.com/openssl/openssl/pull/6746)

---

From: Eric Rescorla <[email-masked]>
Date: Tue, 31 Jul 2018 16:31:16 -0700

Awesome. Thanks!

-Ekr

---

From: Benjamin Kaduk <[email-masked]>
Date: Tue, 31 Jul 2018 18:32:02 -0500

IIUC openssl only tests a subset of the vectors in the document, though.

-Ben

---

From: "Salz, Rich" <[email-masked]>
Date: Wed, 1 Aug 2018 00:01:36 +0000

Good point.  The simple 1RTT handshake I think.

On 7/31/18, 7:32 PM, "Benjamin Kaduk" <[email-masked]> wrote:

    -Ben
    
    >     (Merged from https://github.com/openssl/openssl/pull/6746)
    
    _______________________________________________
    TLS mailing list
    [email-masked]
    https://www.ietf.org/mailman/listinfo/tls

---

From: Martin Thomson <[email-masked]>
Date: Wed, 1 Aug 2018 15:03:13 +1000

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.

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

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

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

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

> This is kind of an odd order.

I don't know how else to order this.

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

> This is the same as the server write side, right?

Same here.

> This isn't calculating the finished but rather the finished keys.

The fix mentioned above should address that.

> You should label this as the binder.

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

> 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.)

> I should have noted this earlier, but it's not really ciphertext.

Fixed. -> "complete record"

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