quinn icon indicating copy to clipboard operation
quinn copied to clipboard

Access Client Hello before accepting connection

Open FlorianUekermann opened this issue 1 year ago • 18 comments

I need to access the rustls::server::ClientHello or at least the content of server_name() before choosing a rustls config. alpn() would also be very useful. Could this be exposed via Incoming or is that too early?

FlorianUekermann avatar Oct 29 '24 20:10 FlorianUekermann

Incoming is too early: it is yielded before the endpoint even decrypts first packet, and a ClientHello might span multiple packets. I think your use case is worth supporting, but it will require an additional intermediate stage between Incoming and Connection, and changes in the proto layer to suspend work once the ClientHello is received.

Ralith avatar Oct 29 '24 23:10 Ralith

I see. Sounds like a non-trivial amount if work. That's unfortunate. Maybe it would be easier to add an accept method, which takes a closure mapping Client Hello to a rustls config as a first step.

FlorianUekermann avatar Oct 30 '24 11:10 FlorianUekermann

We already have a Connecting stage, so this might fit in there? I wonder if we could do this by adding a (provided) method to quinn-proto's ServerConfig trait.

djc avatar Oct 30 '24 13:10 djc

We already have a Connecting stage, so this might fit in there?

Connecting exposes the rustls handshake data (including the negotiated protocol), so I don't think we can get to hat stage without a rustls config. So even changing the semantics of the Connecting stage slightly would be tricky for compatibility reasons.

But the fact that the handshake data is available in Connecting, which you get by calling Incoming::accept() (which is not async, so presumably not waiting for network traffic) is a little hard to reconcile with @Ralith statement. Doesn't that mean the Client Hello would need to be received before we reach the Incoming stage? I'll try to take a look.

FlorianUekermann avatar Oct 30 '24 16:10 FlorianUekermann

Connecting::handshake_data is async. If your Client Hello has not been fully received, it won't be ready yet.

Ralith avatar Oct 31 '24 03:10 Ralith

We already have a Connecting stage, so this might fit in there?

I'm not sure if this would be feasible. The motivation here is to make the connection's configuration depending on the Client Hello, so we have to defer the connection logic until after the application has had its opportunity to weigh in, similar to how Incoming defers all frame processing.

A callback could work, though it's a bit of a compromise ergonomically.

Ralith avatar Oct 31 '24 03:10 Ralith

Adding an async Incoming::client_hello_data, which waits until the data is available similar to Connecting::handshake_data would be best from a user perspective imo. I don't think the public API would benefit from an intermediate stage, because I think it would just duplicate the methods Incoming already has.

FlorianUekermann avatar Nov 02 '24 13:11 FlorianUekermann

That could work. I think it'd need about the same changes at the proto layer: we still want decode a Client Hello but defer all other work, and unless the user calls client_hello_data we shouldn't even do that much. I agree that adding more stages with duplicate APIs is unappealing if it can be avoided, though there might be other viable approaches as well, like a single type parameterized by a stage marker type, or plain composition.

Ralith avatar Nov 03 '24 00:11 Ralith

Stupid question: Reaching Incoming means the "initial" packet (type 0x00) has been received, right? Doesn't that mean the first CRYPTO frame, which includes the Client Hello has already been received as well?

FlorianUekermann avatar Nov 03 '24 16:11 FlorianUekermann

Stupid question: Reaching Incoming means the "initial" packet (type 0x00) has been received, right? Doesn't that mean the first CRYPTO frame, which includes the Client Hello has already been received as well?

Yes, the first CRYPTO frame is included, but there's no guarantee the complete ClientHello has been received (particularly with post-quantum key exchange).

djc avatar Nov 03 '24 16:11 djc

Does quinn accept API breaking change? We are deeply rely on lazy select server_config.

Could we accept callback style like s2n or LazyAcceptor like rustls?

taikulawo avatar Dec 04 '24 09:12 taikulawo

Does quinn accept API breaking change?

We could accept a breaking change, but I'm not sure we'd want this to be breaking? Depends on how it breaks current users.

We are deeply rely on lazy select server_config.

Who's we, what are you using Quinn for?

Could we accept callback style like s2n or LazyAcceptor like rustls?

I think we'd probably prefer something closer to rustls, mainly so callers can asynchronously decide which ServerConfig they want to use.

djc avatar Dec 04 '24 09:12 djc

Our company have rust based gateway, try to add quic/http3 on it.

So I am researching a Rust QUIC library(quinn/msquic/s2n/quiche/quictls) to integrate it into the gateway.

taikulawo avatar Dec 04 '24 09:12 taikulawo

You may know this issue. I solved by patch rustls. https://github.com/rustls/rustls/issues/1976. since it cause breaking api change. I haven't submit PR.

I prefer use quic library which can integrate rustls as tls backend, so we can reuse previous patched rustls.

taikulawo avatar Dec 04 '24 09:12 taikulawo

Quinn integrates rustls already, and also makes the encryption layer pluggable so you could use your own rustls fork instead.

djc avatar Dec 04 '24 09:12 djc

Quinn integrates rustls already, and also makes the encryption layer pluggable so you could use your own rustls fork instead.

Yes, but quinn don't support select ServerConfig. In Tcp tls, we have TlsClientParse, We pre-parse tcp data get SNI before rustls get involved, so we can select different ServerConfig. But quic encapsulate TLS into itself, we can't pre-parse simple

You can think we are NGINX, we have many domain listen on same quic port, we need use different cert/pkey. NGINX use openssl SSL_CTX_set_tlsext_servername_callback callback, so we need find rustls style way to solve it. :)

taikulawo avatar Dec 04 '24 09:12 taikulawo

Quinn integrates rustls already, and also makes the encryption layer pluggable so you could use your own rustls fork instead.

Yes, but quinn don't support select ServerConfig. In Tcp tls, we have TlsClientParse, We pre-parse tcp data get SNI before rustls get involved, so we can select different ServerConfig. But quic encapsulate TLS into itself, we can't pre-parse simple

You can think we are NGINX, we have many domain listen on same quic port, we need use different cert/pkey. NGINX use openssl SSL_CTX_set_tlsext_servername_callback callback, so we need find rustls style way to solve it. :)

I think that's definitely feasible, someone just needs to do the work (or someone can pay me to do the work, happy to discuss).

djc avatar Dec 04 '24 10:12 djc

Our company have rust based gateway, try to add quic/http3 on it.

So I am researching a Rust QUIC library(quinn/msquic/s2n/quiche/quictls) to integrate it into the gateway.

On the first gray version, we will have 1,000,000 qps http/3 traffic which power our core business. So quic library wish stable and reliable.

taikulawo avatar Dec 05 '24 04:12 taikulawo