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

Allow app to accept/reject/retry before handshake begins #1752

Merged
merged 7 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,6 @@ Cargo.lock
.idea
.DS_Store
.vscode

cargo-test-*
tarpaulin-report.html
2 changes: 1 addition & 1 deletion perf/src/bin/perf_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ async fn run(opt: Opt) -> Result<()> {
Ok(())
}

async fn handle(handshake: quinn::Connecting, opt: Arc<Opt>) -> Result<()> {
async fn handle(handshake: quinn::Incoming, opt: Arc<Opt>) -> Result<()> {
let connection = handshake.await.context("handshake failed")?;
debug!("{} connected", connection.remote_address());
tokio::try_join!(
Expand Down
29 changes: 0 additions & 29 deletions quinn-proto/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -757,16 +757,9 @@ pub struct ServerConfig {
/// Used to generate one-time AEAD keys to protect handshake tokens
pub(crate) token_key: Arc<dyn HandshakeTokenKey>,

/// Whether to require clients to prove ownership of an address before committing resources.
///
/// Introduces an additional round-trip to the handshake to make denial of service attacks more difficult.
pub(crate) use_retry: bool,
/// Microseconds after a stateless retry token was issued for which it's considered valid.
pub(crate) retry_token_lifetime: Duration,

/// Maximum number of concurrent connections
pub(crate) concurrent_connections: u32,

/// Whether to allow clients to migrate to new addresses
///
/// Improves behavior for clients that move between different internet connections or suffer NAT
Expand All @@ -785,11 +778,8 @@ impl ServerConfig {
crypto,

token_key,
use_retry: false,
retry_token_lifetime: Duration::from_secs(15),

concurrent_connections: 100_000,

migration: true,
}
}
Expand All @@ -806,29 +796,12 @@ impl ServerConfig {
self
}

/// Whether to require clients to prove ownership of an address before committing resources.
///
/// Introduces an additional round-trip to the handshake to make denial of service attacks more difficult.
pub fn use_retry(&mut self, value: bool) -> &mut Self {
self.use_retry = value;
self
}

/// Duration after a stateless retry token was issued for which it's considered valid.
pub fn retry_token_lifetime(&mut self, value: Duration) -> &mut Self {
self.retry_token_lifetime = value;
self
}

/// Maximum number of simultaneous connections to accept.
///
/// New incoming connections are only accepted if the total number of incoming or outgoing
/// connections is less than this. Outgoing connections are unaffected.
pub fn concurrent_connections(&mut self, value: u32) -> &mut Self {
self.concurrent_connections = value;
self
}

/// Whether to allow clients to migrate to new addresses
///
/// Improves behavior for clients that move between different internet connections or suffer NAT
Expand Down Expand Up @@ -874,9 +847,7 @@ impl fmt::Debug for ServerConfig {
.field("transport", &self.transport)
.field("crypto", &"ServerConfig { elided }")
.field("token_key", &"[ elided ]")
.field("use_retry", &self.use_retry)
.field("retry_token_lifetime", &self.retry_token_lifetime)
.field("concurrent_connections", &self.concurrent_connections)
.field("migration", &self.migration)
.finish()
}
Expand Down
16 changes: 13 additions & 3 deletions quinn-proto/src/connection/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@ impl Connection {
version: u32,
allow_mtud: bool,
rng_seed: [u8; 32],
path_validated: bool,
) -> Self {
let side = if server_config.is_some() {
Side::Server
Expand All @@ -268,7 +269,6 @@ impl Connection {
client_hello: None,
});
let mut rng = StdRng::from_seed(rng_seed);
let path_validated = server_config.as_ref().map_or(true, |c| c.use_retry);
let mut this = Self {
endpoint_config,
server_config,
Expand Down Expand Up @@ -2195,7 +2195,10 @@ impl Connection {
}
ConnectionError::VersionMismatch => State::Draining,
ConnectionError::LocallyClosed => {
unreachable!("LocallyClosed isn't generated by packet processing")
unreachable!("LocallyClosed isn't generated by packet processing");
}
ConnectionError::CidsExhausted => {
unreachable!("CidsExhausted isn't generated by packet processing");
}
};
}
Expand Down Expand Up @@ -3508,6 +3511,11 @@ pub enum ConnectionError {
/// The local application closed the connection
#[error("closed")]
LocallyClosed,
/// The connection could not be created because not enough of the CID space is available
///
/// Try using longer connection IDs.
#[error("CIDs exhausted")]
CidsExhausted,
}

impl From<Close> for ConnectionError {
Expand All @@ -3527,7 +3535,9 @@ impl From<ConnectionError> for io::Error {
TimedOut => io::ErrorKind::TimedOut,
Reset => io::ErrorKind::ConnectionReset,
ApplicationClosed(_) | ConnectionClosed(_) => io::ErrorKind::ConnectionAborted,
TransportError(_) | VersionMismatch | LocallyClosed => io::ErrorKind::Other,
TransportError(_) | VersionMismatch | LocallyClosed | CidsExhausted => {
io::ErrorKind::Other
}
};
Self::new(kind, x)
}
Expand Down
Loading