Thank you for your close review. I have made a PR in response to these comments: PR https://github.com/tlswg/draft-ietf-tls-esni/pull/648
Detailed responses below. ## Editorial > 10.1 »Security and Privacy Goals« starts with definitions > (active/passive) that aren’t really Security/Privacy goals. > Since “passive” is then used distinguishingly in only three places, it > is not clear that this passage is particularly useful. > It is also puzzling to sort filtering middleboxes under “passive”. They are passive from the perspective of ECH's threat model; this is part of why I think this paragraph is useful. > 10.1: I cannot find a discussion of a “threat model” under this name > in RFC 8446. This is a citation for TLS 1.3. I have added a citation for 3552 behind threat model. > 10.1: This section starts using the term “host” as a noun with a > distinguishing quality that in this document is probably related to > server_name, but not explained (“host” as a noun does not occur in RFC > 8446). I changed the term "host" to "server name". > > 10.2 appears to address integrity only, where the heading might > suggest some discussion about confidentiality. The final sentence addresses confidentiality or the lack thereof. > I don’t understand the last sentence of 10.3 (“servers with high > load”…). I rewrote this a bit. > 11.3: After »A "Y" or "N" value indicating if the extension is TLS WG recommends > that the extension be supported.« — can’t parse »Adding a value with a > value of "Y" requires Standards Action .« -- s/value/registration/ ? Fixed. > 10.11: I would have expected a brief discussion on how granularizing > the padding to 32 byte steps cannot be used by an attacker to extract > information beyond that granularity (by causing the client to vary the > size before padding, straddling a step). The attacker doesn't control CH at all, so I'm not sure why this would be needed. > ## Nits > > Please do a pass of replacing “which” with “that” where a restrictive > clause is intended (usually easy to find by the lack of a comma, about > two dozen occurrences) This is one of those folklore grammar rules that does not match actual English usage, like not splitting infinitives. http://itre.cis.upenn.edu/~myl/languagelog/archives/001484.html > Thank you for the many section references; two more could be used in > »<p id="section-10.8-3">Note that, if the cookie includes a key name, > analogous to Section 4 of« and »<p id="section-6.1-7.3.1”>[…] (see > Appendix D.4 of <span>[<a href="#RFC8446" class="cite > xref">RFC8446</a>]</span>) when ECH is negotiated.« Done. > There are at least 17 occurrences of ‘bytes’ for counting lengths in > bytes and 3 of ‘octet’/‘octets’. I changed these all to "bytes". > Please expand HRR on first use. Done. > Please define “ECH key” (8 occurrences) and “ECH record” (5 > occurrences). I am replacing ECH record with HTTPS record, which is already defined. Defined ECH key. > Please define “application profile” and “application profile standard” > (10.4 uses “application using (D)TLS” — is that the same?) I don't believe this is a necessary change. TLS 1.3 already uses this term without definition in RFC 8446 S 9.1. > 1: »the SNI remains the primary explicit signal used to determine the > server's identity.« -- used by whom? By observers. Addresed. > Figure 2 uses private.example.com in the second variant where > private.example.org is used in the first; may want to use .org here as well Fixed. > 5: it is slightly surprising that the definition list explaining > config_id and cipher_suite does this in an order different from the > TPL in the figure above I take your point, but I think this is reflects the difference between code (avoiding forward references) and text (top down). > 5.1: The first figure has a cliff-hanger (length_of_padding); maybe > add a reference to 6.1.3. Done. > 5.2: »This value does not include Handshake structure's four-byte > header in TLS,« Removed the comma. > 6.1.1: »Including ClientHelloOuterAAD as the HPKE AAD binds the > ClientHelloOuter to the ClientHelloInner, this preventing attackers« > s/this/thus/ ? Fixed. > 6.1.3: »through -their length« Fixed. > 6.1.3: »that have sensitive-length fields« (hyphen ➔ space) This appears to be some kind of rendering error. It's fine in the markdown. I'll trust the RPC to fix it. > 6.1.6 »yield a tracking vector« — for whom? > ➔ supply the server with a tracking vector? I think this is OK as-is. > 6.1.7: »reference identity« -- define? Added a reference to RFC 6125. > 6.1.7: »(e.g. [RFC3986], Section 7.4 and [WHATWG-IPV4])« > The "and" does not work here Fixed. > 6.2.2: »Correctly-implemented client will ignore those extensions.« Fixed. > 10.9: could use superscript in place of 2^64 > (This of course only applies to usage in text, which seems to be the > only one here.) Markdown again. I'll let the RPC address. > 10.10.4: »out-of-scope. including, « Fixed. > I assume “page” in 11.3 is what is now called “registry group”? Yes. Fixed. On Fri, Mar 14, 2025 at 5:35 AM Carsten Bormann via Datatracker < nore...@ietf.org> wrote: > Reviewer: Carsten Bormann > Review result: Ready with Nits > > > (Insert ARTART boilerplate here.) > > Thank you for this draft, it is in very good shape. > The document is explicit about the different configurations the > protocol can be run in, the participants, their roles, the security > and privacy objectives that can be achieved as well as the security > considerations, and it discusses (and addresses!) deployment > considerations. > > This specification is ready with nits. > A few editorial observations and a few nits below. > > ## Editorial > > 10.1 »Security and Privacy Goals« starts with definitions > (active/passive) that aren’t really Security/Privacy goals. > Since “passive” is then used distinguishingly in only three places, it > is not clear that this passage is particularly useful. > It is also puzzling to sort filtering middleboxes under “passive”. > > 10.1: I cannot find a discussion of a “threat model” under this name > in RFC 8446. > > 10.1: This section starts using the term “host” as a noun with a > distinguishing quality that in this document is probably related to > server_name, but not explained (“host” as a noun does not occur in RFC > 8446). > > 10.2 appears to address integrity only, where the heading might > suggest some discussion about confidentiality. > > I don’t understand the last sentence of 10.3 (“servers with high > load”…). > > 11.3: After »A "Y" or "N" value indicating if the extension is TLS WG > recommends > that the extension be supported.« — can’t parse »Adding a value with a > value of "Y" requires Standards Action .« -- s/value/registration/ ? > > 10.11: I would have expected a brief discussion on how granularizing > the padding to 32 byte steps cannot be used by an attacker to extract > information beyond that granularity (by causing the client to vary the > size before padding, straddling a step). > > ## Nits > > Please do a pass of replacing “which” with “that” where a restrictive > clause is intended (usually easy to find by the lack of a comma, about > two dozen occurrences) > > Thank you for the many section references; two more could be used in > »<p id="section-10.8-3">Note that, if the cookie includes a key name, > analogous to Section 4 of« and »<p id="section-6.1-7.3.1”>[…] (see > Appendix D.4 of <span>[<a href="#RFC8446" class="cite > xref">RFC8446</a>]</span>) when ECH is negotiated.« > > There are at least 17 occurrences of ‘bytes’ for counting lengths in > bytes and 3 of ‘octet’/‘octets’. > > Please expand HRR on first use. > > Please define “ECH key” (8 occurrences) and “ECH record” (5 > occurrences). > > Please define “application profile” and “application profile standard” > (10.4 uses “application using (D)TLS” — is that the same?) > > 1: »the SNI remains the primary explicit signal used to determine the > server's identity.« -- used by whom? > > Figure 2 uses private.example.com in the second variant where > private.example.org is used in the first; may want to use .org here as > well > > 5: it is slightly surprising that the definition list explaining > config_id and cipher_suite does this in an order different from the > TPL in the figure above > > 5.1: The first figure has a cliff-hanger (length_of_padding); maybe > add a reference to 6.1.3. > > 5.2: »This value does not include Handshake structure's four-byte > header in TLS,« > > 6.1.1: »Including ClientHelloOuterAAD as the HPKE AAD binds the > ClientHelloOuter to the ClientHelloInner, this preventing attackers« > s/this/thus/ ? > > 6.1.3: »through -their length« > > 6.1.3: »that have sensitive-length fields« (hyphen ➔ space) > > 6.1.6 »yield a tracking vector« — for whom? > ➔ supply the server with a tracking vector? > > 6.1.7: »reference identity« -- define? > > 6.1.7: »(e.g. [RFC3986], Section 7.4 and [WHATWG-IPV4])« > The "and" does not work here > > 6.2.2: »Correctly-implemented client will ignore those extensions.« > > 10.9: could use superscript in place of 2^64 > (This of course only applies to usage in text, which seems to be the > only one here.) > > 10.10.4: »out-of-scope. including, « > > I assume “page” in 11.3 is what is now called “registry group”? > > > >
_______________________________________________ TLS mailing list -- tls@ietf.org To unsubscribe send an email to tls-le...@ietf.org
