Skip to content

Commit

Permalink
Merge branch 'fix-override-feature-indicator'
Browse files Browse the repository at this point in the history
  • Loading branch information
@dlon
dlon committed Aug 22, 2024
2 parents 283d1eb + 6d1a486 commit da5f68a
Show file tree
Hide file tree
Showing 9 changed files with 136 additions and 31 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
25 changes: 19 additions & 6 deletions mullvad-daemon/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 @@ -1144,8 +1150,15 @@ impl Daemon {
endpoint,
..
} => {
// 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);
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
39 changes: 35 additions & 4 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 @@ -787,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!(
Expand Down Expand Up @@ -828,9 +847,11 @@ 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!(!exit.overridden_ipv6);
assert_eq!(endpoint.ip(), IpAddr::from(OVERRIDE_IPV4));
}
wrong_relay => panic!(
Expand Down Expand Up @@ -869,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!(
Expand Down Expand Up @@ -1118,6 +1141,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 +1162,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 +1366,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 +1387,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 da5f68a

Please sign in to comment.