diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs index 671361b559..c81a3727c6 100644 --- a/neqo-transport/src/connection/mod.rs +++ b/neqo-transport/src/connection/mod.rs @@ -1937,6 +1937,14 @@ impl Connection { }; let path = close.path().borrow(); + // In some error cases, we will not be able to make a new, permanent path. + // For example, if we run out of connection IDs and the error results from + // a packet on a new path, we avoid sending (and the privacy risk) rather + // than reuse a connection ID. + if path.is_temporary() { + assert!(!cfg!(test), "attempting to close with a temporary path"); + return Err(Error::InternalError); + } let (_, mut builder) = Self::build_packet_header( &path, cspace, diff --git a/neqo-transport/src/connection/tests/migration.rs b/neqo-transport/src/connection/tests/migration.rs index 09a25faa28..405ae161a4 100644 --- a/neqo-transport/src/connection/tests/migration.rs +++ b/neqo-transport/src/connection/tests/migration.rs @@ -6,6 +6,7 @@ use std::{ cell::RefCell, + mem, net::{IpAddr, Ipv6Addr, SocketAddr}, rc::Rc, time::{Duration, Instant}, @@ -950,3 +951,39 @@ fn retire_prior_to_migration_success() { assert_ne!(get_cid(&dgram), original_cid); assert_ne!(get_cid(&dgram), probe_cid); } + +struct GarbageWriter {} + +impl crate::connection::test_internal::FrameWriter for GarbageWriter { + fn write_frames(&mut self, builder: &mut PacketBuilder) { + // Not a valid frame type. + builder.encode_varint(u32::MAX); + } +} + +/// Test the case that we run out of connection ID and receive an invalid frame +/// from a new path. +#[test] +#[should_panic(expected = "attempting to close with a temporary path")] +fn error_on_new_path_with_no_connection_id() { + let mut client = default_client(); + let mut server = default_server(); + connect_force_idle(&mut client, &mut server); + + let cid_gen: Rc> = + Rc::new(RefCell::new(CountingConnectionIdGenerator::default())); + server.test_frame_writer = Some(Box::new(RetireAll { cid_gen })); + let retire_all = send_something(&mut server, now()); + + client.process_input(&retire_all, now()); + + server.test_frame_writer = Some(Box::new(GarbageWriter {})); + let garbage = send_something(&mut server, now()); + + let dgram = change_path(&garbage, DEFAULT_ADDR_V4); + client.process_input(&dgram, now()); + + // See issue #1697. We had a crash when the client had a temporary path and + // process_output is called. + mem::drop(client.process_output(now())); +}