[TLS] Gorry Fairhurst's No Objection on draft-ietf-tls-esni-24: (with COMMENT)

[Gorry Fairhurst via Datatracker <[e-mail]>]

Gorry Fairhurst has entered the following ballot position for
draft-ietf-tls-esni-24: No Objection

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/ 
for more information about how to handle DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-tls-esni/



----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Thank you for preparing this I-D. Thanks also for the TSV-ART review provided
by Tommy Pauly. I agree with that review conclusion: this protocol extension is
defined to work with various transport cases (TLS over TCP, DTLS over UDP,
QUIC, etc), and otherwise does not fundamentally change their properties.

The following comments are not blocking, but I would really appreciate feedback
to help understand what is intended:

1) In section 6.1 what is required for interop? On Offering ECH I see a list of
seven requirements and I thought I saw at least some of these 7 requirements
were necessary for correct operation of the remaining algorithm. (e.g., For me,
Rule 1 suggests a need for TLS 1.3 or higher and if not supplied, a later rule
says this results in a server reject for ECH, etc). However, I found this
sentence after the list: “Note that these rules may change in the presence of
an application profile specifying otherwise.” - I am trying to understand if
any of these requirements are actually required for correct operation: Does
that additional sentence simply mean these are a default and  all these
requirements can be changed ?  or something else?

2) A similar phrase follows the seven requirements in section 6.2. What is
intended to happen here?

===
The following are editorial comments, please consider for the next revision:

1) The appendix contains a normative requirement which seems odd to position
this requirement in an appendix: “Any future information or hints that
influence ClientHelloOuter  SHOULD be specified as ECHConfig extensions. “

2) It would have been nice for me if the acronym KEM was expanded on first use.

3) I would appreciate a forward reference for the first mention of “GREASE ECH”
in the text of 6.1 that says “and sends GREASE ECH otherwise.”  referencing the
text of section 6.3 - I did guess!!!

4) I would appreciate a single sentence or so simply explaining what GREASE ECH
was seeking to achieve in the currently empty text of section 6.2. (A note
saying “see also section 10.10.4” would be super helpful)

5) Please clarify address: “Clients can implement a new transport connection in
a way that best
   suits their deployment.  For example, clients can reuse the same IP
   address when establishing the new transport connection or they can
   choose to use a different IP address if provided with options from
   DNS. “
- I think this means destination IP address (because of the mention of DNS),
but maybe not, because a client can also use alternative valid source
addresses. I would appreciate just a little more clarity - perhaps even just
something like: “Clients can reuse the same IP addresses when establishing the
new transport connection or they can choose to use a different pair of IP
addresses (e.g., if provided with options from DNS).” - if that was what was
intended?

6) To me it is really odd to require a server to be careful!!!
See: “The server MUST be careful not to unnecessarily reject connections if the
same ECHConfig id or keypair is used in multiple ECHConfigs with distinct
public names.” - This seems to me to require some rewording to communicate the
requirement and I assume the care is needed when configuring the server, and
the server MUST NOT unnecessarily reject connections …

7) See: “The design of ECH as specified in this document necessarily requires
changes to client, client-facing server, and backend server.” - would it read
better with the “the” or “a” before client?

8) See: “unencrypted.This means differences” - missing space before period.

9) See: “Notes:  Any notes associated with the entry”  missing final period.

10) Please check use of “which” below and update as needed: One rule I’ve heard
is that uf the details are crucial to the sentence, use that. If they aren’t
crucial, use which: “Section 11.3 describes a set of Reserved extensions which
will never be registered.” /which/that/? “Some use cases which depend on
information ECH encrypts may break
   with the deployment of ECH.”  -  /which/that/ ?
“Depending on implementation details and deployment settings, use
   cases which depend on “ -  /which/that/ ?
“Values which are independent of the true server name, or other
   information the client wishes to protect, MAY be included in
   ClientHelloOuter.”  -  /which/that/ ?
“Values which depend on the contents of ClientHelloInner, such as the
   true server name, can influence how client-facing servers process
   this message.”  -  /which/that/ ?
“A malicious attacker may craft a packet which takes excessive resources
   to decompress”  -  /which/that/ ?
“These requirements prevent an attacker from performing a packet
   amplification attack, by crafting a ClientHelloOuter which
   decompresses to a much larger ClientHelloInner.” -  /which/that/ ?

Thanks again for this detailed and useful specification.
Gorry