Remove dependency on relay-selector in mullvad-api

Author: buggmagnet

mullvad-api/src/access_mode.rs

@@ -15,9 +15,9 @@ use futures::{
     StreamExt,
 };
 use mullvad_encrypted_dns_proxy::state::EncryptedDnsProxyState;
-use mullvad_relay_selector::RelaySelector;
-use mullvad_types::access_method::{
-    AccessMethod, AccessMethodSetting, BuiltInAccessMethod, Id, Settings,
+use mullvad_types::{
+    access_method::{AccessMethod, AccessMethodSetting, BuiltInAccessMethod, Id, Settings},
+    relay_list::ShadowsocksBridgeProvider,
 };
 use std::{marker::PhantomData, net::SocketAddr, path::PathBuf};
 use talpid_types::net::{proxy::CustomProxy, AllowedEndpoint, Endpoint, TransportProtocol};
@@ -251,7 +251,7 @@ pub struct AccessModeSelector<P> {
     cmd_rx: mpsc::UnboundedReceiver<Message>,
     cache_dir: PathBuf,
     /// Used for selecting a Bridge when the `Mullvad Bridges` access method is used.
-    relay_selector: RelaySelector,
+    bridge_provider: Box<dyn ShadowsocksBridgeProvider>,
     /// Used for selecting a config for the 'Encrypted DNS proxy' access method.
     encrypted_dns_proxy_cache: EncryptedDnsProxyState,
     access_method_settings: Settings,
@@ -270,7 +270,7 @@ where
 {
     pub async fn spawn(
         cache_dir: PathBuf,
-        relay_selector: RelaySelector,
+        bridge_provider: Box<dyn ShadowsocksBridgeProvider>,
         #[cfg_attr(not(feature = "api-override"), allow(unused_mut))]
         mut access_method_settings: Settings,
         #[cfg(feature = "api-override")] api_endpoint: ApiEndpoint,
@@ -294,7 +294,7 @@ where
         let (index, next) = Self::find_next_active(0, &access_method_settings);
         let initial_connection_mode = Self::resolve_inner_with_default(
             &next,
-            &relay_selector,
+            &bridge_provider,
             &mut encrypted_dns_proxy_cache,
             &address_cache,
         )
@@ -309,7 +309,7 @@ where
             api_endpoint,
             cmd_rx,
             cache_dir,
-            relay_selector,
+            bridge_provider,
             encrypted_dns_proxy_cache,
             access_method_settings,
             address_cache,
@@ -537,7 +537,7 @@ where
     ) -> Option<ResolvedConnectionMode> {
         Self::resolve_inner(
             &access_method,
-            &self.relay_selector,
+            &self.bridge_provider,
             &mut self.encrypted_dns_proxy_cache,
             &self.address_cache,
         )
@@ -546,13 +546,16 @@ where
 
     async fn resolve_inner(
         access_method: &AccessMethodSetting,
-        relay_selector: &RelaySelector,
+        bridge_provider: &Box<dyn ShadowsocksBridgeProvider>,
         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 connection_mode = Self::resolve_connection_mode(
+            access_method,
+            bridge_provider,
+            encrypted_dns_proxy_cache,
+        )
+        .await?;
         let endpoint =
             resolve_allowed_endpoint::<P>(&connection_mode, address_cache.get_address().await);
         Some(ResolvedConnectionMode {
@@ -570,7 +573,7 @@ where
     ) -> ResolvedConnectionMode {
         Self::resolve_inner_with_default(
             &access_method,
-            &self.relay_selector,
+            &self.bridge_provider,
             &mut self.encrypted_dns_proxy_cache,
             &self.address_cache,
         )
@@ -579,13 +582,13 @@ where
 
     async fn resolve_inner_with_default(
         access_method: &AccessMethodSetting,
-        relay_selector: &RelaySelector,
+        bridge_provider: &Box<dyn ShadowsocksBridgeProvider>,
         encrypted_dns_proxy_cache: &mut EncryptedDnsProxyState,
         address_cache: &AddressCache,
     ) -> ResolvedConnectionMode {
         match Self::resolve_inner(
             access_method,
-            relay_selector,
+            bridge_provider,
             encrypted_dns_proxy_cache,
             address_cache,
         )
@@ -609,14 +612,14 @@ where
 
     async fn resolve_connection_mode(
         access_method: &AccessMethodSetting,
-        relay_selector: &RelaySelector,
+        bridge_provider: &Box<dyn ShadowsocksBridgeProvider>,
         encrypted_dns_proxy_cache: &mut EncryptedDnsProxyState,
     ) -> Option<ApiConnectionMode> {
         let connection_mode = {
             match &access_method.access_method {
                 AccessMethod::BuiltIn(BuiltInAccessMethod::Direct) => ApiConnectionMode::Direct,
                 AccessMethod::BuiltIn(BuiltInAccessMethod::Bridge) => {
-                    let Some(bridge) = relay_selector.get_bridge_forced() else {
+                    let Some(bridge) = bridge_provider.get_bridge_forced() else {
                         log::warn!("Could not select a Mullvad bridge");
                         log::debug!("The relay list might be empty");
                         return None;

mullvad-daemon/src/lib.rs

@@ -706,10 +706,12 @@ impl Daemon {
                 .set_config(SelectorConfig::from_settings(settings));
         });
 
+        let bridge_provider = Box::new(relay_selector.clone());
+
         let (access_mode_handler, access_mode_provider) =
             mullvad_api::access_mode::AccessModeSelector::<AllowedClientsSelector>::spawn(
                 config.cache_dir.clone(),
-                relay_selector.clone(),
+                bridge_provider,
                 settings.api_access_methods.clone(),
                 #[cfg(feature = "api-override")]
                 config.endpoint.clone(),

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

@@ -39,7 +39,7 @@ use mullvad_types::{
         OpenVpnConstraints, RelayConstraints, RelayOverride, RelaySettings, ResolvedBridgeSettings,
         WireguardConstraints,
     },
-    relay_list::{Relay, RelayEndpointData, RelayList},
+    relay_list::{Relay, RelayEndpointData, RelayList, ShadowsocksBridgeProvider},
     settings::Settings,
     wireguard::QuantumResistantState,
     CustomTunnelEndpoint, Intersection,
@@ -53,9 +53,10 @@ use talpid_types::{
     ErrorExt,
 };
 
-/// [`WIREGUARD_RETRY_ORDER`] defines an ordered set of relay parameters which the relay selector should
-/// prioritize on successive connection attempts. Note that these will *never* override user
-/// preferences. See [the documentation on `RelayQuery`][RelayQuery] for further details.
+/// [`WIREGUARD_RETRY_ORDER`] defines an ordered set of relay parameters which the relay selector
+/// should should prioritize on successive connection attempts. Note that these will *never*
+/// override user preferences. See [the documentation on `RelayQuery`][RelayQuery] for further
+/// details.
 ///
 /// This list should be kept in sync with the expected behavior defined in `docs/relay-selector.md`
 pub static WIREGUARD_RETRY_ORDER: LazyLock<Vec<RelayQuery>> = LazyLock::new(|| {
@@ -83,8 +84,8 @@ pub static WIREGUARD_RETRY_ORDER: LazyLock<Vec<RelayQuery>> = LazyLock::new(|| {
     ]
 });
 
-/// [`OPENVPN_RETRY_ORDER`] defines an ordered set of relay parameters which the relay selector should
-/// prioritize on successive connection attempts. Note that these will *never* override user
+/// [`OPENVPN_RETRY_ORDER`] defines an ordered set of relay parameters which the relay selector
+/// should prioritize on successive connection attempts. Note that these will *never* override user
 /// preferences. See [the documentation on `RelayQuery`][RelayQuery] for further details.
 ///
 /// This list should be kept in sync with the expected behavior defined in `docs/relay-selector.md`
@@ -435,6 +436,47 @@ impl<'a> TryFrom<NormalSelectorConfig<'a>> for RelayQuery {
     }
 }
 
+impl ShadowsocksBridgeProvider for RelaySelector {
+    /// Returns a non-custom bridge based on the relay and bridge constraints, ignoring the bridge
+    /// state.
+    fn get_bridge_forced(&self) -> Option<Shadowsocks> {
+        let parsed_relays = &self.parsed_relays.lock().unwrap().parsed_list().clone();
+        let config = self.config.lock().unwrap();
+        let specialized_config = SpecializedSelectorConfig::from(&*config);
+
+        let near_location = match specialized_config {
+            SpecializedSelectorConfig::Normal(config) => RelayQuery::try_from(config.clone())
+                .ok()
+                .and_then(|user_preferences| {
+                    Self::get_relay_midpoint(&user_preferences, parsed_relays, config.custom_lists)
+                }),
+            SpecializedSelectorConfig::Custom(_) => None,
+        };
+
+        let bridge_settings = &config.bridge_settings;
+        let constraints = match bridge_settings.resolve() {
+            Ok(ResolvedBridgeSettings::Normal(settings)) => InternalBridgeConstraints {
+                location: settings.location.clone(),
+                providers: settings.providers.clone(),
+                ownership: settings.ownership,
+                transport_protocol: Constraint::Only(TransportProtocol::Tcp),
+            },
+            _ => InternalBridgeConstraints {
+                location: Constraint::Any,
+                providers: Constraint::Any,
+                ownership: Constraint::Any,
+                transport_protocol: Constraint::Only(TransportProtocol::Tcp),
+            },
+        };
+
+        let custom_lists = &config.custom_lists;
+        Self::get_proxy_settings(parsed_relays, &constraints, near_location, custom_lists)
+            .map(|(settings, _relay)| settings)
+            .inspect_err(|error| log::error!("Failed to get bridge: {error}"))
+            .ok()
+    }
+}
+
 impl RelaySelector {
     /// Returns a new `RelaySelector` backed by relays cached on disk.
     pub fn new(
@@ -507,45 +549,6 @@ impl RelaySelector {
         self.parsed_relays.lock().unwrap().last_updated()
     }
 
-    /// Returns a non-custom bridge based on the relay and bridge constraints, ignoring the bridge
-    /// state.
-    pub fn get_bridge_forced(&self) -> Option<Shadowsocks> {
-        let parsed_relays = &self.parsed_relays.lock().unwrap().parsed_list().clone();
-        let config = self.config.lock().unwrap();
-        let specialized_config = SpecializedSelectorConfig::from(&*config);
-
-        let near_location = match specialized_config {
-            SpecializedSelectorConfig::Normal(config) => RelayQuery::try_from(config.clone())
-                .ok()
-                .and_then(|user_preferences| {
-                    Self::get_relay_midpoint(&user_preferences, parsed_relays, config.custom_lists)
-                }),
-            SpecializedSelectorConfig::Custom(_) => None,
-        };
-
-        let bridge_settings = &config.bridge_settings;
-        let constraints = match bridge_settings.resolve() {
-            Ok(ResolvedBridgeSettings::Normal(settings)) => InternalBridgeConstraints {
-                location: settings.location.clone(),
-                providers: settings.providers.clone(),
-                ownership: settings.ownership,
-                transport_protocol: Constraint::Only(TransportProtocol::Tcp),
-            },
-            _ => InternalBridgeConstraints {
-                location: Constraint::Any,
-                providers: Constraint::Any,
-                ownership: Constraint::Any,
-                transport_protocol: Constraint::Only(TransportProtocol::Tcp),
-            },
-        };
-
-        let custom_lists = &config.custom_lists;
-        Self::get_proxy_settings(parsed_relays, &constraints, near_location, custom_lists)
-            .map(|(settings, _relay)| settings)
-            .inspect_err(|error| log::error!("Failed to get bridge: {error}"))
-            .ok()
-    }
-
     /// Returns random relay and relay endpoint matching `query`.
     pub fn get_relay_by_query(&self, query: RelayQuery) -> Result<GetRelay, Error> {
         let config_guard = self.config.lock().unwrap();
@@ -697,10 +700,10 @@ impl RelaySelector {
         parsed_relays: &RelayList,
         custom_lists: &CustomListsSettings,
     ) -> Result<GetRelay, Error> {
-        // FIXME: A bit of defensive programming - calling `get_wireguard_relay_inner` with a query that
-        // doesn't specify Wireguard as the desired tunnel type is not valid and will lead
-        // to unwanted behavior. This should be seen as a workaround, and it would be nicer
-        // to lift this invariant to be checked by the type system instead.
+        // FIXME: A bit of defensive programming - calling `get_wireguard_relay_inner` with a query
+        // that doesn't specify Wireguard as the desired tunnel type is not valid and will
+        // lead to unwanted behavior. This should be seen as a workaround, and it would be
+        // nicer to lift this invariant to be checked by the type system instead.
         let mut query = query.clone();
         query.set_tunnel_protocol(TunnelType::Wireguard)?;
         Self::get_wireguard_relay_inner(&query, custom_lists, parsed_relays)

mullvad-types/src/relay_list.rs

@@ -251,3 +251,7 @@ impl ShadowsocksEndpointData {
         }
     }
 }
+
+pub trait ShadowsocksBridgeProvider: Send + Sync {
+    fn get_bridge_forced(&self) -> Option<Shadowsocks>;
+}