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

From: Peter Gutmann <[email-masked]>
Date: Wed, 29 Oct 2014 15:30:17 +0000

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

>The *only* difference is (something that doesn't involve the definition of
>length), so the definition of the length is unchanged, therefore it's the one
>from RFC 5246.

No it isn't.  RFC 7366 moves the MAC outside the GenericXXXCipher fragment, so
it's no longer included.  In fact it doesn't even make any sense to include
the MAC length, if you tell your code to MAC the length of the ciphertext plus
the length of the MAC you'll end up MACing 20 or 32 bytes of whatever random
data happens to be present at the end of the ciphertext.  TLSCiphertext.length
is exactly that, the length of the ciphertext, which in RFC 7366 no longer
includes the MAC.  It's not "TLSCiphertext.length + some other data that's
present at the end of the ciphertext".  Even RFC 5246 supports this, it says:

  The encrypted data length (TLSCiphertext.length) [...]

The MAC is no longer encrypted, so it's not part of the encrypted data length.

Peter.

---

From: Nikos Mavrogiannopoulos <[email-masked]>
Date: Wed, 29 Oct 2014 17:04:52 +0100

On Wed, 2014-10-29 at 15:30 +0000, Peter Gutmann wrote:
> it's no longer included.

I don't think it matter that the MAC is outside the GenericXXXCipher
fragment. TLSCiphertext is defined in RFC7366 as follows:

   struct {
          ContentType type;
          ProtocolVersion version;
          uint16 length;
          GenericBlockCipher fragment;
          opaque MAC;
          } TLSCiphertext;

(for block ciphers at least). That TLSCiphertext.length send on the wire
contains both fragment and MAC length, doesn't it?

regards,
Nikos

---

From: [email-masked] (Martin Rex)
Date: Thu, 30 Oct 2014 01:13:46 +0100 (CET)

Nikos Mavrogiannopoulos wrote:
>> No it isn't.  RFC 7366 moves the MAC outside the GenericXXXCipher fragment,
>> so it's no longer included.
> 
> fragment.

On the contrary, this matters a whole lot.

> TLSCiphertext is defined in RFC7366 as follows:
> 
>    struct {
>           ContentType type;
>           ProtocolVersion version;
>           uint16 length;

This is the rfc5246 definition of TLSCiphertext and the explanation
of the "length" element within that structure:

http://tools.ietf.org/html/rfc5246#section-6.2.3

      struct {
          ContentType type;
          ProtocolVersion version;
          uint16 length;
          select (SecurityParameters.cipher_type) {
              case stream: GenericStreamCipher;
              case block:  GenericBlockCipher;
              case aead:   GenericAEADCipher;
          } fragment;
      } TLSCiphertext;

   type
      The type field is identical to TLSCompressed.type.

   version
      The version field is identical to TLSCompressed.version.

   length
      The length (in bytes) of the following TLSCiphertext.fragment.
      The length MUST NOT exceed 2^14 + 2048.

   fragment
      The encrypted form of TLSCompressed.fragment, with the MAC.

In rfc5246, the "length" element of the TLSCiphertext structure
contains the length of the "fragment" element directly following it,
and rfc7366.   But rfc7366 moves the MAC outside of the fragment,
so its length will no longer be included in the TLSCiphertext.length
count.

Curiously, rfc7366 doesn't indicate the real length of the MAC element
in the same fashion that rfc5246 does:

  rfc5246:   opaque MAC[SecurityParameters.mac_length];

  rfc7366:   opaque MAC;

...but then, rfc2246 and rfc4346 used slightly different names/terminology
for the SecurityParameters, and would use:
             opaque MAC[SecurityParameters.hash_size];

-Martin

---

From: Nikos Mavrogiannopoulos <[email-masked]>
Date: Thu, 30 Oct 2014 09:06:01 +0100

> so its length will no longer be included in the TLSCiphertext.length

Correct. Thus the length sent on the wire must also not include the MAC
size. So is my understanding correct that no deployed implementation is
compliant to rfc7366 so far?

regards,
Nikos

---

From: Manuel Pégourié-Gonnard <[email-masked]>
Date: Thu, 30 Oct 2014 10:12:29 +0100

> compliant to rfc7366 so far?

I feel it's the third time this point about what goes on the wire is made (2
times by Nikos and 1 by me) and it didn't get any answer so far, while it is IMO
crucial. I'd really like a direct answer to this.

Let me recap. With RFC 5246 the following are true:
1. TLSCiphertext.length is the length of TLSCiphertext.fragment
2. TLSCiphertext.length is the length that goes on the wire.
3. The length that goes on the wire is the length of the rest of the record¹.

So far I've seen many arguments that it's obvious that (1) must continue to hold
in the context of RFC 7366, but I'd like to see the other points discussed.
Specifically, I hope we all agree that it's impossible for all of the 3 points
to simultaneously remain true in the context of RFC 7366.

The implementation on the eid server made the choice that (1) and (3) remain
true, but (2) becomes false. This contradicts the text of RFC 7366 which says:
"The overall TLS packet [2] is then: [definition of] TLSCiphertext", confirming
that TLSCiphertext is still what goes on the wire.

The choice I spontaneously made when first implementing it (without as much
analysis as I'm doing now) is that (2) and (3) are pretty fundamental
assumptions so it was "obvious" (to me!) that (1) was to be violated by changing
the definition of TLSCiphertext.length to be "length of TLSCiphertext.fragment +
MAC".

Now, I don't want to discuss which of these options is the best² or the most
natural interpretation of RFC 7366. My point is, RFC 7366 is ambiguous since it
forces the reader to guess/choose which of the above three points changes
instead of stating it explicitly. This should be fixed.

Manuel.

-----
¹ While it's not explicitly stated in RFC 5246, it's true of the three record
types it defines, and is a pretty important assumption in many implementations.
Violating this assumption would make any RFC much harder to implement/deploy.

² I think it's clear that unless there already are mutually incompatible
implementations deployed, we really want the new implementations to be
compatible with the deployed ones.

---

From: Manuel Pégourié-Gonnard <[email-masked]>
Date: Thu, 30 Oct 2014 10:19:35 +0100

On 29/10/2014 16:30, Peter Gutmann wrote:
> [...] In fact it doesn't even make any sense to include
> data happens to be present at the end of the ciphertext.

Sure, I'd never think of doing that. But I can certainly tell my code to compute

  MAC_k( seq_num + type + version + length + encrypted_stuff )

with length != length_of(encrypted_stuff). After all, for the MAC function, what
we call "length" is just two meaningless bytes.

Manuel.