From af930ed093f9e37097967a923f017e4ca5c12bc5 Mon Sep 17 00:00:00 2001 From: Markus Pettersson Date: Fri, 22 Dec 2023 13:33:58 +0100 Subject: [PATCH] Re-implement test procedure for access methods Since the `AccessModeSelector` knows how to resolve endpoints on it's own, we no longer have to re-use the existing `MullvadRestHandle` from the daemon. Instead, we may spawn a completely new `ApiProxy` and except the appropriate endpoint in the firewall without affecting the running `MullvadRestHandle`. --- mullvad-daemon/src/access_method.rs | 102 +-------------------------- mullvad-daemon/src/lib.rs | 104 ++++++++++++++++++++++------ 2 files changed, 84 insertions(+), 122 deletions(-) diff --git a/mullvad-daemon/src/access_method.rs b/mullvad-daemon/src/access_method.rs index d5c1bd80e497..4bdbd17f153e 100644 --- a/mullvad-daemon/src/access_method.rs +++ b/mullvad-daemon/src/access_method.rs @@ -1,9 +1,9 @@ use crate::{ - api::{self, AccessModeSelectorHandle}, + api, settings::{self, MadeChanges}, Daemon, EventListener, }; -use mullvad_api::rest::{self, MullvadRestHandle}; +use mullvad_api::rest; use mullvad_types::{ access_method::{self, AccessMethod, AccessMethodSetting}, settings::Settings, @@ -224,102 +224,4 @@ where }; self } - - /// The semantics of the [`Command`] datastructure. - async fn process_command(&mut self, command: Command) -> Result<(), Error> { - match command { - Command::Nothing => Ok(()), - Command::Rotate => self.force_api_endpoint_rotation().await, - Command::Set(id) => self.set_api_access_method(id).await, - } - } -} - -/// Try to reach the Mullvad API using a specific access method, returning -/// an [`Error`] in the case where the test fails to reach the API. -/// -/// Ephemerally sets a new access method (associated with `access_method`) -/// to be used for subsequent API calls, before performing an API call and -/// switching back to the previously active access method. The previous -/// access method is *always* reset. -pub async fn test_access_method( - new_access_method: AccessMethodSetting, - access_mode_selector: AccessModeSelectorHandle, - rest_handle: MullvadRestHandle, -) -> Result { - // Setup test - let previous_access_method = access_mode_selector - .get_access_method() - .await - .map_err(Error::ConnectionMode)?; - - let method_under_test = new_access_method.clone(); - access_mode_selector - .set_access_method(new_access_method) - .await - .map_err(Error::ConnectionMode)?; - - // We need to perform a rotation of API endpoint after a set action - let rotation_handle = rest_handle.clone(); - rotation_handle - .service() - .next_api_endpoint() - .await - .map_err(|err| { - log::error!("Failed to rotate API endpoint: {err}"); - Error::Rest(err) - })?; - - // Set up the reset - // - // In case the API call fails, the next API endpoint will - // automatically be selected, which means that we need to set up - // with the previous API endpoint beforehand. - access_mode_selector - .set_access_method(previous_access_method) - .await - .map_err(|err| { - log::error!( - "Could not reset to previous access - method after API reachability test was carried out. This should only - happen if the previous access method was removed in the meantime." - ); - Error::ConnectionMode(err) - })?; - - // Perform test - // - // Send a HEAD request to some Mullvad API endpoint. We issue a HEAD - // request because we are *only* concerned with if we get a reply from - // the API, and not with the actual data that the endpoint returns. - let result = mullvad_api::ApiProxy::new(rest_handle) - .api_addrs_available() - .await - .map_err(Error::Rest)?; - - // We need to perform a rotation of API endpoint after a set action - // Note that this will be done automatically if the API call fails, - // so it only has to be done if the call succeeded .. - if result { - rotation_handle - .service() - .next_api_endpoint() - .await - .map_err(|err| { - log::error!("Failed to rotate API endpoint: {err}"); - Error::Rest(err) - })?; - } - - log::info!( - "The result of testing {method:?} is {result}", - method = method_under_test.access_method, - result = if result { - "success".to_string() - } else { - "failed".to_string() - } - ); - - Ok(result) } diff --git a/mullvad-daemon/src/lib.rs b/mullvad-daemon/src/lib.rs index ddb3f2fed3d3..bbf710ef75a1 100644 --- a/mullvad-daemon/src/lib.rs +++ b/mullvad-daemon/src/lib.rs @@ -1227,7 +1227,7 @@ where UpdateApiAccessMethod(tx, method) => self.on_update_api_access_method(tx, method).await, GetCurrentAccessMethod(tx) => self.on_get_current_api_access_method(tx), SetApiAccessMethod(tx, method) => self.on_set_api_access_method(tx, method).await, - TestApiAccessMethod(tx, method) => self.on_test_api_access_method(tx, method), + TestApiAccessMethod(tx, method) => self.on_test_api_access_method(tx, method).await, IsPerformingPostUpgrade(tx) => self.on_is_performing_post_upgrade(tx), GetCurrentVersion(tx) => self.on_get_current_version(tx), #[cfg(not(target_os = "android"))] @@ -2436,35 +2436,95 @@ where }); } - fn on_test_api_access_method( + async fn on_test_api_access_method( &mut self, tx: ResponseTx, access_method: mullvad_types::access_method::Id, ) { - // NOTE: Preferably we would block all new API calls until the test is - // done and the previous access method is reset. Otherwise we run the - // risk of errounously triggering a rotation of the currently in-use - // access method. - let api_handle = self.api_handle.clone(); - let handle = self.connection_modes_handler.clone(); - let access_method_lookup = self - .get_api_access_method(access_method) - .map_err(Error::AccessMethodError); + let reply = + |response| Self::oneshot_send(tx, response, "on_test_api_access_method response"); - match access_method_lookup { - Ok(access_method) => { - tokio::spawn(async move { - let result = - access_method::test_access_method(access_method, handle, api_handle) - .await - .map_err(Error::AccessMethodError); - Self::oneshot_send(tx, result, "on_test_api_access_method response"); - }); + let access_method = match self.get_api_access_method(access_method) { + Ok(x) => x, + Err(err) => { + reply(Err(Error::AccessMethodError(err))); + return; } + }; + + let test_subject = match self.connection_modes_handler.resolve(access_method).await { + Ok(test_subject) => test_subject, Err(err) => { - Self::oneshot_send(tx, Err(err), "on_test_api_access_method response"); + reply(Err(Error::ApiConnectionModeError(err))); + return; } - } + }; + + let proxy_provider = test_subject.connection_mode.clone().into_repeat(); + let rest_handle = self.api_runtime.mullvad_rest_handle(proxy_provider).await; + let api_proxy = mullvad_api::ApiProxy::new(rest_handle); + let daemon_event_sender = self.tx.to_specialized_sender(); + let access_method_selector = self.connection_modes_handler.clone(); + + tokio::spawn(async move { + let result = Self::test_api_access_method_inner( + test_subject.clone(), + access_method_selector, + api_proxy, + daemon_event_sender, + ) + .await; + + log::debug!( + "API access method {method} {result}", + method = test_subject.setting.name, + result = if result.as_ref().is_ok_and(|is_true| *is_true) { + "could successfully connect to the Mullvad API".to_string() + } else { + "could not connect to the Mullvad API".to_string() + } + ); + + reply(result); + }); + } + + async fn test_api_access_method_inner( + test_subject: api::ResolvedConnectionMode, + access_method_selector: api::AccessModeSelectorHandle, + api_proxy: mullvad_api::ApiProxy, + daemon_event_sender: DaemonEventSender, + ) -> Result { + // Send an internal daemon event which will punch a hole in the firewall + // for the connection mode we are testing. + let _ = api::NewAccessMethodEvent::new(test_subject.setting, test_subject.endpoint) + .announce(false) + .send(&daemon_event_sender) + .await; + + // Send a HEAD request to some Mullvad API endpoint. We issue a HEAD + // request because we are *only* concerned with if we get a reply from + // the API, and not with the actual data that the endpoint returns. + let result = api_proxy + .api_addrs_available() + .await + .map_err(Error::RestError); + + // Tell the daemon to reset the hole we just punched to whatever was in + // place before. + let api::ResolvedConnectionMode { + endpoint, setting, .. + } = access_method_selector + .get_current() + .await + .map_err(Error::ApiConnectionModeError)?; + + let _ = api::NewAccessMethodEvent::new(setting, endpoint) + .announce(false) + .send(&daemon_event_sender) + .await; + + result } fn on_get_settings(&self, tx: oneshot::Sender) {