Remove fwmark based routing for linux

4 files changed, 65 insertions, 254 deletions

diff --git a/talpid-core/src/routing/linux.rs b/talpid-core/src/routing/linux.rs
index da256a849f..4c4712ac1e 100644
--- a/talpid-core/src/routing/linux.rs
+++ b/talpid-core/src/routing/linux.rs
@@ -1,4 +1,4 @@
-use super::{NetNode, RequiredRoutes};
+use super::{NetNode, RequiredRoutes, Route};
 
 use super::subprocess::{Exec, RunExpr};
 use std::{collections::HashSet, net::IpAddr};
@@ -12,261 +12,75 @@ error_chain! {
         FailedToRemoveRoute {
             description("Failed to remove route")
         }
-
-        FailedToRemoveTable {
-            description("Failed to remove table")
-        }
-
-        FailedToAdjustMainRoutingTable {
-            description("Failed to adjust main routing table")
-        }
-
-        FailedToSetRuleForFwmark {
-            description("Failed to set rule for fwmark")
-        }
-        NoDefaultRoute {
-            description("No default route")
-        }
-
         FailedToGetDefaultRoute {
             description("Failed to get default route")
         }
     }
 }
 
-#[derive(Hash, Eq, PartialEq)]
-enum IpVersion {
-    V4,
-    V6,
-}
-
-impl IpVersion {
-    fn new(ip: IpAddr) -> Self {
-        if ip.is_ipv4() {
-            IpVersion::V4
-        } else {
-            IpVersion::V6
-        }
-    }
-
-    fn is_ipv4(&self) -> bool {
-        match self {
-            IpVersion::V4 => true,
-            _ => false,
-        }
-    }
-}
-
-impl From<IpAddr> for IpVersion {
-    fn from(ip: IpAddr) -> IpVersion {
-        Self::new(ip)
-    }
-}
-
-impl AsRef<str> for IpVersion {
-    fn as_ref(&self) -> &str {
-        match self {
-            IpVersion::V4 => "-4",
-            IpVersion::V6 => "-6",
-        }
-    }
-}
-
-// A record of a table being set by a RouteManager.
-#[derive(Hash, Eq, PartialEq)]
-struct Table {
-    version: IpVersion,
-    fwmark: String,
-}
 
 pub struct RouteManager {
     added_routes: HashSet<super::Route>,
-    added_tables: HashSet<Table>,
-    // the main routing table only has to be adjusted for default routes
-    main_table_suppress_by_prefix_set_v4: bool,
-    main_table_suppress_by_prefix_set_v6: bool,
 }
 
 impl RouteManager {
-    // This function adjusts main routing table to not make any routing decisions based on rules
-    // with a prefix of 0. This is to bypass the main table for default routes.
-    fn set_suppress_prefix_length_on_main_routing_table(
-        &mut self,
-        version: IpVersion,
-        set_rule: bool,
-    ) -> Result<()> {
-        if (version.is_ipv4() && (set_rule == self.main_table_suppress_by_prefix_set_v4))
-            || (!version.is_ipv4() && (set_rule == self.main_table_suppress_by_prefix_set_v6))
-        {
-            return Ok(());
-        }
-        duct::cmd!(
-            "ip",
-            version.as_ref(),
-            "rule",
-            if set_rule { "add" } else { "delete" },
-            "table",
-            "main",
-            "suppress_prefixlength",
-            "0"
-        )
-        .run_expr()
-        .chain_err(|| ErrorKind::FailedToAdjustMainRoutingTable)?;
-        if version.is_ipv4() {
-            self.main_table_suppress_by_prefix_set_v4 = set_rule;
-        } else {
-            self.main_table_suppress_by_prefix_set_v6 = set_rule;
-        }
-        Ok(())
-    }
-
-    fn add_route(&mut self, route: super::Route, fwmark: &Option<String>) -> Result<()> {
+    fn add_route(&mut self, route: super::Route) -> Result<()> {
         if route.prefix.prefix() == 0 {
-            self.set_suppress_prefix_length_on_main_routing_table(route.prefix.ip().into(), true)?;
+            return if route.prefix.is_ipv4() {
+                self.add_route(Route::new("0.0.0.0/1".parse().unwrap(), route.node.clone()))?;
+                self.add_route(Route::new(
+                    "128.0.0.0/1".parse().unwrap(),
+                    route.node.clone(),
+                ))
+            } else {
+                self.add_route(Route::new("::/1".parse().unwrap(), route.node.clone()))?;
+                self.add_route(Route::new("8000::/1".parse().unwrap(), route.node.clone()))
+            };
         }
 
-        let version = IpVersion::new(route.prefix.ip());
-
-        let mut cmd = Exec::cmd("ip")
-            .arg(version.as_ref())
-            .arg("route")
-            .arg("add")
-            .arg(route.prefix.to_string());
-        cmd = match &route.node {
-            NetNode::Address(ref addr) => cmd.arg(addr.to_string()),
-            NetNode::Device(ref device) => cmd.arg("dev").arg(device),
-        };
-
-        if let Some(ref fwmark) = &fwmark {
-            cmd = cmd.arg("table").arg(fwmark);
-        }
+        let cmd = Self::add_route_cmd(&route);
 
         cmd.into_expr()
             .run_expr()
             .chain_err(|| ErrorKind::FailedToAddRoute)?;
 
-        if let Some(fwmark) = &fwmark {
-            self.ensure_table_rules(Table {
-                version,
-                fwmark: fwmark.to_string(),
-            })?;
-        } else {
-            self.added_routes.insert(route);
-        }
-        Ok(())
-    }
-
-    // if a route we're applying is set to a specific table, that table should have it's rules set
-    fn ensure_table_rules(&mut self, added_table: Table) -> Result<()> {
-        if self.added_tables.contains(&added_table) {
-            return Ok(());
-        }
-        duct::cmd!(
-            "ip",
-            added_table.version.as_ref(),
-            "rule",
-            "add",
-            "not",
-            "fwmark",
-            &added_table.fwmark,
-            "table",
-            &added_table.fwmark
-        )
-        .run_expr()
-        .chain_err(|| ErrorKind::FailedToSetRuleForFwmark)?;
-
-
-        self.added_tables.insert(added_table);
+        self.added_routes.insert(route);
         Ok(())
     }
 
-    fn clear_routes(&mut self) -> Result<()> {
-        let mut end_result = Ok(());
-        for route in self.added_routes.drain() {
-            let ip_vers: IpVersion = route.prefix.ip().into();
-            let result = duct::cmd!(
-                "ip",
-                ip_vers.as_ref(),
-                "route",
-                "delete",
-                route.prefix.to_string()
-            )
-            .run_expr()
-            .chain_err(|| ErrorKind::FailedToRemoveRoute);
-            if let Err(e) = result {
-                log::error!("Failed to remove route {} - {}", route.prefix, e);
-                end_result = Err(e);
-            }
+    fn add_route_cmd(route: &Route) -> Exec {
+        let cmd = Exec::cmd("ip")
+            .arg(ip_vers(&route))
+            .arg("route")
+            .arg("add")
+            .arg(route.prefix.to_string());
+        match &route.node {
+            NetNode::Address(ref addr) => cmd.arg("via").arg(addr.to_string()),
+            NetNode::Device(ref device) => cmd.arg("dev").arg(device),
         }
-        end_result
     }
+}
 
-    fn clear_tables(&mut self) -> Result<()> {
-        let mut end_result = Ok(());
-        for table in self.added_tables.drain() {
-            let result = duct::cmd!(
-                "ip",
-                table.version.as_ref(),
-                "rule",
-                "delete",
-                "table",
-                &table.fwmark
-            )
-            .run_expr()
-            .chain_err(|| ErrorKind::FailedToRemoveTable);
-
-            if let Err(e) = result {
-                log::error!("Failed to remove routing table {} - {}", &table.fwmark, e);
-                end_result = Err(e);
-            }
-        }
-
-        if self.main_table_suppress_by_prefix_set_v4 {
-            if let Err(e) =
-                self.set_suppress_prefix_length_on_main_routing_table(IpVersion::V4, false)
-            {
-                log::error!(
-                    "Failed to remove prefix limit for main routing table - {}",
-                    e
-                );
-                end_result = Err(e);
-            } else {
-                self.main_table_suppress_by_prefix_set_v4 = false;
-            }
-        }
-
-        if self.main_table_suppress_by_prefix_set_v6 {
-            if let Err(e) =
-                self.set_suppress_prefix_length_on_main_routing_table(IpVersion::V6, false)
-            {
-                log::error!(
-                    "Failed to remove prefix limit for main routing table - {}",
-                    e
-                );
-                end_result = Err(e);
-            } else {
-                self.main_table_suppress_by_prefix_set_v6 = false;
-            }
-        }
-        end_result
+fn ip_vers(route: &Route) -> &'static str {
+    if route.prefix.is_ipv4() {
+        "-4"
+    } else {
+        "-6"
     }
 }
 
+
 impl super::RoutingT for RouteManager {
     type Error = Error;
     fn new() -> Result<Self> {
         Ok(RouteManager {
             added_routes: HashSet::new(),
-            added_tables: HashSet::new(),
-            // the main routing table only has to be adjusted for default routes
-            main_table_suppress_by_prefix_set_v4: false,
-            main_table_suppress_by_prefix_set_v6: false,
         })
     }
 
     fn add_routes(&mut self, required_routes: RequiredRoutes) -> Result<()> {
         for route in required_routes.routes.into_iter() {
-            if let Err(e) = self.add_route(route, &required_routes.fwmark) {
+            if let Err(e) = self.add_route(route) {
                 let _ = self.delete_routes();
                 return Err(e);
             }
@@ -275,9 +89,23 @@ impl super::RoutingT for RouteManager {
     }
 
     fn delete_routes(&mut self) -> Result<()> {
-        let result = self.clear_routes();
-        let other_result = self.clear_tables();
-        result.and_then(|_| other_result)
+        let mut end_result = Ok(());
+        for route in self.added_routes.drain() {
+            let result = duct::cmd!(
+                "ip",
+                ip_vers(&route),
+                "route",
+                "delete",
+                route.prefix.to_string()
+            )
+            .run_expr()
+            .chain_err(|| ErrorKind::FailedToRemoveRoute);
+            if let Err(e) = result {
+                log::error!("Failed to remove route {} - {}", route.prefix, e);
+                end_result = Err(e);
+            }
+        }
+        end_result
     }
 
     /// Retrieves the gateway for the default route
diff --git a/talpid-core/src/routing/mod.rs b/talpid-core/src/routing/mod.rs
index 68df1872b6..50385a436f 100644
--- a/talpid-core/src/routing/mod.rs
+++ b/talpid-core/src/routing/mod.rs
@@ -41,10 +41,6 @@ pub enum NetNode {
 pub struct RequiredRoutes {
     /// List of routes to be applied to the routing table.
     pub routes: Vec<Route>,
-    /// Optionally apply the routes to a specific table and only apply routes when a firewall mark
-    /// is not used. Currently only used on Linux.
-    #[cfg(target_os = "linux")]
-    pub fwmark: Option<String>,
 }
 
 /// Manages adding and removing routes from the routing table.
@@ -72,6 +68,7 @@ impl RouteManager {
 
     /// Retrieves the gateway for the default route.
     pub fn get_default_route_node(&mut self) -> Result<std::net::IpAddr, imp::Error> {
+        // use routing::RoutingT;
         self.inner.get_default_route_node()
     }
 }
diff --git a/talpid-core/src/tunnel/wireguard/config.rs b/talpid-core/src/tunnel/wireguard/config.rs
index e04a45618d..97d7832a60 100644
--- a/talpid-core/src/tunnel/wireguard/config.rs
+++ b/talpid-core/src/tunnel/wireguard/config.rs
@@ -11,8 +11,6 @@ pub struct Config {
     pub ipv4_gateway: Ipv4Addr,
     pub ipv6_gateway: Option<Ipv6Addr>,
     pub mtu: u16,
-    #[cfg(target_os = "linux")]
-    pub fwmark: i32,
 }
 
 /// Smallest MTU that supports IPv6
@@ -90,8 +88,6 @@ impl Config {
                 None
             },
             mtu,
-            #[cfg(target_os = "linux")]
-            fwmark: wg_options.fwmark,
         })
     }
 
@@ -103,11 +99,6 @@ impl Config {
             .add("private_key", self.tunnel.private_key.as_bytes().as_ref())
             .add("listen_port", "0");
 
-        #[cfg(target_os = "linux")]
-        {
-            wg_conf.add("fwmark", self.fwmark.to_string().as_str());
-        }
-
         wg_conf.add("replace_peers", "true");
 
         for peer in &self.peers {
diff --git a/talpid-core/src/tunnel/wireguard/mod.rs b/talpid-core/src/tunnel/wireguard/mod.rs
index 925552ad5f..2533d2034a 100644
--- a/talpid-core/src/tunnel/wireguard/mod.rs
+++ b/talpid-core/src/tunnel/wireguard/mod.rs
@@ -119,30 +119,25 @@ impl WireguardMonitor {
             })
             .collect();
 
-        if cfg!(target_os = "macos") {
-            // To survive network roaming on osx, we should listen for new routes and reapply them
-            // here - probably would need RouteManager be extended. Or maybe RouteManager can deal
-            // with it on it's own
-            let default_node = self
-                .router
-                .get_default_route_node()
-                .chain_err(|| ErrorKind::SetupRoutingError)?;
+        // To survive network roaming, we should listen for new routes and reapply them
+        // here - probably would need RouteManager be extended. Or maybe RouteManager can deal
+        // with it on it's own
+        let default_node = self
+            .router
+            .get_default_route_node()
+            .chain_err(|| ErrorKind::SetupRoutingError)?;
 
-            // route endpoints with specific routes
-            for peer in config.peers.iter() {
-                let default_route = routing::Route::new(
-                    peer.endpoint.ip().into(),
-                    routing::NetNode::Address(default_node),
-                );
-                routes.push(default_route);
-            }
+        // route endpoints with specific routes
+        for peer in config.peers.iter() {
+            let default_route = routing::Route::new(
+                peer.endpoint.ip().into(),
+                routing::NetNode::Address(default_node),
+            );
+            routes.push(default_route);
         }
 
-        let required_routes = routing::RequiredRoutes {
-            routes,
-            #[cfg(target_os = "linux")]
-            fwmark: Some(config.fwmark.to_string()),
-        };
+        let required_routes = routing::RequiredRoutes { routes };
+
         self.router
             .add_routes(required_routes)
             .chain_err(|| ErrorKind::SetupRoutingError)