# Ben Kaduk's review of draft-ietf-tls-super-jumbo-record-limit CC @kaduk Generally this is in good shape and I support publication, though I do see a couple places to tighten up some potential ambiguity.
## Comments ### Clarify extension non-negotiation in resumption In §3 we note that "This admits the possibility that the extension might not be negotiated during resumption". In conjumction with "Records are subject to the limits that were set in the handshake that produces the keys that are used to protect those records" I *think* that means that if we resume and do not negotiate the extension, the stock (D)TLS 1.3 limits apply (or what we get from "record_size_limit" if negotiated, etc.), but given that this is an edge case that implementors might not test initially, I'd suggest that we dispel any ambiguity. ### Unprotected and handshake-secret messages We say that "Unprotected messages and records protected with early_traffic_secret or handshake_traffic_secret are not subject to the large record size limit", which presumably means (1) they cannot use the TLSLargeCiphertext or varuint-length unified_hdr, and (2) they are subject to the stock (D)TLS 1.3 limits. But, as above, it seems best to dispel ambiguity, especially since this is a divergence from RFC 8449 that affects processing of all protected records. ### Diverging from TLS Presentation Language We informally define in prose the "varuint" type but make no attempt to define it in a way compatible with TLS presentation language, such that we now have an "undocumented" extension to the core TLS presentation language. Do we want to write up a "Variant" structure that formally specifies how varuint works? (I see we reference MLS, RFC 9420, for "as defined in", though MLS does not use the "varuint" name. MLS does not provide a formal description of this type, perhaps because it only appears in the encoding of vector types.) ## Nits Sorry to have these inline rather than via PR; it seemed like a lot of overhead to clone the repo and then I found a couple more. ### Bits vs bytes In §3 we say "The encoded representation MUST use the smallest number of bits necessary to represent the integer value. When decoding, any value that uses more bits than necessary MUST be treated as malformed" but our choices for how many bits to use are only available at byte granularity (admittedly for the overall field size rather than the portion that represents the integer itself); a literal readin gof "smallest number of bits" would have the reader thinking about non-byte-aligned content which is weird. Would we rather talk about "smallest number of octets" here? ### DTLS It's probably worth a pass to check if we want to sprinkle "DTLS" in more places, e.g., the abstract. ### application_traffic_secret I think this was already noted by another reviewer, but when we say "protected with application_traffic_secret" (multiple instances) that's not actually a concept in TLS 1.3; we just have "application_traffic_secret_N". ### length In "MAY generate records protected with application_traffic_secret with inner plaintext that is equal to or smaller than the LargeRecordSizeLimit value it receives from its peer" we're missing a "length" about "inner plaintext length". ### grammar In §4's """When the "large_record_size" has been negotiated record size limit larger than 214 + 1 bytes""" we probably want a "with a" for "negotiated with a record size limit". # Notes This review is formatted in the "IETF Comments" Markdown format, see https://github.com/mnot/ietf-comments . _______________________________________________ TLS mailing list -- [email protected] To unsubscribe send an email to [email protected]
