Skip to content

Commit

Permalink
Fix IP-override feature indicator
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@Serock3 @dlon
Serock3 authored and dlon committed Aug 22, 2024
1 parent 283d1eb commit b42d608
Show file tree
Hide file tree
Showing 9 changed files with 124 additions and 30 deletions.
2 changes: 2 additions & 0 deletions mullvad-api/src/relay_list.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
21 changes: 14 additions & 7 deletions mullvad-daemon/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -980,17 +980,23 @@ 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,
feature_indicators,
}
}
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,
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down
39 changes: 37 additions & 2 deletions mullvad-daemon/src/tunnel.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<GeoIpLocation> {
let inner = self.0.lock().await;
Expand All @@ -106,6 +122,7 @@ impl ParametersGenerator {
wg_entry: entry,
wg_exit: exit,
obfuscator,
..
} => {
entry_hostname = take_hostname(entry);
hostname = exit.hostname.clone();
Expand All @@ -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;
Expand Down Expand Up @@ -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()))
Expand All @@ -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))
Expand Down Expand Up @@ -315,10 +345,15 @@ enum LastSelectedRelays {
wg_entry: Option<Relay>,
wg_exit: Relay,
obfuscator: Option<Relay>,
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<Relay> },
OpenVpn {
relay: Relay,
bridge: Option<Relay>,
server_override: bool,
},
}
2 changes: 2 additions & 0 deletions mullvad-management-interface/src/types/conversions/relay_list.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,8 @@ impl TryFrom<proto::Relay> 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,
Expand Down
5 changes: 3 additions & 2 deletions mullvad-relay-selector/src/relay_selector/matcher.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
30 changes: 28 additions & 2 deletions mullvad-relay-selector/tests/relay_selector.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ static RELAYS: Lazy<RelayList> = 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,
Expand All @@ -66,6 +68,8 @@ static RELAYS: Lazy<RelayList> = 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,
Expand All @@ -85,6 +89,8 @@ static RELAYS: Lazy<RelayList> = 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,
Expand All @@ -97,6 +103,8 @@ static RELAYS: Lazy<RelayList> = 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,
Expand All @@ -109,6 +117,8 @@ static RELAYS: Lazy<RelayList> = 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,
Expand Down Expand Up @@ -182,6 +192,8 @@ static SHADOWSOCKS_RELAY: Lazy<Relay> = 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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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!(
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
Loading

0 comments on commit b42d608

Please sign in to comment.