From 5d69b1dbd8c27e5e5c040bdc7050dae474f299c3 Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Thu, 3 Oct 2024 16:01:54 +0200 Subject: [PATCH 01/11] Replace `smart_routing` with `use_multihop_if_necessary` in daemon Simplify the logic for feature indicators --- mullvad-daemon/src/lib.rs | 36 ++++-- mullvad-daemon/src/management_interface.rs | 2 +- .../proto/management_interface.proto | 1 - .../src/types/conversions/features.rs | 8 +- .../src/types/conversions/wireguard.rs | 4 +- .../src/relay_selector/mod.rs | 23 ++-- .../src/relay_selector/query.rs | 15 ++- .../tests/relay_selector.rs | 20 +-- mullvad-types/src/features.rs | 118 ++++++++++++------ mullvad-types/src/wireguard.rs | 2 +- 10 files changed, 146 insertions(+), 83 deletions(-) diff --git a/mullvad-daemon/src/lib.rs b/mullvad-daemon/src/lib.rs index e833f1b82c9a..312e11d84f35 100644 --- a/mullvad-daemon/src/lib.rs +++ b/mullvad-daemon/src/lib.rs @@ -266,7 +266,7 @@ pub enum DaemonCommand { #[cfg(daita)] SetEnableDaita(ResponseTx<(), settings::Error>, bool), #[cfg(daita)] - SetDaitaSmartRouting(ResponseTx<(), settings::Error>, bool), + SetDaitaUseMultihopIfNecessary(ResponseTx<(), settings::Error>, bool), #[cfg(daita)] SetDaitaSettings(ResponseTx<(), settings::Error>, DaitaSettings), /// Set DNS options or servers to use @@ -1261,7 +1261,9 @@ impl Daemon { #[cfg(daita)] SetEnableDaita(tx, value) => self.on_set_daita_enabled(tx, value).await, #[cfg(daita)] - SetDaitaSmartRouting(tx, value) => self.on_set_daita_smart_routing(tx, value).await, + SetDaitaUseMultihopIfNecessary(tx, value) => { + self.on_set_daita_use_multihop_if_necessary(tx, value).await + } #[cfg(daita)] SetDaitaSettings(tx, daita_settings) => { self.on_set_daita_settings(tx, daita_settings).await @@ -2344,9 +2346,13 @@ impl Daemon { .update(|settings| { settings.tunnel_options.wireguard.daita.enabled = value; - // enable smart-routing automatically with daita + // enable 'use_multihop_if_necessary' automatically with daita if cfg!(not(target_os = "android")) { - settings.tunnel_options.wireguard.daita.smart_routing = value + settings + .tunnel_options + .wireguard + .daita + .use_multihop_if_necessary = value } }) .await; @@ -2376,7 +2382,7 @@ impl Daemon { } #[cfg(daita)] - async fn on_set_daita_smart_routing( + async fn on_set_daita_use_multihop_if_necessary( &mut self, tx: ResponseTx<(), settings::Error>, value: bool, @@ -2385,11 +2391,17 @@ impl Daemon { match self .settings - .update(|settings| settings.tunnel_options.wireguard.daita.smart_routing = value) + .update(|settings| { + settings + .tunnel_options + .wireguard + .daita + .use_multihop_if_necessary = value + }) .await { Ok(settings_changed) => { - Self::oneshot_send(tx, Ok(()), "set_daita_smart_routing response"); + Self::oneshot_send(tx, Ok(()), "set_daita_use_multihop_if_necessary response"); let RelaySettings::Normal(constraints) = &self.settings.relay_settings else { return; // DAITA is not supported for custom relays @@ -2410,7 +2422,7 @@ impl Daemon { } Err(e) => { log::error!("{}", e.display_chain_with_msg("Unable to save settings")); - Self::oneshot_send(tx, Err(e), "set_daita_smart_routing response"); + Self::oneshot_send(tx, Err(e), "set_daita_use_multihop_if_necessary response"); } } } @@ -3024,12 +3036,16 @@ fn new_selector_config(settings: &Settings) -> SelectorConfig { #[cfg(daita)] daita: settings.tunnel_options.wireguard.daita.enabled, #[cfg(daita)] - daita_smart_routing: settings.tunnel_options.wireguard.daita.smart_routing, + daita_use_multihop_if_necessary: settings + .tunnel_options + .wireguard + .daita + .use_multihop_if_necessary, #[cfg(not(daita))] daita: false, #[cfg(not(daita))] - daita_smart_routing: false, + daita_use_multihop_if_necessary: false, quantum_resistant: settings.tunnel_options.wireguard.quantum_resistant, }, diff --git a/mullvad-daemon/src/management_interface.rs b/mullvad-daemon/src/management_interface.rs index b52c214a01ce..36aa168962ae 100644 --- a/mullvad-daemon/src/management_interface.rs +++ b/mullvad-daemon/src/management_interface.rs @@ -356,7 +356,7 @@ impl ManagementService for ManagementServiceImpl { let value = request.into_inner(); log::debug!("set_daita_smart_routing({value})"); let (tx, rx) = oneshot::channel(); - self.send_command_to_daemon(DaemonCommand::SetDaitaSmartRouting(tx, value))?; + self.send_command_to_daemon(DaemonCommand::SetDaitaUseMultihopIfNecessary(tx, value))?; self.wait_for_result(rx).await?.map(Response::new)?; Ok(Response::new(())) } diff --git a/mullvad-management-interface/proto/management_interface.proto b/mullvad-management-interface/proto/management_interface.proto index 70d9e1ad2bd8..9b946d24db45 100644 --- a/mullvad-management-interface/proto/management_interface.proto +++ b/mullvad-management-interface/proto/management_interface.proto @@ -264,7 +264,6 @@ enum FeatureIndicator { CUSTOM_MTU = 11; CUSTOM_MSS_FIX = 12; DAITA = 13; - DAITA_SMART_ROUTING = 14; } message ObfuscationEndpoint { diff --git a/mullvad-management-interface/src/types/conversions/features.rs b/mullvad-management-interface/src/types/conversions/features.rs index 85c85b8b7786..04714218911c 100644 --- a/mullvad-management-interface/src/types/conversions/features.rs +++ b/mullvad-management-interface/src/types/conversions/features.rs @@ -17,8 +17,8 @@ impl From for proto::FeatureIndicator mullvad_types::features::FeatureIndicator::ServerIpOverride => ServerIpOverride, mullvad_types::features::FeatureIndicator::CustomMtu => CustomMtu, mullvad_types::features::FeatureIndicator::CustomMssFix => CustomMssFix, - mullvad_types::features::FeatureIndicator::Daita => Daita, - mullvad_types::features::FeatureIndicator::DaitaSmartRouting => DaitaSmartRouting, + mullvad_types::features::FeatureIndicator::DaitaDirectOnly => Daita, + mullvad_types::features::FeatureIndicator::Daita => DaitaSmartRouting, } } } @@ -39,8 +39,8 @@ impl From for mullvad_types::features::FeatureIndicator proto::FeatureIndicator::ServerIpOverride => Self::ServerIpOverride, proto::FeatureIndicator::CustomMtu => Self::CustomMtu, proto::FeatureIndicator::CustomMssFix => Self::CustomMssFix, - proto::FeatureIndicator::Daita => Self::Daita, - proto::FeatureIndicator::DaitaSmartRouting => Self::DaitaSmartRouting, + proto::FeatureIndicator::Daita => Self::DaitaDirectOnly, + proto::FeatureIndicator::DaitaSmartRouting => Self::Daita, } } } diff --git a/mullvad-management-interface/src/types/conversions/wireguard.rs b/mullvad-management-interface/src/types/conversions/wireguard.rs index 9e40b4b52600..c6bf643664c7 100644 --- a/mullvad-management-interface/src/types/conversions/wireguard.rs +++ b/mullvad-management-interface/src/types/conversions/wireguard.rs @@ -78,7 +78,7 @@ impl From for proto::DaitaSettings { fn from(settings: mullvad_types::wireguard::DaitaSettings) -> Self { proto::DaitaSettings { enabled: settings.enabled, - smart_routing: settings.smart_routing, + smart_routing: settings.use_multihop_if_necessary, } } } @@ -88,7 +88,7 @@ impl From for mullvad_types::wireguard::DaitaSettings { fn from(settings: proto::DaitaSettings) -> Self { mullvad_types::wireguard::DaitaSettings { enabled: settings.enabled, - smart_routing: settings.smart_routing, + use_multihop_if_necessary: settings.smart_routing, } } } diff --git a/mullvad-relay-selector/src/relay_selector/mod.rs b/mullvad-relay-selector/src/relay_selector/mod.rs index 6233e2da0529..b94a5812a011 100644 --- a/mullvad-relay-selector/src/relay_selector/mod.rs +++ b/mullvad-relay-selector/src/relay_selector/mod.rs @@ -126,7 +126,7 @@ pub struct AdditionalWireguardConstraints { /// If true and multihop is disabled, will set up multihop with an automatic entry relay if /// DAITA is enabled. - pub daita_smart_routing: bool, + pub daita_use_multihop_if_necessary: bool, /// If enabled, select relays that support PQ. pub quantum_resistant: QuantumResistantState, @@ -349,7 +349,7 @@ impl<'a> TryFrom> for RelayQuery { } = wireguard_constraints; let AdditionalWireguardConstraints { daita, - daita_smart_routing, + daita_use_multihop_if_necessary, quantum_resistant, } = additional_constraints; WireguardRelayQuery { @@ -359,7 +359,7 @@ impl<'a> TryFrom> for RelayQuery { entry_location, obfuscation: ObfuscationQuery::from(obfuscation_settings), daita: Constraint::Only(daita), - daita_smart_routing: Constraint::Only(daita_smart_routing), + daita_use_multihop_if_necessary: Constraint::Only(daita_use_multihop_if_necessary), quantum_resistant, } } @@ -738,18 +738,23 @@ impl RelaySelector { Result::<_, Error>::Ok(!candidates.is_empty()) }; - // is `smart_routing` enabled? - let smart_routing = || { + // is `use_multihop_if_necessary` enabled? + let use_multihop_if_necessary = || { query .wireguard_constraints() - .daita_smart_routing + .daita_use_multihop_if_necessary .intersection(Constraint::Only(true)) .is_some() }; - // if we found no matching relays because DAITA was enabled, and `smart_routing` is enabled, - // try enabling multihop and connecting using an automatically selected entry relay. - if candidates.is_empty() && using_daita() && no_relay_because_daita()? && smart_routing() { + // if we found no matching relays because DAITA was enabled, and `use_multihop_if_necessary` + // is enabled, try enabling multihop and connecting using an automatically selected + // entry relay. + if candidates.is_empty() + && using_daita() + && no_relay_because_daita()? + && use_multihop_if_necessary() + { return Self::get_wireguard_auto_multihop_config(query, custom_lists, parsed_relays); } diff --git a/mullvad-relay-selector/src/relay_selector/query.rs b/mullvad-relay-selector/src/relay_selector/query.rs index 2dc81b083386..570a2212cb42 100644 --- a/mullvad-relay-selector/src/relay_selector/query.rs +++ b/mullvad-relay-selector/src/relay_selector/query.rs @@ -268,7 +268,7 @@ pub struct WireguardRelayQuery { pub entry_location: Constraint, pub obfuscation: ObfuscationQuery, pub daita: Constraint, - pub daita_smart_routing: Constraint, + pub daita_use_multihop_if_necessary: Constraint, pub quantum_resistant: QuantumResistantState, } @@ -354,7 +354,7 @@ impl WireguardRelayQuery { entry_location: Constraint::Any, obfuscation: ObfuscationQuery::Auto, daita: Constraint::Any, - daita_smart_routing: Constraint::Any, + daita_use_multihop_if_necessary: Constraint::Any, quantum_resistant: QuantumResistantState::Auto, } } @@ -673,9 +673,14 @@ pub mod builder { impl RelayQueryBuilder> { - /// Enable DAITA smart routing. - pub fn daita_smart_routing(mut self, constraint: impl Into>) -> Self { - self.query.wireguard_constraints.daita_smart_routing = constraint.into(); + /// Enable DAITA 'use_multihop_if_necessary'. + pub fn daita_use_multihop_if_necessary( + mut self, + constraint: impl Into>, + ) -> Self { + self.query + .wireguard_constraints + .daita_use_multihop_if_necessary = constraint.into(); self } } diff --git a/mullvad-relay-selector/tests/relay_selector.rs b/mullvad-relay-selector/tests/relay_selector.rs index dc05d04eb2aa..1ce69b0591fb 100644 --- a/mullvad-relay-selector/tests/relay_selector.rs +++ b/mullvad-relay-selector/tests/relay_selector.rs @@ -1451,7 +1451,7 @@ fn test_daita() { let query = RelayQueryBuilder::new() .wireguard() .daita() - .daita_smart_routing(false) + .daita_use_multihop_if_necessary(false) .build(); let relay = unwrap_entry_relay(relay_selector.get_relay_by_query(query).unwrap()); assert!( @@ -1463,23 +1463,23 @@ fn test_daita() { let query = RelayQueryBuilder::new() .wireguard() .daita() - .daita_smart_routing(false) + .daita_use_multihop_if_necessary(false) .location(NON_DAITA_RELAY_LOCATION.clone()) .build(); relay_selector .get_relay_by_query(query) .expect_err("Expected to find no matching relay"); - // Should be able to connect to non-DAITA relay with smart_routing + // Should be able to connect to non-DAITA relay with use_multihop_if_necessary let query = RelayQueryBuilder::new() .wireguard() .daita() - .daita_smart_routing(true) + .daita_use_multihop_if_necessary(true) .location(NON_DAITA_RELAY_LOCATION.clone()) .build(); let relay = relay_selector .get_relay_by_query(query) - .expect("Expected to find a relay with daita_smart_routing"); + .expect("Expected to find a relay with daita_use_multihop_if_necessary"); match relay { GetRelay::Wireguard { inner: WireguardConfig::Multihop { exit, entry }, @@ -1493,16 +1493,16 @@ fn test_daita() { ), } - // Should be able to connect to DAITA relay with smart_routing + // Should be able to connect to DAITA relay with use_multihop_if_necessary let query = RelayQueryBuilder::new() .wireguard() .daita() - .daita_smart_routing(true) + .daita_use_multihop_if_necessary(true) .location(DAITA_RELAY_LOCATION.clone()) .build(); let relay = relay_selector .get_relay_by_query(query) - .expect("Expected to find a relay with daita_smart_routing"); + .expect("Expected to find a relay with daita_use_multihop_if_necessary"); match relay { GetRelay::Wireguard { inner: WireguardConfig::Singlehop { exit }, @@ -1537,7 +1537,7 @@ fn test_daita() { let query = RelayQueryBuilder::new() .wireguard() .daita() - .daita_smart_routing(false) + .daita_use_multihop_if_necessary(false) .multihop() .build(); let relay = relay_selector.get_relay_by_query(query).unwrap(); @@ -1557,7 +1557,7 @@ fn test_daita() { let query = RelayQueryBuilder::new() .wireguard() .daita() - .daita_smart_routing(false) + .daita_use_multihop_if_necessary(false) .multihop() .location(NON_DAITA_RELAY_LOCATION.clone()) .build(); diff --git a/mullvad-types/src/features.rs b/mullvad-types/src/features.rs index ba40e42d1800..6f4ea1e9ccf9 100644 --- a/mullvad-types/src/features.rs +++ b/mullvad-types/src/features.rs @@ -1,6 +1,9 @@ use std::{collections::HashSet, fmt::Display}; -use crate::settings::{DnsState, Settings}; +use crate::{ + relay_constraints::RelaySettings, + settings::{DnsState, Settings}, +}; use serde::{Deserialize, Serialize}; use talpid_types::net::{ObfuscationType, TunnelEndpoint, TunnelType}; @@ -65,14 +68,7 @@ pub enum FeatureIndicator { ServerIpOverride, CustomMtu, CustomMssFix, - - /// Whether DAITA (without smart routing) is in use. - /// Mutually exclusive with [FeatureIndicator::DaitaSmartRouting]. Daita, - - /// Whether DAITA (with smart routing) is in use. - /// Mutually exclusive with [FeatureIndicator::Daita]. - DaitaSmartRouting, } impl FeatureIndicator { @@ -92,7 +88,6 @@ impl FeatureIndicator { FeatureIndicator::CustomMtu => "Custom MTU", FeatureIndicator::CustomMssFix => "Custom MSS", FeatureIndicator::Daita => "DAITA", - FeatureIndicator::DaitaSmartRouting => "DAITA: Smart Routing", } } } @@ -165,28 +160,37 @@ pub fn compute_feature_indicators( let mtu = settings.tunnel_options.wireguard.mtu.is_some(); - let mut daita_smart_routing = false; - let mut multihop = false; - - if let crate::relay_constraints::RelaySettings::Normal(constraints) = - &settings.relay_settings - { - multihop = endpoint.entry_endpoint.is_some() - && constraints.wireguard_constraints.use_multihop; - - #[cfg(daita)] - { - // Detect whether we're using "smart_routing" by checking if multihop is - // in use but not explicitly enabled. - daita_smart_routing = endpoint.daita - && endpoint.entry_endpoint.is_some() - && !constraints.wireguard_constraints.use_multihop - } - }; + let mut daita = false; + let mut multihop = endpoint.entry_endpoint.is_some(); - // Daita is mutually exclusive with DaitaSmartRouting #[cfg(daita)] - let daita = endpoint.daita && !daita_smart_routing; + if endpoint.daita { + daita = true; + + let multihop_setting_enabled = + if let RelaySettings::Normal(constraints) = &settings.relay_settings { + constraints.wireguard_constraints.use_multihop + } else { + false + }; + + let daita_use_multihop_if_necessary = settings + .tunnel_options + .wireguard + .daita + .use_multihop_if_necessary; + + // If multihop is disabled in the settings, but enabled automatically by DAITA, we + // will not show the multihop indicator. + let multihop_enable_automatically = multihop && !multihop_setting_enabled; + if multihop_enable_automatically { + debug_assert!( + daita_use_multihop_if_necessary, + "Multihop can only be enabled automatically if DAITA is enabled" + ); + multihop = false; + } + } vec![ (quantum_resistant, FeatureIndicator::QuantumResistance), @@ -194,9 +198,7 @@ pub fn compute_feature_indicators( (udp_tcp, FeatureIndicator::Udp2Tcp), (shadowsocks, FeatureIndicator::Shadowsocks), (mtu, FeatureIndicator::CustomMtu), - #[cfg(daita)] (daita, FeatureIndicator::Daita), - (daita_smart_routing, FeatureIndicator::DaitaSmartRouting), ] } }; @@ -328,13 +330,18 @@ mod tests { expected_indicators ); + if let RelaySettings::Normal(constraints) = &mut settings.relay_settings { + constraints.wireguard_constraints.use_multihop = true; + }; + assert_eq!( + compute_feature_indicators(&settings, &endpoint, false), + expected_indicators, + "The multihop feature indicator should be enabled by the endpoint, not the settings" + ); endpoint.entry_endpoint = Some(Endpoint { address: SocketAddr::from(([1, 2, 3, 4], 443)), protocol: TransportProtocol::Tcp, }); - if let RelaySettings::Normal(constraints) = &mut settings.relay_settings { - constraints.wireguard_constraints.use_multihop = true; - }; expected_indicators.0.insert(FeatureIndicator::Multihop); assert_eq!( compute_feature_indicators(&settings, &endpoint, false), @@ -370,25 +377,57 @@ mod tests { #[cfg(daita)] { + // Multihop and DAITA on endpoint.daita = true; + settings + .tunnel_options + .wireguard + .daita + .use_multihop_if_necessary = true; + expected_indicators.0.insert(FeatureIndicator::Daita); assert_eq!( compute_feature_indicators(&settings, &endpoint, false), expected_indicators ); + // Should not change regardless of whether `use_multihop_if_necessary` is true, since + // multihop is enabled explicitly + settings + .tunnel_options + .wireguard + .daita + .use_multihop_if_necessary = false; + assert_eq!( + compute_feature_indicators(&settings, &endpoint, false), + expected_indicators, + ); + + // Here we mock that multihop was automatically enabled by DAITA. + // We enable `use_multihop_if_necessary` again and disable the multihop setting, while + // keeping the entry relay In this scenario, we should not get a Multihop + // indicator + settings + .tunnel_options + .wireguard + .daita + .use_multihop_if_necessary = true; if let RelaySettings::Normal(constraints) = &mut settings.relay_settings { constraints.wireguard_constraints.use_multihop = false; }; - expected_indicators - .0 - .insert(FeatureIndicator::DaitaSmartRouting); - expected_indicators.0.remove(&FeatureIndicator::Daita); expected_indicators.0.remove(&FeatureIndicator::Multihop); assert_eq!( compute_feature_indicators(&settings, &endpoint, false), expected_indicators, - "DaitaSmartRouting should be enabled" + "DaitaDirectOnly should be enabled" + ); + + // If we also remove the entry relay, we should still not get a multihop indicator + endpoint.entry_endpoint = None; + assert_eq!( + compute_feature_indicators(&settings, &endpoint, false), + expected_indicators, + "DaitaDirectOnly should be enabled" ); } @@ -409,7 +448,6 @@ mod tests { FeatureIndicator::CustomMtu => {} FeatureIndicator::CustomMssFix => {} FeatureIndicator::Daita => {} - FeatureIndicator::DaitaSmartRouting => {} } } } diff --git a/mullvad-types/src/wireguard.rs b/mullvad-types/src/wireguard.rs index c6ec6cec5f2b..77ed60e1b2d2 100644 --- a/mullvad-types/src/wireguard.rs +++ b/mullvad-types/src/wireguard.rs @@ -86,7 +86,7 @@ pub struct DaitaSettings { pub enabled: bool, #[serde(default)] - pub smart_routing: bool, + pub use_multihop_if_necessary: bool, } /// Contains account specific wireguard data From 4f586f8432372e8e1b6089cd56dc5cf36236f2b5 Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Fri, 4 Oct 2024 11:34:19 +0200 Subject: [PATCH 02/11] Sort feature indicators in alphabetical order in debug fmt --- mullvad-types/src/features.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/mullvad-types/src/features.rs b/mullvad-types/src/features.rs index 6f4ea1e9ccf9..cb6bb23f982f 100644 --- a/mullvad-types/src/features.rs +++ b/mullvad-types/src/features.rs @@ -1,4 +1,7 @@ -use std::{collections::HashSet, fmt::Display}; +use std::{ + collections::HashSet, + fmt::{Debug, Display}, +}; use crate::{ relay_constraints::RelaySettings, @@ -11,9 +14,20 @@ use talpid_types::net::{ObfuscationType, TunnelEndpoint, TunnelType}; /// what is affecting their connection at any given time. /// /// Note that the feature indicators are not ordered. -#[derive(Default, Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Default, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct FeatureIndicators(HashSet); +impl Debug for FeatureIndicators { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let mut indicators: Vec<&str> = self.0.iter().map(|feature| feature.to_str()).collect(); + // Sort the features alphabetically (Just to have some order, arbitrarily chosen) + indicators.sort(); + f.debug_tuple("FeatureIndicators") + .field(&indicators) + .finish() + } +} + impl FeatureIndicators { pub fn is_empty(&self) -> bool { self.0.is_empty() From 0578e79afcaf6ad1158663941e05f01ce3a7c2c3 Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Fri, 4 Oct 2024 16:17:22 +0200 Subject: [PATCH 03/11] Rename "smart routing" to "direct only" in the API Also invert the behavior --- mullvad-cli/src/cmds/tunnel.rs | 18 +++++++------- mullvad-daemon/src/management_interface.rs | 24 +++++++++++-------- .../proto/management_interface.proto | 4 ++-- mullvad-management-interface/src/client.rs | 4 ++-- .../src/types/conversions/features.rs | 6 ++--- .../src/types/conversions/wireguard.rs | 4 ++-- test/test-manager/src/tests/daita.rs | 20 ++++++++-------- 7 files changed, 41 insertions(+), 39 deletions(-) diff --git a/mullvad-cli/src/cmds/tunnel.rs b/mullvad-cli/src/cmds/tunnel.rs index da66cac5d505..118fd63a8dc7 100644 --- a/mullvad-cli/src/cmds/tunnel.rs +++ b/mullvad-cli/src/cmds/tunnel.rs @@ -41,9 +41,9 @@ pub enum TunnelOptions { /// Configure whether to enable DAITA #[arg(long)] daita: Option, - /// Configure whether to enable DAITA smart routing + /// Configure whether to enable DAITA direct only #[arg(long)] - daita_smart_routing: Option, + daita_direct_only: Option, /// The key rotation interval. Number of hours, or 'any' #[arg(long)] rotation_interval: Option>, @@ -138,7 +138,7 @@ impl Tunnel { mtu, quantum_resistant, daita, - daita_smart_routing, + daita_direct_only, rotation_interval, rotate_key, } => { @@ -146,7 +146,7 @@ impl Tunnel { mtu, quantum_resistant, daita, - daita_smart_routing, + daita_direct_only, rotation_interval, rotate_key, ) @@ -178,7 +178,7 @@ impl Tunnel { mtu: Option>, quantum_resistant: Option, daita: Option, - daita_smart_routing: Option, + daita_direct_only: Option, rotation_interval: Option>, rotate_key: Option, ) -> Result<()> { @@ -197,12 +197,12 @@ impl Tunnel { if let Some(enable_daita) = daita { rpc.set_enable_daita(*enable_daita).await?; println!("DAITA setting has been updated"); - println!("Smart routing setting has been updated"); + println!("Direct only setting has been updated"); } - if let Some(daita_smart_routing) = daita_smart_routing { - rpc.set_daita_smart_routing(*daita_smart_routing).await?; - println!("Smart routing setting has been updated"); + if let Some(daita_direct_only) = daita_direct_only { + rpc.set_daita_direct_only(*daita_direct_only).await?; + println!("Direct only setting has been updated"); } if let Some(interval) = rotation_interval { diff --git a/mullvad-daemon/src/management_interface.rs b/mullvad-daemon/src/management_interface.rs index 36aa168962ae..eadfd9980699 100644 --- a/mullvad-daemon/src/management_interface.rs +++ b/mullvad-daemon/src/management_interface.rs @@ -343,20 +343,23 @@ impl ManagementService for ManagementServiceImpl { #[cfg(daita)] async fn set_enable_daita(&self, request: Request) -> ServiceResult<()> { - let value = request.into_inner(); - log::debug!("set_enable_daita({value})"); + let daita_enabled = request.into_inner(); + log::debug!("set_enable_daita({daita_enabled})"); let (tx, rx) = oneshot::channel(); - self.send_command_to_daemon(DaemonCommand::SetEnableDaita(tx, value))?; + self.send_command_to_daemon(DaemonCommand::SetEnableDaita(tx, daita_enabled))?; self.wait_for_result(rx).await?.map(Response::new)?; Ok(Response::new(())) } #[cfg(daita)] - async fn set_daita_smart_routing(&self, request: Request) -> ServiceResult<()> { - let value = request.into_inner(); - log::debug!("set_daita_smart_routing({value})"); + async fn set_daita_direct_only(&self, request: Request) -> ServiceResult<()> { + let direct_only_enabled = request.into_inner(); + log::debug!("set_daita_direct_only({direct_only_enabled})"); let (tx, rx) = oneshot::channel(); - self.send_command_to_daemon(DaemonCommand::SetDaitaUseMultihopIfNecessary(tx, value))?; + self.send_command_to_daemon(DaemonCommand::SetDaitaUseMultihopIfNecessary( + tx, + !direct_only_enabled, + ))?; self.wait_for_result(rx).await?.map(Response::new)?; Ok(Response::new(())) } @@ -381,7 +384,7 @@ impl ManagementService for ManagementServiceImpl { } #[cfg(not(daita))] - async fn set_daita_smart_routing(&self, _: Request) -> ServiceResult<()> { + async fn set_daita_direct_only(&self, _: Request) -> ServiceResult<()> { Ok(Response::new(())) } @@ -1135,8 +1138,9 @@ impl ManagementInterfaceServer { }) } - /// Wait for the server to shut down gracefully. If that does not happend within [`RPC_SERVER_SHUTDOWN_TIMEOUT`], - /// the gRPC server is aborted and we yield the async execution. + /// Wait for the server to shut down gracefully. If that does not happend within + /// [`RPC_SERVER_SHUTDOWN_TIMEOUT`], the gRPC server is aborted and we yield the async + /// execution. pub async fn stop(mut self) { use futures::SinkExt; // Send a singal to the underlying RPC server to shut down. diff --git a/mullvad-management-interface/proto/management_interface.proto b/mullvad-management-interface/proto/management_interface.proto index 9b946d24db45..b57d63dcf3bd 100644 --- a/mullvad-management-interface/proto/management_interface.proto +++ b/mullvad-management-interface/proto/management_interface.proto @@ -49,7 +49,7 @@ service ManagementService { rpc SetEnableIpv6(google.protobuf.BoolValue) returns (google.protobuf.Empty) {} rpc SetQuantumResistantTunnel(QuantumResistantState) returns (google.protobuf.Empty) {} rpc SetEnableDaita(google.protobuf.BoolValue) returns (google.protobuf.Empty) {} - rpc SetDaitaSmartRouting(google.protobuf.BoolValue) returns (google.protobuf.Empty) {} + rpc SetDaitaDirectOnly(google.protobuf.BoolValue) returns (google.protobuf.Empty) {} rpc SetDaitaSettings(DaitaSettings) returns (google.protobuf.Empty) {} rpc SetDnsOptions(DnsOptions) returns (google.protobuf.Empty) {} rpc SetRelayOverride(RelayOverride) returns (google.protobuf.Empty) {} @@ -545,7 +545,7 @@ message QuantumResistantState { message DaitaSettings { bool enabled = 1; - bool smart_routing = 2; + bool direct_only = 2; } message TunnelOptions { diff --git a/mullvad-management-interface/src/client.rs b/mullvad-management-interface/src/client.rs index 73bff561a46e..0b74f9d9d66f 100644 --- a/mullvad-management-interface/src/client.rs +++ b/mullvad-management-interface/src/client.rs @@ -385,9 +385,9 @@ impl MullvadProxyClient { } #[cfg(daita)] - pub async fn set_daita_smart_routing(&mut self, value: bool) -> Result<()> { + pub async fn set_daita_direct_only(&mut self, value: bool) -> Result<()> { self.0 - .set_daita_smart_routing(value) + .set_daita_direct_only(value) .await .map_err(Error::Rpc)?; Ok(()) diff --git a/mullvad-management-interface/src/types/conversions/features.rs b/mullvad-management-interface/src/types/conversions/features.rs index 04714218911c..ac235d81636c 100644 --- a/mullvad-management-interface/src/types/conversions/features.rs +++ b/mullvad-management-interface/src/types/conversions/features.rs @@ -17,8 +17,7 @@ impl From for proto::FeatureIndicator mullvad_types::features::FeatureIndicator::ServerIpOverride => ServerIpOverride, mullvad_types::features::FeatureIndicator::CustomMtu => CustomMtu, mullvad_types::features::FeatureIndicator::CustomMssFix => CustomMssFix, - mullvad_types::features::FeatureIndicator::DaitaDirectOnly => Daita, - mullvad_types::features::FeatureIndicator::Daita => DaitaSmartRouting, + mullvad_types::features::FeatureIndicator::Daita => Daita, } } } @@ -39,8 +38,7 @@ impl From for mullvad_types::features::FeatureIndicator proto::FeatureIndicator::ServerIpOverride => Self::ServerIpOverride, proto::FeatureIndicator::CustomMtu => Self::CustomMtu, proto::FeatureIndicator::CustomMssFix => Self::CustomMssFix, - proto::FeatureIndicator::Daita => Self::DaitaDirectOnly, - proto::FeatureIndicator::DaitaSmartRouting => Self::Daita, + proto::FeatureIndicator::Daita => Self::Daita, } } } diff --git a/mullvad-management-interface/src/types/conversions/wireguard.rs b/mullvad-management-interface/src/types/conversions/wireguard.rs index c6bf643664c7..3e896da22db4 100644 --- a/mullvad-management-interface/src/types/conversions/wireguard.rs +++ b/mullvad-management-interface/src/types/conversions/wireguard.rs @@ -78,7 +78,7 @@ impl From for proto::DaitaSettings { fn from(settings: mullvad_types::wireguard::DaitaSettings) -> Self { proto::DaitaSettings { enabled: settings.enabled, - smart_routing: settings.use_multihop_if_necessary, + direct_only: !settings.use_multihop_if_necessary, } } } @@ -88,7 +88,7 @@ impl From for mullvad_types::wireguard::DaitaSettings { fn from(settings: proto::DaitaSettings) -> Self { mullvad_types::wireguard::DaitaSettings { enabled: settings.enabled, - use_multihop_if_necessary: settings.smart_routing, + use_multihop_if_necessary: !settings.direct_only, } } } diff --git a/test/test-manager/src/tests/daita.rs b/test/test-manager/src/tests/daita.rs index 912c811eccc2..24f5f4b0df78 100644 --- a/test/test-manager/src/tests/daita.rs +++ b/test/test-manager/src/tests/daita.rs @@ -12,10 +12,10 @@ use test_rpc::ServiceClient; use super::{helpers, Error, TestContext}; -/// Test that daita and daita_smart_routing works by connecting +/// Test that daita and daita_direct_only works by connecting /// - to a non-DAITA relay with singlehop (should block) /// - to a DAITA relay with singlehop -/// - to a DAITA relay with auto-multihop using smart_routing +/// - to a DAITA relay with auto-multihop by disabling direct_only /// - to a DAITA relay with explicit multihop /// - to a non-DAITA relay with multihop (should block) /// @@ -90,25 +90,25 @@ pub async fn test_daita( .await? .inspect(|event| log::debug!("New daemon event: {event:?}")); - log::info!("Connecting to non-daita relay with DAITA smart routing"); + log::info!("Connecting to non-daita relay with DAITA by automatically using multihop"); { helpers::set_relay_settings(&mut mullvad_client, non_daita_location_query.clone()).await?; mullvad_client.set_enable_daita(true).await?; mullvad_client.connect_tunnel().await?; let state = wait_for_daemon_reconnect(&mut events) .await - .context("Failed to connect with smart_routing enabled")?; + .context("Failed to connect with 'direct only' disabled")?; let endpoint: &TunnelEndpoint = state.endpoint().ok_or(anyhow!("No endpoint"))?; ensure!(endpoint.daita, "DAITA must be used"); ensure!(endpoint.entry_endpoint.is_some(), "multihop must be used"); - log::info!("Successfully multihopped with use smart_routing"); + log::info!("Successfully multihopped with 'direct only' disabled"); } - log::info!("Connecting to non-daita relay with DAITA but no smart routing"); + log::info!("Connecting to non-daita relay with 'DAITA: direct only'"); { - mullvad_client.set_daita_smart_routing(false).await?; + mullvad_client.set_daita_direct_only(true).await?; let result = wait_for_daemon_reconnect(&mut events).await; let Err(Error::UnexpectedErrorState(state)) = result else { @@ -121,13 +121,13 @@ pub async fn test_daita( log::info!("Failed to connect, this is expected!"); } - log::info!("Connecting to daita relay with smart_routing"); + log::info!("Connecting to daita relay with 'direct_only' disabled"); { helpers::set_relay_settings(&mut mullvad_client, daita_location_query).await?; let state = wait_for_daemon_reconnect(&mut events) .await - .context("Failed to connect to daita location with smart_routing enabled")?; + .context("Failed to connect to daita location with 'direct_only' disabled")?; let endpoint = state.endpoint().context("No endpoint")?; ensure!(endpoint.daita, "DAITA must be used"); @@ -136,7 +136,7 @@ pub async fn test_daita( "multihop must not be used" ); - log::info!("Successfully singlehopped with smart_routing"); + log::info!("Successfully singlehopped with 'direct_only' disabled"); } log::info!("Connecting to daita relay with multihop"); From e7e77cfc14291b4d06291a1fed73a0e56924a40b Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Fri, 4 Oct 2024 17:12:59 +0200 Subject: [PATCH 04/11] Replace "smart routing" with "direct only" in gui code --- gui/src/main/daemon-rpc.ts | 4 +- gui/src/main/grpc-type-convertions.ts | 2 - gui/src/main/settings.ts | 4 +- gui/src/renderer/app.tsx | 4 +- gui/src/renderer/components/DaitaSettings.tsx | 47 +++++++++++-------- .../main-view/FeatureIndicators.tsx | 35 +------------- .../select-location/RelayListContext.tsx | 6 +-- .../select-location/SelectLocation.tsx | 6 +-- gui/src/renderer/lib/filter-locations.ts | 8 ++-- gui/src/shared/daemon-rpc-types.ts | 3 +- gui/src/shared/ipc-schema.ts | 2 +- 11 files changed, 48 insertions(+), 73 deletions(-) diff --git a/gui/src/main/daemon-rpc.ts b/gui/src/main/daemon-rpc.ts index c504efe2b9dc..c86bed047f13 100644 --- a/gui/src/main/daemon-rpc.ts +++ b/gui/src/main/daemon-rpc.ts @@ -464,8 +464,8 @@ export class DaemonRpc extends GrpcClient { await this.callBool(this.client.setEnableDaita, value); } - public async setDaitaSmartRouting(value: boolean): Promise { - await this.callBool(this.client.setDaitaSmartRouting, value); + public async setDaitaDirectOnly(value: boolean): Promise { + await this.callBool(this.client.setDaitaDirectOnly, value); } public async listDevices(accountNumber: AccountNumber): Promise> { diff --git a/gui/src/main/grpc-type-convertions.ts b/gui/src/main/grpc-type-convertions.ts index d37dd3a9b840..6511f10c6735 100644 --- a/gui/src/main/grpc-type-convertions.ts +++ b/gui/src/main/grpc-type-convertions.ts @@ -381,8 +381,6 @@ function convertFromFeatureIndicator( return FeatureIndicator.customMssFix; case grpcTypes.FeatureIndicator.DAITA: return FeatureIndicator.daita; - case grpcTypes.FeatureIndicator.DAITA_SMART_ROUTING: - return FeatureIndicator.daitaSmartRouting; case grpcTypes.FeatureIndicator.SHADOWSOCKS: return FeatureIndicator.shadowsocks; } diff --git a/gui/src/main/settings.ts b/gui/src/main/settings.ts index 03537ba581a6..99fb4f2a5aec 100644 --- a/gui/src/main/settings.ts +++ b/gui/src/main/settings.ts @@ -110,8 +110,8 @@ export default class Settings implements Readonly { IpcMainEventChannel.settings.handleSetEnableDaita((value) => { return this.daemonRpc.setEnableDaita(value); }); - IpcMainEventChannel.settings.handleSetDaitaSmartRouting((value) => { - return this.daemonRpc.setDaitaSmartRouting(value); + IpcMainEventChannel.settings.handleSetDaitaDirectOnly((value) => { + return this.daemonRpc.setDaitaDirectOnly(value); }); IpcMainEventChannel.guiSettings.handleSetEnableSystemNotifications((flag: boolean) => { diff --git a/gui/src/renderer/app.tsx b/gui/src/renderer/app.tsx index 6c40b0e90899..d36c27aa14c8 100644 --- a/gui/src/renderer/app.tsx +++ b/gui/src/renderer/app.tsx @@ -349,8 +349,8 @@ export default class AppRenderer { IpcRendererEventChannel.settings.setObfuscationSettings(obfuscationSettings); public setEnableDaita = (value: boolean) => IpcRendererEventChannel.settings.setEnableDaita(value); - public setDaitaSmartRouting = (value: boolean) => - IpcRendererEventChannel.settings.setDaitaSmartRouting(value); + public setDaitaDirectOnly = (value: boolean) => + IpcRendererEventChannel.settings.setDaitaDirectOnly(value); public collectProblemReport = (toRedact: string | undefined) => IpcRendererEventChannel.problemReport.collectLogs(toRedact); public viewLog = (path: string) => IpcRendererEventChannel.problemReport.viewLog(path); diff --git a/gui/src/renderer/components/DaitaSettings.tsx b/gui/src/renderer/components/DaitaSettings.tsx index b4ee10c94df2..6a9d7fad3fba 100644 --- a/gui/src/renderer/components/DaitaSettings.tsx +++ b/gui/src/renderer/components/DaitaSettings.tsx @@ -94,10 +94,19 @@ export default function DaitaSettings() { 'It does this by carefully adding network noise and making all network packets the same size.', )} + + {sprintf( + messages.pgettext( + 'wireguard-settings-view', + 'Not all our servers are %(daita)s-enabled. We use multihop automatically to use %(daita)s with any server.', + ), + { daita: strings.daita, daitaFull: strings.daitaFull }, + )} + {messages.pgettext( 'wireguard-settings-view', - 'Can only be used with WireGuard. Since this increases your total network traffic, be cautious if you have a limited data plan. It can also negatively impact your network speed.', + 'Attention: Be cautious if you have a limited data plan as this feature will increase your network traffic. This feature can only be used with WireGuard.', )} , @@ -119,11 +128,11 @@ export default function DaitaSettings() { } function DaitaToggle() { - const { setEnableDaita, setDaitaSmartRouting } = useAppContext(); + const { setEnableDaita, setDaitaDirectOnly } = useAppContext(); const relaySettings = useSelector((state) => state.settings.relaySettings); const daita = useSelector((state) => state.settings.wireguard.daita?.enabled ?? false); - const smartRouting = useSelector( - (state) => state.settings.wireguard.daita?.smartRouting ?? false, + const directOnly = useSelector( + (state) => state.settings.wireguard.daita?.directOnly ?? false, ); const [confirmationDialogVisible, showConfirmationDialog, hideConfirmationDialog] = useBoolean(); @@ -135,16 +144,16 @@ function DaitaToggle() { void setEnableDaita(value); }, []); - const setSmartRouting = useCallback((value: boolean) => { + const setDirectOnly = useCallback((value: boolean) => { if (value) { - void setDaitaSmartRouting(value); - } else { showConfirmationDialog(); + } else { + void setDaitaDirectOnly(value); } }, []); - const confirmDisableSmartRouting = useCallback(() => { - void setDaitaSmartRouting(false); + const confirmEnableDirectOnly = useCallback(() => { + void setDaitaDirectOnly(true); hideConfirmationDialog(); }, []); @@ -170,13 +179,13 @@ function DaitaToggle() { - {messages.gettext('Smart routing')} + {messages.gettext('Direct only')} - + - + @@ -185,7 +194,7 @@ function DaitaToggle() { {sprintf( messages.pgettext( 'vpn-settings-view', - 'Makes it possible to use %(daita)s with any server and is automatically enabled.', + 'Manually choose which %(daita)s-enabled server to use.', ), { daita: strings.daita }, )} @@ -199,12 +208,12 @@ function DaitaToggle() { gridButtons={[ - {messages.gettext('Disable anyway')} + {messages.gettext('Enable "Direct only"')} , - {messages.pgettext('wireguard-settings-view', 'Use Smart routing')} + {messages.pgettext('wireguard-settings-view', 'Cancel')} , ]} close={hideConfirmationDialog}> @@ -213,7 +222,7 @@ function DaitaToggle() { // TRANSLATORS: Warning text in a dialog that is displayed after a setting is toggled. messages.pgettext( 'wireguard-settings-view', - 'Not all our servers are %(daita)s-enabled. In order to use the internet, you might have to select a new location after disabling, or you can continue using %(daita)s with Smart routing.', + 'Not all our servers are %(daita)s-enabled. In order to use the internet, you might have to select a new location after enabling.', ), { daita: strings.daita }, )} @@ -223,13 +232,13 @@ function DaitaToggle() { ); } -export function SmartRoutingModalMessage() { +function DirectOnlyModalMessage() { return ( {sprintf( messages.pgettext( 'wireguard-settings-view', - 'Not all our servers are %(daita)s-enabled. Smart routing allows %(daita)s to be used at any location. It does this by using multihop in the background to route your traffic via the closest %(daita)s-enabled server first.', + 'By enabling “Direct only” you will have to manually select a server that is %(daita)s-enabled. This can cause you to end up in a blocked state until you have selected a compatible server in the “Select location” view.', ), { daita: strings.daita, diff --git a/gui/src/renderer/components/main-view/FeatureIndicators.tsx b/gui/src/renderer/components/main-view/FeatureIndicators.tsx index a4bf8659c65b..2d7914fd8742 100644 --- a/gui/src/renderer/components/main-view/FeatureIndicators.tsx +++ b/gui/src/renderer/components/main-view/FeatureIndicators.tsx @@ -5,13 +5,10 @@ import styled from 'styled-components'; import { colors, strings } from '../../../config.json'; import { FeatureIndicator } from '../../../shared/daemon-rpc-types'; import { messages } from '../../../shared/gettext'; -import { useBoolean, useStyledRef } from '../../lib/utilityHooks'; +import { useStyledRef } from '../../lib/utilityHooks'; import { useSelector } from '../../redux/store'; import { tinyText } from '../common-styles'; -import { SmartRoutingModalMessage } from '../DaitaSettings'; import { InfoIcon } from '../InfoButton'; -import { ModalAlert, ModalAlertType } from '../Modal'; -import { SmallButton, SmallButtonColor } from '../SmallButton'; import { ConnectionPanelAccordion } from './styles'; const LINE_HEIGHT = 22; @@ -102,11 +99,6 @@ interface FeatureIndicatorsProps { // we can count those and add another ellipsis element which is visible and place it after the last // visible indicator. export default function FeatureIndicators(props: FeatureIndicatorsProps) { - const [ - daitaSmartRoutingDialogueVisible, - showDaitaSmartRoutingDialogue, - hideDaitaSmartRoutingDialogue, - ] = useBoolean(); const tunnelState = useSelector((state) => state.connection.status); const ellipsisRef = useStyledRef(); const ellipsisSpacerRef = useStyledRef(); @@ -127,9 +119,8 @@ export default function FeatureIndicators(props: FeatureIndicatorsProps) { // Returns an optional callback for clickable feature indicators, or undefined. const getFeatureIndicatorOnClick = (indicator: FeatureIndicator) => { + // NOTE: With the "smart routing" feature indicator removed, this function now does nothing, should it be removed? switch (indicator) { - case FeatureIndicator.daitaSmartRouting: - return showDaitaSmartRoutingDialogue; default: return undefined; } @@ -231,21 +222,6 @@ export default function FeatureIndicators(props: FeatureIndicatorsProps) { /> - - - {messages.gettext('Got it!')} - , - ]} - close={hideDaitaSmartRoutingDialogue}> - - ); } @@ -276,13 +252,6 @@ function getFeatureIndicatorLabel(indicator: FeatureIndicator) { switch (indicator) { case FeatureIndicator.daita: return strings.daita; - case FeatureIndicator.daitaSmartRouting: - return sprintf( - // TRANSLATORS: This refers to the Smart Routing setting in the VPN settings view. - // TRANSLATORS: This is displayed when both Smart Routing and DAITA features are on. - messages.gettext('%(daita)s: Smart routing'), - { daita: strings.daita }, - ); case FeatureIndicator.udp2tcp: case FeatureIndicator.shadowsocks: return messages.pgettext('wireguard-settings-view', 'Obfuscation'); diff --git a/gui/src/renderer/components/select-location/RelayListContext.tsx b/gui/src/renderer/components/select-location/RelayListContext.tsx index 241d11fbdcf0..ab568a088d07 100644 --- a/gui/src/renderer/components/select-location/RelayListContext.tsx +++ b/gui/src/renderer/components/select-location/RelayListContext.tsx @@ -62,8 +62,8 @@ interface RelayListContextProviderProps { export function RelayListContextProvider(props: RelayListContextProviderProps) { const { locationType, searchTerm } = useSelectLocationContext(); const daita = useSelector((state) => state.settings.wireguard.daita?.enabled ?? false); - const smartRouting = useSelector( - (state) => state.settings.wireguard.daita?.smartRouting ?? false, + const directOnly = useSelector( + (state) => state.settings.wireguard.daita?.directOnly ?? false, ); const fullRelayList = useSelector((state) => state.settings.relayLocations); @@ -81,7 +81,7 @@ export function RelayListContextProvider(props: RelayListContextProviderProps) { return filterLocationsByDaita( relayListForEndpointType, daita, - smartRouting, + directOnly, locationType, relaySettings?.tunnelProtocol ?? 'any', relaySettings?.wireguard.useMultihop ?? false, diff --git a/gui/src/renderer/components/select-location/SelectLocation.tsx b/gui/src/renderer/components/select-location/SelectLocation.tsx index 9cd66f98f435..f61f5144e104 100644 --- a/gui/src/renderer/components/select-location/SelectLocation.tsx +++ b/gui/src/renderer/components/select-location/SelectLocation.tsx @@ -68,12 +68,12 @@ export default function SelectLocation() { const providers = relaySettings?.providers ?? []; const filteredProviders = useFilteredProviders(providers, ownership); const daita = useSelector((state) => state.settings.wireguard.daita?.enabled ?? false); - const smartRouting = useSelector( - (state) => state.settings.wireguard.daita?.smartRouting ?? false, + const directOnly = useSelector( + (state) => state.settings.wireguard.daita?.directOnly ?? false, ); const showDaitaFilter = daitaFilterActive( daita, - smartRouting, + directOnly, locationType, relaySettings?.tunnelProtocol ?? 'any', relaySettings?.wireguard.useMultihop ?? false, diff --git a/gui/src/renderer/lib/filter-locations.ts b/gui/src/renderer/lib/filter-locations.ts index 661d8ecc2950..e559e6409665 100644 --- a/gui/src/renderer/lib/filter-locations.ts +++ b/gui/src/renderer/lib/filter-locations.ts @@ -37,19 +37,19 @@ export function filterLocationsByEndPointType( export function filterLocationsByDaita( locations: IRelayLocationCountryRedux[], daita: boolean, - smartRouting: boolean, + directOnly: boolean, locationType: LocationType, tunnelProtocol: LiftedConstraint, multihop: boolean, ): IRelayLocationCountryRedux[] { - return daitaFilterActive(daita, smartRouting, locationType, tunnelProtocol, multihop) + return daitaFilterActive(daita, directOnly, locationType, tunnelProtocol, multihop) ? filterLocationsImpl(locations, (relay: IRelayLocationRelayRedux) => relay.daita) : locations; } export function daitaFilterActive( daita: boolean, - smartRouting: boolean, + directOnly: boolean, locationType: LocationType, tunnelProtocol: LiftedConstraint, multihop: boolean, @@ -57,7 +57,7 @@ export function daitaFilterActive( const isEntry = multihop ? locationType === LocationType.entry : locationType === LocationType.exit; - return daita && (!smartRouting || multihop) && isEntry && tunnelProtocol !== 'openvpn'; + return daita && (directOnly || multihop) && isEntry && tunnelProtocol !== 'openvpn'; } export function filterLocations( diff --git a/gui/src/shared/daemon-rpc-types.ts b/gui/src/shared/daemon-rpc-types.ts index a22021a1d0f0..bd8f99711b27 100644 --- a/gui/src/shared/daemon-rpc-types.ts +++ b/gui/src/shared/daemon-rpc-types.ts @@ -183,7 +183,6 @@ export interface ITunnelStateRelayInfo { // The order of the variants match the priority order and can be sorted on. export enum FeatureIndicator { daita, - daitaSmartRouting, quantumResistance, multihop, bridgeMode, @@ -552,7 +551,7 @@ export interface RelayOverride { export interface IDaitaSettings { enabled: boolean; - smartRouting: boolean; + directOnly: boolean; } export function parseSocketAddress(socketAddrStr: string): ISocketAddress { diff --git a/gui/src/shared/ipc-schema.ts b/gui/src/shared/ipc-schema.ts index b10c79623e6f..954dce168014 100644 --- a/gui/src/shared/ipc-schema.ts +++ b/gui/src/shared/ipc-schema.ts @@ -195,7 +195,7 @@ export const ipcSchema = { testCustomApiAccessMethod: invoke(), clearAllRelayOverrides: invoke(), setEnableDaita: invoke(), - setDaitaSmartRouting: invoke(), + setDaitaDirectOnly: invoke(), }, guiSettings: { '': notifyRenderer(), From b512af0b3afa7d709d229b44973a834c3340b971 Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Mon, 7 Oct 2024 09:32:48 +0200 Subject: [PATCH 05/11] Simplify the DAITA + multihop feature indicator logic We now simply show the "multihop" indicator there is an entry endpoint, regardless of whether it was activated manually or through DAITA. This reflects the intent to base the feature indicators on the current connection and not the user settings. There is no special indicator for "smart routing" or "direct only". --- .../lib/daemon/grpc/mapper/ToDomain.kt | 1 - .../mullvadvpn/lib/model/FeatureIndicator.kt | 1 - mullvad-types/src/features.rs | 39 +++---------------- 3 files changed, 6 insertions(+), 35 deletions(-) diff --git a/android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/mapper/ToDomain.kt b/android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/mapper/ToDomain.kt index 4a8ee04683e0..2f6a08eede39 100644 --- a/android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/mapper/ToDomain.kt +++ b/android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/mapper/ToDomain.kt @@ -621,7 +621,6 @@ internal fun ManagementInterface.FeatureIndicator.toDomain() = ManagementInterface.FeatureIndicator.CUSTOM_MTU -> FeatureIndicator.CUSTOM_MTU ManagementInterface.FeatureIndicator.DAITA -> FeatureIndicator.DAITA ManagementInterface.FeatureIndicator.SHADOWSOCKS -> FeatureIndicator.SHADOWSOCKS - ManagementInterface.FeatureIndicator.DAITA_SMART_ROUTING, ManagementInterface.FeatureIndicator.LOCKDOWN_MODE, ManagementInterface.FeatureIndicator.MULTIHOP, ManagementInterface.FeatureIndicator.BRIDGE_MODE, diff --git a/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/FeatureIndicator.kt b/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/FeatureIndicator.kt index 74fea073267f..3c8df824f48a 100644 --- a/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/FeatureIndicator.kt +++ b/android/lib/model/src/main/kotlin/net/mullvad/mullvadvpn/lib/model/FeatureIndicator.kt @@ -3,7 +3,6 @@ package net.mullvad.mullvadvpn.lib.model // The order of the variants match the priority order and can be sorted on. enum class FeatureIndicator { DAITA, - // DAITA_SMART_ROUTING QUANTUM_RESISTANCE, // MULTIHOP, SPLIT_TUNNELING, diff --git a/mullvad-types/src/features.rs b/mullvad-types/src/features.rs index cb6bb23f982f..0e25f5a59ceb 100644 --- a/mullvad-types/src/features.rs +++ b/mullvad-types/src/features.rs @@ -3,10 +3,7 @@ use std::{ fmt::{Debug, Display}, }; -use crate::{ - relay_constraints::RelaySettings, - settings::{DnsState, Settings}, -}; +use crate::settings::{DnsState, Settings}; use serde::{Deserialize, Serialize}; use talpid_types::net::{ObfuscationType, TunnelEndpoint, TunnelType}; @@ -175,35 +172,11 @@ pub fn compute_feature_indicators( let mtu = settings.tunnel_options.wireguard.mtu.is_some(); let mut daita = false; - let mut multihop = endpoint.entry_endpoint.is_some(); + let multihop = endpoint.entry_endpoint.is_some(); #[cfg(daita)] if endpoint.daita { daita = true; - - let multihop_setting_enabled = - if let RelaySettings::Normal(constraints) = &settings.relay_settings { - constraints.wireguard_constraints.use_multihop - } else { - false - }; - - let daita_use_multihop_if_necessary = settings - .tunnel_options - .wireguard - .daita - .use_multihop_if_necessary; - - // If multihop is disabled in the settings, but enabled automatically by DAITA, we - // will not show the multihop indicator. - let multihop_enable_automatically = multihop && !multihop_setting_enabled; - if multihop_enable_automatically { - debug_assert!( - daita_use_multihop_if_necessary, - "Multihop can only be enabled automatically if DAITA is enabled" - ); - multihop = false; - } } vec![ @@ -419,8 +392,8 @@ mod tests { // Here we mock that multihop was automatically enabled by DAITA. // We enable `use_multihop_if_necessary` again and disable the multihop setting, while - // keeping the entry relay In this scenario, we should not get a Multihop - // indicator + // keeping the entry relay. In this scenario, we should still get a Multihop + // indicator. settings .tunnel_options .wireguard @@ -429,15 +402,15 @@ mod tests { if let RelaySettings::Normal(constraints) = &mut settings.relay_settings { constraints.wireguard_constraints.use_multihop = false; }; - expected_indicators.0.remove(&FeatureIndicator::Multihop); assert_eq!( compute_feature_indicators(&settings, &endpoint, false), expected_indicators, "DaitaDirectOnly should be enabled" ); - // If we also remove the entry relay, we should still not get a multihop indicator + // If we also remove the entry relay, we should not get a multihop indicator endpoint.entry_endpoint = None; + expected_indicators.0.remove(&FeatureIndicator::Multihop); assert_eq!( compute_feature_indicators(&settings, &endpoint, false), expected_indicators, From bf109a9e85a6cf71679595b5b3ff053e805a30a4 Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Mon, 7 Oct 2024 10:53:30 +0200 Subject: [PATCH 06/11] Do not toggle "direct only" with DAITA --- mullvad-daemon/src/lib.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/mullvad-daemon/src/lib.rs b/mullvad-daemon/src/lib.rs index 312e11d84f35..dd6c828802da 100644 --- a/mullvad-daemon/src/lib.rs +++ b/mullvad-daemon/src/lib.rs @@ -2345,15 +2345,6 @@ impl Daemon { .settings .update(|settings| { settings.tunnel_options.wireguard.daita.enabled = value; - - // enable 'use_multihop_if_necessary' automatically with daita - if cfg!(not(target_os = "android")) { - settings - .tunnel_options - .wireguard - .daita - .use_multihop_if_necessary = value - } }) .await; From 8ace1c33558a97cfdf3f683f63e3534a0e10560f Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Mon, 7 Oct 2024 11:36:50 +0200 Subject: [PATCH 07/11] Set "Direct only" to `false` as default For android, it is set to true, as multihop is not supported. Note that in the daemon, the setting is called `use_multihop_if_necessary` and has the inverse meaning. --- .../lib/daemon/grpc/ManagementService.kt | 12 ++++++--- mullvad-types/src/wireguard.rs | 26 +++++++++++++++++-- 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/ManagementService.kt b/android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/ManagementService.kt index 7961b5460c0a..851c92dad443 100644 --- a/android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/ManagementService.kt +++ b/android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/ManagementService.kt @@ -510,10 +510,14 @@ class ManagementService( suspend fun setDaitaEnabled(enabled: Boolean): Either = Either.catch { - val daitaSettings = - ManagementInterface.DaitaSettings.newBuilder().setEnabled(enabled).build() - grpc.setDaitaSettings(daitaSettings) - } + val daitaSettings = + ManagementInterface.DaitaSettings.newBuilder().setEnabled(enabled) + // TODO: Before Multihop is supported on Android, calling `setDirectOnly` with false + // will cause undefined behaviour. + .setDirectOnly(true) + .build() + grpc.setDaitaSettings(daitaSettings) + } .mapLeft(SetDaitaSettingsError::Unknown) .mapEmpty() diff --git a/mullvad-types/src/wireguard.rs b/mullvad-types/src/wireguard.rs index 77ed60e1b2d2..12f68566bd80 100644 --- a/mullvad-types/src/wireguard.rs +++ b/mullvad-types/src/wireguard.rs @@ -81,14 +81,36 @@ impl FromStr for QuantumResistantState { pub struct QuantumResistantStateParseError; #[cfg(daita)] -#[derive(Debug, Default, Clone, Serialize, Deserialize, PartialEq)] +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] pub struct DaitaSettings { pub enabled: bool, - #[serde(default)] + #[cfg_attr(target_os = "android", serde(skip))] + #[serde(default = "DaitaSettings::default_use_multihop_if_necessary")] + /// Whether to use multihop if the selected relay is not DAITA-compatible. Note that this is + /// the inverse of of "Direct only" in the GUI. pub use_multihop_if_necessary: bool, } +#[cfg(daita)] +impl DaitaSettings { + /// This setting should be enabled by default, expect on Android where multihop is not + /// supported. + const fn default_use_multihop_if_necessary() -> bool { + cfg!(not(target_os = "android")) + } +} + +#[cfg(daita)] +impl Default for DaitaSettings { + fn default() -> Self { + Self { + enabled: false, + use_multihop_if_necessary: Self::default_use_multihop_if_necessary(), + } + } +} + /// Contains account specific wireguard data #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)] pub struct WireguardData { From 74f1ae59d98fdd12c598fccb25aa686ac01e1c91 Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Mon, 7 Oct 2024 12:22:35 +0200 Subject: [PATCH 08/11] Fix `eslint` complaints --- gui/src/renderer/components/DaitaSettings.tsx | 4 +--- .../renderer/components/select-location/RelayListContext.tsx | 4 +--- .../renderer/components/select-location/SelectLocation.tsx | 4 +--- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/gui/src/renderer/components/DaitaSettings.tsx b/gui/src/renderer/components/DaitaSettings.tsx index 6a9d7fad3fba..2d09a3996e68 100644 --- a/gui/src/renderer/components/DaitaSettings.tsx +++ b/gui/src/renderer/components/DaitaSettings.tsx @@ -131,9 +131,7 @@ function DaitaToggle() { const { setEnableDaita, setDaitaDirectOnly } = useAppContext(); const relaySettings = useSelector((state) => state.settings.relaySettings); const daita = useSelector((state) => state.settings.wireguard.daita?.enabled ?? false); - const directOnly = useSelector( - (state) => state.settings.wireguard.daita?.directOnly ?? false, - ); + const directOnly = useSelector((state) => state.settings.wireguard.daita?.directOnly ?? false); const [confirmationDialogVisible, showConfirmationDialog, hideConfirmationDialog] = useBoolean(); diff --git a/gui/src/renderer/components/select-location/RelayListContext.tsx b/gui/src/renderer/components/select-location/RelayListContext.tsx index ab568a088d07..e41b04c17e86 100644 --- a/gui/src/renderer/components/select-location/RelayListContext.tsx +++ b/gui/src/renderer/components/select-location/RelayListContext.tsx @@ -62,9 +62,7 @@ interface RelayListContextProviderProps { export function RelayListContextProvider(props: RelayListContextProviderProps) { const { locationType, searchTerm } = useSelectLocationContext(); const daita = useSelector((state) => state.settings.wireguard.daita?.enabled ?? false); - const directOnly = useSelector( - (state) => state.settings.wireguard.daita?.directOnly ?? false, - ); + const directOnly = useSelector((state) => state.settings.wireguard.daita?.directOnly ?? false); const fullRelayList = useSelector((state) => state.settings.relayLocations); const relaySettings = useNormalRelaySettings(); diff --git a/gui/src/renderer/components/select-location/SelectLocation.tsx b/gui/src/renderer/components/select-location/SelectLocation.tsx index f61f5144e104..01297f56309a 100644 --- a/gui/src/renderer/components/select-location/SelectLocation.tsx +++ b/gui/src/renderer/components/select-location/SelectLocation.tsx @@ -68,9 +68,7 @@ export default function SelectLocation() { const providers = relaySettings?.providers ?? []; const filteredProviders = useFilteredProviders(providers, ownership); const daita = useSelector((state) => state.settings.wireguard.daita?.enabled ?? false); - const directOnly = useSelector( - (state) => state.settings.wireguard.daita?.directOnly ?? false, - ); + const directOnly = useSelector((state) => state.settings.wireguard.daita?.directOnly ?? false); const showDaitaFilter = daitaFilterActive( daita, directOnly, From dcd425fa0fcc7ad1d89d9d1132b57fd9b0c95d70 Mon Sep 17 00:00:00 2001 From: Markus Pettersson Date: Mon, 7 Oct 2024 14:08:41 +0200 Subject: [PATCH 09/11] Run `kotlinfmt` --- .../lib/daemon/grpc/ManagementService.kt | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/ManagementService.kt b/android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/ManagementService.kt index 851c92dad443..67719e0a9108 100644 --- a/android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/ManagementService.kt +++ b/android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/ManagementService.kt @@ -510,14 +510,15 @@ class ManagementService( suspend fun setDaitaEnabled(enabled: Boolean): Either = Either.catch { - val daitaSettings = - ManagementInterface.DaitaSettings.newBuilder().setEnabled(enabled) - // TODO: Before Multihop is supported on Android, calling `setDirectOnly` with false - // will cause undefined behaviour. - .setDirectOnly(true) - .build() - grpc.setDaitaSettings(daitaSettings) - } + val daitaSettings = + ManagementInterface.DaitaSettings.newBuilder() + .setEnabled(enabled) + // TODO: Before Multihop is supported on Android, calling `setDirectOnly` + // with false will cause undefined behaviour. + .setDirectOnly(true) + .build() + grpc.setDaitaSettings(daitaSettings) + } .mapLeft(SetDaitaSettingsError::Unknown) .mapEmpty() From cf55c3c28c5e2151240d0b735160c2dce52e771a Mon Sep 17 00:00:00 2001 From: Markus Pettersson Date: Mon, 7 Oct 2024 14:27:05 +0200 Subject: [PATCH 10/11] Do not use forbidden word in Android code --- .../mullvad/mullvadvpn/lib/daemon/grpc/ManagementService.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/ManagementService.kt b/android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/ManagementService.kt index 67719e0a9108..024091c9dc79 100644 --- a/android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/ManagementService.kt +++ b/android/lib/daemon-grpc/src/main/kotlin/net/mullvad/mullvadvpn/lib/daemon/grpc/ManagementService.kt @@ -513,8 +513,9 @@ class ManagementService( val daitaSettings = ManagementInterface.DaitaSettings.newBuilder() .setEnabled(enabled) - // TODO: Before Multihop is supported on Android, calling `setDirectOnly` - // with false will cause undefined behaviour. + // Before Multihop is supported on Android, calling `setDirectOnly` with + // false will cause undefined behaviour. Will be fixed by as part of + // DROID-1412. .setDirectOnly(true) .build() grpc.setDaitaSettings(daitaSettings) From 777013135b01d1c4d13961802db511b0e06e90e9 Mon Sep 17 00:00:00 2001 From: Sebastian Holmin Date: Mon, 7 Oct 2024 15:31:26 +0200 Subject: [PATCH 11/11] Update translation files with "direct only" --- gui/locales/messages.pot | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/gui/locales/messages.pot b/gui/locales/messages.pot index e9ef237a3a7d..174688cf27f7 100644 --- a/gui/locales/messages.pot +++ b/gui/locales/messages.pot @@ -5,11 +5,6 @@ msgstr "" msgid "%(amount)d more..." msgstr "" -#. This refers to the Smart Routing setting in the VPN settings view. -#. This is displayed when both Smart Routing and DAITA features are on. -msgid "%(daita)s: Smart routing" -msgstr "" - msgid "%(duration)s was added, account paid until %(expiry)s." msgstr "" @@ -127,10 +122,10 @@ msgstr "" msgid "Delete list" msgstr "" -msgid "Disable" +msgid "Direct only" msgstr "" -msgid "Disable anyway" +msgid "Disable" msgstr "" msgid "Disconnect" @@ -157,6 +152,9 @@ msgstr "" msgid "Enable" msgstr "" +msgid "Enable \"Direct only\"" +msgstr "" + msgid "Enable anyway" msgstr "" @@ -253,9 +251,6 @@ msgstr "" msgid "Settings" msgstr "" -msgid "Smart routing" -msgstr "" - msgid "System default" msgstr "" @@ -1956,13 +1951,13 @@ msgctxt "vpn-settings-view" msgid "Lockdown mode" msgstr "" +#. Label for settings that enables malware blocking. msgctxt "vpn-settings-view" -msgid "Makes it possible to use %(daita)s with any server and is automatically enabled." +msgid "Malware" msgstr "" -#. Label for settings that enables malware blocking. msgctxt "vpn-settings-view" -msgid "Malware" +msgid "Manually choose which %(daita)s-enabled server to use." msgstr "" msgctxt "vpn-settings-view" @@ -2071,7 +2066,15 @@ msgid "%(wireguard)s settings" msgstr "" msgctxt "wireguard-settings-view" -msgid "Can only be used with WireGuard. Since this increases your total network traffic, be cautious if you have a limited data plan. It can also negatively impact your network speed." +msgid "Attention: Be cautious if you have a limited data plan as this feature will increase your network traffic. This feature can only be used with WireGuard." +msgstr "" + +msgctxt "wireguard-settings-view" +msgid "By enabling “Direct only” you will have to manually select a server that is %(daita)s-enabled. This can cause you to end up in a blocked state until you have selected a compatible server in the “Select location” view." +msgstr "" + +msgctxt "wireguard-settings-view" +msgid "Cancel" msgstr "" msgctxt "wireguard-settings-view" @@ -2104,11 +2107,11 @@ msgstr "" #. Warning text in a dialog that is displayed after a setting is toggled. msgctxt "wireguard-settings-view" -msgid "Not all our servers are %(daita)s-enabled. In order to use the internet, you might have to select a new location after disabling, or you can continue using %(daita)s with Smart routing." +msgid "Not all our servers are %(daita)s-enabled. In order to use the internet, you might have to select a new location after enabling." msgstr "" msgctxt "wireguard-settings-view" -msgid "Not all our servers are %(daita)s-enabled. Smart routing allows %(daita)s to be used at any location. It does this by using multihop in the background to route your traffic via the closest %(daita)s-enabled server first." +msgid "Not all our servers are %(daita)s-enabled. We use multihop automatically to use %(daita)s with any server." msgstr "" msgctxt "wireguard-settings-view" @@ -2181,10 +2184,6 @@ msgctxt "wireguard-settings-view" msgid "UDP-over-TCP port" msgstr "" -msgctxt "wireguard-settings-view" -msgid "Use Smart routing" -msgstr "" - #. Text describing the valid port range for a port selector. msgctxt "wireguard-settings-view" msgid "Valid range: %(min)s - %(max)s"