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

A MUST for inclusion of public_name in outer SNI seems wrong #396

Closed
sftcd opened this issue Mar 10, 2021 · 20 comments · Fixed by #456
Closed

A MUST for inclusion of public_name in outer SNI seems wrong #396

sftcd opened this issue Mar 10, 2021 · 20 comments · Fixed by #456
Labels
design

Comments

@sftcd
Copy link
Collaborator

@sftcd sftcd commented Mar 10, 2021

For at least command line clients (e.g. curl) and perhaps for censorship circumvention tools it makes sense to allow the outer SNI to contain nothing or something that is not the public_name from the SVCB RR. It's possible that any tool that acquires an ECHConfig from a non-DNS source would benefit from that option too.

The offending (for me:-) text is in section 6.1, 2nd list, bullet 7 where it says: 'The value of ECHConfig.contents.public_name MUST be placed in the "server_name" extension.'

There may be clients that do need the above. I'm not sure how to chacterise those in a short phrase, but that's likely possible. At the IETF-110 TLS session mention was made of clients that need to fallback but I'm not sure what reference would be correct for that if e.g. we appended something like "....for clients that need to be able to fallback. [ref]"

@cbartle891
Copy link
Collaborator

@cbartle891 cbartle891 commented Mar 10, 2021

and perhaps for censorship circumvention tools

Censorship circumvention isn't one of the stated goals of ECH.

It's possible that any tool that acquires an ECHConfig from a non-DNS source would benefit from that option too.

Even if an ECHConfig is acquired from a non-DNS source, it still has the ECHConfig.contents.public_name field, which, as specified in 6.1.3.3, is necessary because:

"The client MUST verify that the certificate is valid for ECHConfig.contents.public_name. If invalid, it MUST abort the connection with the appropriate alert."

@ekr
Copy link
Collaborator

@ekr ekr commented Mar 10, 2021

I believe we should keep this as-is. Fallback is for the server's benefit and the client has no way of knowing whether it will be needed.

@chris-wood
Copy link
Collaborator

@chris-wood chris-wood commented Mar 10, 2021

+1 -- for robustness reasons, I believe we should keep this as-is.

@sayrer
Copy link
Contributor

@sayrer sayrer commented Mar 10, 2021

I also found this requirement odd. Is it really necessary with the changes to config_id in draft -10? If someone could explain why this should be mandatory, that would be fine. I just don't understand the requirement.

@davidben
Copy link
Collaborator

@davidben davidben commented Mar 10, 2021

When there is a mismatch between DNS and the server configuration, the server will handshake with the outer ClientHello. The expectation when that happens is that the connection is authenticated for the public name, which gives an authenticated recovery signal. That means the server needs to, e.g., select a certificate that covers the public name. Thus, the outer SNI selection.

@sayrer
Copy link
Contributor

@sayrer sayrer commented Mar 10, 2021

OK, and a server that didn't need the outer SNI name would be identifiable by IP address anyway. Makes sense.

@sftcd
Copy link
Collaborator Author

@sftcd sftcd commented Mar 11, 2021

@sftcd
Copy link
Collaborator Author

@sftcd sftcd commented Mar 11, 2021

@sftcd
Copy link
Collaborator Author

@sftcd sftcd commented Mar 11, 2021

@dmcardle
Copy link
Contributor

@dmcardle dmcardle commented Mar 11, 2021

One oddity of this requirement is that it presents a GREASE distinguisher when the client is connecting to an IP address with no server name. In the GREASE case, the ClientHelloOuter.server_name is absent because SNI only supports host names. In the real-ECH case, ClientHelloOuter.server_name is present and contains ECHConfig.public_name.

However, why use ECH at all if the IP address uniquely identifies the desired service? I think I support keeping the requirement for the sake of the fallback/retry mechanism.

Was there some other scenario you're thinking of?

@davidben
Copy link
Collaborator

@davidben davidben commented Mar 11, 2021

(The outer name pretty fundamentally interacts with GREASE because the distribution of true SNIs is different from the distribution of cleartext ones, but the fact that IP addresses don't get SNIs is an insignificant component of that, given how rare they are. Any reasonable model of GREASE distinguishability would need to be a bit more subtle.)

@cjpatton
Copy link
Contributor

@cjpatton cjpatton commented Mar 23, 2021

If possible, I'd like to see if we can drill down on where folks disagree.

  • Fact 1: The public name specifies the name that the client expects the client-facing server to verify in case of ECH rejection.
  • Fact 2: The outer SNI specifies the name the client expects the client-facing server to verify, regardless of whether ECH was offered.

It seems like the point of disagreement is how these two extensions --- ECH and SNI --- are meant to be composed. It seems to me that @davidben believes that SNI takes precedence: if we allowed public name != outer SNI, then this would violate the rules of the SNI extension, It seems like @sftcd believes that ECH can take precedence: allowing public name != outer SNI is OK because the client already knows what name the client-facing server will verify. Am I understanding this all correctly?

@davidben
Copy link
Collaborator

@davidben davidben commented Mar 23, 2021

I'm very confused what facts 1 and 2 are trying to say.

The server_name extension, in any ClientHello, is the name of the server you're trying to contact with that ClientHello. This figures into certificate selection, and you could even imagine a protocol which uses it in lieu of an in-band Host header for virtual hosting. (Although HTTP does it in-band and doesn't care much about the server_name.)

In the ClientHelloInner, you are trying to contact the true name, so you put the true name in server_name.
In the ClientHelloOuter, you are trying to contact the public name, so you put the public name in server_name.

Independent of ECH, if you have external knowledge that such-and-such field will be interpreted in a different way and the server doesn't need the server_name, then you can go against the spec. If that external knowledge applies to the ClientHelloOuter, then you can apply that to the ClientHelloOuter. But that's not how the field is meant to work, so the spec should describe the intended semantics.

@chris-wood
Copy link
Collaborator

@chris-wood chris-wood commented Mar 23, 2021

Independent of ECH, if you have external knowledge that such-and-such field will be interpreted in a different way and the server doesn't need the server_name, then you can go against the spec. If that external knowledge applies to the ClientHelloOuter, then you can apply that to the ClientHelloOuter. But that's not how the field is meant to work, so the spec should describe the intended semantics.

This seems to advocate that a SHOULD, rather than a MUST, is appropriate in choosing what name goes in ClientHelloOuter, right? In particular, the spec does not accommodate external knowledge about what goes in the field. It requires it be set to ECHConfig.public_name.

@sftcd
Copy link
Collaborator Author

@sftcd sftcd commented Mar 23, 2021

@cjpatton
Copy link
Contributor

@cjpatton cjpatton commented Mar 23, 2021

I'm very confused what facts 1 and 2 are trying to say.

Fact 1 says that public_name is the name of the server when ECH is rejected (i.e., the client-facing server). Fact 2 says that server_name is the name of the server. My point was merely that these facts appear to contradict one another if public_name != CHOuter.server_name. Hence, we need to decide how to compose ECH with SNI in a way that resolves the contradiction.

The answer might simply be that public_name MUST equal CHOuter.server_name. Or, we might do say something else.

@davidben
Copy link
Collaborator

@davidben davidben commented Mar 24, 2021

(I don't particular care about MUST vs. SHOULD. Whatever we say elsewhere for server_name I guess.)

@cjpatton I'm not following. I think talking about public_name != CHOuter.server_name is getting causality the wrong way around.

CHOuter is a construct we are inventing in ECH. That means we get to define the semantics. The semantics we've defined are that CHOuter, along with the result of handshaking with CHOuter, is intended for the ECH rejection case. How does the client know how the ECH rejection case works? Well, we also need to define that in ECH. Our definition is that you handshake against ECHConfig.public_name. Finally, server_name is a construct that has already been defined. What goes in there? RFC6066 and RFC8446 tell you. It's the name of the server you are trying to contact.

With all that, what do you put in CHOuter.server_name? Well, you put the rules together. It's the name of the server you are trying to contact in the ECH rejection case, which is ECHConfig.public_name.

@sftcd
Copy link
Collaborator Author

@sftcd sftcd commented Mar 24, 2021

@davidben
Copy link
Collaborator

@davidben davidben commented Mar 24, 2021

Not really - CHOuter has to be a "working" CH with semantics defined by 8446, so we're a lot more constrained in reality.

Oh sure. I wasn't suggesting CHOuter and the resulting handshake get to ignore 8446. But the parameters to that handshake are not set and that's what ECH get to define.

That's too absolute IMO. ECHConfig.public_name tells you a name that should work for the CHOuter fallback. There will be many situations where another name can work as well, though using ECHConfig.public_name is a fine default/SHOULD.

Hrm, in which link of the chain were you looking to add flexibility? There's:

  • The implementation wants to set server_name to some value other than the name they are contacting
  • The implementation wants to set the name they are contacting to some value other than ECHConfig.public_name

Weakening the first to SHOULD is perhaps plausible, especially if 6066 and 8446 already are less prescriptive. Weakening the second to SHOULD is much more dangerous because you lose the chain of authenticity up from the TLS handshake through X.509 to the name specified by the ECHConfig.

@sftcd
Copy link
Collaborator Author

@sftcd sftcd commented Mar 25, 2021

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants