From b42d608e4f666c0d3cf020395db2e7b42963e6f2 Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Wed, 21 Aug 2024 14:06:08 +0200 Subject: [PATCH 1/3] Fix IP-override feature indicator It was trigger by any overrides existing in the settings, not by the current endpoint being overridden. Add flag to `Relay` to specify if its IPv4 and/or IPv6 has been overridden and use that in combination with the endpoint IP version to derive if the current connection is overridden. --- mullvad-api/src/relay_list.rs | 2 + mullvad-daemon/src/lib.rs | 21 ++++++---- mullvad-daemon/src/tunnel.rs | 39 ++++++++++++++++++- .../src/types/conversions/relay_list.rs | 2 + .../src/relay_selector/matcher.rs | 5 ++- .../tests/relay_selector.rs | 30 +++++++++++++- mullvad-types/src/features.rs | 33 +++++++++------- mullvad-types/src/relay_constraints.rs | 4 +- mullvad-types/src/relay_list.rs | 18 +++++++++ 9 files changed, 124 insertions(+), 30 deletions(-) diff --git a/mullvad-api/src/relay_list.rs b/mullvad-api/src/relay_list.rs index 22410549b885..afe71c829d1a 100644 --- a/mullvad-api/src/relay_list.rs +++ b/mullvad-api/src/relay_list.rs @@ -154,6 +154,8 @@ fn into_mullvad_relay( hostname: relay.hostname, ipv4_addr_in: relay.ipv4_addr_in, ipv6_addr_in: relay.ipv6_addr_in, + overridden_ipv4: false, + overridden_ipv6: false, include_in_country: relay.include_in_country, active: relay.active, owned: relay.owned, diff --git a/mullvad-daemon/src/lib.rs b/mullvad-daemon/src/lib.rs index cb1f489f114b..926946dc7b79 100644 --- a/mullvad-daemon/src/lib.rs +++ b/mullvad-daemon/src/lib.rs @@ -958,7 +958,7 @@ impl Daemon { DeviceMigrationEvent(event) => self.handle_device_migration_event(event), LocationEvent(location_data) => self.handle_location_event(location_data), SettingsChanged => { - self.update_feature_indicators_on_settings_changed(); + self.update_feature_indicators_on_settings_changed().await; } #[cfg(any(windows, target_os = "android", target_os = "macos"))] ExcludedPathsEvent(update, tx) => self.handle_new_excluded_paths(update, tx).await, @@ -980,8 +980,11 @@ impl Daemon { locked_down, }, TunnelStateTransition::Connecting(endpoint) => { - let feature_indicators = - compute_feature_indicators(&self.settings.to_settings(), &endpoint); + let feature_indicators = compute_feature_indicators( + &self.settings.to_settings(), + &endpoint, + self.parameters_generator.last_relay_was_overridden().await, + ); TunnelState::Connecting { endpoint, location: self.parameters_generator.get_last_location().await, @@ -989,8 +992,11 @@ impl Daemon { } } TunnelStateTransition::Connected(endpoint) => { - let feature_indicators = - compute_feature_indicators(&self.settings.to_settings(), &endpoint); + let feature_indicators = compute_feature_indicators( + &self.settings.to_settings(), + &endpoint, + self.parameters_generator.last_relay_was_overridden().await, + ); TunnelState::Connected { endpoint, location: self.parameters_generator.get_last_location().await, @@ -1129,7 +1135,7 @@ impl Daemon { } /// Update the set of feature indicators based on the new settings. - fn update_feature_indicators_on_settings_changed(&mut self) { + async fn update_feature_indicators_on_settings_changed(&mut self) { // Updated settings may affect the feature indicators, even if they don't change the tunnel // state (e.g. activating lockdown mode). Note that only the connected and connecting states // have feature indicators. @@ -1144,8 +1150,9 @@ impl Daemon { endpoint, .. } => { + let ip_override = self.parameters_generator.last_relay_was_overridden().await; let new_feature_indicators = - compute_feature_indicators(&self.settings.to_settings(), endpoint); + compute_feature_indicators(&self.settings.to_settings(), endpoint, ip_override); // Update and broadcast the new feature indicators if they have changed if *feature_indicators != new_feature_indicators { // Make sure to update the daemon's actual tunnel state. Otherwise, feature diff --git a/mullvad-daemon/src/tunnel.rs b/mullvad-daemon/src/tunnel.rs index 839f07e29909..54bee64e7b5c 100644 --- a/mullvad-daemon/src/tunnel.rs +++ b/mullvad-daemon/src/tunnel.rs @@ -87,6 +87,22 @@ impl ParametersGenerator { self.0.lock().await.tunnel_options = tunnel_options.clone(); } + pub async fn last_relay_was_overridden(&self) -> bool { + let inner = self.0.lock().await; + let Some(relays) = inner.last_generated_relays.as_ref() else { + return false; + }; + match relays { + LastSelectedRelays::WireGuard { + server_override, .. + } => *server_override, + #[cfg(not(target_os = "android"))] + LastSelectedRelays::OpenVpn { + server_override, .. + } => *server_override, + } + } + /// Gets the location associated with the last generated tunnel parameters. pub async fn get_last_location(&self) -> Option { let inner = self.0.lock().await; @@ -106,6 +122,7 @@ impl ParametersGenerator { wg_entry: entry, wg_exit: exit, obfuscator, + .. } => { entry_hostname = take_hostname(entry); hostname = exit.hostname.clone(); @@ -114,7 +131,7 @@ impl ParametersGenerator { location = exit.location.as_ref().cloned().unwrap(); } #[cfg(not(target_os = "android"))] - LastSelectedRelays::OpenVpn { relay, bridge } => { + LastSelectedRelays::OpenVpn { relay, bridge, .. } => { hostname = relay.hostname.clone(); bridge_hostname = take_hostname(bridge); entry_hostname = None; @@ -158,9 +175,15 @@ impl InnerParametersGenerator { bridge, } => { let bridge_relay = bridge.as_ref().and_then(|bridge| bridge.relay()); + let server_override = { + let first_relay = bridge_relay.unwrap_or(&exit); + (first_relay.overridden_ipv4 && endpoint.address.is_ipv4()) + || (first_relay.overridden_ipv6 && endpoint.address.is_ipv6()) + }; self.last_generated_relays = Some(LastSelectedRelays::OpenVpn { relay: exit.clone(), bridge: bridge_relay.cloned(), + server_override, }); let bridge_settings = bridge.as_ref().map(|bridge| bridge.settings()); Ok(self.create_openvpn_tunnel_parameters(endpoint, data, bridge_settings.cloned())) @@ -179,10 +202,17 @@ impl InnerParametersGenerator { WireguardConfig::Singlehop { exit } => (None, exit), WireguardConfig::Multihop { exit, entry } => (Some(entry), exit), }; + let server_override = { + let first_relay = wg_entry.as_ref().unwrap_or(&wg_exit); + (first_relay.overridden_ipv4 && endpoint.peer.endpoint.is_ipv4()) + || (first_relay.overridden_ipv6 && endpoint.peer.endpoint.is_ipv6()) + }; + self.last_generated_relays = Some(LastSelectedRelays::WireGuard { wg_entry, wg_exit, obfuscator: obfuscator_relay, + server_override, }); Ok(self.create_wireguard_tunnel_parameters(endpoint, data, obfuscator_config)) @@ -315,10 +345,15 @@ enum LastSelectedRelays { wg_entry: Option, wg_exit: Relay, obfuscator: Option, + server_override: bool, }, /// Represents all relays generated for an OpenVPN tunnel. /// The traffic flows like this: /// client -> bridge -> relay -> internet #[cfg(not(target_os = "android"))] - OpenVpn { relay: Relay, bridge: Option }, + OpenVpn { + relay: Relay, + bridge: Option, + server_override: bool, + }, } diff --git a/mullvad-management-interface/src/types/conversions/relay_list.rs b/mullvad-management-interface/src/types/conversions/relay_list.rs index e38b32eeed3d..a8b69d87fafe 100644 --- a/mullvad-management-interface/src/types/conversions/relay_list.rs +++ b/mullvad-management-interface/src/types/conversions/relay_list.rs @@ -291,6 +291,8 @@ impl TryFrom for mullvad_types::relay_list::Relay { FromProtobufTypeError::InvalidArgument("invalid relay IPv4 address") })?, ipv6_addr_in, + overridden_ipv4: false, + overridden_ipv6: false, include_in_country: relay.include_in_country, active: relay.active, owned: relay.owned, diff --git a/mullvad-relay-selector/src/relay_selector/matcher.rs b/mullvad-relay-selector/src/relay_selector/matcher.rs index 8277619cb6a7..7f5607dfe50c 100644 --- a/mullvad-relay-selector/src/relay_selector/matcher.rs +++ b/mullvad-relay-selector/src/relay_selector/matcher.rs @@ -163,8 +163,9 @@ fn filter_on_shadowsocks( let ip_version = super::detailer::resolve_ip_version(*ip_version); match (settings, &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. + // 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. ( ShadowsocksSettings { port: Constraint::Only(desired_port), diff --git a/mullvad-relay-selector/tests/relay_selector.rs b/mullvad-relay-selector/tests/relay_selector.rs index ee0716aed12e..f0e48641c9a1 100644 --- a/mullvad-relay-selector/tests/relay_selector.rs +++ b/mullvad-relay-selector/tests/relay_selector.rs @@ -47,6 +47,8 @@ static RELAYS: Lazy = Lazy::new(|| RelayList { hostname: "se9-wireguard".to_string(), ipv4_addr_in: "185.213.154.68".parse().unwrap(), ipv6_addr_in: Some("2a03:1b20:5:f011::a09f".parse().unwrap()), + overridden_ipv4: false, + overridden_ipv6: false, include_in_country: true, active: true, owned: true, @@ -66,6 +68,8 @@ static RELAYS: Lazy = Lazy::new(|| RelayList { hostname: "se10-wireguard".to_string(), ipv4_addr_in: "185.213.154.69".parse().unwrap(), ipv6_addr_in: Some("2a03:1b20:5:f011::a10f".parse().unwrap()), + overridden_ipv4: false, + overridden_ipv6: false, include_in_country: true, active: true, owned: false, @@ -85,6 +89,8 @@ static RELAYS: Lazy = Lazy::new(|| RelayList { hostname: "se-got-001".to_string(), ipv4_addr_in: "185.213.154.131".parse().unwrap(), ipv6_addr_in: None, + overridden_ipv4: false, + overridden_ipv6: false, include_in_country: true, active: true, owned: true, @@ -97,6 +103,8 @@ static RELAYS: Lazy = Lazy::new(|| RelayList { hostname: "se-got-002".to_string(), ipv4_addr_in: "1.2.3.4".parse().unwrap(), ipv6_addr_in: None, + overridden_ipv4: false, + overridden_ipv6: false, include_in_country: true, active: true, owned: true, @@ -109,6 +117,8 @@ static RELAYS: Lazy = Lazy::new(|| RelayList { hostname: "se-got-br-001".to_string(), ipv4_addr_in: "1.3.3.7".parse().unwrap(), ipv6_addr_in: None, + overridden_ipv4: false, + overridden_ipv6: false, include_in_country: true, active: true, owned: true, @@ -182,6 +192,8 @@ static SHADOWSOCKS_RELAY: Lazy = Lazy::new(|| Relay { .to_owned(), ipv4_addr_in: SHADOWSOCKS_RELAY_IPV4, ipv6_addr_in: Some(SHADOWSOCKS_RELAY_IPV6), + overridden_ipv4: false, + overridden_ipv6: false, include_in_country: true, active: true, owned: true, @@ -463,6 +475,8 @@ fn test_wireguard_entry() { hostname: "se9-wireguard".to_string(), ipv4_addr_in: "185.213.154.68".parse().unwrap(), ipv6_addr_in: Some("2a03:1b20:5:f011::a09f".parse().unwrap()), + overridden_ipv4: false, + overridden_ipv6: false, include_in_country: true, active: true, owned: true, @@ -482,6 +496,8 @@ fn test_wireguard_entry() { hostname: "se10-wireguard".to_string(), ipv4_addr_in: "185.213.154.69".parse().unwrap(), ipv6_addr_in: Some("2a03:1b20:5:f011::a10f".parse().unwrap()), + overridden_ipv4: false, + overridden_ipv6: false, include_in_country: true, active: true, owned: false, @@ -746,7 +762,8 @@ fn test_selecting_any_relay_will_consider_multihop() { } } -/// Test whether Shadowsocks is always selected as the obfuscation protocol when Shadowsocks is selected. +/// Test whether Shadowsocks is always selected as the obfuscation protocol when Shadowsocks is +/// selected. #[test] fn test_selecting_wireguard_over_shadowsocks() { let relay_selector = RelaySelector::from_list(SelectorConfig::default(), RELAYS.clone()); @@ -828,9 +845,10 @@ fn test_selecting_wireguard_ignore_extra_ips_override_v4() { match relay { GetRelay::Wireguard { obfuscator: Some(SelectedObfuscator { config: ObfuscatorConfig::Shadowsocks { endpoint }, .. }), - inner: WireguardConfig::Singlehop { .. }, + inner: WireguardConfig::Singlehop { exit }, .. } => { + assert!(exit.overridden_ipv4); assert_eq!(endpoint.ip(), IpAddr::from(OVERRIDE_IPV4)); } wrong_relay => panic!( @@ -1118,6 +1136,8 @@ fn test_include_in_country() { hostname: "se9-wireguard".to_string(), ipv4_addr_in: "185.213.154.68".parse().unwrap(), ipv6_addr_in: Some("2a03:1b20:5:f011::a09f".parse().unwrap()), + overridden_ipv4: false, + overridden_ipv6: false, include_in_country: false, active: true, owned: true, @@ -1137,6 +1157,8 @@ fn test_include_in_country() { hostname: "se10-wireguard".to_string(), ipv4_addr_in: "185.213.154.69".parse().unwrap(), ipv6_addr_in: Some("2a03:1b20:5:f011::a10f".parse().unwrap()), + overridden_ipv4: false, + overridden_ipv6: false, include_in_country: false, active: true, owned: false, @@ -1339,6 +1361,8 @@ fn test_daita() { hostname: "se9-wireguard".to_string(), ipv4_addr_in: "185.213.154.68".parse().unwrap(), ipv6_addr_in: Some("2a03:1b20:5:f011::a09f".parse().unwrap()), + overridden_ipv4: false, + overridden_ipv6: false, include_in_country: true, active: true, owned: true, @@ -1358,6 +1382,8 @@ fn test_daita() { hostname: "se10-wireguard".to_string(), ipv4_addr_in: "185.213.154.69".parse().unwrap(), ipv6_addr_in: Some("2a03:1b20:5:f011::a10f".parse().unwrap()), + overridden_ipv4: false, + overridden_ipv6: false, include_in_country: true, active: true, owned: false, diff --git a/mullvad-types/src/features.rs b/mullvad-types/src/features.rs index 95caade0f86a..5cefbc9ea377 100644 --- a/mullvad-types/src/features.rs +++ b/mullvad-types/src/features.rs @@ -78,9 +78,13 @@ impl std::fmt::Display for FeatureIndicator { /// /// Note that [`FeatureIndicators`] are only applicable for the connected and connecting states, and /// this function should not be called with arguments from different tunnel states. +/// +/// Server ip override cannot be determined from the settings and endpoint, and has to be fetched +/// from the relay selector parameter generator. pub fn compute_feature_indicators( settings: &Settings, endpoint: &TunnelEndpoint, + server_ip_override: bool, ) -> FeatureIndicators { #[cfg(any(windows, target_os = "android", target_os = "macos"))] let split_tunneling = settings.split_tunnel.enable_exclusions; @@ -95,7 +99,6 @@ pub fn compute_feature_indicators( .default_options .any_blockers_enabled(); let custom_dns = settings.tunnel_options.dns_options.state == DnsState::Custom; - let server_ip_override = !settings.relay_overrides.is_empty(); // TODO: Should check if actually used let generic_features = [ (split_tunneling, FeatureIndicator::SplitTunneling), @@ -187,7 +190,7 @@ mod tests { let mut expected_indicators: FeatureIndicators = [].into_iter().collect(); assert_eq!( - compute_feature_indicators(&settings, &endpoint), + compute_feature_indicators(&settings, &endpoint, false), expected_indicators, "The default settings and TunnelEndpoint should not have any feature indicators. \ If this is not true anymore, please update this test." @@ -197,7 +200,7 @@ mod tests { expected_indicators.0.insert(FeatureIndicator::LockdownMode); assert_eq!( - compute_feature_indicators(&settings, &endpoint), + compute_feature_indicators(&settings, &endpoint, false), expected_indicators ); @@ -212,7 +215,7 @@ mod tests { .insert(FeatureIndicator::DnsContentBlockers); assert_eq!( - compute_feature_indicators(&settings, &endpoint), + compute_feature_indicators(&settings, &endpoint, false), expected_indicators ); @@ -221,13 +224,13 @@ mod tests { expected_indicators.0.insert(FeatureIndicator::LanSharing); assert_eq!( - compute_feature_indicators(&settings, &endpoint), + compute_feature_indicators(&settings, &endpoint, false), expected_indicators ); settings.tunnel_options.openvpn.mssfix = Some(1300); assert_eq!( - compute_feature_indicators(&settings, &endpoint), + compute_feature_indicators(&settings, &endpoint, false), expected_indicators, "Setting mssfix without having an openVPN endpoint should not result in an indicator" ); @@ -236,7 +239,7 @@ mod tests { expected_indicators.0.insert(FeatureIndicator::CustomMssFix); assert_eq!( - compute_feature_indicators(&settings, &endpoint), + compute_feature_indicators(&settings, &endpoint, false), expected_indicators ); @@ -250,7 +253,7 @@ mod tests { expected_indicators.0.insert(FeatureIndicator::BridgeMode); assert_eq!( - compute_feature_indicators(&settings, &endpoint), + compute_feature_indicators(&settings, &endpoint, false), expected_indicators ); @@ -260,7 +263,7 @@ mod tests { .remove(&FeatureIndicator::CustomMssFix); expected_indicators.0.remove(&FeatureIndicator::BridgeMode); assert_eq!( - compute_feature_indicators(&settings, &endpoint), + compute_feature_indicators(&settings, &endpoint, false), expected_indicators ); @@ -269,7 +272,7 @@ mod tests { .0 .insert(FeatureIndicator::QuantumResistance); assert_eq!( - compute_feature_indicators(&settings, &endpoint), + compute_feature_indicators(&settings, &endpoint, false), expected_indicators ); @@ -279,7 +282,7 @@ mod tests { }); expected_indicators.0.insert(FeatureIndicator::Multihop); assert_eq!( - compute_feature_indicators(&settings, &endpoint), + compute_feature_indicators(&settings, &endpoint, false), expected_indicators ); @@ -292,21 +295,21 @@ mod tests { }); expected_indicators.0.insert(FeatureIndicator::Udp2Tcp); assert_eq!( - compute_feature_indicators(&settings, &endpoint), + compute_feature_indicators(&settings, &endpoint, false), expected_indicators ); endpoint.obfuscation.as_mut().unwrap().obfuscation_type = ObfuscationType::Shadowsocks; expected_indicators.0.remove(&FeatureIndicator::Udp2Tcp); expected_indicators.0.insert(FeatureIndicator::Shadowsocks); assert_eq!( - compute_feature_indicators(&settings, &endpoint), + compute_feature_indicators(&settings, &endpoint, false), expected_indicators ); settings.tunnel_options.wireguard.mtu = Some(1300); expected_indicators.0.insert(FeatureIndicator::CustomMtu); assert_eq!( - compute_feature_indicators(&settings, &endpoint), + compute_feature_indicators(&settings, &endpoint, false), expected_indicators ); @@ -315,7 +318,7 @@ mod tests { endpoint.daita = true; expected_indicators.0.insert(FeatureIndicator::Daita); assert_eq!( - compute_feature_indicators(&settings, &endpoint), + compute_feature_indicators(&settings, &endpoint, false), expected_indicators ); } diff --git a/mullvad-types/src/relay_constraints.rs b/mullvad-types/src/relay_constraints.rs index b348727685bd..f3e38adc7943 100644 --- a/mullvad-types/src/relay_constraints.rs +++ b/mullvad-types/src/relay_constraints.rs @@ -658,14 +658,14 @@ impl RelayOverride { "Overriding ipv4_addr_in for {}: {ipv4_addr_in}", relay.hostname ); - relay.ipv4_addr_in = ipv4_addr_in; + relay.override_ipv4(ipv4_addr_in); } if let Some(ipv6_addr_in) = self.ipv6_addr_in { log::debug!( "Overriding ipv6_addr_in for {}: {ipv6_addr_in}", relay.hostname ); - relay.ipv6_addr_in = Some(ipv6_addr_in); + relay.override_ipv6(ipv6_addr_in); } // Additional IPs should be ignored when overrides are present diff --git a/mullvad-types/src/relay_list.rs b/mullvad-types/src/relay_list.rs index 173b50b4f9de..1693720d4af1 100644 --- a/mullvad-types/src/relay_list.rs +++ b/mullvad-types/src/relay_list.rs @@ -80,6 +80,10 @@ pub struct Relay { pub hostname: String, pub ipv4_addr_in: Ipv4Addr, pub ipv6_addr_in: Option, + // NOTE: Probably a better design choice would be to store the overridden IP addresses + // instead of a boolean override flags. This would allow us to access the original IPs. + pub overridden_ipv4: bool, + pub overridden_ipv6: bool, pub include_in_country: bool, pub active: bool, pub owned: bool, @@ -89,6 +93,18 @@ pub struct Relay { pub location: Option, } +impl Relay { + pub fn override_ipv4(&mut self, new_ipv4: Ipv4Addr) { + self.ipv4_addr_in = new_ipv4; + self.overridden_ipv4 = true; + } + + pub fn override_ipv6(&mut self, new_ipv6: Ipv6Addr) { + self.ipv6_addr_in = Some(new_ipv6); + self.overridden_ipv6 = true; + } +} + impl PartialEq for Relay { /// Hostnames are assumed to be unique per relay, i.e. a relay can be uniquely identified by its /// hostname. @@ -103,6 +119,8 @@ impl PartialEq for Relay { /// hostname: "se9-wireguard".to_string(), /// ipv4_addr_in: "185.213.154.68".parse().unwrap(), /// # ipv6_addr_in: None, + /// # overridden_ipv4: false, + /// # overridden_ipv6: false, /// # include_in_country: true, /// # active: true, /// # owned: true, From dfe166d16fd87198546bb631fca38e08d473fb7c Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Wed, 21 Aug 2024 17:45:23 +0200 Subject: [PATCH 2/3] Improve IP override testing --- mullvad-relay-selector/tests/relay_selector.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/mullvad-relay-selector/tests/relay_selector.rs b/mullvad-relay-selector/tests/relay_selector.rs index f0e48641c9a1..9b178c3a4482 100644 --- a/mullvad-relay-selector/tests/relay_selector.rs +++ b/mullvad-relay-selector/tests/relay_selector.rs @@ -804,9 +804,11 @@ fn test_selecting_wireguard_over_shadowsocks_extra_ips() { match relay { GetRelay::Wireguard { obfuscator: Some(SelectedObfuscator { config: ObfuscatorConfig::Shadowsocks { endpoint }, .. }), - inner: WireguardConfig::Singlehop { .. }, + inner: WireguardConfig::Singlehop { exit }, .. } => { + assert!(!exit.overridden_ipv4); + assert!(!exit.overridden_ipv6); assert!(SHADOWSOCKS_RELAY_EXTRA_ADDRS.contains(&endpoint.ip()), "{} is not an additional IP", endpoint); } wrong_relay => panic!( @@ -849,6 +851,7 @@ fn test_selecting_wireguard_ignore_extra_ips_override_v4() { .. } => { assert!(exit.overridden_ipv4); + assert!(!exit.overridden_ipv6); assert_eq!(endpoint.ip(), IpAddr::from(OVERRIDE_IPV4)); } wrong_relay => panic!( @@ -887,9 +890,11 @@ fn test_selecting_wireguard_ignore_extra_ips_override_v6() { match relay { GetRelay::Wireguard { obfuscator: Some(SelectedObfuscator { config: ObfuscatorConfig::Shadowsocks { endpoint }, .. }), - inner: WireguardConfig::Singlehop { .. }, + inner: WireguardConfig::Singlehop { exit }, .. } => { + assert!(exit.overridden_ipv6); + assert!(!exit.overridden_ipv4); assert_eq!(endpoint.ip(), IpAddr::from(OVERRIDE_IPV6)); } wrong_relay => panic!( From 6d1a486fadde25716de7f0c0527925b86bb67ab6 Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Wed, 21 Aug 2024 19:16:45 +0200 Subject: [PATCH 3/3] Use previous IP override indicator on settings change --- mullvad-daemon/src/lib.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/mullvad-daemon/src/lib.rs b/mullvad-daemon/src/lib.rs index 926946dc7b79..c52972c4e12e 100644 --- a/mullvad-daemon/src/lib.rs +++ b/mullvad-daemon/src/lib.rs @@ -53,7 +53,7 @@ use mullvad_types::{ auth_failed::AuthFailed, custom_list::CustomList, device::{Device, DeviceEvent, DeviceEventCause, DeviceId, DeviceState, RemoveDeviceEvent}, - features::{compute_feature_indicators, FeatureIndicators}, + features::{compute_feature_indicators, FeatureIndicator, FeatureIndicators}, location::{GeoIpLocation, LocationEventData}, relay_constraints::{ BridgeSettings, BridgeState, BridgeType, ObfuscationSettings, RelayOverride, RelaySettings, @@ -958,7 +958,7 @@ impl Daemon { DeviceMigrationEvent(event) => self.handle_device_migration_event(event), LocationEvent(location_data) => self.handle_location_event(location_data), SettingsChanged => { - self.update_feature_indicators_on_settings_changed().await; + self.update_feature_indicators_on_settings_changed(); } #[cfg(any(windows, target_os = "android", target_os = "macos"))] ExcludedPathsEvent(update, tx) => self.handle_new_excluded_paths(update, tx).await, @@ -1135,7 +1135,7 @@ impl Daemon { } /// Update the set of feature indicators based on the new settings. - async fn update_feature_indicators_on_settings_changed(&mut self) { + fn update_feature_indicators_on_settings_changed(&mut self) { // Updated settings may affect the feature indicators, even if they don't change the tunnel // state (e.g. activating lockdown mode). Note that only the connected and connecting states // have feature indicators. @@ -1150,7 +1150,13 @@ impl Daemon { endpoint, .. } => { - let ip_override = self.parameters_generator.last_relay_was_overridden().await; + // The server IP override feature indicator can only be changed when the tunnels + // state changes and it is updated in `handle_tunnel_state_transition`. We must rely + // on this value being up to date as we need the relay to know if + // the IP override is active. + let ip_override = feature_indicators + .active_features() + .any(|f| matches!(&f, FeatureIndicator::ServerIpOverride)); let new_feature_indicators = compute_feature_indicators(&self.settings.to_settings(), endpoint, ip_override); // Update and broadcast the new feature indicators if they have changed