Skip to main content

Last Call Review of draft-ietf-tls-esni-23
review-ietf-tls-esni-23-artart-lc-bormann-2025-03-14-00

Request Review of draft-ietf-tls-esni
Requested revision No specific revision (document currently at 24)
Type IETF Last Call Review
Team ART Area Review Team (artart)
Deadline 2025-03-13
Requested 2025-02-20
Authors Eric Rescorla , Kazuho Oku , Nick Sullivan , Christopher A. Wood
I-D last updated 2025-03-23 (Latest revision 2025-03-20)
Completed reviews Dnsdir IETF Last Call review of -23 by R. (Miek) Gieben (diff)
Artart IETF Last Call review of -23 by Carsten Bormann (diff)
Secdir IETF Last Call review of -23 by Adam W. Montville (diff)
Tsvart IETF Last Call review of -23 by Tommy Pauly (diff)
Genart IETF Last Call review of -23 by Stewart Bryant (diff)
Opsdir IETF Last Call review of -24 by Giuseppe Fioccola
Dnsdir Telechat review of -24 by R. (Miek) Gieben
Intdir Telechat review of -24 by Tommy Pauly
Assignment Reviewer Carsten Bormann
State Completed
Request IETF Last Call review on draft-ietf-tls-esni by ART Area Review Team Assigned
Posted at https://mailarchive.ietf.org/arch/msg/art/UPKRBAismhdcLHD-tWeWn0Bl69g
Reviewed revision 23 (document currently at 24)
Result Ready w/nits
Completed 2025-03-14
review-ietf-tls-esni-23-artart-lc-bormann-2025-03-14-00
(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”?