Initial ESNI/ECH client prototype

[sayrer]

This client almost works--it has a few interoperability problems with some servers, but the handshake extension it sends seems credible. It's based on draft -02 of the spec: https://tools.ietf.org/html/draft-ietf-tls-esni-02 . There are newer ones, but Firefox and Cloudflare ship -02 for now. I think there will be some fresh movement around -05 in a few weeks.

I thought I would create this as discussion point, since I found it difficult to guess how you'd like this feature factored into the existing code. I tried to keep most of the ESNI-specific stuff in esni.rs (for now), and I think the stuff in msgs/handshake.rs should be uncontroversial. I'm sure you'll want to change what I've done to sessions, configs, etc.

(edit: I see I've slightly broken the contribution rules here, by starting without opening an issue, although there is one already: issue #199. anyway, there's lots of work to do yet, this is just a small start. still need a server side, some work on what other messages need to be in the handshake, a tool for generating the DNS record, and updates to newer drafts).

[ctz]

Thanks for working on this; this is very definitely a feature I'd like in rustls.

Very broadly what I had in mind for fitting this into the API was something like:

pub struct EncryptedHostname {
  hostname: webpki::DNSNameRef,
  (...)
}

impl EncryptedHostname {
  fn new(hostname: webpki::DNSNameRef, esni_data: ESNIHandshakeData ...) -> EncryptedHostname {}
}

pub enum ClientPeerName /* or something */ {
  Hostname(webpki::DNSNameRef),
  EncryptedHostname(EncryptedHostname),
  // space for IpAddress(std::net::IpAddr) in the future
}

impl ClientSession {
  pub fn new(config: &Arc<ClientConfig>, peername: ClientPeerName) -> ClientSession {}
}

It seems like a good idea to make space for future support for non-SNI IP address peers.

[sayrer]

I got the client side of this interoperating with other implementations. However, it looks like the TLS WG is going to make some substantial changes soon. I'll wait for those before I try to clean everything up for a merge.

[coveralls]

Coverage decreased (-1.7%) to 94.703% when pulling aa6317d on sayrer:master into d132d48 on ctz:master.

[codecov-io]

Codecov Report

Merging #318 into master will decrease coverage by 1.68%.
The diff coverage is 56.94%.

@@            Coverage Diff            @@
##           master    #318      +/-   ##
=========================================
- Coverage   96.38%   94.7%   -1.69%     
=========================================
  Files          49      50       +1     
  Lines        8307    8628     +321     
=========================================
+ Hits         8007    8171     +164     
- Misses        300     457     +157

Impacted Files	Coverage Δ
rustls/src/suites.rs	87.82% <ø> (-0.08%)	⬇️
rustls/src/cipher.rs	96.35% <ø> (-0.02%)	⬇️
rustls/src/client/common.rs	100% <100%> (ø)	⬆️
rustls/src/msgs/handshake_test.rs	99.8% <100%> (ø)	⬆️
rustls/src/client/tls13.rs	98.29% <100%> (-0.01%)	⬇️
rustls/src/quic.rs	92.1% <100%> (ø)	⬆️
rustls/src/msgs/handshake.rs	92% <41.29%> (-5.74%)	⬇️
rustls/src/client/hs.rs	95.31% <60%> (-2.12%)	⬇️
rustls/src/client/mod.rs	90.79% <66.66%> (-1.44%)	⬇️
rustls/src/esni.rs	67.87% <67.87%> (ø)
... and 24 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d132d48...aa6317d. Read the comment docs.

[vinhjaxt]

Any update?

[sayrer]

waiting for draft -06 here: https://datatracker.ietf.org/doc/draft-ietf-tls-esni/

[djc]

(Looks like draft -06 has been published on March 9.)

[sayrer]

draft-06 had a lot of things in it that I knew were going to change (I follow the WG pretty closely).

draft-07 is just out and seems almost ready.

[sayrer]

Cloudflare now has a reasonably complete Go implementation, so I'll update this patch to work with it.

cloudflare/go#30

[sayrer]

The TLS WG has stabilized on version -09. There had been a lot of churn up until now.

https://mailarchive.ietf.org/arch/msg/tls/ZtLcWScp8u-7YkDEpZJrHm5low8/

[sayrer]

Have talked to a few folks about this. Given the other work going in this repo, I think it would be best to split this out into at least four tasks.

1/ Parsing ECHConfig retrieved from DNS in handshake.rs (no conflict with other work)

2/ The actual crypto to create the ECH messages. This is something close to the esni.rs file in my patch. (no major conflict, may need Ring changes for the cfrg-hpke draft)

3/ Factoring the handshake/session creation API for the various results that can happen (the needed edits seem not great given the changes suggested in #461, which is a good idea). See ctz's comment here for an older take on this problem: #318 (comment). #461 would probably obsolete that API surface.

4/ Misc changes that make TLS 1.3 required for ECH, banning 1.2 (easy to hack in, but might be better to more thoroughly disentangle 1.2). I have no use for 1.2, so I kind of wish I could turn it off as a crate feature.