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/5] 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 1118737bcf..ad8089a4a9 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 109f6fb7fb..a2a9f996b8 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/5] 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 32774de36b..a87b74761c 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 ad8089a4a9..cecb7206d7 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(); From a072a93bbbe27e19ee88358e9a3000e00b66659b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philipp=20Kr=C3=BCger?= Date: Thu, 18 Jun 2026 12:07:31 +0200 Subject: [PATCH 3/5] Add a comment for future us --- noq-proto/src/tests/multipath.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/noq-proto/src/tests/multipath.rs b/noq-proto/src/tests/multipath.rs index cecb7206d7..d93d299416 100644 --- a/noq-proto/src/tests/multipath.rs +++ b/noq-proto/src/tests/multipath.rs @@ -2144,7 +2144,10 @@ fn regression_delayed_path_cids_blocked() -> TestResult { 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 + // Capture the server's PATH_NEW_CONNECTION_ID frames so the client generates a PATH_CIDS_BLOCKED frame on the next open_path call. + // This only works, because the server's outbound packet is constructed inefficiently: + // The PATH_NEW_CONNECTION_ID frames should *actually* be coaleced together with the server's handshake response instead of + // being put into a separate datagram. See also 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(); From 61c3898c9dbafc5124cc3eed044a88d6e8b8ae2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philipp=20Kr=C3=BCger?= Date: Thu, 18 Jun 2026 12:08:27 +0200 Subject: [PATCH 4/5] fix(proto): Clear pending PATH_CIDS_BLOCKED upon receiving CIDs --- noq-proto/src/connection/mod.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index a87b74761c..8ad3eeb93d 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -5246,6 +5246,20 @@ impl Connection { } }; + // No need to send/resend PATH_CIDS_BLOCKED anymore + self.spaces[SpaceId::Data] + .pending + .path_cids_blocked + .retain(|&id| id != path_id); + + for space in self.spaces[SpaceId::Data].iter_paths_mut() { + for sent_packet in space.sent_packets.values_mut() { + if let Some(retransmits) = sent_packet.retransmits.get_mut() { + retransmits.path_cids_blocked.retain(|&id| id != path_id); + } + } + } + if self.side.is_server() && path_id == PathId::ZERO && self From cbb45a7ab8931d65e3392b0db4660e112d041a54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philipp=20Kr=C3=BCger?= Date: Thu, 18 Jun 2026 12:15:14 +0200 Subject: [PATCH 5/5] fix(proto): Avoid retransmitting PATHS_BLOCKED after receiving MAX_PATH_ID --- noq-proto/src/connection/mod.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index 8ad3eeb93d..1b22b5880b 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -5459,6 +5459,16 @@ impl Connection { // frames that do not increase the path id are ignored if path_id > self.remote_max_path_id { self.remote_max_path_id = path_id; + // Avoid retransmitting PATHS_BLOCKED in case it got lost and we got the MAX_PATH_ID frame in the mean time. + self.spaces[SpaceId::Data].pending.paths_blocked = false; + for space in self.spaces[SpaceId::Data].iter_paths_mut() { + for sent_packet in space.sent_packets.values_mut() { + if let Some(retransmits) = sent_packet.retransmits.get_mut() { + retransmits.paths_blocked = false; + } + } + } + self.issue_first_path_cids(now); self.open_nat_traversed_paths(now); }