Ignore family-irrelevant extra Shadowsocks addresses

Author: dlon

mullvad-relay-selector/src/relay_selector/detailer.rs

@@ -159,15 +159,22 @@ fn get_address_for_wireguard_relay(
     query: &WireguardRelayQuery,
     relay: &Relay,
 ) -> Result<IpAddr, Error> {
-    match query.ip_version {
-        Constraint::Any | Constraint::Only(IpVersion::V4) => Ok(relay.ipv4_addr_in.into()),
-        Constraint::Only(IpVersion::V6) => relay
+    match resolve_ip_version(query.ip_version) {
+        IpVersion::V4 => Ok(relay.ipv4_addr_in.into()),
+        IpVersion::V6 => relay
             .ipv6_addr_in
             .map(|addr| addr.into())
             .ok_or(Error::NoIPv6(Box::new(relay.clone()))),
     }
 }
 
+pub fn resolve_ip_version(ip_version: Constraint<IpVersion>) -> IpVersion {
+    match ip_version {
+        Constraint::Any | Constraint::Only(IpVersion::V4) => IpVersion::V4,
+        Constraint::Only(IpVersion::V6) => IpVersion::V6,
+    }
+}
+
 /// Try to pick a valid Wireguard port.
 fn get_port_for_wireguard_relay(
     query: &WireguardRelayQuery,

mullvad-relay-selector/src/relay_selector/helpers.rs

@@ -139,6 +139,13 @@ fn get_shadowsocks_obfuscator_inner(
     extra_in_addrs: &[IpAddr],
     desired_port: Constraint<u16>,
 ) -> Option<SocketAddr> {
+    // Filter out addresses for the wrong address family
+    let extra_in_addrs: Vec<_> = extra_in_addrs
+        .iter()
+        .filter(|addr| addr.is_ipv4() == wg_in_addr.is_ipv4())
+        .copied()
+        .collect();
+
     let in_ip = extra_in_addrs
         .iter()
         .choose(&mut rand::thread_rng())
@@ -283,4 +290,51 @@ mod tests {
             "expected selected port, got {selected_addr:?}"
         );
     }
+
+    /// Extra addresses that belong to the wrong IP family should be ignored
+    #[test]
+    fn test_shadowsocks_irrelevant_extra_addrs() {
+        const PORT_RANGES: &[(u16, u16)] = &[(100, 200), (1000, 2000)];
+        const IN_RANGE_PORT: u16 = 100;
+        const OUT_OF_RANGE_PORT: u16 = 1;
+        let wg_in_ip: IpAddr = "1.2.3.4".parse().unwrap();
+
+        let extra_in_addrs: &[IpAddr] = &["::2".parse().unwrap()];
+
+        let selected_addr = get_shadowsocks_obfuscator_inner(
+            wg_in_ip,
+            PORT_RANGES,
+            extra_in_addrs,
+            Constraint::Any,
+        )
+        .expect("should find valid port without constraint");
+
+        assert_eq!(
+            selected_addr.ip(),
+            wg_in_ip,
+            "expected extra IP to be ignored"
+        );
+
+        let selected_addr = get_shadowsocks_obfuscator_inner(
+            wg_in_ip,
+            PORT_RANGES,
+            extra_in_addrs,
+            Constraint::Only(OUT_OF_RANGE_PORT),
+        );
+        assert!(
+            selected_addr.is_none(),
+            "expected no match for out-of-range port"
+        );
+
+        let selected_addr = get_shadowsocks_obfuscator_inner(
+            wg_in_ip,
+            PORT_RANGES,
+            extra_in_addrs,
+            Constraint::Only(IN_RANGE_PORT),
+        );
+        assert!(
+            selected_addr.is_some(),
+            "expected match for within-range port"
+        );
+    }
 }

mullvad-relay-selector/src/relay_selector/matcher.rs

@@ -10,7 +10,7 @@ use mullvad_types::{
     },
     relay_list::{Relay, RelayEndpointData, WireguardRelayEndpointData},
 };
-use talpid_types::net::TunnelType;
+use talpid_types::net::{IpVersion, TunnelType};
 
 use super::{
     parsed_relays::ParsedRelays,
@@ -142,6 +142,7 @@ fn filter_on_obfuscation(
             let wg_data = &relay_list.parsed_list().wireguard;
             filter_on_shadowsocks(
                 &wg_data.shadowsocks_port_ranges,
+                &query.ip_version,
                 &query.shadowsocks_port,
                 relay,
             )
@@ -155,20 +156,31 @@ fn filter_on_obfuscation(
 /// Returns whether `relay` satisfies the Shadowsocks filter posed by `port`.
 fn filter_on_shadowsocks(
     port_ranges: &[(u16, u16)],
-    settings: &Constraint<ShadowsocksSettings>,
+    ip_version: &Constraint<IpVersion>,
+    shadowsocks: &Constraint<ShadowsocksSettings>,
     relay: &Relay,
 ) -> bool {
-    match (&settings, &relay.endpoint_data) {
+    let ip_version = super::detailer::resolve_ip_version(*ip_version);
+
+    match (&shadowsocks, &relay.endpoint_data) {
         // If Shadowsocks is specifically asked for, we must check if the specific relay supports our port.
         // If there are extra addresses, then all ports are available, so we do not need to do this.
         (
             Constraint::Only(ShadowsocksSettings {
                 port: Constraint::Only(desired_port),
             }),
             RelayEndpointData::Wireguard(wg_data),
-        ) if wg_data.shadowsocks_extra_addr_in.is_empty() => port_ranges
-            .iter()
-            .any(|(begin, end)| (*begin..=*end).contains(&desired_port)),
+        ) => {
+            let filtered_extra_addrs = wg_data
+                .shadowsocks_extra_addr_in
+                .iter()
+                .find(|&&addr| IpVersion::from(addr) == ip_version);
+
+            filtered_extra_addrs.is_some()
+                || port_ranges
+                    .iter()
+                    .any(|(begin, end)| (*begin..=*end).contains(desired_port))
+        }
 
         // Otherwise, any relay works.
         _ => true,

talpid-types/src/net/mod.rs

@@ -438,6 +438,15 @@ pub enum IpVersion {
     V6,
 }
 
+impl From<IpAddr> for IpVersion {
+    fn from(value: IpAddr) -> Self {
+        match value {
+            IpAddr::V4(_) => IpVersion::V4,
+            IpAddr::V6(_) => IpVersion::V6,
+        }
+    }
+}
+
 impl fmt::Display for IpVersion {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
         match *self {