diff --git a/deny.toml b/deny.toml index 49ef4ea547..45a2f90173 100644 --- a/deny.toml +++ b/deny.toml @@ -20,8 +20,8 @@ license-files = [{ path = "LICENSE", hash = 0xbd0eed23 }] [advisories] ignore = [ - # `paste` is unmaintained - "RUSTSEC-2024-0436", + # async-std is unmaintained + "RUSTSEC-2025-0052", ] [sources] diff --git a/quinn-proto/src/connection/mod.rs b/quinn-proto/src/connection/mod.rs index 3a953a9ede..0457cfa8df 100644 --- a/quinn-proto/src/connection/mod.rs +++ b/quinn-proto/src/connection/mod.rs @@ -339,6 +339,7 @@ impl Connection { rem_cid_set: side.is_server(), expected_token: Bytes::new(), client_hello: None, + allow_server_migration: side.is_client(), }); let local_cid_state = FxHashMap::from_iter([( PathId(0), @@ -1637,7 +1638,7 @@ impl Connection { // forbids migration, drop the datagram. This could be relaxed to heuristically // permit NAT-rebinding-like migration. if let Some(known_remote) = self.path(path_id).map(|path| path.remote) { - if remote != known_remote && !self.side.remote_may_migrate() { + if remote != known_remote && !self.side.remote_may_migrate(&self.state) { trace!("discarding packet from unrecognized peer {}", remote); return; } @@ -3093,9 +3094,16 @@ impl Connection { debug!(%remote, %path_id, "discarding multipath packet during handshake"); return; } - if remote != self.path_data(path_id).remote { - debug!("discarding packet with unexpected remote during handshake"); - return; + if remote != self.path_data_mut(path_id).remote { + match self.state { + State::Handshake(ref hs) if hs.allow_server_migration => { + self.path_data_mut(path_id).remote = remote; + } + _ => { + debug!("discarding packet with unexpected remote during handshake"); + return; + } + } } } @@ -3285,7 +3293,9 @@ impl Connection { match packet.header { Header::Retry { - src_cid: rem_cid, .. + src_cid: rem_cid, + dst_cid: loc_cid, + .. } => { debug_assert_eq!(path_id, PathId(0)); if self.side.is_server() { @@ -3328,6 +3338,10 @@ impl Connection { .update_initial_cid(rem_cid); self.rem_handshake_cid = rem_cid; + if loc_cid.len() >= 64 && loc_cid == self.handshake_cid { + self.on_path_validated(path_id); + } + let space = &mut self.spaces[SpaceId::Initial]; if let Some(info) = space.for_path(PathId(0)).take(0) { self.on_packet_acked(now, PathId(0), 0, info); @@ -3363,10 +3377,16 @@ impl Connection { unreachable!("we already short-circuited if we're server"); }; *token = packet.payload.freeze().split_to(token_len); + + // If the retry packet validated the server's address, the server is no + // longer allowed to migrate. + let allow_server_migration = + matches!(self.state, State::Handshake(ref hs) if hs.allow_server_migration); self.state = State::Handshake(state::Handshake { expected_token: Bytes::new(), rem_cid_set: false, client_hello: None, + allow_server_migration, }); Ok(()) } @@ -5234,14 +5254,23 @@ impl Connection { /// Mark the path as validated, and enqueue NEW_TOKEN frames to be sent as appropriate fn on_path_validated(&mut self, path_id: PathId) { self.path_data_mut(path_id).validated = true; - let ConnectionSide::Server { server_config } = &self.side else { - return; - }; - let remote_addr = self.path_data(path_id).remote; - let new_tokens = &mut self.spaces[SpaceId::Data as usize].pending.new_tokens; - new_tokens.clear(); - for _ in 0..server_config.validation_token.sent { - new_tokens.push(remote_addr); + match &self.side { + ConnectionSide::Client { .. } => { + // If we are still handshaking we've just validated the first path. From + // now on we should not allow the server to migrate address anymore. + if let State::Handshake(ref mut hs) = self.state { + hs.allow_server_migration = false; + } + } + ConnectionSide::Server { server_config } => { + // enqueue NEW_TOKEN frames + let remote_addr = self.path_data(path_id).remote; + let new_tokens = &mut self.spaces[SpaceId::Data as usize].pending.new_tokens; + new_tokens.clear(); + for _ in 0..server_config.validation_token.sent { + new_tokens.push(remote_addr); + } + } } } @@ -5304,10 +5333,13 @@ enum ConnectionSide { } impl ConnectionSide { - fn remote_may_migrate(&self) -> bool { + fn remote_may_migrate(&self, state: &State) -> bool { match self { Self::Server { server_config } => server_config.migration, - Self::Client { .. } => false, + Self::Client { .. } => match state { + State::Handshake(handshake) => handshake.allow_server_migration, + _ => false, + }, } } @@ -5527,6 +5559,20 @@ mod state { /// /// Only set for clients pub(super) client_hello: Option, + /// Whether the server address is allowed to migrate + /// + /// We allow the server to migrate during the handshake as long as we have not + /// validated it's address: it can send a response from a different address than we + /// sent the initial to. This allows us to send the inial over multiple paths - by + /// means of an IPv6 ULA address that copies the packets sent to it to multiple + /// destinations - and accept one response. + /// + /// This is only ever set to true if for a client which hasn't validated the server + /// address yet. It is set back to false in [`Connection::on_path_validated`]. + /// + /// THIS IS NOT RFC 9000 COMPLIANT! A server is not allowed to migrate addresses, + /// other than using the preferred-address transport parameter. + pub(super) allow_server_migration: bool, } #[allow(unreachable_pub)] // fuzzing only diff --git a/quinn-proto/src/connection/streams/mod.rs b/quinn-proto/src/connection/streams/mod.rs index 53e42815ee..3e769c74f9 100644 --- a/quinn-proto/src/connection/streams/mod.rs +++ b/quinn-proto/src/connection/streams/mod.rs @@ -125,7 +125,7 @@ impl RecvStream<'_> { /// control window is filled. On any given stream, you can switch from ordered to unordered /// reads, but ordered reads on streams that have seen previous unordered reads will return /// `ReadError::IllegalOrderedRead`. - pub fn read(&mut self, ordered: bool) -> Result { + pub fn read(&mut self, ordered: bool) -> Result, ReadableError> { Chunks::new(self.id, ordered, self.state, self.pending) } diff --git a/quinn/src/mutex.rs b/quinn/src/mutex.rs index 7c24df4685..29b257412b 100644 --- a/quinn/src/mutex.rs +++ b/quinn/src/mutex.rs @@ -133,7 +133,7 @@ mod non_tracking { /// Acquires the lock for a certain purpose /// /// The purpose will be recorded in the list of last lock owners - pub(crate) fn lock(&self, _purpose: &'static str) -> MutexGuard { + pub(crate) fn lock(&self, _purpose: &'static str) -> MutexGuard<'_, T> { MutexGuard { guard: self.inner.lock().unwrap(), }