Subject: Re: [TLS] rfc7366: is encrypt-then-mac implemented?

From: Peter Gutmann <[email-masked]>
Date: Thu, 30 Oct 2014 13:40:11 +0000

Manuel Pégourié-Gonnard <[email-masked]> writes:

>Are you refering the "normal" (MtE) case or the EtM case here? 

The EtM case.  In the MtE case what gets MAC'd is the plaintext, i.e. only
TLSCompressed.fragment.  In the EtM case what gets MAC'd is the ciphertext
including padding and other stuff, so GenericBlockCipher.fragment.

>This is supported by the fact that both (what get's MAC'd and what goes in
>the wire) are written as TLSCiphertext.length in the RFC.

The correct notation to use is quite confusing, intuitively you can say "the
length used in the MAC calculation is the length of the data that gets MAC'd",
but specifying it in notation isn't so easy, it's the length of the padded
encrypted TLSCompressed value as well as the length of the IV if TLS 1.1 or
newer is used.  If there's some clear way of saying that that, say, at least
three different people can agree on I'll use it.

>Again, I have no preference on which interpretation is chosen, as long as
>there remains only one arguably valid way to interpret the text.

Sure.

>   MAC(MAC_write_key, seq_num +
>       TLSCipherText.type +
>       TLSCipherText.version +
>       length of ENC(content + padding + padding_length) +
>       IV +
>       ENC(content + padding + padding_length));
>
>    where the length is encoded as two bytes in the usual way.

That's still not quite right though because it excludes the length of the IV
when it's present in TLS 1.1+ (see above).  The intent of using
TLSCipherText.length was to accommodate the different protocol-version-
dependent options.

What about defining it the other way round, instead of saying "the length used
is this and that and sometimes something else", say "the length value used for
the MAC computation is the length as encoded on the wire minus
SecurityParameters.mac_length, since the MAC value is not included in the MAC
computation"?  So perhaps:

  Note that the value of the 'uint16 length' field in the TLSCiphertext record
  differs from the length value used in the MAC calculation, since the
  TLSCiphertext record contains both the ciphtertext and the MAC, while the
  MAC calculation is performed only over the ciphertext.  So the length value
  encoded in the TLSCiphertext record is 'length' while the length value used
  in the MAC calculation is 'length - SecurityParameters.mac_length'.

Peter.

---

From: Henrick Hellström <[email-masked]>
Date: Thu, 30 Oct 2014 14:45:01 +0100

> three different people can agree on I'll use it.

Would it be overkill to define a new TLSIntermediaryCiphertext record type?

---

From: Manuel Pégourié-Gonnard <[email-masked]>
Date: Thu, 30 Oct 2014 14:57:20 +0100

> including padding and other stuff, so GenericBlockCipher.fragment.
Ok. I'm not sure if it's a good idea to use GenericBlockCipher.fragment to
denote this. GenericBlockCipher is a struct defined with a different meaning in
RFC 5246.

> when it's present in TLS 1.1+ (see above).

You're right obviously. So I change my suggestion to:

           In TLS [2] notation, the MAC calculation for TLS 1.0 without
   the explicit Initialization Vector (IV) is:

   MAC(MAC_write_key, seq_num +
       TLSCipherText.type +
       TLSCipherText.version +
       length of (ENC(content + padding + padding_length)) +
       ENC(content + padding + padding_length));

   and for TLS 1.1 and greater with an explicit IV is:

   MAC(MAC_write_key, seq_num +
       TLSCipherText.type +
       TLSCipherText.version +
       length of (IV + ENC(content + padding + padding_length)) +
       IV +
       ENC(content + padding + padding_length));

   MAC(MAC_write_key, seq_num +
       TLSCipherText.type +
       TLSCipherText.version +
       length of (IV + ENC(content + padding + padding_length)) +
       IV +
       ENC(content + padding + padding_length));

since the text is already distinguishing between the two.

>  The intent of using
Ok. I'm afraid here we have a conflict between being concise and being
unambiguous, and I'd err on the side of unambiguous.

> computation"?

Sounds like a good idea, but there is the minor problem that currently the
length on the wire is defined after the MAC computation is defined.

>  So perhaps:
That sounds pretty clear to me. A belt-and-braces approach could even combine
this paragraph with my above suggestion...

Manuel.

---

From: Manuel Pégourié-Gonnard <[email-masked]>
Date: Thu, 30 Oct 2014 15:47:31 +0100

> Would it be overkill to define a new TLSIntermediaryCiphertext record type?
Another (related) idea: explicitly change the definition of
GenericBlockCipher when EtM is in use (which is already implicitly done
at some point). Eg (unchanged text is indicated by a | in the margin,
left there for context at the beginning):

 | Once the use of encrypt-then-MAC has been negotiated, processing of
 | TLS/DTLS packets switches from the standard:
 |
 | encrypt( data || MAC || pad )
 |
 | to the new:
 |
 | encrypt( data || pad ) || MAC
 |
   with the MAC covering the encrypted data as well as metadata, as
   described below.  In TLS [2] notation, the definition of
   GenericBlockCipher is changed, for TLS 1.0 without the explicit
   Initialization Vector (IV), to:

      struct {
          block-ciphered struct {
              opaque content[TLSCompressed.length];
              uint8 padding[GenericBlockCipher.padding_length];
              uint8 padding_length;
          } ciphertext;
          opaque MAC[CipherSpec.hash_size];
      } GenericBlockCipher;

   and for TLS 1.1 and greater with an explicit IV to:

      struct {
          struct {
              opaque IV[SecurityParameters.record_iv_length];
              block-ciphered struct {
                  opaque content[TLSCompressed.length];
                  uint8 padding[GenericBlockCipher.padding_length];
                  uint8 padding_length;
              };
          } ciphertext;
          opaque MAC[SecurityParameters.mac_length];
      } GenericBlockCipher;

   All fields have the same definition as in RFC 2246 and RFC 5246
   respectively, except the MAC which is:

 | MAC(MAC_write_key, seq_num +
 |     TLSCipherText.type +
 |     TLSCipherText.version +
       length of GenericBlockCipher.ciphertext +
       GenericBlockCipher.ciphertext);

   where the length is encoded as two bytes in the usual way.
 | (For DTLS, the sequence number is replaced by the combined epoch and
 | sequence number as per DTLS [4].)
   The input to the MAC function differs from the data sent on the wire
   in two few ways: the first is the prepended sequence number (which,
   with TLS is not on the wire, and with DTLS is on the wire in a
   different position); the second is that the length on the wire
   includes the length of the MAC while the lenght in the input of the
   MAC function does not.

 | Note from the GenericBlockCipher annotation that this only applies to
 | standard block ciphers that have distinct encrypt and MAC operations.
   [rest of the section unchanged]
   [note: no need to recall/redefine TLSCiphertext any more]

Please criticize! (An obvious drawback is it looks quite heavy for an erratum,
compared to the other idea that changes only a few lines.)

Manuel.