Remove dependency on relay-selector in mullvad-api

Author: buggmagnet

Cargo.lock

@@ -52,7 +52,6 @@ mullvad-fs = { path = "../mullvad-fs" }
 mullvad-types = { path = "../mullvad-types" }
 talpid-types = { path = "../talpid-types" }
 talpid-time = { path = "../talpid-time" }
-mullvad-relay-selector = { path = "../mullvad-relay-selector" }
 
 shadowsocks = { workspace = true, features = ["stream-cipher"] }
 

mullvad-api/Cargo.toml

@@ -7,7 +7,7 @@
 #[cfg(feature = "api-override")]
 use crate::ApiEndpoint;
 use crate::{
-    proxy::{AllowedClientsProvider, ApiConnectionMode, ConnectionModeProvider},
+    proxy::{ApiConnectionMode, ConnectionModeProvider},
     AddressCache,
 };
 use async_trait::async_trait;
@@ -16,8 +16,8 @@ use futures::{
     StreamExt,
 };
 use mullvad_types::access_method::{AccessMethod, AccessMethodSetting, Id, Settings};
-use std::{marker::PhantomData, net::SocketAddr, path::PathBuf};
-use talpid_types::net::{AllowedEndpoint, Endpoint, TransportProtocol};
+use std::{net::SocketAddr, path::PathBuf};
+use talpid_types::net::{AllowedClients, AllowedEndpoint, Endpoint, TransportProtocol};
 
 pub enum Message {
     Get(ResponseTx<ResolvedConnectionMode>),
@@ -242,29 +242,25 @@ impl ConnectionModeProvider for AccessModeConnectionModeProvider {
 /// [`ApiConnectionMode::Direct`]) via a bridge ([`ApiConnectionMode::Proxied`])
 /// or via any supported custom proxy protocol
 /// ([`talpid_types::net::proxy::CustomProxy`]).
-pub struct AccessModeSelector<P> {
+pub struct AccessModeSelector<B: AccessMethodResolver> {
     #[cfg(feature = "api-override")]
     api_endpoint: ApiEndpoint,
     cmd_rx: mpsc::UnboundedReceiver<Message>,
     cache_dir: PathBuf,
-    bridge_dns_proxy_provider: Box<dyn BridgeAndDNSProxy>,
+    bridge_dns_proxy_provider: B,
     access_method_settings: Settings,
     address_cache: AddressCache,
     access_method_event_sender: mpsc::UnboundedSender<(AccessMethodEvent, oneshot::Sender<()>)>,
     connection_mode_provider_sender: mpsc::UnboundedSender<ApiConnectionMode>,
     current: ResolvedConnectionMode,
     /// `index` is used to keep track of the [`AccessMethodSetting`] to use.
     index: usize,
-    provider: PhantomData<P>,
 }
 
-impl<P> AccessModeSelector<P>
-where
-    P: AllowedClientsProvider + 'static,
-{
+impl<B: AccessMethodResolver + 'static> AccessModeSelector<B> {
     pub async fn spawn(
         cache_dir: PathBuf,
-        mut bridge_dns_proxy_provider: Box<dyn BridgeAndDNSProxy>,
+        mut bridge_dns_proxy_provider: B,
         #[cfg_attr(not(feature = "api-override"), allow(unused_mut))]
         mut access_method_settings: Settings,
         #[cfg(feature = "api-override")] api_endpoint: ApiEndpoint,
@@ -283,18 +279,15 @@ where
 
         // 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_with_default(
-            &next,
-            &address_cache,
-            &mut *bridge_dns_proxy_provider,
-        )
-        .await;
+        let initial_connection_mode =
+            Self::resolve_inner_with_default(&next, &address_cache, &mut bridge_dns_proxy_provider)
+                .await;
 
         let (change_tx, change_rx) = mpsc::unbounded();
 
         let api_connection_mode = initial_connection_mode.connection_mode.clone();
 
-        let selector: AccessModeSelector<P> = AccessModeSelector {
+        let selector = AccessModeSelector {
             #[cfg(feature = "api-override")]
             api_endpoint,
             cmd_rx,
@@ -306,7 +299,6 @@ where
             connection_mode_provider_sender: change_tx,
             current: initial_connection_mode,
             index,
-            provider: PhantomData,
         };
 
         tokio::spawn(selector.into_future());
@@ -527,25 +519,25 @@ where
         Self::resolve_inner(
             &access_method,
             &self.address_cache,
-            &mut *self.bridge_dns_proxy_provider,
+            &mut self.bridge_dns_proxy_provider,
         )
         .await
     }
 
     async fn resolve_inner(
-        access_method: &AccessMethodSetting,
+        method_setting: &AccessMethodSetting,
         address_cache: &AddressCache,
-        bridge_dns_proxy_provider: &mut dyn BridgeAndDNSProxy,
+        bridge_dns_proxy_provider: &mut B,
     ) -> Option<ResolvedConnectionMode> {
         let connection_mode = bridge_dns_proxy_provider
-            .match_access_method(access_method)
+            .resolve_access_method_setting(method_setting)
             .await?;
         let endpoint =
-            resolve_allowed_endpoint::<P>(&connection_mode, address_cache.get_address().await);
+            resolve_allowed_endpoint::<B>(&connection_mode, address_cache.get_address().await);
         Some(ResolvedConnectionMode {
             connection_mode,
             endpoint,
-            setting: access_method.clone(),
+            setting: method_setting.clone(),
         })
     }
 
@@ -558,21 +550,21 @@ where
         Self::resolve_inner_with_default(
             &access_method,
             &self.address_cache,
-            &mut *self.bridge_dns_proxy_provider,
+            &mut self.bridge_dns_proxy_provider,
         )
         .await
     }
 
     async fn resolve_inner_with_default(
         access_method: &AccessMethodSetting,
         address_cache: &AddressCache,
-        bridge_dns_proxy_provider: &mut dyn BridgeAndDNSProxy,
+        bridge_dns_proxy_provider: &mut B,
     ) -> ResolvedConnectionMode {
         match Self::resolve_inner(access_method, address_cache, bridge_dns_proxy_provider).await {
             Some(resolved) => resolved,
             None => {
                 log::trace!("Defaulting to direct API connection");
-                let endpoint = resolve_allowed_endpoint::<P>(
+                let endpoint = resolve_allowed_endpoint::<B>(
                     &ApiConnectionMode::Direct,
                     address_cache.get_address().await,
                 );
@@ -587,24 +579,26 @@ where
 }
 
 #[async_trait]
-pub trait BridgeAndDNSProxy: Send + Sync {
-    async fn match_access_method(
+pub trait AccessMethodResolver: Send + Sync {
+    async fn resolve_access_method_setting(
         &mut self,
         access_method: &AccessMethodSetting,
     ) -> Option<ApiConnectionMode>;
+
+    fn allowed_clients(connection_mode: &ApiConnectionMode) -> AllowedClients;
 }
 
-pub fn resolve_allowed_endpoint<P>(
+pub fn resolve_allowed_endpoint<B>(
     connection_mode: &ApiConnectionMode,
     fallback: SocketAddr,
 ) -> AllowedEndpoint
 where
-    P: AllowedClientsProvider,
+    B: AccessMethodResolver,
 {
     let endpoint = match connection_mode.get_endpoint() {
         Some(endpoint) => endpoint,
         None => Endpoint::from_socket_address(fallback, TransportProtocol::Tcp),
     };
-    let clients = P::allowed_clients(connection_mode);
+    let clients = B::allowed_clients(connection_mode);
     AllowedEndpoint { endpoint, clients }
 }

mullvad-api/src/access_mode.rs

@@ -8,7 +8,7 @@ use std::{
     task::{self, Poll},
 };
 use talpid_types::{
-    net::{proxy, AllowedClients, Endpoint, TransportProtocol},
+    net::{proxy, Endpoint, TransportProtocol},
     ErrorExt,
 };
 use tokio::{
@@ -53,10 +53,6 @@ impl ConnectionModeProvider for StaticConnectionModeProvider {
     }
 }
 
-pub trait AllowedClientsProvider: Send + Sync {
-    fn allowed_clients(connection_mode: &ApiConnectionMode) -> AllowedClients;
-}
-
 #[derive(Clone, Debug, PartialEq, Deserialize, Serialize)]
 pub enum ApiConnectionMode {
     /// Connect directly to the target.

mullvad-api/src/proxy.rs

@@ -4,9 +4,9 @@ use crate::DaemonCommand;
 use crate::DaemonEventSender;
 use futures::{channel::mpsc, StreamExt};
 use mullvad_api::{
-    access_mode::BridgeAndDNSProxy,
+    access_mode::AccessMethodResolver,
     availability::ApiAvailability,
-    proxy::{AllowedClientsProvider, ApiConnectionMode, ProxyConfig},
+    proxy::{ApiConnectionMode, ProxyConfig},
 };
 use mullvad_encrypted_dns_proxy::state::EncryptedDnsProxyState;
 use mullvad_management_interface::async_trait;
@@ -16,12 +16,12 @@ use mullvad_types::access_method::{AccessMethod, AccessMethodSetting, BuiltInAcc
 use talpid_core::mpsc::Sender;
 use talpid_types::net::{proxy::CustomProxy, AllowedClients, Connectivity};
 
-pub struct BridgeAndDNSProxyProvider {
+pub struct DaemonAccessMethodResolver {
     relay_selector: RelaySelector,
     encrypted_dns_proxy_cache: EncryptedDnsProxyState,
 }
 
-impl BridgeAndDNSProxyProvider {
+impl DaemonAccessMethodResolver {
     pub fn new(
         relay_selector: RelaySelector,
         encrypted_dns_proxy_cache: EncryptedDnsProxyState,
@@ -34,8 +34,8 @@ impl BridgeAndDNSProxyProvider {
 }
 
 #[async_trait]
-impl BridgeAndDNSProxy for BridgeAndDNSProxyProvider {
-    async fn match_access_method(
+impl AccessMethodResolver for DaemonAccessMethodResolver {
+    async fn resolve_access_method_setting(
         &mut self,
         access_method: &AccessMethodSetting,
     ) -> Option<ApiConnectionMode> {
@@ -73,12 +73,7 @@ impl BridgeAndDNSProxy for BridgeAndDNSProxyProvider {
         };
         Some(connection_mode)
     }
-}
-
-#[derive(Clone, Copy)]
-pub struct AllowedClientsSelector {}
 
-impl AllowedClientsProvider for AllowedClientsSelector {
     #[cfg(unix)]
     fn allowed_clients(connection_mode: &ApiConnectionMode) -> AllowedClients {
         match connection_mode {

mullvad-daemon/src/api.rs

@@ -31,7 +31,7 @@ pub mod version;
 mod version_check;
 
 use crate::target_state::PersistentTargetState;
-use api::{AllowedClientsSelector, BridgeAndDNSProxyProvider};
+use api::DaemonAccessMethodResolver;
 use device::{AccountEvent, PrivateAccountAndDevice, PrivateDeviceEvent};
 use futures::{
     channel::{mpsc, oneshot},
@@ -41,7 +41,10 @@ use futures::{
 use geoip::GeoIpHandler;
 use leak_checker::{LeakChecker, LeakInfo};
 use management_interface::ManagementInterfaceServer;
-use mullvad_api::{access_mode::AccessMethodEvent, proxy::AllowedClientsProvider, ApiEndpoint};
+use mullvad_api::{
+    access_mode::{AccessMethodEvent, AccessMethodResolver},
+    ApiEndpoint,
+};
 use mullvad_encrypted_dns_proxy::state::EncryptedDnsProxyState;
 use mullvad_relay_selector::{RelaySelector, SelectorConfig};
 #[cfg(target_os = "android")]
@@ -709,12 +712,12 @@ impl Daemon {
 
         let encrypted_dns_proxy_cache = EncryptedDnsProxyState::default();
         let bridge_dns_proxy_provider =
-            BridgeAndDNSProxyProvider::new(relay_selector.clone(), encrypted_dns_proxy_cache);
+            DaemonAccessMethodResolver::new(relay_selector.clone(), encrypted_dns_proxy_cache);
 
         let (access_mode_handler, access_mode_provider) =
-            mullvad_api::access_mode::AccessModeSelector::<AllowedClientsSelector>::spawn(
+            mullvad_api::access_mode::AccessModeSelector::spawn(
                 config.cache_dir.clone(),
-                Box::new(bridge_dns_proxy_provider),
+                bridge_dns_proxy_provider,
                 settings.api_access_methods.clone(),
                 #[cfg(feature = "api-override")]
                 config.endpoint.clone(),
@@ -2855,7 +2858,7 @@ impl Daemon {
         let api_proxy = self.create_limited_api_proxy(connection_mode.clone());
         let proxy_endpoint = AllowedEndpoint {
             endpoint: proxy.get_remote_endpoint().endpoint,
-            clients: AllowedClientsSelector::allowed_clients(&connection_mode),
+            clients: DaemonAccessMethodResolver::allowed_clients(&connection_mode),
         };
 
         let daemon_event_sender = self.tx.to_specialized_sender();

mullvad-daemon/src/lib.rs

@@ -53,10 +53,9 @@ use talpid_types::{
     ErrorExt,
 };
 
-/// [`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.
+/// [`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.
 ///
 /// 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(|| {
@@ -82,8 +81,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`
@@ -694,10 +693,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)