Configure firewall rules to allow proxy clients

The default setting will (always) be to only allow processes with
root-privilege to send/receive traffic from an allowed endpoint.

This change is only supposed to be used with the local SOCKS5 api access
method.

7 files changed, 174 insertions, 61 deletions

diff --git a/mullvad-api/src/lib.rs b/mullvad-api/src/lib.rs
index 267fdfd811..5a436fe1bf 100644
--- a/mullvad-api/src/lib.rs
+++ b/mullvad-api/src/lib.rs
@@ -21,7 +21,7 @@ use std::{
     ops::Deref,
     path::Path,
 };
-use talpid_types::{net::Endpoint, ErrorExt};
+use talpid_types::{net::AllowedEndpoint, ErrorExt};
 
 pub mod availability;
 use availability::{ApiAvailability, ApiAvailabilityHandle};
@@ -221,13 +221,13 @@ pub enum Error {
 
 /// Closure that receives the next API (real or proxy) endpoint to use for `api.mullvad.net`.
 /// It should return a future that determines whether to reject the new endpoint or not.
-pub trait ApiEndpointUpdateCallback: Fn(Endpoint) -> Self::AcceptedNewEndpoint {
+pub trait ApiEndpointUpdateCallback: Fn(AllowedEndpoint) -> Self::AcceptedNewEndpoint {
     type AcceptedNewEndpoint: Future<Output = bool> + Send;
 }
 
 impl<U, T: Future<Output = bool> + Send> ApiEndpointUpdateCallback for U
 where
-    U: Fn(Endpoint) -> T,
+    U: Fn(AllowedEndpoint) -> T,
 {
     type AcceptedNewEndpoint = T;
 }
diff --git a/mullvad-api/src/proxy.rs b/mullvad-api/src/proxy.rs
index 0727628596..783cc8a157 100644
--- a/mullvad-api/src/proxy.rs
+++ b/mullvad-api/src/proxy.rs
@@ -9,7 +9,7 @@ use std::{
     task::{self, Poll},
 };
 use talpid_types::{
-    net::{Endpoint, TransportProtocol},
+    net::{AllowedClients, Endpoint, TransportProtocol},
     ErrorExt,
 };
 use tokio::{
@@ -143,6 +143,36 @@ impl ApiConnectionMode {
         }
     }
 
+    #[cfg(unix)]
+    pub fn allowed_clients(&self) -> AllowedClients {
+        use access_method::Socks5;
+        match self {
+            ApiConnectionMode::Proxied(ProxyConfig::Socks(Socks5::Local(_))) => AllowedClients::All,
+            ApiConnectionMode::Direct | ApiConnectionMode::Proxied(_) => AllowedClients::Root,
+        }
+    }
+
+    #[cfg(windows)]
+    pub fn allowed_clients(&self) -> AllowedClients {
+        use access_method::Socks5;
+        match self {
+            ApiConnectionMode::Proxied(ProxyConfig::Socks(Socks5::Local(_))) => {
+                AllowedClients::all()
+            }
+            ApiConnectionMode::Direct | ApiConnectionMode::Proxied(_) => {
+                let daemon_exe = std::env::current_exe().expect("failed to obtain executable path");
+                vec![
+                    daemon_exe
+                        .parent()
+                        .expect("missing executable parent directory")
+                        .join("mullvad-problem-report.exe"),
+                    daemon_exe,
+                ]
+                .into()
+            }
+        }
+    }
+
     pub fn is_proxy(&self) -> bool {
         *self != ApiConnectionMode::Direct
     }
diff --git a/mullvad-api/src/rest.rs b/mullvad-api/src/rest.rs
index a973c32f2a..2484bec64b 100644
--- a/mullvad-api/src/rest.rs
+++ b/mullvad-api/src/rest.rs
@@ -25,7 +25,7 @@ use std::{
     time::Duration,
 };
 use talpid_types::{
-    net::{Endpoint, TransportProtocol},
+    net::{AllowedEndpoint, Endpoint, TransportProtocol},
     ErrorExt,
 };
 
@@ -217,8 +217,10 @@ impl<
                             TransportProtocol::Tcp,
                         ),
                     };
+                    let clients = new_config.allowed_clients();
+                    let allowed_endpoint = AllowedEndpoint { endpoint, clients };
                     // Switch to new connection mode unless rejected by address change callback
-                    if (self.new_address_callback)(endpoint).await {
+                    if (self.new_address_callback)(allowed_endpoint).await {
                         self.connector_handle.set_connection_mode(new_config);
                     }
                 }
diff --git a/mullvad-daemon/src/api.rs b/mullvad-daemon/src/api.rs
index a6dd0dead6..3ac526e688 100644
--- a/mullvad-daemon/src/api.rs
+++ b/mullvad-daemon/src/api.rs
@@ -239,7 +239,7 @@ impl ApiEndpointUpdaterHandle {
 
     pub fn callback(&self) -> impl ApiEndpointUpdateCallback {
         let tunnel_tx = self.tunnel_cmd_tx.clone();
-        move |endpoint: Endpoint| {
+        move |allowed_endpoint: AllowedEndpoint| {
             let inner_tx = tunnel_tx.clone();
             async move {
                 let tunnel_tx = if let Some(tunnel_tx) = { inner_tx.lock().unwrap().as_ref() }
@@ -252,12 +252,15 @@ impl ApiEndpointUpdaterHandle {
                 };
                 let (result_tx, result_rx) = oneshot::channel();
                 let _ = tunnel_tx.unbounded_send(TunnelCommand::AllowEndpoint(
-                    get_allowed_endpoint(endpoint),
+                    allowed_endpoint.clone(),
                     result_tx,
                 ));
                 // Wait for the firewall policy to be updated.
                 let _ = result_rx.await;
-                log::debug!("API endpoint: {endpoint}");
+                log::debug!(
+                    "API endpoint: {endpoint}",
+                    endpoint = allowed_endpoint.endpoint
+                );
                 true
             }
         }
@@ -265,22 +268,22 @@ impl ApiEndpointUpdaterHandle {
 }
 
 pub(super) fn get_allowed_endpoint(endpoint: Endpoint) -> AllowedEndpoint {
+    #[cfg(unix)]
+    let clients = talpid_types::net::AllowedClients::Root;
     #[cfg(windows)]
-    let daemon_exe = std::env::current_exe().expect("failed to obtain executable path");
-    #[cfg(windows)]
-    let clients = vec![
-        daemon_exe
-            .parent()
-            .expect("missing executable parent directory")
-            .join("mullvad-problem-report.exe"),
-        daemon_exe,
-    ];
+    let clients = {
+        let daemon_exe = std::env::current_exe().expect("failed to obtain executable path");
+        vec![
+            daemon_exe
+                .parent()
+                .expect("missing executable parent directory")
+                .join("mullvad-problem-report.exe"),
+            daemon_exe,
+        ]
+        .into()
+    };
 
-    AllowedEndpoint {
-        #[cfg(windows)]
-        clients,
-        endpoint,
-    }
+    AllowedEndpoint { endpoint, clients }
 }
 
 pub(crate) fn forward_offline_state(
diff --git a/talpid-core/src/firewall/linux.rs b/talpid-core/src/firewall/linux.rs
index 1b74fb30bd..a74aa1ce4a 100644
--- a/talpid-core/src/firewall/linux.rs
+++ b/talpid-core/src/firewall/linux.rs
@@ -14,7 +14,7 @@ use std::{
     fs, io,
     net::{IpAddr, Ipv4Addr},
 };
-use talpid_types::net::{AllowedTunnelTraffic, Endpoint, TransportProtocol};
+use talpid_types::net::{AllowedEndpoint, AllowedTunnelTraffic, Endpoint, TransportProtocol};
 
 /// Priority for rules that tag split tunneling packets. Equals NF_IP_PRI_MANGLE.
 const MANGLE_CHAIN_PRIORITY: i32 = libc::NF_IP_PRI_MANGLE;
@@ -519,7 +519,7 @@ impl<'a> PolicyBatch<'a> {
                 allowed_tunnel_traffic,
             } => {
                 self.add_allow_tunnel_endpoint_rules(peer_endpoint, fwmark);
-                self.add_allow_endpoint_rules(&allowed_endpoint.endpoint);
+                self.add_allow_endpoint_rules(allowed_endpoint);
 
                 // Important to block DNS after allow relay rule (so the relay can operate
                 // over port 53) but before allow LAN (so DNS does not leak to the LAN)
@@ -568,7 +568,7 @@ impl<'a> PolicyBatch<'a> {
                 allowed_endpoint,
             } => {
                 if let Some(endpoint) = allowed_endpoint {
-                    self.add_allow_endpoint_rules(&endpoint.endpoint);
+                    self.add_allow_endpoint_rules(endpoint);
                 }
 
                 // Important to drop DNS before allowing LAN (to stop DNS leaking to the LAN)
@@ -628,24 +628,28 @@ impl<'a> PolicyBatch<'a> {
 
     /// Adds firewall rules allow traffic to flow to the API. Allows the app to reach the API in
     /// blocked states.
-    fn add_allow_endpoint_rules(&mut self, endpoint: &Endpoint) {
+    fn add_allow_endpoint_rules(&mut self, endpoint: &AllowedEndpoint) {
         let mut in_rule = Rule::new(&self.in_chain);
-        check_endpoint(&mut in_rule, End::Src, endpoint);
+        check_endpoint(&mut in_rule, End::Src, &endpoint.endpoint);
         let allowed_states = nftnl::expr::ct::States::ESTABLISHED.bits();
         in_rule.add_expr(&nft_expr!(ct state));
         in_rule.add_expr(&nft_expr!(bitwise mask allowed_states, xor 0u32));
         in_rule.add_expr(&nft_expr!(cmp != 0u32));
-        in_rule.add_expr(&nft_expr!(meta skuid));
-        in_rule.add_expr(&nft_expr!(cmp == super::ROOT_UID));
+        if !endpoint.clients.allow_all() {
+            in_rule.add_expr(&nft_expr!(meta skuid));
+            in_rule.add_expr(&nft_expr!(cmp == super::ROOT_UID));
+        }
 
         add_verdict(&mut in_rule, &Verdict::Accept);
 
         self.batch.add(&in_rule, nftnl::MsgType::Add);
 
         let mut out_rule = Rule::new(&self.out_chain);
-        check_endpoint(&mut out_rule, End::Dst, endpoint);
-        out_rule.add_expr(&nft_expr!(meta skuid));
-        out_rule.add_expr(&nft_expr!(cmp == super::ROOT_UID));
+        check_endpoint(&mut out_rule, End::Dst, &endpoint.endpoint);
+        if !endpoint.clients.allow_all() {
+            out_rule.add_expr(&nft_expr!(meta skuid));
+            out_rule.add_expr(&nft_expr!(cmp == super::ROOT_UID));
+        }
         add_verdict(&mut out_rule, &Verdict::Accept);
 
         self.batch.add(&out_rule, nftnl::MsgType::Add);
diff --git a/talpid-core/src/firewall/macos.rs b/talpid-core/src/firewall/macos.rs
index 4f95309890..54f72a79c2 100644
--- a/talpid-core/src/firewall/macos.rs
+++ b/talpid-core/src/firewall/macos.rs
@@ -6,7 +6,7 @@ use std::{
     net::{IpAddr, Ipv4Addr},
 };
 use subslice::SubsliceExt;
-use talpid_types::net::{self, AllowedTunnelTraffic};
+use talpid_types::net::{self, AllowedEndpoint, AllowedTunnelTraffic};
 
 pub use pfctl::Error;
 
@@ -122,7 +122,7 @@ impl Firewall {
                 allowed_tunnel_traffic,
             } => {
                 let mut rules = vec![self.get_allow_relay_rule(*peer_endpoint)?];
-                rules.push(self.get_allowed_endpoint_rule(allowed_endpoint.endpoint)?);
+                rules.push(self.get_allowed_endpoint_rule(allowed_endpoint)?);
 
                 // Important to block DNS after allow relay rule (so the relay can operate
                 // over port 53) but before allow LAN (so DNS does not leak to the LAN)
@@ -175,7 +175,7 @@ impl Firewall {
             } => {
                 let mut rules = Vec::new();
                 if let Some(allowed_endpoint) = allowed_endpoint {
-                    rules.push(self.get_allowed_endpoint_rule(allowed_endpoint.endpoint)?);
+                    rules.push(self.get_allowed_endpoint_rule(allowed_endpoint)?);
                 }
 
                 if *allow_lan {
@@ -287,22 +287,26 @@ impl Firewall {
             .build()
     }
 
-    /// Produces a rule that allows traffic to flow to the API. Allows the app to reach the API in
-    /// blocked states.
+    /// Produces a rule that allows traffic to flow to the API. Allows the app (or other apps if configured)
+    /// to reach the API in blocked states.
     fn get_allowed_endpoint_rule(
         &self,
-        allowed_endpoint: net::Endpoint,
+        allowed_endpoint: &AllowedEndpoint,
     ) -> Result<pfctl::FilterRule> {
-        let pfctl_proto = as_pfctl_proto(allowed_endpoint.protocol);
+        let pfctl_proto = as_pfctl_proto(allowed_endpoint.endpoint.protocol);
 
-        self.create_rule_builder(FilterRuleAction::Pass)
-            .direction(pfctl::Direction::Out)
-            .to(allowed_endpoint.address)
+        let mut rule = self.create_rule_builder(FilterRuleAction::Pass);
+        rule.direction(pfctl::Direction::Out)
+            .to(allowed_endpoint.endpoint.address)
             .proto(pfctl_proto)
             .keep_state(pfctl::StatePolicy::Keep)
-            .user(Uid::from(super::ROOT_UID))
-            .quick(true)
-            .build()
+            .quick(true);
+
+        if !allowed_endpoint.clients.allow_all() {
+            rule.user(Uid::from(super::ROOT_UID)).build()?;
+        }
+
+        rule.build()
     }
 
     fn get_block_dns_rules(&self) -> Result<Vec<pfctl::FilterRule>> {
diff --git a/talpid-types/src/net/mod.rs b/talpid-types/src/net/mod.rs
index 6ff56175cc..cd8d5987a9 100644
--- a/talpid-types/src/net/mod.rs
+++ b/talpid-types/src/net/mod.rs
@@ -276,10 +276,10 @@ impl fmt::Display for Endpoint {
 /// Host that should be reachable in any tunnel state.
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub struct AllowedEndpoint {
-    /// Paths that should be allowed to communicate with `endpoint`.
-    #[cfg(windows)]
-    pub clients: Vec<PathBuf>,
+    /// How to connect to a certain `endpoint`.
     pub endpoint: Endpoint,
+    /// Clients that should be allowed to communicate with `endpoint`.
+    pub clients: AllowedClients,
 }
 
 impl fmt::Display for AllowedEndpoint {
@@ -288,23 +288,93 @@ impl fmt::Display for AllowedEndpoint {
         write!(f, "{}", self.endpoint)?;
         #[cfg(windows)]
         {
-            write!(f, "{} for", self.endpoint)?;
-            #[cfg(windows)]
-            for client in &self.clients {
-                write!(
-                    f,
-                    " {}",
-                    client
-                        .file_name()
-                        .map(|s| s.to_string_lossy())
-                        .unwrap_or(std::borrow::Cow::Borrowed("<UNKNOWN>"))
-                )?;
-            }
+            let clients = if self.clients.allow_all() {
+                "any executable".to_string()
+            } else {
+                self.clients
+                    .iter()
+                    .map(|client| {
+                        client
+                            .file_name()
+                            .map(|s| s.to_string_lossy())
+                            .unwrap_or(std::borrow::Cow::Borrowed("<UNKNOWN>"))
+                    })
+                    .collect::<Vec<_>>()
+                    .join(" ")
+            };
+            write!(
+                f,
+                "{endpoint} for {clients}",
+                endpoint = self.endpoint,
+                clients = clients
+            )?;
         }
         Ok(())
     }
 }
 
+/// Clients which should be able to reach an allowed host in any tunnel state.
+///
+/// # Note
+/// On Windows, there is no predetermined binary which should be allowed to leak
+/// traffic outside of the tunnel. Thus, [`std::default::Default`] is not
+/// implemented for [`AllowedClients`].
+#[cfg(windows)]
+#[derive(Debug, Clone, Eq, PartialEq)]
+pub struct AllowedClients(std::sync::Arc<[PathBuf]>);
+
+#[cfg(windows)]
+impl std::ops::Deref for AllowedClients {
+    type Target = [PathBuf];
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+#[cfg(windows)]
+impl From<Vec<PathBuf>> for AllowedClients {
+    fn from(value: Vec<PathBuf>) -> Self {
+        Self(value.into())
+    }
+}
+
+#[cfg(windows)]
+impl AllowedClients {
+    /// Allow all clients to leak traffic to an allowed [`Endpoint`].
+    pub fn all() -> Self {
+        vec![].into()
+    }
+
+    pub fn allow_all(&self) -> bool {
+        self.is_empty()
+    }
+}
+
+/// Clients which should be able to reach an allowed host in any tunnel state.
+#[cfg(unix)]
+#[derive(Debug, Clone, Eq, PartialEq)]
+pub enum AllowedClients {
+    /// Allow only clients running as `root` to leak traffic to an allowed [`Endpoint`].
+    ///
+    /// # Note
+    /// The most secure client(s) is our own, which runs as root.
+    Root,
+    /// Allow *all* clients to leak traffic to an allowed [`Endpoint`].
+    ///
+    /// This is necessary on platforms which does not have proper support for
+    /// split-tunneling, but which wants to support running local proxies which
+    /// may not run as root.
+    All,
+}
+
+#[cfg(unix)]
+impl AllowedClients {
+    pub fn allow_all(&self) -> bool {
+        matches!(self, AllowedClients::All)
+    }
+}
+
 #[derive(Debug, Clone, Eq, PartialEq, Hash)]
 pub enum AllowedTunnelTraffic {
     None,