Return an error if the path is not permanent (#1698)

* Return an error if the path is not permanent

* address comments

* address comments and make sure the test case really triggers the crash

* clippy

* clippy again

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,

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<RefCell<dyn ConnectionIdGenerator>> =
+        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()));
+}