Add staged ClientHello acceptor for ServerConfig selection#2671
Add staged ClientHello acceptor for ServerConfig selection#2671iadev09 wants to merge 11 commits into
Conversation
190980e to
738f9f7
Compare
738f9f7 to
7dbd839
Compare
| # Enable rustls with the `aws-lc-rs` crypto provider | ||
| rustls-aws-lc-rs = ["dep:rustls", "aws-lc-rs", "proto/rustls-aws-lc-rs", "proto/aws-lc-rs"] | ||
| rustls-aws-lc-rs-fips = ["dep:rustls", "aws-lc-rs-fips", "proto/rustls-aws-lc-rs-fips", "proto/aws-lc-rs-fips"] | ||
| rustls-aws-lc-rs = ["__rustls", "dep:rustls-aws-lc-rs", "aws-lc-rs", "proto/rustls-aws-lc-rs", "proto/aws-lc-rs"] |
There was a problem hiding this comment.
I think this feature refactoring can be done separately, earlier (suggest in a separate PR). rustls 0.24 requires that callers provide a crypto provider explicitly. Since rustls is a public dependency for Quinn anyway, I suggest that we drop the provider-specific features in favor of requiring an explicit provider, too.
@Ralith do you agree?
If so, I'd suggest that the explicit provider passing can also become a separate commit that can be merged ahead of this one. (May obviate the need for a __rustls feature?)
There was a problem hiding this comment.
I suggest that we drop the provider-specific features in favor of requiring an explicit provider, too.
Yep, that should be much more composable. We may still want to spend some effort having a reasonable default, but we should try to avoid needing to know about other providers.
There was a problem hiding this comment.
I guess the challenge here is that we still have the raw primitives in use for packet protection. Maybe the first objective is finishing main...ctz:quinn:jbp-retry-calculation-uses-rustls...
7dbd839 to
86ccb0f
Compare
e67f0ff to
6f479c7
Compare
6f479c7 to
e42adb1
Compare
|
So I think if you want to make progress on getting this merged, this needs to be done as a separate PR:
I think the upgrade to 0.24 should also be a separate PR -- not sure which of these two makes sense to merge first. |
|
I split the rustls 0.24 upgrade out into a separate branch first. #2701 After that, I can look at the retry packet protection cleanup as another small PR before rebasing the staged ClientHello acceptor work on top. |
Closes #2024.
Adds a staged ClientHello accept path so a server can inspect the rustls
ClientHellobefore finalizing the QuinnServerConfig.Commit Structure
1. rustls 0.24 upgrade path
Kept as one compatibility commit. This is where the temporary rustls git dependencies live, including the rustls-platform-verifier 0.8 branch.
Related rustls 0.24 PR outside Quinn: rustls/rustls-platform-verifier#233.
2. staged ClientHello acceptor primitive
3. selector helper
Adds a
ServerConfigSelectorhelper trait on top of the staged acceptor API.Tests
Bench
https://github.com/iadev09/quinn-acceptor-bench
In local 1000-sample smoke runs, the selected path stayed in the same range as continuing with the default
ServerConfig.Comparison
Compared with the TCP/rustls shape, this exposes the staged primitive directly.
For TCP, config selection usually needs an intermediate acceptor/future shape:
For Quinn, the proposed primitive lets the caller inspect the
ClientHelloand choose thequinn::ServerConfigdirectly.Because the
Endpointalready owns the Initial/CRYPTO handling, it does not need aLazyConfigAcceptor-style stream wrapper.Same scenario on TCP/rustls
LazyConfigAcceptor:https://github.com/iadev09/lazy-config-acceptor-bench