search

tlswg / draft-ietf-tls-esni

TLS Encrypted Client Hello
https://tlswg.github.io/draft-ietf-tls-esni/#go.draft-ietf-tls-esni.html
Other
231 stars 56 forks source link

Are we quite right now with "MUST use public_name"? #572

Closed sftcd closed 8 months ago

sftcd commented 8 months ago

The spec currently says:

'The value of ECHConfig.contents.public_name MUST be placed in the "server_name" extension.'

I've never been fond of that, but I think it's also overly restrictive. For example, tests I've done with my and others' servers indicate that servers do not insist on this, if ECH decryption worked, and I think the code is doing the right thing there.

I've also included the capability for a command-line override of public_name (in the outer SNI) in the cURL code I've proposed to that project. The logic there is that could be useful for some circumvention applications in future - I don't know that is or will be true, but it seems good to preserve the flexibility. (That said only my client library code seems to have an API that allows that override as of now.)

So I'd suggest we modify the MUST quoted above and add a SHOULD NOT (or make any equivalent change), to e.g.:

I don't think we need to say anything new on this topic related to ECH decryption failures.

chris-wood commented 8 months ago

This is a dupe of #396, which we already closed in the past, perhaps maybe prematurely. Relaxing this to a SHOULD NOT would effectively reintroduce domain fronting, which is not something I think we want to encourage. If anything, I would suggest that servers willing to accept the risk of not enforcing this MUST opt in via an ECHConfig extension.

sftcd commented 8 months ago

Looking back at #396 (which I'd forgotten was an issue on here, sorry;-) I don't think it was clear that a MUST (as we have now) was really preferred over a SHOULD.

The suggested SHOULD NOT for servers above is based on just testing what the ones I know of, do now. (And of course it matches what I'd like too:-)

I've no preference as to whether to close this and re-open #396 or not but I do think it's worth revisiting/rechecking this topic before we're done.

chris-wood commented 8 months ago

Based on your testing, what server implementations do not enforce this requirement?

sftcd commented 8 months ago

All few of them. Meaning those using my OpenSSL fork, those using CF's server code and tls-ech.dev which uses boringssl. I added some tests for this to my cURL test script. I checked with wireshark that the outer SNI was as expected (value == "override") when sending to CF.

If anyone knows of additional servers to add to test, I'd be happy to add 'em to my list.

martinthomson commented 8 months ago

If the connection works without one sure. But there are two reasons that a client might want to include a public name.

  1. The server just doesn't work without one. I can easily imagine a server implementation that routes to ECH logic based on the public SNI and fails otherwise.
  2. Fallback.

Maybe you can argue that a server can execute fallback with the public name without the client telling it what that public name is. So maybe this is just one reason.

With a single reason you could imagine allowing the public name to be empty in the config. The problem there would be if there is some sort of system that depends on having an SNI, which is likely. In that case, you could use a fixed string instead.

sftcd commented 8 months ago

No, the argument here is not to omit the public_name, but rather to say, for clients, the outer SNI SHOULD be the public_name, and for servers, when ECH decryption worked, they SHOULD NOT care. When ECH decryption fails, then using the public_name will get better interop, hence the SHOULD for clients.

chris-wood commented 8 months ago

and for servers, when ECH decryption worked, they SHOULD NOT care

This just isn't true. Servers will care about this because, again, not caring about it just opens up the door to effectively domain front.

sftcd commented 8 months ago

If a server cares, that's fine. Saying all servers MUST care seem wrong. Esp since none currently seem to care.

chris-wood commented 8 months ago

If a server cares, that's fine. Saying all servers MUST care seem wrong. Esp since none currently seem to care.

I agree with this. I would be fine just relaxing this MUST to a SHOULD and describing the consequences of not enforcing this check for clients and servers.

sftcd commented 8 months ago

I'd be ok with that.

ckcr4lyf commented 8 months ago

I'd talked about this a bit on the mailing list, but didn't receive much response there. (See: https://mailarchive.ietf.org/arch/msg/tls/HUG1CU0Q4PorZ7fD0yafVfj7VUY/)

Is github the preferred discussion point for ECH rather than the mailing list? If so I will state my view of it here as well for those not on the mailing list

ckcr4lyf commented 8 months ago

FYI: My main point revolves around the public name "leaking" which website you're connecting to anyway, which was one of the problems with SNI initially (and how some ISPs such as Jio in India were blocking websites).

Using the public_name field only is viable if you're behind a big corporation like Cloudflare, but if you want to do it for your personal website it gets tricky (which 涛叔 mentioned here: https://mailarchive.ietf.org/arch/msg/tls/0yxB-VwM8ayf6QLWpqCBcrSCFT4/)