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

[Eric Rescorla <[e-mail]>]

Thank you for your review. I have created a PR to address
some of these comments.

https://github.com/tlswg/draft-ietf-tls-esni/pull/650

> 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?

I don't actually see the text in 6.2. Can you quote
the larger passage?

This is a good catch. Obviously, anything here that isn't a MUST
is not truly necessary. This leaves us with:

> 1.  It MUST offer to negotiate TLS 1.3 or above.

This isn's strictly required for interop but I don't think but will
produce a non-conformant appearing transcript because you will get TLS
1.3 in the ServerHello but not in the ClientHello.


>  2.  If it compressed any extensions in EncodedClientHelloInner, it
>      MUST copy the corresponding extensions from ClientHelloInner.
>      The copied extensions additionally MUST be in the same relative
>      order as in ClientHelloInner.

I think this is strictly required, because otherwise things won't
actually work.

>   3.  It MUST copy the legacy_session_id field from ClientHelloInner.
>       This allows the server to echo the correct session ID for TLS
>       1.3's compatibility mode (see Appendix D.4 of [RFC8446]) when ECH
>       is negotiated.

This actually isn't required for DTLS, as DTLS does not use
this feature (See RFC 9147; Section 5).


 >  6.  When the client offers the "pre_shared_key" extension in
 >       ClientHelloInner, it SHOULD also include a GREASE
 >       "pre_shared_key" extension in ClientHelloOuter, generated in the
 >       manner described in Section 6.1.2.  The client MUST NOT use this
 >       extension to advertise a PSK to the client-facing server.  (See
 >       Section 10.12.3.)  When the client includes a GREASE
 >       "pre_shared_key" extension, it MUST also copy the
 >       "psk_key_exchange_modes" from the ClientHelloInner into the
 >       ClientHelloOuter.

I think this MUST is required for the same reason as 1, but won't
cause an interop problem if you don't.

>   7.  When the client offers the "early_data" extension in
>       ClientHelloInner, it MUST also include the "early_data" extension
>       in ClientHelloOuter.  This allows servers that reject ECH and use
>       ClientHelloOuter to safely ignore any early data sent by the
>       client per [RFC8446], Section 4.2.10.

I think this is required for interop.


In any case, I think we should remove the waffle text, as
updated RFCs can change anything. This is a question for
the WG, however. I have filed
https://github.com/tlswg/draft-ietf-tls-esni/issues/651
for the WG to consider.


> ===
> 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. “

Agreed. I moved this up to the extensions text.

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

Fixed.

> 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!!!

Fixed.


> 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)

Added soem text.


> 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?

It's server IP. The client's IP is generally not withing  the client's
control. I added some text.


> 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 …

Rewritten.

> 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?

Added "the".

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

Fixed.

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

Fixed, along with another period in the previous entry.


> 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/ ?

This is one of those folklore rules of English like split
infinitives that is not actually reflective of actual
writing. See http://itre.cis.upenn.edu/~myl/languagelog/archives/001484.html
.

I think these are all sufficiently clear whichever word is used, but
if you disagree on one, then please flag it and I'll take a look.

-Ekr



On Fri, Apr 25, 2025 at 11:23 PM Gorry Fairhurst via Datatracker <
[e-mail]> wrote:

> 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
>
>
>
>