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

Access Client Hello before accepting connection #2024

Open
FlorianUekermann opened this issue Oct 29, 2024 · 10 comments
Open

Access Client Hello before accepting connection #2024

FlorianUekermann opened this issue Oct 29, 2024 · 10 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@FlorianUekermann
Copy link
Contributor

FlorianUekermann commented Oct 29, 2024

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?

@Ralith
Copy link
Collaborator

Ralith commented Oct 29, 2024

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 Ralith added enhancement New feature or request help wanted Extra attention is needed labels Oct 29, 2024
@FlorianUekermann
Copy link
Contributor Author

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.

@djc
Copy link
Member

djc commented Oct 30, 2024

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.

@FlorianUekermann
Copy link
Contributor Author

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.

@Ralith
Copy link
Collaborator

Ralith commented Oct 31, 2024

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

@Ralith
Copy link
Collaborator

Ralith commented Oct 31, 2024

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.

@FlorianUekermann
Copy link
Contributor Author

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.

@Ralith
Copy link
Collaborator

Ralith commented Nov 3, 2024

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.

@FlorianUekermann
Copy link
Contributor Author

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?

@djc
Copy link
Member

djc commented Nov 3, 2024

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants