forked from quinn-rs/quinn
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Early data clarity #5
Draft
gretchenfrage
wants to merge
20
commits into
1661
Choose a base branch
from
early-data-clarity
base: 1661
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Subsequent commits will allow the user to achieve the removed behavior manually and more flexibly. - Removes concurrent_connections from ServerConfig, as well as corresponding check in early_validate_first_packet. - Adds ConnectionError::CidsExhausted error, although it is not yet instantiated. - Renames ConnectError variant TooManyConnections to CidsExhausted. - Renames proto Endpoint internal method is_full to cids_exhausted. - Adds method open_connections to Endpoint (both proto and quinn). - Removes Endpoint method reject_new_connection from Endpoint (both proto and quinn). - Deletes obselete tests concurrent_connections_full and reject_new_connections.
This commit factors out the two fields of a quinn::Endpoint's State necessary to process a proto::Transmit into a new sub-struct, TransmitState. This is to alleviate borrowing issues, because proto::Transmit will soon be called from more call sites than previously. The bulk of this code change is just moving around existing code. Co-authored-by: Dirkjan Ochtman <[email protected]>
This commit refactors the logic for a quinn_proto::Endpoint accepting an incoming connection so that it constructs an explicit Incoming struct containing all the necessary state to accept/reject/retry the connection-creating packet. However, the external API stays the same. The bulk of this code change is just moving around existing code. Additionally, adds some gitignore lines I was using for coverage testing.
This commit removes use_retry from the server config and provides a public API for the user to manually accept/reject/retry incoming connections before a handshake begins, and inspect properties such as an incoming connection's remote address and whether that address is validated when doing so. In quinn-proto, Incoming is made public, as well as Endpoint's accept/ reject/retry methods which operate on it. The DatagramEvent::NewConnection event is modified to return an incoming but not yet accepted connection. In quinn, awaiting Endpoint::accept now yields a new quinn::Incoming type, rather than quinn::Connecting. The new quinn::Incoming type has all the methods its quinn_proto equivalent has, as well as an accept method to (fallibly) transition it into a Connecting, and also reject, retry, and ignore methods. Furthermore, quinn::Incoming implements IntoFuture with the output type Result<Connection, ConnectionError>>, which is the same as the Future output type of Connecting. This lets server code which was straightforwardly awaiting the result of quinn::Endpoint::accept work with little to no modification. The test accept_after_close was removed because the functionality it was testing for no longer exists.
This commit adds a new --block option to the server example to illustate in a simplified way the general structure one would use to implement IP address blocking with the new accept/reject/retry API. For example: cargo run --example server ./ --listen 127.0.0.1:4433 --stateless-retry --block 127.0.0.1:8065 cargo run --example client https://127.0.0.1:4433/Cargo.toml --host localhost --bind 127.0.0.1:8065 One thing to note is that that example places the reject condition before the retry condition. This expends slightly less effort rejecting connections, but does create a blocked IP address oracle for an attacker who can do address spoofing.
This commit adds a new --connection-limit option to the server example to illustrate how a user could implement a limit to the number of connections open at a time with the new "incoming" API and Endpoint::open_connections method rather than with the now-removed concurrent_connections ServerConfig parameter.
Changes response_start to track the time of the first byte received.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I had a hard time getting 0-RTT actually working in practice. It escalated to me forking rustls so I could put in a bunch of print statements. In hopes of improving understandability and discoverability of this, this PR demonstrates the usage of 0-RTT in the client/server example, adds other features to the client/server example to facilitate 0-RTT testing, and adds and rewrites some of the doc comments. It does not actually change any quinn code or APIs.
Tests with emulated localhost latency set to 100 ms.
Baseline no-0RTT test
Basic 0RTT test
Test: 0-RTT, but not 0.5-RTT
Test: 0.5-RTT, but not 0-RTT
Test: 0-RTT rejected by server