From d2d97cf5c1a1db7edea7e8e64863e25d5798cf6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philipp=20Kr=C3=BCger?= Date: Wed, 17 Jun 2026 13:23:25 +0200 Subject: [PATCH 1/2] test(proto): Write regression test for receiving delayed PATH_CIDS_BLOCKED --- noq-proto/src/tests/multipath.rs | 70 ++++++++++++++++++++++++++++++++ noq-proto/src/tests/util.rs | 19 ++++++++- 2 files changed, 88 insertions(+), 1 deletion(-) diff --git a/noq-proto/src/tests/multipath.rs b/noq-proto/src/tests/multipath.rs index 1118737bc..ad8089a4a 100644 --- a/noq-proto/src/tests/multipath.rs +++ b/noq-proto/src/tests/multipath.rs @@ -2123,3 +2123,73 @@ fn on_path_challenge_lost_backoff() { let duration = pair.time.duration_since(last_challenge_send); assert_eq!(duration, Duration::from_millis(1)); } + +#[test] +fn regression_delayed_path_cids_blocked() -> TestResult { + let _guard = subscribe(); + + let (mut pair, client_cfg) = ConnPair::builder().enable_multipath().build_pair(); + info!("connecting"); + let client_ch = pair.begin_connect(client_cfg); + pair.drive_client(); // Client sends Initial + pair.drive_server(); // Server receives Initial, sends Handshake + pair.drive_client(); // Client receives Handshake, sends handshake confirmed + pair.drive_server(); // Server receives handshake confirmed, sends PATH_NEW_CONNECTION_ID and confirms handshake itself + // capture the server's PATH_NEW_CONNECTION_ID frames so the client generates a PATH_CIDS_BLOCKED frame on the next open_path call + let captured_server_cids = pair.client.inbound.pop_back().unwrap(); + pair.drive_client(); // Client receives confirmed handshake, but not PATH_NEW_CONNECTION_ID frames + let server_ch = pair.server.assert_accept(); + pair.finish_connect(client_ch, server_ch); + + // Attempting to open a path on the client side will fail, because we're missing the CIDs for path 1 and more + let mut pair = ConnPair::new(pair, client_ch, server_ch); + let server_addr = pair.routes.public_server_addr(); + pair.open_path( + Client, + FourTuple::from_remote(server_addr), + PathStatus::Available, + ) + .expect_err("expected RemoteCidsExhausted error"); + + pair.drive_client(); // Client generates PATH_CIDS_BLOCKED + // We intentionally drop the client's PATH_CIDS_BLOCKED frame. + pair.server.inbound.pop_back().unwrap(); + // After the client has sent the PATH_CIDS_BLOCKED frame, we give it all the server's CIDs. + pair.client.inbound.push_back(captured_server_cids); + pair.drive_client(); // Client processes the delayed server's PATH_NEW_CONNECTION_ID + + info!("Skipping forward 80ms"); + pair.time += Duration::from_millis(80); // Trigger the client's loss detection timer for the PATH_CIDS_BLOCKED frame + pair.drive_client(); // Client generates another PATH_CIDS_BLOCKED + // This is now an encrypted datagram containing a PATH_CIDS_BLOCKED path_id=1 next_seq=1 frame. + let captured_client_cids_blocked = pair.server.inbound.pop_back().unwrap(); + + let path_id = pair.open_path( + Client, + FourTuple::from_remote(server_addr), + PathStatus::Available, + )?; + pair.drive(); // Fully open the path on both ends + pair.close_path(Client, path_id, 42u32.into())?; + pair.drive(); // Fully process closing the path on both ends, including discarding path state + + // Now we send the delayed PATH_CIDS_BLOCKED frame, and it'll trigger a protocol violation + pair.server.inbound.push_front(captured_client_cids_blocked); + pair.drive(); + + // The server must not close the connection over the stale frame. + while let Some(event) = pair.poll(Server) { + if let Event::ConnectionLost { + reason: crate::ConnectionError::TransportError(error), + } = event + { + assert_ne!( + error.code, + TransportErrorCode::PROTOCOL_VIOLATION, + "stale PATH_CIDS_BLOCKED should not trigger PROTOCOL_VIOLATION: {}", + error.reason + ); + } + } + Ok(()) +} diff --git a/noq-proto/src/tests/util.rs b/noq-proto/src/tests/util.rs index 109f6fb7f..a2a9f996b 100644 --- a/noq-proto/src/tests/util.rs +++ b/noq-proto/src/tests/util.rs @@ -313,7 +313,12 @@ impl Pair { client_ch } - fn finish_connect(&mut self, client_ch: ConnectionHandle, server_ch: ConnectionHandle) { + /// Asserts connection events expected for a finished connection are emitted. + pub(super) fn finish_connect( + &mut self, + client_ch: ConnectionHandle, + server_ch: ConnectionHandle, + ) { assert_matches!( self.client_conn_mut(client_ch).poll(), Some(Event::HandshakeDataReady) @@ -564,6 +569,18 @@ impl ConnPair { Default::default() } + pub(super) fn new( + pair: Pair, + client_ch: ConnectionHandle, + server_ch: ConnectionHandle, + ) -> Self { + Self { + pair, + client_ch, + server_ch, + } + } + fn connect_with(mut pair: Pair, client_cfg: ClientConfig) -> Self { let (client_ch, server_ch) = pair.connect_with(client_cfg); Self { From c49030feba544e762115a262ef73c9d310e07cf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philipp=20Kr=C3=BCger?= Date: Wed, 17 Jun 2026 13:39:18 +0200 Subject: [PATCH 2/2] fix(proto): Ignore PATH_CIDS_BLOCKED referring to abandoned paths --- noq-proto/src/connection/mod.rs | 13 +++++++------ noq-proto/src/tests/multipath.rs | 9 +++++++++ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index 32774de36..a87b74761 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -5481,12 +5481,13 @@ impl Connection { "PATH_CIDS_BLOCKED path identifier was larger than local maximum", )); } - if next_seq.0 - > self - .local_cid_state - .get(&path_id) - .map(|cid_state| cid_state.active_seq().1 + 1) - .unwrap_or_default() + if !self.abandoned_paths.contains(&path_id) + && next_seq.0 + > self + .local_cid_state + .get(&path_id) + .map(|cid_state| cid_state.active_seq().1 + 1) + .unwrap_or_default() { return Err(TransportError::PROTOCOL_VIOLATION( "PATH_CIDS_BLOCKED next sequence number larger than in local state", diff --git a/noq-proto/src/tests/multipath.rs b/noq-proto/src/tests/multipath.rs index ad8089a4a..cecb7206d 100644 --- a/noq-proto/src/tests/multipath.rs +++ b/noq-proto/src/tests/multipath.rs @@ -2124,6 +2124,15 @@ fn on_path_challenge_lost_backoff() { assert_eq!(duration, Duration::from_millis(1)); } +/// This test used to generate a PROTOCOL_VIOLATION error from just packet loss and delayed packets. +/// +/// The problem was receiving a PATH_CIDS_BLOCKED frame with path_id=1 and next_seq=1 when the server +/// side had already abandoned and discarded path 1. +/// In that case, we assumed the other side would violate the protocol, whereas in reality it's just +/// a (very) delayed packet. +/// +/// The fix was to properly check if the path was already abandoned that the PATH_CIDS_BLOCKED frame +/// referred to. #[test] fn regression_delayed_path_cids_blocked() -> TestResult { let _guard = subscribe();