Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define HelloRetryRequestInner/Outer messages #407

Closed
wants to merge 5 commits into from

Conversation

cjpatton
Copy link
Contributor

@cjpatton cjpatton commented Mar 25, 2021

The design is based on @davidben's sketch.

Closes #374.
Closes #373.
Closes #358.
Closes #333.

@chris-wood chris-wood requested review from ekr and chris-wood Mar 25, 2021
ekr
ekr approved these changes Mar 26, 2021
draft-ietf-tls-esni.md Outdated Show resolved Hide resolved
cjpatton and others added 2 commits Mar 26, 2021
draft-ietf-tls-esni.md Outdated Show resolved Hide resolved
draft-ietf-tls-esni.md Outdated Show resolved Hide resolved

The payload of the "encrypted_client_hello" extension is constructed as follows.
The client-facing server begins by computing `hrr_inner_key` and
`hrr_inner_nonce` as described in {{client-hrr-accepted-ech}}. The extension
Copy link
Collaborator

@cbartle891 cbartle891 Mar 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(This is a note not just about this PR but the document in general.) I'm finding it confusing for all the client behavior discussed in one section and the server behavior discussed in another. I'm bouncing back and forth from one to the other to get the actual ordering of the series of events. Would it not be less confusing to put things in that order than to describe all the client behavior and then all the server behavior?

Copy link
Contributor Author

@cjpatton cjpatton Mar 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, that's an interesting idea. I wonder which is easier from the perspective of someone who is implementing this for the first time. My assumption has been that it's better to separate client behavior and server behavior into two section so that the implementer needs only to refer to one section when implementing the client (resp. server) bits. But given how complex ECH is, perhaps a chronological description is better. I think this sort of editorial re-work is worth filing an issue for discussion.

HelloRetryRequestInner to construct ClientHelloInner per the rules of
{{RFC8446}}. It then encodes the second ClientHelloInner as in
{{encoding-inner}}, except that it does not incorporate any extensions from the
ClientHelloOuter (i.e., the "ech_outer_extensions" extension is not used). It
Copy link
Collaborator

@cbartle891 cbartle891 Mar 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(i.e., the "ech_outer_extensions" extension is not used)

Why is this?

Copy link
Contributor Author

@cjpatton cjpatton Mar 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea here is that there's no need to incorporate outer extensions because since the inner handshake was taken. However, it occurs to me that this assumption might not be correct, depending on how the "TODO" above is resolved. I wonder, are there any extensions that MUST appear in CHOuter (according to the HRR rules prescribed by 8446), but which one would want to incorporate here? "supported_versions" is the only extension I know of that MUST appear in the CHOuter, but there's no reason to incorporate it.

@cjpatton cjpatton requested a review from cbartle891 Mar 30, 2021
draft-ietf-tls-esni.md Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants