From 77d41767571f6e1b0d3f096c67410932dd1db90d Mon Sep 17 00:00:00 2001 From: Markus Pettersson Date: Wed, 18 Oct 2023 11:50:54 +0200 Subject: [PATCH 1/2] Add configurable transport protocol to local SOCKS5 access method --- mullvad-api/src/lib.rs | 6 +-- mullvad-api/src/proxy.rs | 39 ++++++++------- mullvad-api/src/rest.rs | 10 +++- mullvad-cli/src/cmds/api_access.rs | 48 ++++++++++++++++--- mullvad-daemon/src/api.rs | 13 ++--- mullvad-daemon/src/lib.rs | 5 +- .../proto/management_interface.proto | 3 +- .../src/types/conversions/access_method.rs | 25 ++++++---- mullvad-types/src/access_method.rs | 15 +++++- 9 files changed, 113 insertions(+), 51 deletions(-) diff --git a/mullvad-api/src/lib.rs b/mullvad-api/src/lib.rs index 91e2bc524acc..267fdfd8114e 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::ErrorExt; +use talpid_types::{net::Endpoint, 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(SocketAddr) -> Self::AcceptedNewEndpoint { +pub trait ApiEndpointUpdateCallback: Fn(Endpoint) -> Self::AcceptedNewEndpoint { type AcceptedNewEndpoint: Future + Send; } impl + Send> ApiEndpointUpdateCallback for U where - U: Fn(SocketAddr) -> T, + U: Fn(Endpoint) -> T, { type AcceptedNewEndpoint = T; } diff --git a/mullvad-api/src/proxy.rs b/mullvad-api/src/proxy.rs index 3193fdc19b53..07276285969e 100644 --- a/mullvad-api/src/proxy.rs +++ b/mullvad-api/src/proxy.rs @@ -4,12 +4,14 @@ use mullvad_types::access_method; use serde::{Deserialize, Serialize}; use std::{ fmt, io, - net::SocketAddr, path::Path, pin::Pin, task::{self, Poll}, }; -use talpid_types::ErrorExt; +use talpid_types::{ + net::{Endpoint, TransportProtocol}, + ErrorExt, +}; use tokio::{ fs, io::{AsyncRead, AsyncWrite, AsyncWriteExt, ReadBuf}, @@ -41,13 +43,17 @@ pub enum ProxyConfig { } impl ProxyConfig { - /// Returns the remote address to reach the proxy. - fn get_endpoint(&self) -> SocketAddr { + /// Returns the remote endpoint describing how to reach the proxy. + fn get_endpoint(&self) -> Endpoint { match self { - ProxyConfig::Shadowsocks(ss) => ss.peer, + ProxyConfig::Shadowsocks(shadowsocks) => { + Endpoint::from_socket_address(shadowsocks.peer, TransportProtocol::Tcp) + } ProxyConfig::Socks(socks) => match socks { - access_method::Socks5::Local(s) => s.remote_peer, - access_method::Socks5::Remote(s) => s.peer, + access_method::Socks5::Local(local) => local.remote_endpoint, + access_method::Socks5::Remote(remote) => { + Endpoint::from_socket_address(remote.peer, TransportProtocol::Tcp) + } }, } } @@ -55,18 +61,14 @@ impl ProxyConfig { impl fmt::Display for ProxyConfig { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> { + let endpoint = self.get_endpoint(); match self { - // TODO: Do not hardcode TCP - ProxyConfig::Shadowsocks(ss) => write!(f, "Shadowsocks {}/TCP", ss.peer), + ProxyConfig::Shadowsocks(_) => write!(f, "Shadowsocks {}", endpoint), ProxyConfig::Socks(socks) => match socks { - access_method::Socks5::Local(s) => { - write!( - f, - "Socks5 {}/TCP via localhost:{}", - s.remote_peer, s.local_port - ) + access_method::Socks5::Remote(_) => write!(f, "Socks5 {}", endpoint), + access_method::Socks5::Local(local) => { + write!(f, "Socks5 {} via localhost:{}", endpoint, local.local_port) } - access_method::Socks5::Remote(s) => write!(f, "Socks5 {}/TCP", s.peer), }, } } @@ -132,8 +134,9 @@ impl ApiConnectionMode { } } - /// Returns the remote address required to reach the API, or `None` for `ApiConnectionMode::Direct`. - pub fn get_endpoint(&self) -> Option { + /// Returns the remote endpoint required to reach the API, or `None` for + /// `ApiConnectionMode::Direct`. + pub fn get_endpoint(&self) -> Option { match self { ApiConnectionMode::Direct => None, ApiConnectionMode::Proxied(proxy_config) => Some(proxy_config.get_endpoint()), diff --git a/mullvad-api/src/rest.rs b/mullvad-api/src/rest.rs index 63c909507cea..a973c32f2a75 100644 --- a/mullvad-api/src/rest.rs +++ b/mullvad-api/src/rest.rs @@ -24,7 +24,10 @@ use std::{ sync::{Arc, Weak}, time::Duration, }; -use talpid_types::ErrorExt; +use talpid_types::{ + net::{Endpoint, TransportProtocol}, + ErrorExt, +}; #[cfg(feature = "api-override")] use crate::API; @@ -209,7 +212,10 @@ impl< if let Some(new_config) = self.proxy_config_provider.next().await { let endpoint = match new_config.get_endpoint() { Some(endpoint) => endpoint, - None => self.address_cache.get_address().await, + None => Endpoint::from_socket_address( + self.address_cache.get_address().await, + TransportProtocol::Tcp, + ), }; // Switch to new connection mode unless rejected by address change callback if (self.new_address_callback)(endpoint).await { diff --git a/mullvad-cli/src/cmds/api_access.rs b/mullvad-cli/src/cmds/api_access.rs index 55c4a9b51626..9ad481c4efa6 100644 --- a/mullvad-cli/src/cmds/api_access.rs +++ b/mullvad-cli/src/cmds/api_access.rs @@ -4,7 +4,7 @@ use mullvad_types::access_method::{AccessMethod, AccessMethodSetting, CustomAcce use std::net::IpAddr; use clap::{Args, Subcommand}; -use talpid_types::net::openvpn::SHADOWSOCKS_CIPHERS; +use talpid_types::net::{openvpn::SHADOWSOCKS_CIPHERS, TransportProtocol}; #[derive(Subcommand, Debug, Clone)] pub enum ApiAccess { @@ -118,10 +118,21 @@ impl ApiAccess { } CustomAccessMethod::Socks5(socks) => match socks { Socks5::Local(local) => { - let remote_ip = cmd.params.ip.unwrap_or(local.remote_peer.ip()); - let remote_port = cmd.params.port.unwrap_or(local.remote_peer.port()); + let remote_ip = cmd.params.ip.unwrap_or(local.remote_endpoint.address.ip()); + let remote_port = cmd + .params + .port + .unwrap_or(local.remote_endpoint.address.port()); let local_port = cmd.params.local_port.unwrap_or(local.local_port); - AccessMethod::from(Socks5Local::new((remote_ip, remote_port), local_port)) + let remote_peer_transport_protocol = cmd + .params + .transport_protocol + .unwrap_or(local.remote_endpoint.protocol); + AccessMethod::from(Socks5Local::new_with_transport_protocol( + (remote_ip, remote_port), + local_port, + remote_peer_transport_protocol, + )) } Socks5::Remote(remote) => { let ip = cmd.params.ip.unwrap_or(remote.peer.ip()); @@ -306,6 +317,14 @@ pub enum AddSocks5Commands { remote_ip: IpAddr, /// The port of the remote peer remote_port: u16, + /// The Mullvad App can not know which transport protocol that the + /// remote peer accepts, but it needs to know this in order to correctly + /// exempt the connection traffic in the firewall. + /// + /// By default, the transport protocol is assumed to be `TCP`, but it + /// can optionally be set to `UDP` as well. + #[arg(long, default_value_t = TransportProtocol::Tcp)] + transport_protocol: TransportProtocol, /// Disable the use of this custom access method. It has to be manually /// enabled at a later stage to be used when accessing the Mullvad API. #[arg(default_value_t = false, short, long)] @@ -398,6 +417,9 @@ pub struct EditParams { /// The port that the server on localhost is listening on [Socks5 (Local proxy)] #[arg(long)] local_port: Option, + /// The transport protocol used by the remote proxy [Socks5 (Local proxy)] + #[arg(long)] + transport_protocol: Option, } /// Implement conversions from CLI types to Daemon types. @@ -418,9 +440,15 @@ mod conversions { remote_port, name: _, disabled: _, + transport_protocol, } => { - println!("Adding SOCKS5-proxy: localhost:{local_port} => {remote_ip}:{remote_port}"); - daemon_types::Socks5Local::new((remote_ip, remote_port), local_port).into() + println!("Adding SOCKS5-proxy: localhost:{local_port} => {remote_ip}:{remote_port}/{transport_protocol}"); + daemon_types::Socks5Local::new_with_transport_protocol( + (remote_ip, remote_port), + local_port, + transport_protocol, + ) + .into() } AddSocks5Commands::Remote { remote_ip, @@ -559,7 +587,13 @@ mod pp { } writeln!(f)?; print_option!("Protocol", "Socks5 (local)"); - print_option!("Peer", local.remote_peer); + print_option!( + "Peer", + format!( + "{}/{}", + local.remote_endpoint.address, local.remote_endpoint.protocol + ) + ); print_option!("Local port", local.local_port); Ok(()) } diff --git a/mullvad-daemon/src/api.rs b/mullvad-daemon/src/api.rs index c548f0a293c4..a6dd0dead66b 100644 --- a/mullvad-daemon/src/api.rs +++ b/mullvad-daemon/src/api.rs @@ -12,7 +12,6 @@ use mullvad_api::{ use mullvad_relay_selector::RelaySelector; use mullvad_types::access_method::{AccessMethod, AccessMethodSetting, BuiltInAccessMethod}; use std::{ - net::SocketAddr, path::PathBuf, pin::Pin, sync::{Arc, Mutex, Weak}, @@ -22,7 +21,7 @@ use std::{ use talpid_core::mpsc::Sender; use talpid_core::tunnel_state_machine::TunnelCommand; use talpid_types::{ - net::{openvpn::ProxySettings, AllowedEndpoint, Endpoint, TransportProtocol}, + net::{openvpn::ProxySettings, AllowedEndpoint, Endpoint}, ErrorExt, }; @@ -240,7 +239,7 @@ impl ApiEndpointUpdaterHandle { pub fn callback(&self) -> impl ApiEndpointUpdateCallback { let tunnel_tx = self.tunnel_cmd_tx.clone(); - move |address: SocketAddr| { + move |endpoint: Endpoint| { let inner_tx = tunnel_tx.clone(); async move { let tunnel_tx = if let Some(tunnel_tx) = { inner_tx.lock().unwrap().as_ref() } @@ -253,21 +252,19 @@ impl ApiEndpointUpdaterHandle { }; let (result_tx, result_rx) = oneshot::channel(); let _ = tunnel_tx.unbounded_send(TunnelCommand::AllowEndpoint( - get_allowed_endpoint(address), + get_allowed_endpoint(endpoint), result_tx, )); // Wait for the firewall policy to be updated. let _ = result_rx.await; - log::debug!("API endpoint: {}", address); + log::debug!("API endpoint: {endpoint}"); true } } } } -pub(super) fn get_allowed_endpoint(api_address: SocketAddr) -> AllowedEndpoint { - let endpoint = Endpoint::from_socket_address(api_address, TransportProtocol::Tcp); - +pub(super) fn get_allowed_endpoint(endpoint: Endpoint) -> AllowedEndpoint { #[cfg(windows)] let daemon_exe = std::env::current_exe().expect("failed to obtain executable path"); #[cfg(windows)] diff --git a/mullvad-daemon/src/lib.rs b/mullvad-daemon/src/lib.rs index 8ee8e58fd676..77304ae75f8b 100644 --- a/mullvad-daemon/src/lib.rs +++ b/mullvad-daemon/src/lib.rs @@ -726,7 +726,10 @@ where }; let initial_api_endpoint = - api::get_allowed_endpoint(api_runtime.address_cache.get_address().await); + api::get_allowed_endpoint(talpid_types::net::Endpoint::from_socket_address( + api_runtime.address_cache.get_address().await, + talpid_types::net::TransportProtocol::Tcp, + )); let parameters_generator = tunnel::ParametersGenerator::new( account_manager.clone(), relay_selector.clone(), diff --git a/mullvad-management-interface/proto/management_interface.proto b/mullvad-management-interface/proto/management_interface.proto index 057718bdb137..0eefd9a1db53 100644 --- a/mullvad-management-interface/proto/management_interface.proto +++ b/mullvad-management-interface/proto/management_interface.proto @@ -335,7 +335,8 @@ message AccessMethod { message Socks5Local { string remote_ip = 1; uint32 remote_port = 2; - uint32 local_port = 3; + TransportProtocol remote_transport_protocol = 3; + uint32 local_port = 4; } message SocksAuth { string username = 1; diff --git a/mullvad-management-interface/src/types/conversions/access_method.rs b/mullvad-management-interface/src/types/conversions/access_method.rs index b4a4547fdf93..425e0a70da86 100644 --- a/mullvad-management-interface/src/types/conversions/access_method.rs +++ b/mullvad-management-interface/src/types/conversions/access_method.rs @@ -144,15 +144,19 @@ mod data { type Error = FromProtobufTypeError; fn try_from(value: proto::access_method::Socks5Local) -> Result { + use crate::types::conversions::net::try_transport_protocol_from_i32; let remote_ip = value.remote_ip.parse::().map_err(|_| { FromProtobufTypeError::InvalidArgument( "Could not parse Socks5 (local) message from protobuf", ) })?; - Ok(AccessMethod::from(Socks5Local::new( - (remote_ip, value.remote_port as u16), - value.local_port as u16, - ))) + Ok(AccessMethod::from( + Socks5Local::new_with_transport_protocol( + (remote_ip, value.remote_port as u16), + value.local_port as u16, + try_transport_protocol_from_i32(value.remote_transport_protocol)?, + ), + )) } } @@ -229,13 +233,16 @@ mod data { ) } CustomAccessMethod::Socks5(Socks5::Local(Socks5Local { - remote_peer: peer, - local_port: port, + remote_endpoint, + local_port, })) => proto::access_method::AccessMethod::Socks5local( proto::access_method::Socks5Local { - remote_ip: peer.ip().to_string(), - remote_port: peer.port() as u32, - local_port: port as u32, + remote_ip: remote_endpoint.address.ip().to_string(), + remote_port: remote_endpoint.address.port() as u32, + remote_transport_protocol: i32::from(proto::TransportProtocol::from( + remote_endpoint.protocol, + )), + local_port: local_port as u32, }, ), CustomAccessMethod::Socks5(Socks5::Remote(Socks5Remote { diff --git a/mullvad-types/src/access_method.rs b/mullvad-types/src/access_method.rs index f4a76392fa3c..b714a79edd05 100644 --- a/mullvad-types/src/access_method.rs +++ b/mullvad-types/src/access_method.rs @@ -2,6 +2,7 @@ use std::str::FromStr; use serde::{Deserialize, Serialize}; use std::net::SocketAddr; +use talpid_types::net::{Endpoint, TransportProtocol}; /// Daemon settings for API access methods. #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] @@ -205,7 +206,7 @@ pub struct Shadowsocks { #[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq, Hash)] pub struct Socks5Local { - pub remote_peer: SocketAddr, + pub remote_endpoint: Endpoint, /// Port on localhost where the SOCKS5-proxy listens to. pub local_port: u16, } @@ -252,8 +253,18 @@ impl Shadowsocks { impl Socks5Local { pub fn new>(remote_peer: I, local_port: u16) -> Self { + let transport_protocol = TransportProtocol::Tcp; + Self::new_with_transport_protocol(remote_peer, local_port, transport_protocol) + } + + pub fn new_with_transport_protocol>( + remote_peer: I, + local_port: u16, + transport_protocol: TransportProtocol, + ) -> Self { + let remote_endpoint = Endpoint::from_socket_address(remote_peer.into(), transport_protocol); Self { - remote_peer: remote_peer.into(), + remote_endpoint, local_port, } } From 288a29de4d1ba2c0771afa37b64c39f727b13706 Mon Sep 17 00:00:00 2001 From: Markus Pettersson Date: Tue, 24 Oct 2023 15:36:39 +0200 Subject: [PATCH 2/2] 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. --- mullvad-api/src/lib.rs | 6 +- mullvad-api/src/proxy.rs | 32 +++++++++- mullvad-api/src/rest.rs | 6 +- mullvad-daemon/src/api.rs | 37 ++++++----- talpid-core/src/firewall/linux.rs | 24 ++++--- talpid-core/src/firewall/macos.rs | 30 +++++---- talpid-types/src/net/mod.rs | 100 +++++++++++++++++++++++++----- 7 files changed, 174 insertions(+), 61 deletions(-) diff --git a/mullvad-api/src/lib.rs b/mullvad-api/src/lib.rs index 267fdfd8114e..5a436fe1bf7f 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 + Send; } impl + 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 07276285969e..783cc8a157e7 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 a973c32f2a75..2484bec64b01 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 a6dd0dead66b..3ac526e688f4 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 1b74fb30bd80..a74aa1ce4a9c 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 4f95309890ca..54f72a79c26a 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 { - 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> { diff --git a/talpid-types/src/net/mod.rs b/talpid-types/src/net/mod.rs index 6ff56175cc65..cd8d5987a912 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, + /// 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("")) - )?; - } + 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("")) + }) + .collect::>() + .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> for AllowedClients { + fn from(value: Vec) -> 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,