Merge branch 'linux-remove-use-fwmark'

6 files changed, 48 insertions, 46 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7f70ad51bb..edaa39109c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -59,6 +59,10 @@ Line wrap the file at 100 chars.                                              Th
 #### Windows
 - Upgrade Wintun from 0.9.2 to 0.10.1.
 
+#### Linux
+- Only allow packets with the mark set to `0x6d6f6c65` to communicate with the relay server.
+  Previously, bridges were expected to run as root instead.
+
 ### Fixed
 - Fix delay in showing/hiding update notification when toggling beta program.
 - Improve responsiveness when reconnecting after some failed connection attempts.
diff --git a/mullvad-cli/src/cmds/bridge.rs b/mullvad-cli/src/cmds/bridge.rs
index d55e08a946..1e2a223817 100644
--- a/mullvad-cli/src/cmds/bridge.rs
+++ b/mullvad-cli/src/cmds/bridge.rs
@@ -66,31 +66,48 @@ fn create_bridge_set_subcommand() -> clap::App<'static, 'static> {
 
 
 fn create_set_custom_settings_subcommand() -> clap::App<'static, 'static> {
+    #[allow(unused_mut)]
+    let mut local_subcommand = clap::SubCommand::with_name("local")
+        .about("Registers a local SOCKS5 proxy")
+        .arg(
+            clap::Arg::with_name("local-port")
+                .help("Specifies the port the local proxy server is listening on")
+                .required(true)
+                .index(1),
+        )
+        .arg(
+            clap::Arg::with_name("remote-ip")
+                .help("Specifies the IP of the proxy server peer")
+                .required(true)
+                .index(2),
+        )
+        .arg(
+            clap::Arg::with_name("remote-port")
+                .help("Specifies the port of the proxy server peer")
+                .required(true)
+                .index(3),
+        );
+
+    #[cfg(target_os = "linux")]
+    {
+        local_subcommand = local_subcommand.about(
+            "Registers a local SOCKS5 proxy. The server must be excluded using \
+           'mullvad-exclude', or `SO_MARK` must be set to '0x6d6f6c65', in order \
+           to bypass firewall restrictions",
+        );
+    }
+    #[cfg(target_os = "macos")]
+    {
+        local_subcommand = local_subcommand.help(
+            "Registers a local SOCKS5 proxy. The server must run as root to bypass \
+            firewall restrictions",
+        );
+    }
+
     clap::SubCommand::with_name("custom")
         .about("Configure a SOCKS5 proxy")
         .setting(clap::AppSettings::SubcommandRequiredElseHelp)
-        .subcommand(
-            clap::SubCommand::with_name("local")
-                .about("Registers a local SOCKS5 proxy")
-                .arg(
-                    clap::Arg::with_name("local-port")
-                        .help("Specifies the port the local proxy server is listening on")
-                        .required(true)
-                        .index(1),
-                )
-                .arg(
-                    clap::Arg::with_name("remote-ip")
-                        .help("Specifies the IP of the proxy server peer")
-                        .required(true)
-                        .index(2),
-                )
-                .arg(
-                    clap::Arg::with_name("remote-port")
-                        .help("Specifies the port of the proxy server peer")
-                        .required(true)
-                        .index(3),
-                ),
-        )
+        .subcommand(local_subcommand)
         .subcommand(
             clap::SubCommand::with_name("remote")
                 .about("Registers a remote SOCKS5 proxy")
diff --git a/talpid-core/src/firewall/linux.rs b/talpid-core/src/firewall/linux.rs
index 95e7f5e46b..19aa94a54c 100644
--- a/talpid-core/src/firewall/linux.rs
+++ b/talpid-core/src/firewall/linux.rs
@@ -575,10 +575,9 @@ impl<'a> PolicyBatch<'a> {
                 pingable_hosts,
                 allow_lan,
                 allowed_endpoint,
-                use_fwmark,
             } => {
                 self.add_allow_icmp_pingable_hosts(&pingable_hosts);
-                self.add_allow_tunnel_endpoint_rules(peer_endpoint, *use_fwmark);
+                self.add_allow_tunnel_endpoint_rules(peer_endpoint);
                 self.add_allow_endpoint_rules(allowed_endpoint);
 
                 // Important to block DNS after allow relay rule (so the relay can operate
@@ -591,9 +590,8 @@ impl<'a> PolicyBatch<'a> {
                 tunnel,
                 allow_lan,
                 dns_servers,
-                use_fwmark,
             } => {
-                self.add_allow_tunnel_endpoint_rules(peer_endpoint, *use_fwmark);
+                self.add_allow_tunnel_endpoint_rules(peer_endpoint);
                 self.add_allow_dns_rules(tunnel, &dns_servers, TransportProtocol::Udp)?;
                 self.add_allow_dns_rules(tunnel, &dns_servers, TransportProtocol::Tcp)?;
                 // Important to block DNS *before* we allow the tunnel and allow LAN. So DNS
@@ -632,7 +630,7 @@ impl<'a> PolicyBatch<'a> {
         Ok(())
     }
 
-    fn add_allow_tunnel_endpoint_rules(&mut self, endpoint: &Endpoint, use_fwmark: bool) {
+    fn add_allow_tunnel_endpoint_rules(&mut self, endpoint: &Endpoint) {
         let mut prerouting_rule = Rule::new(&self.prerouting_chain);
         check_endpoint(&mut prerouting_rule, End::Src, endpoint);
         prerouting_rule.add_expr(&nft_expr!(immediate data crate::linux::TUNNEL_FW_MARK));
@@ -657,13 +655,8 @@ impl<'a> PolicyBatch<'a> {
 
         let mut out_rule = Rule::new(&self.out_chain);
         check_endpoint(&mut out_rule, End::Dst, endpoint);
-        if use_fwmark {
-            out_rule.add_expr(&nft_expr!(meta mark));
-            out_rule.add_expr(&nft_expr!(cmp == crate::linux::TUNNEL_FW_MARK));
-        } else {
-            out_rule.add_expr(&nft_expr!(meta skuid));
-            out_rule.add_expr(&nft_expr!(cmp == 0u32));
-        }
+        out_rule.add_expr(&nft_expr!(meta mark));
+        out_rule.add_expr(&nft_expr!(cmp == crate::linux::TUNNEL_FW_MARK));
         add_verdict(&mut out_rule, &Verdict::Accept);
 
         self.batch.add(&out_rule, nftnl::MsgType::Add);
diff --git a/talpid-core/src/firewall/mod.rs b/talpid-core/src/firewall/mod.rs
index 83a112ce88..1e782e2e6a 100644
--- a/talpid-core/src/firewall/mod.rs
+++ b/talpid-core/src/firewall/mod.rs
@@ -112,10 +112,6 @@ pub enum FirewallPolicy {
         /// A process that is allowed to send packets to the relay.
         #[cfg(windows)]
         relay_client: PathBuf,
-        /// Whether rule for allowing traffic to endpoint should match a firewall mark or match on
-        /// root UID.
-        #[cfg(target_os = "linux")]
-        use_fwmark: bool,
     },
 
     /// Allow traffic only to server and over tunnel interface
@@ -132,10 +128,6 @@ pub enum FirewallPolicy {
         /// A process that is allowed to send packets to the relay.
         #[cfg(windows)]
         relay_client: PathBuf,
-        /// Whether rule for allowing traffic to endpoint should match a firewall mark or match on
-        /// root UID.
-        #[cfg(target_os = "linux")]
-        use_fwmark: bool,
     },
 
     /// Block all network traffic in and out from the computer.
diff --git a/talpid-core/src/tunnel_state_machine/connected_state.rs b/talpid-core/src/tunnel_state_machine/connected_state.rs
index 2f1d53fd9a..dd2102127e 100644
--- a/talpid-core/src/tunnel_state_machine/connected_state.rs
+++ b/talpid-core/src/tunnel_state_machine/connected_state.rs
@@ -113,8 +113,6 @@ impl ConnectedState {
                 &shared_values.resource_dir,
                 &self.tunnel_parameters,
             ),
-            #[cfg(target_os = "linux")]
-            use_fwmark: self.tunnel_parameters.get_proxy_endpoint().is_none(),
         }
     }
 
diff --git a/talpid-core/src/tunnel_state_machine/connecting_state.rs b/talpid-core/src/tunnel_state_machine/connecting_state.rs
index 3416fd6095..f9f4a00764 100644
--- a/talpid-core/src/tunnel_state_machine/connecting_state.rs
+++ b/talpid-core/src/tunnel_state_machine/connecting_state.rs
@@ -69,8 +69,6 @@ impl ConnectingState {
             allowed_endpoint: shared_values.allowed_endpoint.clone(),
             #[cfg(windows)]
             relay_client: TunnelMonitor::get_relay_client(&shared_values.resource_dir, &params),
-            #[cfg(target_os = "linux")]
-            use_fwmark: params.get_proxy_endpoint().is_none(),
         };
         shared_values
             .firewall