diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index b12e7c2ab6..139cfaba7b 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -2327,7 +2327,18 @@ impl Connection { // forbids migration, drop the datagram. This could be relaxed to heuristically // permit NAT-rebinding-like migration. if let Some(known_path) = self.path_mut(path_id) { - if network_path.remote != known_path.network_path.remote && !peer_may_probe { + // noq#738: like the local_ip comparison below, dual-stack sockets can report + // the remote peer's address as an IPv4-mapped-IPv6 address (`::ffff:a.b.c.d`) + // on one side of this comparison and a plain IPv4 address on the other, + // depending on how the underlying fd was created (observed on Android when a + // physical-interface-bound socket is handed to noq via an abstract socket). + // Canonicalize both sides' IPs before comparing so this doesn't spuriously + // trip the "unrecognized peer" discard. + let remote_matches = network_path.remote.port() + == known_path.network_path.remote.port() + && network_path.remote.ip().to_canonical() + == known_path.network_path.remote.ip().to_canonical(); + if !remote_matches && !peer_may_probe { trace!( %path_id, %network_path, @@ -2339,7 +2350,15 @@ impl Connection { if known_path.network_path.local_ip.is_some() && network_path.local_ip.is_some() - && known_path.network_path.local_ip != network_path.local_ip + && known_path + .network_path + .local_ip + .as_ref() + .map(std::net::IpAddr::to_canonical) + != network_path + .local_ip + .as_ref() + .map(std::net::IpAddr::to_canonical) && !local_ip_may_migrate { trace!( diff --git a/noq-proto/src/tests/multipath.rs b/noq-proto/src/tests/multipath.rs index 2b00fd542c..b6061f7846 100644 --- a/noq-proto/src/tests/multipath.rs +++ b/noq-proto/src/tests/multipath.rs @@ -1,6 +1,6 @@ //! Tests for multipath -use std::net::SocketAddr; +use std::net::{Ipv6Addr, SocketAddr}; use std::num::NonZeroU32; use std::sync::Arc; use std::time::Duration; @@ -2293,3 +2293,77 @@ fn regression_discarded_path_stats_are_up_to_date() -> TestResult { Ok(()) } + +/// Regression test for issue #738. +/// +/// When a client opens a new path with an explicit `local_ip` set in the +/// [`FourTuple`], the path should validate successfully. On real devices the +/// `PATH_RESPONSE` was never matched to the outstanding `PATH_CHALLENGE` on +/// such paths, causing them to be abandoned with +/// [`PathAbandonReason::ValidationFailed`]. +/// +/// This test sets up a routing table where the client has a second interface +/// that can reach the *same* server address as the initial connection, and +/// opens a path on that second interface with an explicit `local_ip`. +/// +/// See +#[test] +fn open_path_with_explicit_local_ip() -> TestResult { + let _guard = subscribe(); + let mut pair = ConnPair::builder().enable_multipath().connect(); + + // Set up routing with a second client interface that can reach the same + // server as the first interface. Both server routes point to the same + // server address but link to different client interfaces, so the server + // can respond on either client interface. + let first_client_addr = pair.routes.as_basic().client_addr; + let server_addr = pair.routes.as_basic().server_addr; + let second_client_addr = { + let mut addr = first_client_addr; + if let SocketAddr::V6(v6) = &mut addr { + let s = v6.ip().segments(); + v6.set_ip(Ipv6Addr::new( + s[0], + s[1], + s[2], + s[3], + s[4], + s[5], + s[6], + s[7] + 1, + )); + } + addr + }; + + // `ManyToManyRouting::from_routes` rejects duplicate interface addresses + // (added by #721, after this test was originally written), which trips on + // the same `server_addr` appearing twice. `add_client_route`/`add_server_route` + // don't have that check, so build the same routing table incrementally instead. + let mut routing = ManyToManyRouting::simple_symmetric([first_client_addr], [server_addr]); + routing.add_client_route(second_client_addr, 0); + routing.add_server_route(server_addr, 1); + pair.routes = routing.into(); + + // Open a path with an explicit local_ip, targeting the same server as + // the initial connection (path 0). + let new_path = FourTuple { + local_ip: Some(second_client_addr.ip()), + remote: server_addr, + }; + let path_id = pair.open_path(Client, new_path, PathStatus::Available)?; + pair.drive(); + + // The path should be established on both sides, not abandoned with + // ValidationFailed. + assert_matches!( + pair.poll(Client), + Some(Event::Path(crate::PathEvent::Established { id })) if id == path_id + ); + assert_matches!( + pair.poll(Server), + Some(Event::Path(crate::PathEvent::Established { id })) if id == path_id + ); + + Ok(()) +}