diff --git a/mullvad-cli/src/cmds/status.rs b/mullvad-cli/src/cmds/status.rs index a4a8add15e5d..aad6182df6ee 100644 --- a/mullvad-cli/src/cmds/status.rs +++ b/mullvad-cli/src/cmds/status.rs @@ -2,7 +2,7 @@ use anyhow::Result; use clap::{Args, Subcommand}; use futures::StreamExt; use mullvad_management_interface::{client::DaemonEvent, MullvadProxyClient}; -use mullvad_types::{device::DeviceState, states::TunnelState}; +use mullvad_types::{device::DeviceState, location::GeoIpLocation, states::TunnelState}; use crate::format; @@ -38,13 +38,21 @@ impl Status { format::print_state(&new_state, args.verbose); } - match new_state { - TunnelState::Connected { .. } | TunnelState::Disconnected => { - if args.location { - print_location(&mut rpc).await?; - } + let location = match new_state { + TunnelState::Connected { + location, + endpoint: _, + } => location, + TunnelState::Disconnected(location) => location, + _ => return Ok(()), /* No location data is available during + * connecting/disconnecting/error states */ + }; + if args.location { + if let Some(location) = location { + print_location(&location); + } else { + eprintln!("No location available") } - _ => {} } } DaemonEvent::Settings(settings) => { @@ -88,30 +96,17 @@ pub async fn handle(cmd: Option, args: StatusArgs) -> Result<()> { if args.debug { println!("Tunnel state: {state:#?}"); } else { + // TODO: respect location arg? format::print_state(&state, args.verbose); } - if args.location { - print_location(&mut rpc).await?; - } - if cmd == Some(Status::Listen) { Status::listen(rpc, args).await?; } Ok(()) } -async fn print_location(rpc: &mut MullvadProxyClient) -> Result<()> { - let location = match rpc.get_current_location().await { - Ok(location) => location, - Err(error) => match &error { - mullvad_management_interface::Error::NoLocationData => { - println!("Location data unavailable"); - return Ok(()); - } - _ => return Err(error.into()), - }, - }; +pub fn print_location(location: &GeoIpLocation) { if let Some(ipv4) = location.ipv4 { println!("IPv4: {ipv4}"); } @@ -123,7 +118,6 @@ async fn print_location(rpc: &mut MullvadProxyClient) -> Result<()> { "Position: {:.5}°N, {:.5}°W", location.latitude, location.longitude ); - Ok(()) } fn print_account_loggedout(state: &TunnelState, device: &DeviceState) { @@ -137,6 +131,6 @@ fn print_account_loggedout(state: &TunnelState, device: &DeviceState) { DeviceState::LoggedIn(_) => (), } } - TunnelState::Disconnected | TunnelState::Disconnecting(_) => (), + TunnelState::Disconnected(_) | TunnelState::Disconnecting(_) => (), } } diff --git a/mullvad-cli/src/format.rs b/mullvad-cli/src/format.rs index 2938842cefcf..68d9679d0522 100644 --- a/mullvad-cli/src/format.rs +++ b/mullvad-cli/src/format.rs @@ -4,6 +4,8 @@ use talpid_types::{ tunnel::ErrorState, }; +use crate::cmds::status::print_location; + #[macro_export] macro_rules! print_option { ($value:expr $(,)?) => {{ @@ -37,7 +39,12 @@ pub fn print_state(state: &TunnelState, verbose: bool) { format_relay_connection(endpoint, location.as_ref(), verbose) ); } - Disconnected => println!("Disconnected"), + Disconnected(disconnected_location) => { + println!("Disconnected"); + if let Some(disconnected_location) = disconnected_location { + print_location(disconnected_location); + } + } Disconnecting(_) => println!("Disconnecting..."), } } diff --git a/mullvad-daemon/src/geoip.rs b/mullvad-daemon/src/geoip.rs index 0af495e8bd83..b4ed662fa1e1 100644 --- a/mullvad-daemon/src/geoip.rs +++ b/mullvad-daemon/src/geoip.rs @@ -44,13 +44,13 @@ const LOCATION_RETRY_STRATEGY: Jittered = Jittered::jitter(ExponentialBackoff::new(Duration::from_secs(1), 4)); /// Fetch the current `GeoIpLocation` with retrys -pub async fn get_geo_location( +pub async fn get_geo_location_with_retry( rest_service: RequestServiceHandle, use_ipv6: bool, api_handle: ApiAvailabilityHandle, -) -> Option { +) -> Result { log::debug!("Fetching GeoIpLocation"); - match retry_future( + retry_future( move || send_location_request(rest_service.clone(), use_ipv6), move |result| match result { Err(error) if error.is_network_error() => !api_handle.get_state().is_offline(), @@ -59,13 +59,6 @@ pub async fn get_geo_location( LOCATION_RETRY_STRATEGY, ) .await - { - Ok(loc) => Some(loc), - Err(e) => { - log::warn!("Unable to fetch GeoIP location: {}", e.display_chain()); - None - } - } } async fn send_location_request( diff --git a/mullvad-daemon/src/lib.rs b/mullvad-daemon/src/lib.rs index 65f2c56654e2..b53975a2a777 100644 --- a/mullvad-daemon/src/lib.rs +++ b/mullvad-daemon/src/lib.rs @@ -27,7 +27,7 @@ mod tunnel; pub mod version; mod version_check; -use crate::{geoip::get_geo_location, target_state::PersistentTargetState}; +use crate::target_state::PersistentTargetState; use device::{AccountEvent, PrivateAccountAndDevice, PrivateDeviceEvent}; use futures::{ channel::{mpsc, oneshot}, @@ -200,8 +200,6 @@ pub enum DaemonCommand { Reconnect(oneshot::Sender), /// Request the current state. GetState(oneshot::Sender), - /// Get the current geographical location. - GetCurrentLocation(oneshot::Sender>), CreateNewAccount(ResponseTx), /// Request the metadata for an account. GetAccountData( @@ -417,7 +415,7 @@ impl DaemonExecutionState { match self { Running => { match tunnel_state { - TunnelState::Disconnected => mem::replace(self, Finished), + TunnelState::Disconnected(_) => mem::replace(self, Finished), _ => mem::replace(self, Exiting), }; } @@ -614,6 +612,7 @@ pub struct Daemon { tunnel_state_machine_handle: TunnelStateMachineHandle, #[cfg(target_os = "windows")] volume_update_tx: mpsc::UnboundedSender<()>, + location_abort_handle: Option, } impl Daemon @@ -847,7 +846,7 @@ where relay_list_updater.update().await; let daemon = Daemon { - tunnel_state: TunnelState::Disconnected, + tunnel_state: TunnelState::Disconnected(None), target_state, state: DaemonExecutionState::Running, #[cfg(target_os = "linux")] @@ -873,6 +872,7 @@ where tunnel_state_machine_handle, #[cfg(target_os = "windows")] volume_update_tx, + location_abort_handle: None, }; api_availability.unsuspend(); @@ -971,20 +971,33 @@ where &mut self, tunnel_state_transition: TunnelStateTransition, ) { + // Abort any ongoing calls to `self.notify_tunnel_state_when_ip_arrives` from + // previous tunnel state transitions to avoid sending an outdated information. + if let Some(handle) = self.location_abort_handle.take() { + handle.abort(); + } self.reset_rpc_sockets_on_tunnel_state_transition(&tunnel_state_transition); self.device_checker .handle_state_transition(&tunnel_state_transition); let tunnel_state = match tunnel_state_transition { - TunnelStateTransition::Disconnected => TunnelState::Disconnected, + TunnelStateTransition::Disconnected => { + self.notify_tunnel_state_when_ip_arrives(TunnelState::Disconnected) + .await; + TunnelState::Disconnected(self.parameters_generator.get_last_location().await) + } + TunnelStateTransition::Connecting(endpoint) => TunnelState::Connecting { endpoint, location: self.parameters_generator.get_last_location().await, }, - TunnelStateTransition::Connected(endpoint) => TunnelState::Connected { - endpoint, - location: self.parameters_generator.get_last_location().await, - }, + TunnelStateTransition::Connected(endpoint) => { + let make_tunnel_state = |location| TunnelState::Connected { endpoint, location }; + self.notify_tunnel_state_when_ip_arrives(make_tunnel_state.clone()) + .await; + + make_tunnel_state(self.parameters_generator.get_last_location().await) + } TunnelStateTransition::Disconnecting(after_disconnect) => { TunnelState::Disconnecting(after_disconnect) } @@ -1001,7 +1014,7 @@ where log::debug!("New tunnel state: {:?}", tunnel_state); match tunnel_state { - TunnelState::Disconnected => { + TunnelState::Disconnected(_) => { self.api_handle.availability.reset_inactivity_timer(); } _ => { @@ -1010,7 +1023,7 @@ where } match &tunnel_state { - TunnelState::Disconnected => self.state.disconnected(), + TunnelState::Disconnected(_) => self.state.disconnected(), TunnelState::Connecting { .. } => { log::debug!("Settings: {}", self.settings.summary()); } @@ -1040,6 +1053,29 @@ where self.event_listener.notify_new_state(tunnel_state); } + /// Get the out IP from am.i.mullvad.net. When it arrives, send another + /// TunnelState with the IP filled in. + async fn notify_tunnel_state_when_ip_arrives( + &mut self, + make_tunnel_state: impl FnOnce(Option) -> TunnelState + Send + 'static, + ) { + let rest_service = self.api_runtime.rest_handle().await; + let use_ipv6 = self.settings.tunnel_options.generic.enable_ipv6; + let api_handle = self.api_handle.availability.clone(); + let even_listener = self.event_listener.clone(); + + self.location_abort_handle = Some( + tokio::spawn(async move { + let location = + geoip::get_geo_location_with_retry(rest_service, use_ipv6, api_handle) + .await + .ok(); + even_listener.notify_new_state(make_tunnel_state(location)); + }) + .abort_handle(), + ); + } + fn reset_rpc_sockets_on_tunnel_state_transition( &mut self, tunnel_state_transition: &TunnelStateTransition, @@ -1091,7 +1127,6 @@ where SetTargetState(tx, state) => self.on_set_target_state(tx, state).await, Reconnect(tx) => self.on_reconnect(tx), GetState(tx) => self.on_get_state(tx), - GetCurrentLocation(tx) => self.on_get_current_location(tx).await, CreateNewAccount(tx) => self.on_create_new_account(tx), GetAccountData(tx, account_token) => self.on_get_account_data(tx, account_token), GetWwwAuthToken(tx) => self.on_get_www_auth_token(tx).await, @@ -1342,52 +1377,6 @@ where Self::oneshot_send(tx, performing_post_upgrade, "performing post upgrade"); } - async fn on_get_current_location(&mut self, tx: oneshot::Sender>) { - use self::TunnelState::*; - - match &self.tunnel_state { - Disconnected => self.update_and_send_geo_location(tx, None).await, - Connecting { location, .. } => { - Self::oneshot_send(tx, location.clone(), "current location") - } - Disconnecting(..) => Self::oneshot_send( - tx, - self.parameters_generator.get_last_location().await, - "current location", - ), - Connected { location, .. } => { - self.update_and_send_geo_location(tx, location.clone()) - .await - } - // We are not online at all at this stage so no location data is available. - Error(_) => Self::oneshot_send(tx, None, "current location"), - }; - } - - /// Fetch the current `GeoIpLocation` and send it on the return channel, - /// in a non-blocking fashion. Optionally give a chached previous location. - async fn update_and_send_geo_location( - &mut self, - tx: oneshot::Sender>, - current_relay_location: Option, - ) { - let rest_service = self.api_runtime.rest_handle().await; - let use_ipv6 = self.settings.tunnel_options.generic.enable_ipv6; - let api_handle = self.api_handle.availability.clone(); - tokio::spawn(async move { - let new_location = get_geo_location(rest_service, use_ipv6, api_handle).await; - Self::oneshot_send( - tx, - new_location.map(|fetched_location| GeoIpLocation { - ipv4: fetched_location.ipv4, - ipv6: fetched_location.ipv6, - ..current_relay_location.unwrap_or(fetched_location) - }), - "current location", - ); - }); - } - fn on_create_new_account(&mut self, tx: ResponseTx) { let account_manager = self.account_manager.clone(); tokio::spawn(async move { diff --git a/mullvad-daemon/src/management_interface.rs b/mullvad-daemon/src/management_interface.rs index f042a923e51e..75a9b0bf6fec 100644 --- a/mullvad-daemon/src/management_interface.rs +++ b/mullvad-daemon/src/management_interface.rs @@ -191,17 +191,6 @@ impl ManagementService for ManagementServiceImpl { .map(|relays| Response::new(types::RelayList::from(relays))) } - async fn get_current_location(&self, _: Request<()>) -> ServiceResult { - log::debug!("get_current_location"); - let (tx, rx) = oneshot::channel(); - self.send_command_to_daemon(DaemonCommand::GetCurrentLocation(tx))?; - let result = self.wait_for_result(rx).await?; - match result { - Some(geoip) => Ok(Response::new(types::GeoIpLocation::from(geoip))), - None => Err(Status::not_found("no location was found")), - } - } - async fn set_bridge_settings( &self, request: Request, diff --git a/mullvad-management-interface/proto/management_interface.proto b/mullvad-management-interface/proto/management_interface.proto index a27698f3176b..6f44c5f36ba8 100644 --- a/mullvad-management-interface/proto/management_interface.proto +++ b/mullvad-management-interface/proto/management_interface.proto @@ -29,7 +29,6 @@ service ManagementService { // Relays and tunnel constraints rpc UpdateRelayLocations(google.protobuf.Empty) returns (google.protobuf.Empty) {} rpc GetRelayLocations(google.protobuf.Empty) returns (RelayList) {} - rpc GetCurrentLocation(google.protobuf.Empty) returns (GeoIpLocation) {} rpc SetRelaySettings(RelaySettings) returns (google.protobuf.Empty) {} rpc SetBridgeSettings(BridgeSettings) returns (google.protobuf.Empty) {} rpc SetBridgeState(BridgeState) returns (google.protobuf.Empty) {} @@ -174,7 +173,7 @@ message ErrorState { } message TunnelState { - message Disconnected {} + message Disconnected { GeoIpLocation disconnected_location = 1; } message Connecting { TunnelStateRelayInfo relay_info = 1; } message Connected { TunnelStateRelayInfo relay_info = 1; } message Disconnecting { AfterDisconnect after_disconnect = 1; } diff --git a/mullvad-management-interface/src/client.rs b/mullvad-management-interface/src/client.rs index 64ee088d1877..e4071c8b084a 100644 --- a/mullvad-management-interface/src/client.rs +++ b/mullvad-management-interface/src/client.rs @@ -7,7 +7,6 @@ use mullvad_types::{ account::{AccountData, AccountToken, VoucherSubmission}, custom_list::{CustomList, Id}, device::{Device, DeviceEvent, DeviceId, DeviceState, RemoveDeviceEvent}, - location::GeoIpLocation, relay_constraints::{ BridgeSettings, BridgeState, ObfuscationSettings, RelayOverride, RelaySettings, }, @@ -236,16 +235,6 @@ impl MullvadProxyClient { Ok(()) } - pub async fn get_current_location(&mut self) -> Result { - let location = self - .0 - .get_current_location(()) - .await - .map_err(map_location_error)? - .into_inner(); - GeoIpLocation::try_from(location).map_err(Error::InvalidResponse) - } - pub async fn set_bridge_settings(&mut self, settings: BridgeSettings) -> Result<()> { let settings = types::BridgeSettings::from(settings); self.0 @@ -573,10 +562,9 @@ impl MullvadProxyClient { /// Set the [`AccessMethod`] which [`ApiConnectionModeProvider`] should /// pick. /// - /// - `access_method`: If `Some(access_method)`, [`ApiConnectionModeProvider`] will skip - /// ahead and return `access_method` when asked for a new access method. - /// If `None`, [`ApiConnectionModeProvider`] will pick the next access - /// method "randomly" + /// - `access_method`: If `Some(access_method)`, [`ApiConnectionModeProvider`] will skip ahead + /// and return `access_method` when asked for a new access method. If `None`, + /// [`ApiConnectionModeProvider`] will pick the next access method "randomly" /// /// [`ApiConnectionModeProvider`]: mullvad_daemon::api::ApiConnectionModeProvider pub async fn set_access_method(&mut self, api_access_method: access_method::Id) -> Result<()> { @@ -698,13 +686,6 @@ fn map_device_error(status: Status) -> Error { } } -fn map_location_error(status: Status) -> Error { - match status.code() { - Code::NotFound => Error::NoLocationData, - _other => Error::Rpc(status), - } -} - fn map_custom_list_error(status: Status) -> Error { match status.code() { Code::NotFound => { diff --git a/mullvad-management-interface/src/types/conversions/states.rs b/mullvad-management-interface/src/types/conversions/states.rs index b742aa7b004c..2541859e2745 100644 --- a/mullvad-management-interface/src/types/conversions/states.rs +++ b/mullvad-management-interface/src/types/conversions/states.rs @@ -32,8 +32,10 @@ impl From for proto::TunnelState { }; let state = match state { - MullvadTunnelState::Disconnected => { - proto::tunnel_state::State::Disconnected(proto::tunnel_state::Disconnected {}) + MullvadTunnelState::Disconnected(disconnected_location) => { + proto::tunnel_state::State::Disconnected(proto::tunnel_state::Disconnected { + disconnected_location: disconnected_location.map(proto::GeoIpLocation::from), + }) } MullvadTunnelState::Connecting { endpoint, location } => { proto::tunnel_state::State::Connecting(proto::tunnel_state::Connecting { @@ -189,7 +191,13 @@ impl TryFrom for mullvad_types::states::TunnelState { use talpid_types::{net as talpid_net, tunnel as talpid_tunnel}; let state = match state.state { - Some(proto::tunnel_state::State::Disconnected(_)) => MullvadState::Disconnected, + Some(proto::tunnel_state::State::Disconnected(proto::tunnel_state::Disconnected { + disconnected_location, + })) => MullvadState::Disconnected( + disconnected_location + .map(mullvad_types::location::GeoIpLocation::try_from) + .transpose()?, + ), Some(proto::tunnel_state::State::Connecting(proto::tunnel_state::Connecting { relay_info: Some(proto::TunnelStateRelayInfo { diff --git a/mullvad-types/src/states.rs b/mullvad-types/src/states.rs index f6131711cbcd..4a266d1d6e90 100644 --- a/mullvad-types/src/states.rs +++ b/mullvad-types/src/states.rs @@ -34,7 +34,7 @@ impl fmt::Display for TargetState { #[cfg_attr(target_os = "android", derive(IntoJava))] #[cfg_attr(target_os = "android", jnix(package = "net.mullvad.mullvadvpn.model"))] pub enum TunnelState { - Disconnected, + Disconnected(Option), Connecting { endpoint: TunnelEndpoint, location: Option, @@ -60,6 +60,6 @@ impl TunnelState { /// Returns true if the tunnel state is in the disconnected state. pub fn is_disconnected(&self) -> bool { - matches!(self, TunnelState::Disconnected) + matches!(self, TunnelState::Disconnected(_)) } }