Subject: [TLS] draft-ietf-tls-record-limit-00

From: Hannes Tschofenig <[email-masked]>
Date: Thu, 31 Aug 2017 12:58:07 +0200

Hi Martin,

In Section 4 you define what is meant by the "maximum size of record"
and in TLS 1.3 it refers to the complete length of TLSInnerPlaintext,
namely

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

For TLS 1.2 you are less explicit. Is it the complete length of the
TLSPlaintext or just the fragment?

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

I prefer it to be the TLSPlaintext structure.

You write:
"an endpoint MUST NOT generate a protected
   record with plaintext that is larger than the RecordSizeLimit value
   it receives from its peer.
"

I think it would be good to say that a TLS stack has to check this limit
and to return an error to the application through the API so that the
application has a possibility to correct the size of the transmitted
data or to select other ways to recover from the situation.

You write:

"Unprotected messages - handshake messages
   in particular - are not subject to this limit.
"

I believe what you are trying to say is that this restriction is not
applicable to the handshake protocol and also not to the alert protocol.
Whether messages are protected or unprotected does not really matter for
the code idea of this document (which is about the buffer sizes).

It might also be good to indicate a recommended value. A meaningful
value is one that is equal to or larger than the handshake messages. If
you have buffer to store the large handshake messages, such as the
Certificate message then you will also be able to have the buffer for
the subsequent data packets. Currently, the minimum size is indicated as
64 bytes, which I believe is a bit unrealistic since even a PSK-based
exchange will need larger handshake message sizes than that.

The RecordSizeLimit is indicated in bytes, correct?

I also think it might be worthwhile to move the following paragraph to
the beginning of the document since it motivates the use of this extension:

   An Authentication Encryption with Additional Data (AEAD) ciphers (see
   [RFC5116]) API requires that an entire record be present to decrypt
   and authenticate it.  Some implementations choose not to implement an
   AEAD interface in this way to avoid this problem, but that exposes
   them to risks that an AEAD is intended to protect against.

Ciao
Hannes

---

From: Martin Thomson <[email-masked]>
Date: Fri, 1 Sep 2017 12:01:29 +1000

Hi Hannes,

Thanks for taking a look at this.

> TLSPlaintext or just the fragment?

The problem here is that TLS 1.2 doesn't have a structure that
corresponds to the thing that we are describing.  As you can see,
TLSPlaintext is an entire plaintext (unprotected) record.  In TLS 1.3,
we don't cover the record header.

> I prefer it to be the TLSPlaintext structure.

I think that the best would could do is identify
TlsPlaintext.fragment.length for TLS 1.2.  That would be more
consistent with the description in 1.3.

Proposed text (since we don't have a repo at this moment):

> In TLS 1.2 and earlier, the limit covers all input to compression and encryption, that is the data that ultimately produces  TLSCiphertext.fragment.

> data or to select other ways to recover from the situation.

I don't think that's necessary.  Today, if an application writes 60k
to the stack, the stack is free to fragment that data as it chooses.
(e.g., OpenSSL has a strategy for fragmentation, NSS fragments at the
maximum record size.)  This draft shouldn't change that.

> the code idea of this document (which is about the buffer sizes).

No.  This is explicitly about protected vs. unprotected.  In TLS 1.2,
this means that you can send the server's first flight in a single
record.  The problem that this solves is the forced blocking of a
connection until an entire encrypted record is available for
decryption.  That doesn't apply for cleartext data.  (Also, the client
can't know the server's value when it sends the ClientHello.)

> exchange will need larger handshake message sizes than that.

Yes, 64 bytes is ridiculously small, but the protocol can work with
that.  I know, my tests verify this.  You just end up sending lots and
lots of fragmented handshake messages.

> The RecordSizeLimit is indicated in bytes, correct?

I can't believe that I missed that.

>    them to risks that an AEAD is intended to protect against.

Good idea.

I have changes for all of these comments staged on a branch on my
local copy.  When our repository-deficit is corrected, I will push
this so that you can review all the changes.

Now I have to go change my patch so that it generates a
record_overflow alert when someone sends a too-large record.  I
completely forgot to add that (I have a patch for that too).