Skip to main content

Telechat Review of draft-ietf-tls-record-limit-02

Request Review of draft-ietf-tls-record-limit
Requested revision No specific revision (document currently at 03)
Type Telechat Review
Team Security Area Directorate (secdir)
Deadline 2018-03-06
Requested 2018-02-16
Authors Martin Thomson
I-D last updated 2018-03-01
Completed reviews Opsdir Telechat review of -02 by Éric Vyncke (diff)
Secdir Telechat review of -02 by Alan DeKok (diff)
Genart Telechat review of -02 by Francis Dupont (diff)
Assignment Reviewer Alan DeKok
State Completed
Request Telechat review on draft-ietf-tls-record-limit by Security Area Directorate Assigned
Reviewed revision 02 (document currently at 03)
Result Has nits
Completed 2018-03-01
  I have reviewed this document as part of the security directorate's ongoing
  effort to review all IETF documents being processed by the IESG. These
  comments were written primarily for the benefit of the security area
  directors. Document editors and WG chairs should treat these comments just
  like any other last call comments.

  The summary of the review is ready with nits.

4.  The "record_size_limit" Extension
        ... an endpoint
   MUST NOT send a value higher than the protocol-defined maximum record
   size ...

Comment: That's good, so later we have:

   The record size limit can interact with the maximum transmission unit
   (MTU) in DTLS, but it is a separate and independent constraint on
   record size.

Nit:  perhaps say that this is an *additional* constraint.  Which is (to me) a
clearer indication that both constraints must be matched.  "independent"
constraints sound like not only they vary independently, but that they can be
applied independently (i.e. separately).  Saying "additional" constraint is a
clearer indication that both constraints must be applied at the same time.

   In particular, it is not appropriate to use the record
   size limit in place of path MTU detection.

Q: How would that be done?  I don't mean that the document needs to explain how
to do something wrong.  I mean that it would be good to explain the
misunderstanding which would lead to using record size limit in place of path
MTU detection.

  e.g. "the reception of an illegal_parameter error on a session gives no
  information about the allowed MTU size"

   The record size limit is
   a fixed property of an endpoint that is set during the handshake and
   fixed thereafter.  In comparison, the MTU is determined by the
   network path and can change dynamically over time.

Comment:  it would be good to give guidance on what to do here, and what
happens in error cases.

  e.g. should the record size limit to be set at the start of a session to the
  *smallest expected MTU*?  If so, what are the side effects?

  Or what about this - the MTU is large at the start of a session, and a record
  size limit is negotiated which matches that MTU.  At some later time, the MTU
  changes to a lower value than the negotiated record size limit.

  What happens then?  The application may receive an ICMP message indicating
  destination unreachable, with a code indicating fragmentation needed.  If
  Don't Fragment (DF) is not set.. the packet cannot be sent.

  Should the session be closed?  If not, why not?  If so, should the
  application keep track of minimum MTU across multiple sessions, and negotiate
  a value of record size limit which is more likely to work?

  It would be good to have guidance for edge / error conditions.  They tend to
  be a source of network problems and interoperability issues.

5.  Deprecating "max_fragment_length"
        ...A server that supports the "record_size_limit"
   extension MUST ignore and "max_fragment_length"

Nit: this is probably "any" instead of "and".

  7.  IANA Considerations
   This document registers the "record_size_limit" extension in the TLS
   "ExtensionType Values" registry ...

   In the same registry, the "max_fragment_length" [[has been|will be]]
   changed to a status of not recommended.

Comment: the registry has no "status" column.

  It may be useful to update that registry to add a "deprecated" note, perhaps
  as is done with the ICMP parameters registry:

  In which case the "max_fragment_length" entry should be changed to:

Value                           1
Extension name          max_fragment_length (deprecated)
Reference                       RFC6066, draft-ietf-tls-record-limit