Do not fallback to direct API connections when testing access methods

mullvad-api/src/proxy.rs

@@ -98,6 +98,12 @@ impl From<proxy::CustomProxy> for ProxyConfig {
     }
 }
 
+impl From<mullvad_encrypted_dns_proxy::config::ProxyConfig> for ProxyConfig {
+    fn from(value: mullvad_encrypted_dns_proxy::config::ProxyConfig) -> Self {
+        ProxyConfig::EncryptedDnsProxy(value)
+    }
+}
+
 impl ApiConnectionMode {
     /// Reads the proxy config from `CURRENT_CONFIG_FILENAME`.
     /// This returns `ApiConnectionMode::Direct` if reading from disk fails for any reason.

mullvad-daemon/src/api.rs

@@ -23,15 +23,18 @@ use mullvad_types::access_method::{
 use std::{net::SocketAddr, path::PathBuf};
 use talpid_core::mpsc::Sender;
 use talpid_types::net::{
-    AllowedClients, AllowedEndpoint, Connectivity, Endpoint, TransportProtocol,
+    proxy::CustomProxy, AllowedClients, AllowedEndpoint, Connectivity, Endpoint, TransportProtocol,
 };
 
 pub enum Message {
     Get(ResponseTx<ResolvedConnectionMode>),
     Use(ResponseTx<()>, Id),
     Rotate(ResponseTx<ApiConnectionMode>),
     Update(ResponseTx<()>, Settings),
-    Resolve(ResponseTx<ResolvedConnectionMode>, AccessMethodSetting),
+    Resolve(
+        ResponseTx<Option<ResolvedConnectionMode>>,
+        AccessMethodSetting,
+    ),
 }
 
 /// Calling [`AccessMethodEvent::send`] will cause a
@@ -102,6 +105,8 @@ pub struct ResolvedConnectionMode {
 pub enum Error {
     #[error("No access methods were provided.")]
     NoAccessMethods,
+    #[error("Could not resolve access method {access_method:#?}")]
+    Resolve { access_method: AccessMethod },
     #[error("AccessModeSelector is not receiving any messages.")]
     SendFailed(#[from] mpsc::TrySendError<Message>),
     #[error("AccessModeSelector is not receiving any messages.")]
@@ -175,7 +180,14 @@ impl AccessModeSelectorHandle {
             })
     }
 
-    pub async fn resolve(&self, setting: AccessMethodSetting) -> Result<ResolvedConnectionMode> {
+    /// Try to resolve an access method into a set of connection details.
+    ///
+    /// This might fail if the underlying store/cache where `setting` is the key is empty.
+    /// In that case, `Ok(None)` is returned.
+    pub async fn resolve(
+        &self,
+        setting: AccessMethodSetting,
+    ) -> Result<Option<ResolvedConnectionMode>> {
         self.send_command(|tx| Message::Resolve(tx, setting))
             .await
             .inspect_err(|_| {
@@ -275,8 +287,8 @@ impl AccessModeSelector {
 
         // Always start looking from the position of `Direct`.
         let (index, next) = Self::find_next_active(0, &access_method_settings);
-        let initial_connection_mode = Self::resolve_inner(
-            next,
+        let initial_connection_mode = Self::resolve_inner_with_default(
+            &next,
             &relay_selector,
             &mut encrypted_dns_proxy_cache,
             &address_cache,
@@ -397,7 +409,7 @@ impl AccessModeSelector {
     }
 
     async fn set_current(&mut self, access_method: AccessMethodSetting) {
-        let resolved = self.resolve(access_method).await;
+        let resolved = self.resolve_with_default(access_method).await;
 
         // Note: If the daemon is busy waiting for a call to this function
         // to complete while we wait for the daemon to fully handle this
@@ -497,16 +509,19 @@ impl AccessModeSelector {
 
     pub async fn on_resolve_access_method(
         &mut self,
-        tx: ResponseTx<ResolvedConnectionMode>,
+        tx: ResponseTx<Option<ResolvedConnectionMode>>,
         setting: AccessMethodSetting,
     ) -> Result<()> {
         let reply = self.resolve(setting).await;
         self.reply(tx, reply)
     }
 
-    async fn resolve(&mut self, access_method: AccessMethodSetting) -> ResolvedConnectionMode {
+    async fn resolve(
+        &mut self,
+        access_method: AccessMethodSetting,
+    ) -> Option<ResolvedConnectionMode> {
         Self::resolve_inner(
-            access_method,
+            &access_method,
             &self.relay_selector,
             &mut self.encrypted_dns_proxy_cache,
             &self.address_cache,
@@ -515,50 +530,100 @@ impl AccessModeSelector {
     }
 
     async fn resolve_inner(
+        access_method: &AccessMethodSetting,
+        relay_selector: &RelaySelector,
+        encrypted_dns_proxy_cache: &mut EncryptedDnsProxyState,
+        address_cache: &AddressCache,
+    ) -> Option<ResolvedConnectionMode> {
+        let connection_mode =
+            Self::resolve_connection_mode(access_method, relay_selector, encrypted_dns_proxy_cache)
+                .await?;
+        let endpoint =
+            resolve_allowed_endpoint(&connection_mode, address_cache.get_address().await);
+        Some(ResolvedConnectionMode {
+            connection_mode,
+            endpoint,
+            setting: access_method.clone(),
+        })
+    }
+
+    /// Resolve an access method into a set of connection details - fall back to
+    /// [`ApiConnectionMode::Direct`] in case `access_method` does not yield anything.
+    async fn resolve_with_default(
+        &mut self,
         access_method: AccessMethodSetting,
+    ) -> ResolvedConnectionMode {
+        Self::resolve_inner_with_default(
+            &access_method,
+            &self.relay_selector,
+            &mut self.encrypted_dns_proxy_cache,
+            &self.address_cache,
+        )
+        .await
+    }
+
+    async fn resolve_inner_with_default(
+        access_method: &AccessMethodSetting,
         relay_selector: &RelaySelector,
         encrypted_dns_proxy_cache: &mut EncryptedDnsProxyState,
         address_cache: &AddressCache,
     ) -> ResolvedConnectionMode {
+        match Self::resolve_inner(
+            access_method,
+            relay_selector,
+            encrypted_dns_proxy_cache,
+            address_cache,
+        )
+        .await
+        {
+            Some(resolved) => resolved,
+            None => {
+                log::trace!("Defaulting to direct API connection");
+                ResolvedConnectionMode {
+                    connection_mode: ApiConnectionMode::Direct,
+                    endpoint: resolve_allowed_endpoint(
+                        &ApiConnectionMode::Direct,
+                        address_cache.get_address().await,
+                    ),
+                    setting: access_method.clone(),
+                }
+            }
+        }
+    }
+
+    async fn resolve_connection_mode(
+        access_method: &AccessMethodSetting,
+        relay_selector: &RelaySelector,
+        encrypted_dns_proxy_cache: &mut EncryptedDnsProxyState,
+    ) -> Option<ApiConnectionMode> {
         let connection_mode = {
-            let access_method = access_method.access_method.clone();
-            match access_method {
+            match &access_method.access_method {
                 AccessMethod::BuiltIn(BuiltInAccessMethod::Direct) => ApiConnectionMode::Direct,
-                AccessMethod::BuiltIn(BuiltInAccessMethod::Bridge) => relay_selector
-                    .get_bridge_forced()
-                    .map(ProxyConfig::from)
-                    .map(ApiConnectionMode::Proxied)
-                    .unwrap_or_else(|| {
-                        log::warn!(
-                            "Received unexpected proxy settings type. Defaulting to direct API connection"
-                        );
-                        log::debug!("Defaulting to direct API connection");
-                        ApiConnectionMode::Direct
-                    }),
+                AccessMethod::BuiltIn(BuiltInAccessMethod::Bridge) => {
+                    let Some(bridge) = relay_selector.get_bridge_forced() else {
+                        log::warn!("Selected bridge unexpected proxy settings type");
+                        return None;
+                    };
+                    let proxy = CustomProxy::Shadowsocks(bridge);
+                    ApiConnectionMode::Proxied(ProxyConfig::from(proxy))
+                }
                 AccessMethod::BuiltIn(BuiltInAccessMethod::EncryptedDnsProxy) => {
                     if let Err(error) = encrypted_dns_proxy_cache.fetch_configs().await {
                         log::warn!("Failed to fetch new Encrypted DNS Proxy configurations");
                         log::debug!("{error:#?}");
                     }
-                    encrypted_dns_proxy_cache
-                    .next_configuration()
-                    .map(ProxyConfig::EncryptedDnsProxy)
-                    .map(ApiConnectionMode::Proxied)
-                    .unwrap_or_else(|| {
+                    let Some(edp) = encrypted_dns_proxy_cache.next_configuration() else {
                         log::warn!("Could not select next Encrypted DNS proxy config");
-                        log::debug!("Defaulting to direct API connection");
-                        ApiConnectionMode::Direct
-                    })},
-                AccessMethod::Custom(config) => ApiConnectionMode::Proxied(ProxyConfig::from(config)),
+                        return None;
+                    };
+                    ApiConnectionMode::Proxied(ProxyConfig::from(edp))
+                }
+                AccessMethod::Custom(config) => {
+                    ApiConnectionMode::Proxied(ProxyConfig::from(config.clone()))
+                }
             }
         };
-        let endpoint =
-            resolve_allowed_endpoint(&connection_mode, address_cache.get_address().await);
-        ResolvedConnectionMode {
-            connection_mode,
-            endpoint,
-            setting: access_method,
-        }
+        Some(connection_mode)
     }
 }
 

mullvad-daemon/src/lib.rs

@@ -2752,8 +2752,19 @@ impl Daemon {
             }
         };
 
-        let test_subject = match self.access_mode_handler.resolve(access_method).await {
-            Ok(test_subject) => test_subject,
+        let test_subject = match self
+            .access_mode_handler
+            .resolve(access_method.clone())
+            .await
+        {
+            Ok(Some(test_subject)) => test_subject,
+            Ok(None) => {
+                let error = Error::ApiConnectionModeError(self::api::Error::Resolve {
+                    access_method: access_method.access_method,
+                });
+                reply(Err(error));
+                return;
+            }
             Err(err) => {
                 reply(Err(Error::ApiConnectionModeError(err)));
                 return;

mullvad-daemon/src/tunnel.rs

@@ -185,8 +185,8 @@ impl InnerParametersGenerator {
                     bridge: bridge_relay.cloned(),
                     server_override,
                 });
-                let bridge_settings = bridge.as_ref().map(|bridge| bridge.settings());
-                Ok(self.create_openvpn_tunnel_parameters(endpoint, data, bridge_settings.cloned()))
+                let bridge_settings = bridge.map(|bridge| bridge.to_proxy());
+                Ok(self.create_openvpn_tunnel_parameters(endpoint, data, bridge_settings))
             }
             GetRelay::Wireguard {
                 endpoint,

mullvad-relay-selector/src/relay_selector/detailer.rs

@@ -21,7 +21,7 @@ use mullvad_types::{
 };
 use talpid_types::net::{
     all_of_the_internet,
-    proxy::CustomProxy,
+    proxy::Shadowsocks,
     wireguard::{PeerConfig, PublicKey},
     Endpoint, IpVersion, TransportProtocol,
 };
@@ -266,7 +266,7 @@ fn compatible_openvpn_port_combo(
 }
 
 /// Picks a random bridge from a relay.
-pub fn bridge_endpoint(data: &BridgeEndpointData, relay: &Relay) -> Option<CustomProxy> {
+pub fn bridge_endpoint(data: &BridgeEndpointData, relay: &Relay) -> Option<Shadowsocks> {
     use rand::seq::SliceRandom;
     if relay.endpoint_data != RelayEndpointData::Bridge {
         return None;

mullvad-relay-selector/src/relay_selector/mod.rs

@@ -46,7 +46,9 @@ use mullvad_types::{
 };
 use talpid_types::{
     net::{
-        obfuscation::ObfuscatorConfig, proxy::CustomProxy, Endpoint, TransportProtocol, TunnelType,
+        obfuscation::ObfuscatorConfig,
+        proxy::{CustomProxy, Shadowsocks},
+        Endpoint, TransportProtocol, TunnelType,
     },
     ErrorExt,
 };
@@ -228,15 +230,19 @@ pub enum GetRelay {
 
 #[derive(Clone, Debug)]
 pub enum SelectedBridge {
-    Normal { settings: CustomProxy, relay: Relay },
+    /// TODO: Document this variant, why there is only a Shadowsocks
+    Normal {
+        settings: Shadowsocks,
+        relay: Relay,
+    },
     Custom(CustomProxy),
 }
 
 impl SelectedBridge {
     /// Get the bridge settings.
-    pub fn settings(&self) -> &CustomProxy {
+    pub fn to_proxy(self) -> CustomProxy {
         match self {
-            SelectedBridge::Normal { settings, .. } => settings,
+            SelectedBridge::Normal { settings, .. } => CustomProxy::Shadowsocks(settings),
             SelectedBridge::Custom(settings) => settings,
         }
     }
@@ -444,7 +450,7 @@ impl RelaySelector {
 
     /// Returns a non-custom bridge based on the relay and bridge constraints, ignoring the bridge
     /// state.
-    pub fn get_bridge_forced(&self) -> Option<CustomProxy> {
+    pub fn get_bridge_forced(&self) -> Option<Shadowsocks> {
         let parsed_relays = &self.parsed_relays.lock().unwrap();
         let config = self.config.lock().unwrap();
         let specialized_config = SpecializedSelectorConfig::from(&*config);
@@ -1049,7 +1055,7 @@ impl RelaySelector {
         constraints: &InternalBridgeConstraints,
         location: Option<T>,
         custom_lists: &CustomListsSettings,
-    ) -> Result<(CustomProxy, Relay), Error> {
+    ) -> Result<(Shadowsocks, Relay), Error> {
         let bridges = filter_matching_bridges(constraints, parsed_relays.relays(), custom_lists);
         let bridge_data = &parsed_relays.parsed_list().bridge;
         let bridge = match location {

mullvad-types/src/relay_list.rs

@@ -4,10 +4,7 @@ use std::{
     net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr},
     ops::RangeInclusive,
 };
-use talpid_types::net::{
-    proxy::{CustomProxy, Shadowsocks},
-    wireguard, TransportProtocol,
-};
+use talpid_types::net::{proxy::Shadowsocks, wireguard, TransportProtocol};
 
 /// Stores a list of relays for each country obtained from the API using
 /// `mullvad_api::RelayListProxy`. This can also be passed to frontends.
@@ -246,11 +243,11 @@ pub struct ShadowsocksEndpointData {
 }
 
 impl ShadowsocksEndpointData {
-    pub fn to_proxy_settings(&self, addr: IpAddr) -> CustomProxy {
-        CustomProxy::Shadowsocks(Shadowsocks {
+    pub fn to_proxy_settings(&self, addr: IpAddr) -> Shadowsocks {
+        Shadowsocks {
             endpoint: SocketAddr::new(addr, self.port),
             password: self.password.clone(),
             cipher: self.cipher.clone(),
-        })
+        }
     }
 }