TLS Encrypted Client Hello
draft-ietf-tls-esni-25

Thanks to Adam Montville for their secdir review.

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

I've previously reviewed this document and have very few additional comments; these comments can be incorporated or ignored at the authors' and responsible AD's discretion.

6.1.8: "has been forced to change" imputes external events that aren't relevant to the protocol. The server's configuration may have changed since the client received the retry configs; the client doesn't need to speculate on why.

10.9 notes that there's no collision between ECH acceptance (in 1.3) and downgrade protection (in <1.3) because of the version scoping. It's worth noting, however, that this forecloses using the same approach to guard against downgrades to 1.3 from future TLS versions.

Hi Eric, Kazuho, Nick, and Christopher,

Thank you for the effort put into this specification.

Also, thanks to Giuseppe Fioccola for the OPSDIR review.

The document is well-written with a good discussion on deployment considerations and impacts on some existing use of SNI (network management, attack detection, etc.). Thanks for the clarification provided in follow-up discussion. I trust the authors are tracking the changes.

===OLD BALLOT===

Please find below some few DISCUSS points and a set of comments.

# Require or not require DNS/9460+ECH-IN-DNS

## Recommendation?

CURRENT:
   This document defines the ECH configuration's format, but
   delegates DNS publication details to [RFC9460].   

9460/ECH-IN-DNS are cited as normative. This "seems" to assume that making use of ECH requires this specific discovery. If that is a recommended deployment approach, then the text should say so explicitly. 

Such recommendation does not prevent use of ECH independent of the ECH-IN-DNS. Existing text is clear on this matter, Section 3.2:

   Other delivery mechanisms are also possible.  For example,
   the client may have the ECH configuration preconfigured.

## ECH use with Encrypted DNS Server

Note also that other mechanisms such as DNR (RFC9463) can be used to learn ECH service parameter to connect to a DoH server itself. This is not possible directly with 9460 for the connection with an encrypted DNS resolver.

# Per-server configuration

The spec defines a generic ECH Config structure that can be used by clients. However, there is no discussion how this can be indexed locally and bind it to a server. IMO, this should be independent of the ech discovery mechanism.

# (apparent) Inconsistency vs ECH-IN-DNS?

ECH spec says the following in Section 8.1

   Thus server operators SHOULD ensure servers understand a given set of ECH keys before advertising
   them.  

ECH-IN-DNS says the following in Section 4:

   When publishing a record containing an "ech" parameter, the publisher
   MUST ensure that all IP addresses of TargetName correspond to servers
   that have access to the corresponding private key or are
   authoritative for the public name

Avoiding failures is the main motivation for both “ensure” behaviors. Is there a reason why one spec uses SHOULD while the other uses a MUST?

Please check. Thanks.
# Section 1

## Newer versions

CURRENT:
   ECH is supported in TLS 1.3 [RFC8446], DTLS 1.3 [RFC9147], and newer
   versions of the TLS and DTLS protocols.

Do we mean that future versions must support this?

# Section 5.1

## Contiguous

CURRENT:
  The
   values MUST be ordered contiguously in ClientHelloInner, and the

Unless I missed it, I didn’t see any check on this at the receiver side? Should we have one?

## Substitute

CURRENT:
   the client MAY substitute
   extensions which it knows will be duplicated in ClientHelloOuter.  

Is this substitution triggered by configuration? Can a client make an autonomous decision here? If no, please explicit that this is based on an instruction/configuration.

# Mysterious “network attacker” 

There are some few uses of such mention in the spec. 

For example, Section 5.2 has the following:

   To prevent a network attacker from modifying the ClientHelloOuter
   while keeping the same encrypted ClientHelloInner 

Also, Section 8.1.1 says:

   Unless ECH is disabled as a result of successfully establishing a
   connection to the public name, the client MUST NOT fall back to using
   unencrypted ClientHellos, as this allows a network attacker to
   disclose the contents of this ClientHello, including the SNI.  

What is a “network attacker”?

Attacks can be even from within the infrastructure that hosts the client-facing server/backend server! Not sure if your use of “network attacker” covers that case as well. 

Please reword for clarity.

# Section 6.1.7

## An obsolete RFC

CURRENT:
   In verifying the client-facing server certificate, the client MUST
   interpret the public name as a DNS-based reference identity
   [RFC6125].  

Any reason why RFC9125 is not used here?

## Layer

CURRENT:
   Clients that enforce this by checking ECHConfig.contents.public_name
   do not need to repeat the check at this layer.

Which layer?

## Reasoning

CURRENT:
   Prior to attempting a connection, a client SHOULD validate the
   ECHConfig.  Clients SHOULD ignore any ECHConfig structure with a
   public_name that is not a valid host name in preferred name syntax
   (see Section 2 of [DNS-TERMS]).  

Any reason why invalid configuration are not unconditionally ignored?

#  How to retrieve the value used by an implementation for the following?

CURRENT:
   Clients SHOULD impose the same lifetime and scope
   restrictions that they apply to other server-based tracking vectors
   such as PSKs.

This may be used for troubleshooting/diagnostic.

# On Middleboxes in Section 8.1.2

Any impact to record for load-balancers that used to rely in SNI to distribute requests? 

# Legitimate use of SNI may break

CURRENT:
   Some use cases which depend on information ECH encrypts may break
   with the deployment of ECH.  

We may cite RFC9065 here.

# Do we have record for the “common scenario” claim in Section 10.2?

CURRENT:
   This means that any attacker which can
   inject DNS responses or poison DNS caches, which is a common scenario
   in client access networks, 

# What is meant by “transit mechanisms” in Section 10.2?  

CURRENT:
   Moreover, as noted in the introduction, SNI encryption is less useful
   without encryption of DNS queries in transit mechanisms.

# Section 10.10.5: Regularly

CURRENT:
   This design does not provide forward secrecy for the inner
   ClientHello because the server's ECH key is static.  However, the
   window of exposure is bound by the key lifetime.  It is RECOMMENDED
   that servers rotate keys regularly.

Any guidance on how frequent key rotation should be done to avoid impact service stability and ensure service continuity? Any pointer to cite on such matters?

Adam raised a more general comment: 

  “If it is possible (possibly not in this drat) to offer more detailed
  operational guidance on key rotation, that would be helpful. There are some
  points in the document that might allude to implementation-specific
  configuration choices. Implementations would ideally expose these choices to
  operators so they can make the best possible choices for their needs.”

Some words on these matters would be helpful. Thanks.

# Section 10.11: Redundant padding requirement with the SHOULDs in 6.1.3

OLD:
   Variations in the length of the ClientHelloInner ciphertext could
   leak information about the corresponding plaintext.  Section 6.1.3
   describes a RECOMMENDED padding mechanism for clients aimed at
   reducing potential information leakage.

NEW:
   Variations in the length of the ClientHelloInner ciphertext could
   leak information about the corresponding plaintext.  Section 6.1.3
   describes a recommended padding mechanism for clients aimed at
   reducing potential information leakage.

# Any reason why this text is not included in the main body?

CURRENT:
  Appendix A.  ECHConfig Extension Guidance

Cheers,
Med

# Internet AD comments for draft-ietf-tls-esni-24
CC @ekline

* comment syntax:
  - https://github.com/mnot/ietf-comments/blob/main/format.md

* "Handling Ballot Positions":
  - https://ietf.org/about/groups/iesg/statements/handling-ballot-positions/

## Comments

### S3.1

* Please stick with RFC 5952 S4.3 (lowercase the [a-f] characters in IPv6
  addresses).

Thank you to Carsten Bormann for the ARTART review, and to EKR for addressing his comments.