diff --git a/CHANGELOG.md b/CHANGELOG.md index 45ddb8fbb6cb..d3b172714f0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,8 +23,7 @@ Line wrap the file at 100 chars. Th ## [Unreleased] ### Added -- Add customizable relay lists to the CLI on desktop. Custom lists can be managed through - `mullvad custom-lists` and can be selected through `mullvad relay set` and `mullvad bridge set`. +- Add customizable relay lists to the CLI on desktop. Custom lists can be managed through `mullvad custom-lists` and can be selected through `mullvad relay set` and `mullvad bridge set`. - Add custom lists to location selector in desktop app. - Add custom API access methods to the CLI on desktop. Custom API access methods allow the user to proxy API traffic through a peer before connecting to a tunnel. They are managed through @@ -36,6 +35,7 @@ Line wrap the file at 100 chars. Th ### Changed - Update Electron from 25.2.0 to 26.3.0. +- CLI command `mullvad relay set tunnel wireguard entry-location` changed to `mullvad relay set tunnel wireguard entry location`, as the `location` subcommand can now be swapped for `custom-list` to select entry relays using a custom list. #### Android - Migrate welcome view to compose. @@ -59,6 +59,7 @@ Line wrap the file at 100 chars. Th - Remove wireguard-go (userspace WireGuard) support. ### Fixed +- Validate that hostname matches correct server type for CLI commands `mullvad relay set location`, `mullvad bridge set location` and `mullvad relay set tunnel wireguard entry location` - Show correct endpoint in CLI for custom relays. - Lower risk of being rate limited. - Fix error dialog when failing to write to console by handling the thrown error. diff --git a/Cargo.lock b/Cargo.lock index 391a7179b7a9..ae39b48befd5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1817,7 +1817,6 @@ dependencies = [ "chrono", "clap", "clap_complete", - "env_logger 0.10.0", "futures", "itertools", "mullvad-management-interface", diff --git a/mullvad-cli/Cargo.toml b/mullvad-cli/Cargo.toml index ca08e5f7258e..60ef5faddffd 100644 --- a/mullvad-cli/Cargo.toml +++ b/mullvad-cli/Cargo.toml @@ -17,7 +17,6 @@ path = "src/main.rs" anyhow = "1.0" chrono = { workspace = true } clap = { workspace = true } -env_logger = { workspace = true } futures = "0.3" natord = "1.0.9" itertools = "0.10" diff --git a/mullvad-cli/src/cmds/bridge.rs b/mullvad-cli/src/cmds/bridge.rs index 50c242cefd3b..4abccb99e839 100644 --- a/mullvad-cli/src/cmds/bridge.rs +++ b/mullvad-cli/src/cmds/bridge.rs @@ -11,7 +11,7 @@ use mullvad_types::{ use std::net::{IpAddr, SocketAddr}; use talpid_types::net::openvpn::{self, SHADOWSOCKS_CIPHERS}; -use super::{relay::find_relay_by_hostname, relay_constraints::LocationArgs}; +use super::{relay::resolve_location_constraint, relay_constraints::LocationArgs}; #[derive(Subcommand, Debug)] pub enum Bridge { @@ -160,16 +160,15 @@ impl Bridge { println!("Updated bridge state"); Ok(()) } - SetCommands::Location(location) => { - let countries = rpc.get_relay_locations().await?.countries; - let location = - if let Some(relay) = find_relay_by_hostname(&countries, &location.country) { - Constraint::Only(relay) - } else { - Constraint::from(location) - }; - let location = location.map(LocationConstraint::Location); - Self::update_bridge_settings(&mut rpc, Some(location), None, None).await + SetCommands::Location(location_constraint_args) => { + let relay_filter = |relay: &mullvad_types::relay_list::Relay| { + relay.active && relay.endpoint_data == RelayEndpointData::Bridge + }; + let location_constraint = + resolve_location_constraint(&mut rpc, location_constraint_args, relay_filter) + .await? + .map(LocationConstraint::from); + Self::update_bridge_settings(&mut rpc, Some(location_constraint), None, None).await } SetCommands::CustomList { custom_list_name } => { let list = diff --git a/mullvad-cli/src/cmds/custom_list.rs b/mullvad-cli/src/cmds/custom_list.rs index d70a96a28aa7..3dc4445f8f2c 100644 --- a/mullvad-cli/src/cmds/custom_list.rs +++ b/mullvad-cli/src/cmds/custom_list.rs @@ -1,8 +1,5 @@ -use super::{ - relay::{find_relay_by_hostname, get_filtered_relays}, - relay_constraints::LocationArgs, -}; -use anyhow::{anyhow, Result}; +use super::{relay::resolve_location_constraint, relay_constraints::LocationArgs}; +use anyhow::{anyhow, bail, Result}; use clap::Subcommand; use mullvad_management_interface::MullvadProxyClient; use mullvad_types::{ @@ -108,31 +105,49 @@ impl CustomList { } async fn add_location(name: String, location_args: LocationArgs) -> Result<()> { - let countries = get_filtered_relays().await?; - let location = find_relay_by_hostname(&countries, &location_args.country) - .map_or(Constraint::from(location_args), Constraint::Only) - .option() - .ok_or(anyhow!("\"any\" is not a valid location"))?; - let mut rpc = MullvadProxyClient::new().await?; - let mut list = find_list_by_name(&mut rpc, &name).await?; - list.locations.insert(location); - rpc.update_custom_list(list).await?; + // Don't filter out any hosts, i.e. allow adding even inactive ones + let relay_filter = |_: &_| true; + let location_constraint = + resolve_location_constraint(&mut rpc, location_args, relay_filter).await?; + + match location_constraint { + Constraint::Any => bail!("\"any\" is not a valid location"), + Constraint::Only(location) => { + let mut list = find_list_by_name(&mut rpc, &name).await?; + if list.locations.insert(location) { + rpc.update_custom_list(list).await?; + println!("Location added to custom-list") + } else { + bail!("Provided location is already present in custom-list") + }; + } + } Ok(()) } async fn remove_location(name: String, location_args: LocationArgs) -> Result<()> { - let location = Constraint::::from(location_args) - .option() - .ok_or(anyhow!("\"any\" is not a valid location"))?; - let mut rpc = MullvadProxyClient::new().await?; - let mut list = find_list_by_name(&mut rpc, &name).await?; - list.locations.remove(&location); - rpc.update_custom_list(list).await?; + // Don't filter out any hosts, i.e. allow adding even inactive ones + let relay_filter = |_: &_| true; + let location_constraint = + resolve_location_constraint(&mut rpc, location_args, relay_filter).await?; + + match location_constraint { + Constraint::Any => bail!("\"any\" is not a valid location"), + Constraint::Only(location) => { + let mut list = find_list_by_name(&mut rpc, &name).await?; + if list.locations.remove(&location) { + rpc.update_custom_list(list).await?; + println!("Location removed from custom-list") + } else { + bail!("Provided location was not present in custom-list") + }; + } + } Ok(()) } diff --git a/mullvad-cli/src/cmds/debug.rs b/mullvad-cli/src/cmds/debug.rs new file mode 100644 index 000000000000..47db11ee0576 --- /dev/null +++ b/mullvad-cli/src/cmds/debug.rs @@ -0,0 +1,40 @@ +use anyhow::Result; +use mullvad_management_interface::MullvadProxyClient; +use mullvad_types::relay_constraints::{Constraint, RelayConstraints, RelaySettings}; + +#[derive(clap::Subcommand, Debug)] +pub enum DebugCommands { + /// Block all internet connection by setting an invalid relay constraint. + BlockConnection, +} + +impl DebugCommands { + pub async fn handle(self) -> Result<()> { + match self { + DebugCommands::BlockConnection => { + let mut rpc = MullvadProxyClient::new().await?; + let settings = rpc.get_settings().await?; + + let relay_settings = settings.get_relay_settings(); + let mut constraints = match relay_settings { + RelaySettings::Normal(normal) => normal, + RelaySettings::CustomTunnelEndpoint(_custom) => { + println!("Removing custom relay settings"); + RelayConstraints::default() + } + }; + constraints.location = Constraint::Only( + mullvad_types::relay_constraints::LocationConstraint::Location( + mullvad_types::relay_constraints::GeographicLocationConstraint::Country( + "xx".into(), + ), + ), + ); + rpc.set_relay_settings(RelaySettings::Normal(constraints)) + .await?; + eprintln!("WARNING: ENTERED BLOCKED MODE"); + Ok(()) + } + } + } +} diff --git a/mullvad-cli/src/cmds/mod.rs b/mullvad-cli/src/cmds/mod.rs index 7944e8bdc07a..1984f1493bc2 100644 --- a/mullvad-cli/src/cmds/mod.rs +++ b/mullvad-cli/src/cmds/mod.rs @@ -1,6 +1,5 @@ use clap::builder::{PossibleValuesParser, TypedValueParser, ValueParser}; -use std::io::stdin; -use std::ops::Deref; +use std::{io::stdin, ops::Deref}; pub mod account; pub mod api_access; @@ -8,6 +7,7 @@ pub mod auto_connect; pub mod beta_program; pub mod bridge; pub mod custom_list; +pub mod debug; pub mod dns; pub mod import_settings; pub mod lan; diff --git a/mullvad-cli/src/cmds/relay.rs b/mullvad-cli/src/cmds/relay.rs index 273b6f3395da..3c14456518ed 100644 --- a/mullvad-cli/src/cmds/relay.rs +++ b/mullvad-cli/src/cmds/relay.rs @@ -1,4 +1,4 @@ -use anyhow::{anyhow, Context, Result}; +use anyhow::{anyhow, bail, Context, Result}; use clap::Subcommand; use itertools::Itertools; use mullvad_management_interface::MullvadProxyClient; @@ -134,14 +134,21 @@ pub enum SetTunnelCommands { use_multihop: Option, #[clap(subcommand)] - entry_location: Option, + entry: Option, }, } #[derive(Subcommand, Debug, Clone)] -pub enum EntryLocation { - /// Entry endpoint to use. This can be 'any' or any location that is valid with 'set location', - /// such as 'se got'. +pub enum EntryCommands { + /// Set wireguard entry relay constraints + #[clap(subcommand)] + Entry(EntryArgs), +} + +#[derive(Subcommand, Debug, Clone)] +pub enum EntryArgs { + /// Location of entry relay. This can be 'any' or any location that is valid with 'set + /// location', such as 'se got'. #[command( override_usage = "mullvad relay set tunnel wireguard entry-location [CITY] [HOSTNAME] | @@ -161,7 +168,7 @@ pub enum EntryLocation { \tmullvad relay set tunnel wireguard entry-location se-got-wg-004" )] - EntryLocation(LocationArgs), + Location(LocationArgs), /// Name of custom list to use to pick entry endpoint. CustomList { custom_list_name: String }, } @@ -335,7 +342,7 @@ impl Relay { } async fn list() -> Result<()> { - let mut countries = get_filtered_relays().await?; + let mut countries = get_active_relays().await?; countries.sort_by(|c1, c2| natord::compare_ignore_case(&c1.name, &c2.name)); for mut country in countries { country @@ -432,10 +439,10 @@ impl Relay { port, ip_version, use_multihop, - entry_location, + entry, } => { - Self::set_wireguard_constraints(port, ip_version, use_multihop, entry_location) - .await + let entry = entry.map(|EntryCommands::Entry(entry)| entry); + Self::set_wireguard_constraints(port, ip_version, use_multihop, entry).await } } } @@ -546,35 +553,42 @@ impl Relay { } async fn set_location(location_constraint_args: LocationArgs) -> Result<()> { - let countries = get_filtered_relays().await?; - let constraint = - if let Some(relay) = - // The country field is assumed to be hostname due to CLI argument parsing - find_relay_by_hostname(&countries, &location_constraint_args.country) - { - Constraint::Only(LocationConstraint::Location(relay)) - } else { - let location_constraint: Constraint = - Constraint::from(location_constraint_args); - match &location_constraint { - Constraint::Any => (), - Constraint::Only(constraint) => { - let found = countries - .into_iter() - .flat_map(|country| country.cities) - .flat_map(|city| city.relays) - .any(|relay| constraint.matches(&relay)); - - if !found { - eprintln!("Warning: No matching relay was found."); - } - } + let mut rpc = MullvadProxyClient::new().await?; + let relay_settings = rpc.get_settings().await?.get_relay_settings(); + let constraints = match relay_settings { + RelaySettings::Normal(constraints) => constraints, + RelaySettings::CustomTunnelEndpoint(_custom) => { + bail!("Cannot change location while custom endpoint is set") + } + }; + + // Depending on the current configured tunnel protocol, we filter only the relevant hosts + let location_constraint = match constraints.tunnel_protocol { + Constraint::Any => { + resolve_location_constraint(&mut rpc, location_constraint_args, |relay| { + relay.active && relay.endpoint_data != RelayEndpointData::Bridge + }) + .await + } + Constraint::Only(tunnel) => match tunnel { + TunnelType::OpenVpn => { + resolve_location_constraint(&mut rpc, location_constraint_args, |relay| { + relay.active && relay.endpoint_data == RelayEndpointData::Openvpn + }) + .await } - location_constraint.map(LocationConstraint::Location) - }; + TunnelType::Wireguard => { + resolve_location_constraint(&mut rpc, location_constraint_args, |relay| { + relay.active + && matches!(relay.endpoint_data, RelayEndpointData::Wireguard(_)) + }) + .await + } + }, + }?; Self::update_constraints(|constraints| { - constraints.location = constraint; + constraints.location = location_constraint.map(LocationConstraint::from); }) .await } @@ -639,7 +653,7 @@ impl Relay { port: Option>, ip_version: Option>, use_multihop: Option, - entry_location: Option, + entry_location: Option, ) -> Result<()> { let mut rpc = MullvadProxyClient::new().await?; let wireguard = rpc.get_relay_locations().await?.wireguard; @@ -668,17 +682,17 @@ impl Relay { wireguard_constraints.use_multihop = *use_multihop; } match entry_location { - Some(EntryLocation::EntryLocation(entry)) => { - let countries = get_filtered_relays().await?; - // The country field is assumed to be hostname due to CLI argument parsing + Some(EntryArgs::Location(location_args)) => { + let relay_filter = |relay: &mullvad_types::relay_list::Relay| { + relay.active && matches!(relay.endpoint_data, RelayEndpointData::Wireguard(_)) + }; + let location_constraint = + resolve_location_constraint(&mut rpc, location_args, relay_filter).await?; + wireguard_constraints.entry_location = - if let Some(relay) = find_relay_by_hostname(&countries, &entry.country) { - Constraint::Only(LocationConstraint::Location(relay)) - } else { - Constraint::from(entry) - }; + location_constraint.map(LocationConstraint::from); } - Some(EntryLocation::CustomList { custom_list_name }) => { + Some(EntryArgs::CustomList { custom_list_name }) => { let list_id = super::custom_list::find_list_by_name(&mut rpc, &custom_list_name) .await? .id; @@ -722,10 +736,14 @@ impl Relay { let settings = rpc.get_settings().await?; if warn_non_existent_hostname { - let countries = get_filtered_relays_with_client(&mut rpc).await?; - if find_relay_by_hostname(&countries, hostname).is_none() { + let relay_list = rpc.get_relay_locations().await?; + if !relay_list.relays().any(|relay| { + relay.active + && relay.endpoint_data != RelayEndpointData::Bridge + && relay.hostname.to_lowercase() == hostname.to_lowercase() + }) { eprintln!("Warning: Setting overrides for an unrecognized server"); - } + }; } let mut relay_overrides = settings.relay_overrides; @@ -764,7 +782,7 @@ impl Relay { } let mut countries_with_overrides = vec![]; - for country in get_filtered_relays().await? { + for country in get_active_relays().await? { let mut country_with_overrides = Country { name: country.name, code: country.code, @@ -898,63 +916,94 @@ fn parse_transport_port( } } -/// Lookup a relay among a list of [`RelayListCountry`]s by hostname. -/// The matching is exact, bar capitalization. -pub fn find_relay_by_hostname( - countries: &[RelayListCountry], - hostname: &str, +fn relay_to_geographical_constraint( + relay: mullvad_types::relay_list::Relay, ) -> Option { - countries - .iter() - .flat_map(|country| country.cities.clone()) - .flat_map(|city| city.relays) - .find(|relay| relay.hostname.to_lowercase() == hostname.to_lowercase()) - .and_then(|relay| { - relay.location.map( - |Location { - country_code, - city_code, - .. - }| { - GeographicLocationConstraint::Hostname(country_code, city_code, relay.hostname) - }, - ) - }) + relay.location.map( + |Location { + country_code, + city_code, + .. + }| { + GeographicLocationConstraint::Hostname(country_code, city_code, relay.hostname) + }, + ) } -/// Return a list of all active non-bridge relays -pub async fn get_filtered_relays() -> Result> { - let mut rpc = MullvadProxyClient::new().await?; - get_filtered_relays_with_client(&mut rpc).await -} - -/// Return a list of all active non-bridge relays -async fn get_filtered_relays_with_client( +/// Parses the [`LocationArgs`] into a [`Constraint`]. +/// +/// See the documentation of [`mullvad relay set location`](SetCommands) for a description +/// of what arguments are valid. +/// +/// Usually, only a subset of relays are relevant, e.g. only active server of a certain type. +/// Use `relay_filter` to pass in this requirement. If the user gives a host not matching the +/// filter an appropriate error is given. +pub async fn resolve_location_constraint( rpc: &mut MullvadProxyClient, -) -> Result> { - let relay_list = rpc.get_relay_locations().await?; + location_constraint_args: LocationArgs, + relay_filter: impl FnOnce(&mullvad_types::relay_list::Relay) -> bool, +) -> Result> { + let relay_iter = rpc.get_relay_locations().await?.into_relays(); + if let Some(matching_relay) = relay_iter + .clone() + .find(|relay| relay.hostname.to_lowercase() == location_constraint_args.country) + { + if relay_filter(&matching_relay) { + Ok(Constraint::Only( + relay_to_geographical_constraint(matching_relay) + .context("Selected relay did not contain a valid location")?, + )) + } else { + bail!( + "The relay `{}` is not valid for this operation", + location_constraint_args.country + ) + } + } else { + // The Constraint was not a relay, assuming it to be a location + let location_constraint: Constraint = + Constraint::from(location_constraint_args); - let mut countries = vec![]; + // If the location constraint was not "any", then validate the country/city + if let Constraint::Only(constraint) = &location_constraint { + let found = relay_iter.clone().any(|relay| constraint.matches(&relay)); - for mut country in relay_list.countries { - country.cities = country - .cities - .into_iter() - .filter_map(|mut city| { - city.relays.retain(|relay| { - relay.active && relay.endpoint_data != RelayEndpointData::Bridge - }); - if !city.relays.is_empty() { - Some(city) - } else { - None - } - }) - .collect(); - if !country.cities.is_empty() { - countries.push(country); + if !found { + bail!("Invalid location argument"); + } } + + Ok(location_constraint) } +} - Ok(countries) +/// Return a list of all relays that are active and not bridges +pub async fn get_active_relays() -> Result> { + let mut rpc = MullvadProxyClient::new().await?; + let relay_list = rpc.get_relay_locations().await?; + Ok(relay_list + .countries + .into_iter() + .filter_map(|mut country| { + country.cities = country + .cities + .into_iter() + .filter_map(|mut city| { + city.relays.retain(|relay| { + relay.active && relay.endpoint_data != RelayEndpointData::Bridge + }); + if !city.relays.is_empty() { + Some(city) + } else { + None + } + }) + .collect(); + if !country.cities.is_empty() { + Some(country) + } else { + None + } + }) + .collect_vec()) } diff --git a/mullvad-cli/src/cmds/relay_constraints.rs b/mullvad-cli/src/cmds/relay_constraints.rs index 626754edcb58..4e09e5b880a0 100644 --- a/mullvad-cli/src/cmds/relay_constraints.rs +++ b/mullvad-cli/src/cmds/relay_constraints.rs @@ -20,18 +20,15 @@ impl From for Constraint { return Constraint::Any; } - match (value.country, value.city, value.hostname) { - (country, None, None) => { - Constraint::Only(GeographicLocationConstraint::Country(country)) - } - (country, Some(city), None) => { - Constraint::Only(GeographicLocationConstraint::City(country, city)) + Constraint::Only(match (value.country, value.city, value.hostname) { + (country, None, None) => GeographicLocationConstraint::Country(country), + (country, Some(city), None) => GeographicLocationConstraint::City(country, city), + (country, Some(city), Some(hostname)) => { + GeographicLocationConstraint::Hostname(country, city, hostname) } - (country, Some(city), Some(hostname)) => Constraint::Only( - GeographicLocationConstraint::Hostname(country, city, hostname), - ), + _ => unreachable!("invalid location arguments"), - } + }) } } diff --git a/mullvad-cli/src/main.rs b/mullvad-cli/src/main.rs index d1c518119cf6..83bd28389223 100644 --- a/mullvad-cli/src/main.rs +++ b/mullvad-cli/src/main.rs @@ -31,6 +31,13 @@ enum Cli { #[clap(subcommand)] LockdownMode(lockdown::LockdownMode), + /// Debug commands used for internal testing of the app. + /// + /// These commands will likely set the app in an invalid state, which is + /// used to test security under various edge cases. + #[clap(subcommand, hide = true)] + Debug(debug::DebugCommands), + /// Configure DNS servers to use when connected #[clap(subcommand)] Dns(dns::Dns), @@ -70,7 +77,6 @@ enum Cli { /// Manage relay and tunnel constraints #[clap(subcommand)] Relay(relay::Relay), - /// Manage Mullvad API access methods. /// /// Access methods are used to connect to the the Mullvad API via one of @@ -143,13 +149,12 @@ enum Cli { #[tokio::main] async fn main() -> Result<()> { - env_logger::init(); - match Cli::parse() { Cli::Account(cmd) => cmd.handle().await, Cli::Bridge(cmd) => cmd.handle().await, Cli::Connect { wait } => tunnel_state::connect(wait).await, Cli::Reconnect { wait } => tunnel_state::reconnect(wait).await, + Cli::Debug(cmd) => cmd.handle().await, Cli::Disconnect { wait } => tunnel_state::disconnect(wait).await, Cli::AutoConnect(cmd) => cmd.handle().await, Cli::BetaProgram(cmd) => cmd.handle().await, diff --git a/mullvad-types/src/relay_list.rs b/mullvad-types/src/relay_list.rs index 94f4a9d59d8d..0edf4f575203 100644 --- a/mullvad-types/src/relay_list.rs +++ b/mullvad-types/src/relay_list.rs @@ -36,13 +36,21 @@ impl RelayList { .find(|country| country.code == country_code) } - /// Return a flat iterator with all relays + /// Return a flat iterator of all [`Relay`]s pub fn relays(&self) -> impl Iterator + Clone + '_ { self.countries .iter() .flat_map(|country| country.cities.iter()) .flat_map(|city| city.relays.iter()) } + + /// Return a consuming flat iterator of all [`Relay`]s + pub fn into_relays(self) -> impl Iterator + Clone { + self.countries + .into_iter() + .flat_map(|country| country.cities) + .flat_map(|city| city.relays) + } } /// A list of [`RelayListCity`]s within a country. Used by [`RelayList`].