Subject: [TLS] TLS 1.3 presentation language

From: Stephen Checkoway <[email-masked]>
Date: Mon, 24 Jul 2017 12:27:11 -0500

For the most part, the presentation language (somewhat informally) described in section 3 and its use throughout the document is clear, but the use doesn't always match the description and some things are written in different ways. I have a few examples below of the issues I've noticed. After the examples, I make concrete suggestions. To keep the following text as uncluttered as possible but still provide easy reference, I've copied the relevant structures definitions from Appendix B at the bottom of this message.

Array lengths have four different formats.

- Literal constants. For example, `opaque Random[32];` or `uint8 CipherSuite[2];`.
- Named, implied constants. For example `coordinate_length` in `UncompressedPointRepresentation` which doesn't appear anywhere else, but is informally described in the following paragraphs. A variant of this is `Hash.length` in the `Finished` struct.
- Qualified field names. I.e., `struct.field`. For example `TLSPlaintext.length` in the `TLSPlaintext` and `TLSInnerPlaintext` structs
- Unqualified field names. I.e., `field` where `field` is in the same structure as the array. For example `length` in `TLSCiphertext`.

I think the unqualified field names use should be removed. We should write `TLSCiphertext.length` instead. I think this is the only place that the unqualified field name appears.

`Hash.length` and `coordinate_length` are both doing the same thing. I think they should be written in the same way. `Hash` isn't a defined structure that has fields so it makes sense to me to go with `hash_length`. (This would also simplify parsing the presentation language.)

Next, most uses of `select` don't match the presentation language's definition. Here's the general definition.

      struct {
          T1 f1;
          T2 f2;
          ....
          Tn fn;
          select (E) {
              case e1: Te1;
              case e2: Te2;
              ....
              case en: Ten;
          } [[fv]];
      } [[Tv]];

The definition isn't explicit, but it appears that the `Te1`, ..., `Ten` should be types but most uses outside of section 3 are _not_ types. The only use of `select` that appears to match the example is in the `Handshake` struct. `KeyShare` and `PreSharedKeyExtension` have  additional fields in their `case` statements and, somewhat confusingly, `EarlyDataIndication`'s `case` statements contain both fields and types.

In addition, the only one of those structs to use the optional label on the `select` is `Handshake`.

I think we should make the presentation language definition actually conform to its use which means we either need to change the definition or change the uses. Options include

1. Add anonymous structs around all of the fields in each `case` statement.
2. Add auxiliary structure definitions (like we have for `struct {} Empty;` already) and use the type names for each `case` statement.
2. Change the definition of the `case` statements from types to a sequence of 0 or more fields. Something like
      struct {
          T1 f1;
          T2 f2;
          ....
          Tn fn;
          select (E) {
              case e1:
                  Te1_1 fe1_1;
                  Te1_2 fe1_2;
                  ....
                  Te1_m fe1_m;
              case e2:
                  Te2_1 fe2_1;
                  Te2_2 fe2_2;
                  ....
                  Te2_m fe2_m;
              ....
              case en:
                  Ten_1 fen_1;
                  Ten_2 fen_2;
                  ....
                  Ten_m fen_m;
          } [[fv]];
      } [[Tv]];

Option 1 adds a bunch of otherwise-useless `struct`s inline with the case statements making them harder to read. Option 2 is going to be slightly easier to read but still adds a bunch of structs. Option 3 complicates the definition of `select` but better conforms with what we actually do.

Two additional benefits of option 3 are that we could remove the optional `select` field label (the `[[fv]]`) and we can get rid of the `Empty` structure. The only `select` that actually uses that label is `Handshake` and I don't think that label is ever referred to.

Concretely, I think we should make the following changes.
1. Replace `length` with `TLSCiphertext.length` in the definition of `TLSCiphertext`.
2. Replace `Hash.length` with `hash_length` throughout (9 instances).
3. Change the definition of `select`'s `case` statements to have 0 or more fields (types and names) and remove the optional label.
4. Change the `select` example to match the new definition.
5. Change `Handshake` by adding field names to each `case` statement. (These could all be `body` or they could be unique.) Remove the `body` label.
6. Delete the `Empty` structure and replace both current uses with the comment `/* Empty */`.

Thoughts?

Copied from Appendix B for reference.

      struct {
          uint8                legacy_form = 4;
          opaque               X[coordinate_length];
          opaque               Y[coordinate_length];
      } UncompressedPointRepresentation;

      struct {
          opaque verify_data[Hash.length];
      } Finished;

      struct {
          ContentType type;
          ProtocolVersion legacy_record_version;
          uint16 length;
          opaque fragment[TLSPlaintext.length];
      } TLSPlaintext;

      struct {
          opaque content[TLSPlaintext.length];
          ContentType type;
          uint8 zeros[length_of_padding];
      } TLSInnerPlaintext;

      struct {
          ContentType opaque_type = 23; /* application_data */
          ProtocolVersion legacy_record_version = 0x0301; /* TLS v1.x */
          uint16 length;
          opaque encrypted_record[length];
      } TLSCiphertext;

      struct {
          HandshakeType msg_type;    /* handshake type */
          uint24 length;             /* bytes in message */
          select (Handshake.msg_type) {
              case client_hello:          ClientHello;
              case server_hello:          ServerHello;
              case end_of_early_data:     EndOfEarlyData;
              case hello_retry_request:   HelloRetryRequest;
              case encrypted_extensions:  EncryptedExtensions;
              case certificate_request:   CertificateRequest;
              case certificate:           Certificate;
              case certificate_verify:    CertificateVerify;
              case finished:              Finished;
              case new_session_ticket:    NewSessionTicket;
              case key_update:            KeyUpdate;
          } body;
      } Handshake;

   struct {
       select (Handshake.msg_type) {
           case client_hello:
               KeyShareEntry client_shares<0..2^16-1>;

           case hello_retry_request:
               NamedGroup selected_group;

           case server_hello:
               KeyShareEntry server_share;
       };
   } KeyShare;

   struct {
       select (Handshake.msg_type) {
           case client_hello:
               PskIdentity identities<7..2^16-1>;
               PskBinderEntry binders<33..2^16-1>;

           case server_hello:
               uint16 selected_identity;
       };

   } PreSharedKeyExtension;

   struct {
       select (Handshake.msg_type) {
           case new_session_ticket:   uint32 max_early_data_size;
           case client_hello:         Empty;
           case encrypted_extensions: Empty;
       };
   } EarlyDataIndication;

-- 
Stephen Checkoway

---

From: Kazu Yamamoto (山本和彦) <[email-masked]>
Date: Tue, 25 Jul 2017 15:29:47 +0900 (JST)

Hi Stephen,

Thank you for your nice work.

> 1. Replace `length` with `TLSCiphertext.length` in the definition of `TLSCiphertext`.

I agree.

> 2. Replace `Hash.length` with `hash_length` throughout (9 instances).

I agree.

> 4. Change the `select` example to match the new definition.

I agree if this means 1 or more. (See below)

> 5. Change `Handshake` by adding field names to each `case` statement. (These could all be `body` or they could be unique.)

Would you show me a concrete definition?

> Remove the `body` label.

I agree.

> 6. Delete the `Empty` structure and replace both current uses with the comment `/* Empty */`.

Empty has a long story.

I proposed None (namely Empty) first but it was not accepted:

	https://github.com/tlswg/tls13-spec/issues/630

But when we discussed EarlyDataIndication, this idea came back:

	https://github.com/tlswg/tls13-spec/issues/861

So, people would want to avoid removing Empty again.

--Kazu

---

From: Stephen Checkoway <[email-masked]>
Date: Tue, 25 Jul 2017 15:28:56 -0500

> I agree if this means 1 or more. (See below)

I went with 0 precisely to avoid Empty, but 1 or more seems fine too.

> Would you show me a concrete definition?

I give three straw men proposals below, proposal I shows this.

> [...]

Okay, I wasn't aware of this discussion. There's no real issue here except that the two `Empty` structures would need to be given field names. See the proposal I below.

Here are three proposals and the impact on the structures containing `select`.

Proposal I. Case statements contain 1 or more fields.

      struct {
          HandshakeType msg_type;    /* handshake type */
          uint24 length;             /* bytes in message */
          select (Handshake.msg_type) {
              case client_hello:          ClientHello         ch;
              case server_hello:          ServerHello         sh;
              case end_of_early_data:     EndOfEarlyData      eed;
              case hello_retry_request:   HelloRetryRequest   hrr;
              case encrypted_extensions:  EncryptedExtensions ee;
              case certificate_request:   CertificateRequest  cr;
              case certificate:           Certificate         c;
              case certificate_verify:    CertificateVerify   cv;
              case finished:              Finished            f;
              case new_session_ticket:    NewSessionTicket    nst;
              case key_update:            KeyUpdate           ku;
          };
      } Handshake;

   struct {

           case server_hello:

   struct {} Empty;

   struct {
       select (Handshake.msg_type) {
           case new_session_ticket:   uint32 max_early_data_size;
           case client_hello:         Empty  empty;
           case encrypted_extensions: Empty  empty;
       };
   } EarlyDataIndication;

   struct {

           case server_hello:

      struct {
          select (certificate_type){
              case RawPublicKey:
                // From RFC 7250 ASN.1_subjectPublicKeyInfo
                opaque ASN1_subjectPublicKeyInfo<1..2^24-1>;

              case X509:
                opaque cert_data<1..2^24-1>;
          };
          Extension extensions<0..2^16-1>;
      } CertificateEntry;

In `Handshake`, the field names themselves are unimportant since they're never referenced. Similarly, the `empty` names aren't important. I find this mildly unpleasant. This involves the fewest changes to the actual structure definitions.

Proposal II. Case statements contain a type. Here, fields are replaced by anonymous structs.

      struct {

   struct {
       select (Handshake.msg_type) {
           case client_hello: struct {
               KeyShareEntry client_shares<0..2^16-1>;
           };
           case hello_retry_request: struct {
               NamedGroup selected_group;
           };
           case server_hello: struct {
               KeyShareEntry server_share;
           };
       };
   } KeyShare;

   struct {} Empty;

   struct {
       select (Handshake.msg_type) {
           case new_session_ticket:   struct { uint32 max_early_data_size; };
           case client_hello:         Empty;
           case encrypted_extensions: Empty;
       };
   } EarlyDataIndication;

   struct {
       select (Handshake.msg_type) {
           case client_hello: struct {
               PskIdentity identities<7..2^16-1>;
               PskBinderEntry binders<33..2^16-1>;
           };
           case server_hello: struct {
               uint16 selected_identity;
           };
       };
   } PreSharedKeyExtension;

      struct {
          select (certificate_type){
              case RawPublicKey: struct {
                // From RFC 7250 ASN.1_subjectPublicKeyInfo
                opaque ASN1_subjectPublicKeyInfo<1..2^24-1>;
              };
              case X509: struct {
                opaque cert_data<1..2^24-1>;
              };
          };
          Extension extensions<0..2^16-1>;
      } CertificateEntry;

This involves no changes to the presentation language but inserts a bunch of otherwise useless `structs`s and braces. Those that have just a single field could be replaced by the field's type, but this is almost certainly more confusing and loses the field's name.

Proposal III. Case statements contain a structure type's name.

      struct {

   struct {
   } ClientKeyShares;

   struct {
       NamedGroup selected_group;
   } HelloRetryRequestGroup;

   struct {
   } ServerKeyShare;

   struct {
       select (Handshake.msg_type) {
           case client_hello:        ClientKeyShares;
           case hello_retry_request: HelloRetryRequestGroup;
           case server_hello:        ServerKeyShare;
       };
   } KeyShare;

   struct {} Empty;

   struct {
       uint32 max_early_data_size;
   } MaxEarlyDataSize;

   struct {
       select (Handshake.msg_type) {
           case new_session_ticket:   MaxEarlyDataSize;
           case client_hello:         Empty;
           case encrypted_extensions: Empty;
       };
   } EarlyDataIndication;

   struct {
   } OfferedIdentities;

   struct {
   } SelectedIdentity;

   struct {
       select (Handshake.msg_type) {
           case client_hello: OfferedIdentities;
           case server_hello: SelectedIdentity;
       };
   } PreSharedKeyExtension;

      struct {
          // From RFC 7250 ASN.1_subjectPublicKeyInfo
          opaque ASN1_subjectPublicKeyInfo<1..2^24-1>;
      } SubjectPublicKeyInfo;

      struct {
          opaque cert_data<1..2^24-1>;
      } CertData;

      struct {
          select (certificate_type){
              case RawPublicKey: SubjectPublicKeyInfo;
              case X509:         CertData;
          };
          Extension extensions<0..2^16-1>;
      } CertificateEntry;

This creates a bunch of auxiliary, named structures to hold the field(s). The `ServerKeyShare` struct could be replaced with `KeyShareEntry`, but this loses the field name.

Having written these all out, I like proposal II the least. Proposals I and III have the advantage that anonymous structures can be removed from the presentation language whereas proposal II introduces the only use of them.

Initially, I thought proposal I would be the best, but now I have a mild preference for proposal III. I think it makes the language simpler over all. It essentially becomes two primitive types (`opaque` and `uint8`) and four type definitions:

1. Fixed-length vectors: `T1 T[v];` (numbers are a special case where `T1` is `uint8`);
2. Variable-length vectors: `T1 T<v1..v2>;`;
3. Enumerated types: `enum { e1(r1), ..., en(rn) [[, (vm)]] } T;`; and
4. Structures:
       struct {
           F1;
           F2;
           ...
           Fm;
           [[
           select (Ts.fs) {
               case e1: S1;
               case e2: S2;
               ...
               case en: Sn;
           };
           ]]
       } T;
   Each field Fi is one of
   - `Ti fi`         (normal fields);
   - `Ti fi = v`     (constants where `Ti` is a number type);
   - `Ti fi[v]`      (fixed-length vectors); or
   - `Ti fi<v1..v2>` (variable-length vectors).

where the symbols have the following meanings:
- S: structure type identifier
- T: type identifier
- e: enum element
- f: field identifier
- r: enum value, either a number or a range `v1..v2`
- v: number

Note that currently, the presentation language doesn't describe fixed- nor variable-length vectors as _structure fields_, it only describes them in the context of defining new types.

Neither the current presentation language nor my proposed one above gives meaning to `uint16 ProtocolVersion;`. Absent a compelling reason for type aliases, I think this should be written as `uint8 ProtocolVersion[2];`

-- 
Stephen Checkoway

---

From: Kazu Yamamoto (山本和彦) <[email-masked]>
Date: Wed, 26 Jul 2017 17:08:56 +0900 (JST)

Hi Stephen,

> Initially, I thought proposal I would be the best, but now I have a
> mild preference for proposal III. I think it makes the language
> simpler over all. It essentially becomes two primitive types
> (`opaque` and `uint8`) and four type definitions:

I like III, too. But I don't want to keep unused labels such as
'server_share'. So, I like the following definition.

   struct {

   struct {
       select (Handshake.msg_type) {
           case client_hello:        ClientKeyShares;
           case hello_retry_request: NamedGroup;
           case server_hello:        KeyShareEntry;
       };
   } KeyShare;

> Neither the current presentation language nor my proposed one above
> gives meaning to `uint16 ProtocolVersion;`. Absent a compelling
> reason for type aliases, I think this should be written as `uint8
> ProtocolVersion[2];`

If I remember correctly, ProtocolVersion is defined:

      struct {
          uint8 major;
          uint8 minor;
      } ProtocolVersion;

And its value is described as { 3, 1 }.

Now, the notation of 0x0301 is used. So, that's why unit16 is used.

--Kazu

---

From: Stephen Checkoway <[email-masked]>
Date: Wed, 26 Jul 2017 10:00:02 -0500

> On Jul 26, 2017, at 03:08, Kazu Yamamoto (山本和彦) <[email-masked]> wrote:
> 
> Hi Stephen,
> 

That would be fine too, but `server_share` really is described in the text and there's no good way to discuss it without having a field name.

> And its value is described as { 3, 1 }.

I believe that's how it was defined in previous versions. Now it's defined (see B.3.1) as

    uint16 ProtocolVersion;

But this notation (a type alias/typedef) isn't defined in the presentation language and isn't, to the best of my knowledge, used anywhere else.

-- 
Stephen Checkoway