fix(proto): canonicalize remote IP in early_discard_packet's peer check (noq#738)

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!(

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 <https://github.com/n0-computer/noq/issues/738>
+#[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(())
+}