Telechat Review of draft-ietf-tls-record-limit-02
review-ietf-tls-record-limit-02-secdir-telechat-dekok-2018-03-01-00
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 |
review-ietf-tls-record-limit-02-secdir-telechat-dekok-2018-03-01-00
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: https://www.iana.org/assignments/icmp-parameters/icmp-parameters.xhtml 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