Subject: [TLS] Adam Roach's Yes on draft-ietf-tls-record-limit-02: (with COMMENT)

From: Adam Roach <[email-masked]>
Date: Tue, 03 Apr 2018 22:58:40 -0700

Adam Roach has entered the following ballot position for
draft-ietf-tls-record-limit-02: Yes

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-record-limit/

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

Thanks to everyone who contributed work towards this document.

---------------------------------------------------------------------------

§4:

>  MUST NOT send a value higher than the protocol-defined maximum record
>  size unless explicitly allowed by such a future version or extension.

Presumably, recipients MUST gracefully accept values higher than the maximum
record size?  That is implied by this text (and the text that follows), but
given how TLS frequently aborts connections at the first sign of any
irregularity, it's probably worth saying explicitly.

---------------------------------------------------------------------------

§4:

>  a DTLS endpoint that
>  receives a record larger than its advertised limit MAY either
>  generate a fatal "record_overflow" alert or discard the record.

I'm concerned about the interaction between the option to discard the record and
protocols that perform retransmission of lost packets over DTLS (e.g., proposals
such as draft-rescorla-quic-over-dtls). In the case that an oversized packet is
simply discarded, retransmissions of that (presumably still oversized) packet
will take a while to time out (I'm not particularly well-versed in QUIC, but
assume it has characteristics similar to TCP's ~nine-minute timeout), which
would result in really bad user experiences.  Is there rationale for this optionality?
It would seem to be cleaner if the response were simply to always send a fatal
error.

If this optionality is retained, please at least consider adding text describing
the impact of discarding oversized packets when used with a reliable protocol.

---------------------------------------------------------------------------

§4.1:

>  In TLS 1.2, block ciphers allow between 1 and 256 octets of padding.

nit: The formulation "between X and Y" is ambiguous as to whether X and Y are
included in the range. Consider rephrasing as "...from 1 to 256...".

---

From: Martin Thomson <[email-masked]>
Date: Wed, 4 Apr 2018 16:44:36 +1000

Hi Adam,

Thanks for the review.  You picked up on something that was a little
sloppy there.

PR: https://github.com/tlswg/tls-record-limit/pull/19

On Wed, Apr 4, 2018 at 3:58 PM, Adam Roach <[email-masked]> wrote:>
Adam Roach has entered the following ballot position for

I thought that the following text was doing precisely that:

> A server MUST NOT enforce this restriction; a client might advertise a higher limit that is enabled by an extension or version the server does not understand.

A client can enforce the restriction, because it knows the entire set
of possible extensions that determine the maximum size.  I'll concede
that it's a little sloppy in that it doesn't explicitly say whether a
client is expected to police the value.  Is that a change you would
like to see?  As in:

> A client MAY abort the handshake with an illegal_parameter alert if the record_size_limit extension includes a value greater than the maximum record size permitted by the negotiated protocol version and extensions.

Note that this wasn't made a MUST because if there is an extension
that raises the limit, the client has to do a second pass over the
extensions and that's awkward.

> It would seem to be cleaner if the response were simply to always send a fatal

The problem is that you only want to abort if you decrypt the record.
DTLS doesn't kill connections if it receives junk.  In this case, you
would only want to kill the connection if the record is authentic.
But if you have a record size limit, it's usually because you don't
want to decrypt big records.  So you probably won't bother even trying
to decrypt the big packet, even if you could.

So yes, discard ends up looking like packet loss, akin to the loss you
get when a path doesn't support the MTU.  Except that you don't have
any hope of receiving feedback (inasmuch as ICMP is available to DTLS
anyway...).

Given that it's a protocol error, I'm not all that enthusiastic about
fixing the problem.  Even writing it down seems wrong.  "If an
endpoint violates its negotiated constraints, then it can induce
denial of service on itself."  There's only so much defensive design
we can build in.

FWIW, QUIC has the exact same mechanism built in, but you don't
retransmit packets verbatim, so it could rectify itself if it was a
one-off.  More likely, it would compound when more outstanding data is
available to send in every subsequent packet, perpetuating the use of
over-large packets.

---

From: Adam Roach <[email-masked]>
Date: Wed, 4 Apr 2018 02:44:16 -0500

>> A server MUST NOT enforce this restriction; a client might advertise a higher limit that is enabled by an extension or version the server does not understand.

It would, if it were present. The IESG is reviewing version -02 of the 
document, in which that text does not appear. (I see it's in your repo, 
so presumably the -03 version will address the issue I highlight).

> extensions and that's awkward.

I'm okay both with and without the change. What I really wanted was text 
equivalent to what you quote above.

> DTLS doesn't kill connections if it receives junk.

Ah, okay. That makes perfect sense. Under such circumstances, I agree 
that dropping such packets seems to be the least bad path. No document 
change is necessary, although I wouldn't object if you added an 
explanation to the document that says exactly what you say above.

/a

---

From: Martin Thomson <[email-masked]>
Date: Wed, 4 Apr 2018 18:19:27 +1000

On Wed, Apr 4, 2018 at 5:44 PM, Adam Roach <[email-masked]> wrote:
>>> A server MUST NOT enforce this restriction; a client might advertise a
>>> higher limit that is enabled by an extension or version the server does not
>>> understand.
>
> document, in which that text does not appear. (I see it's in your repo, so
> presumably the -03 version will address the issue I highlight).

My fault.  Sean prompted me to push out a new version, then IESG
reviews started coming in, so I held back.  That was the result of a
secdir review.