Merge branch 'linux-prefer-netlink-wireguard' into master

7 files changed, 61 insertions, 68 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 38cb16bd10..c168fcb13e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -33,7 +33,7 @@ Line wrap the file at 100 chars.                                              Th
 - Add support for custom DNS resolvers (CLI only).
 
 #### Linux
-- Use NetworkManager to create a WireGuard interface.
+- Optionally use NetworkManager to create WireGuard devices.
 - Add support for custom DNS resolvers (CLI only).
 
 #### macOS
@@ -82,6 +82,7 @@ Line wrap the file at 100 chars.                                              Th
 - Fix routing rules sometimes being duplicated.
 - When NetworkManager is managing /etc/resolv.conf but ultimately using systemd-resolved, use
   systemd-resolved directly to manage DNS.
+- Only use WireGuard kernel implementation if DNS isn't managed via NetworkManager.
 
 
 ### Security
diff --git a/README.md b/README.md
index f1e1cdbf61..fcf17da5a2 100644
--- a/README.md
+++ b/README.md
@@ -364,6 +364,13 @@ echo "org.gradle.jvmargs=-Xmx4608M" >> ~/.gradle/gradle.properties
     * `"systemd"`: use systemd's `resolved` service through DBus
     * `"network-manager"`: use `NetworkManager` service through DBus
 
+* `TALPID_FORCE_USERSPACE_WIREGUARD` - Forces the daemon to use the userspace implementation of
+   WireGuard on Linux.
+
+* `TALPID_FORCE_NM_WIREGUARD` - Forces the daemon to use NetworkManager to create a WireGuard
+  device instead of relying on netlink. This is highly inadvisable currently, as NetworkManager is
+  exhibiting buggy behavior.
+
 
 ## Building and running the desktop Electron GUI app
 
diff --git a/talpid-core/src/dns/linux/mod.rs b/talpid-core/src/dns/linux/mod.rs
index b0293590bc..9eef807b71 100644
--- a/talpid-core/src/dns/linux/mod.rs
+++ b/talpid-core/src/dns/linux/mod.rs
@@ -104,7 +104,15 @@ impl DnsMonitorHolder {
     fn with_detected_dns_manager() -> Result<Self> {
         SystemdResolved::new()
             .map(DnsMonitorHolder::SystemdResolved)
-            .or_else(|_| NetworkManager::new().map(DnsMonitorHolder::NetworkManager))
+            .or_else(|err| {
+                match err {
+                    systemd_resolved::Error::NoSystemdResolved(_) => (),
+                    other_error => {
+                        log::debug!("systemd-resolved is not being used because {}", other_error)
+                    }
+                }
+                NetworkManager::new().map(DnsMonitorHolder::NetworkManager)
+            })
             .or_else(|_| Resolvconf::new().map(DnsMonitorHolder::Resolvconf))
             .or_else(|_| StaticResolvConf::new().map(DnsMonitorHolder::StaticResolvConf))
             .map_err(|_| Error::NoDnsMonitor)
@@ -138,3 +146,9 @@ impl DnsMonitorHolder {
         Ok(())
     }
 }
+
+/// Returns true if DnsMonitor will use NetworkManager to manage DNS.
+pub fn will_use_nm() -> bool {
+    crate::dns::imp::SystemdResolved::new().is_err()
+        && crate::dns::imp::NetworkManager::new().is_ok()
+}
diff --git a/talpid-core/src/dns/linux/systemd_resolved.rs b/talpid-core/src/dns/linux/systemd_resolved.rs
index 6abdd4990e..4150bb8a6a 100644
--- a/talpid-core/src/dns/linux/systemd_resolved.rs
+++ b/talpid-core/src/dns/linux/systemd_resolved.rs
@@ -85,32 +85,18 @@ pub struct SystemdResolved {
 
 impl SystemdResolved {
     pub fn new() -> Result<Self> {
-        let result = (|| {
-            let dbus_connection =
-                Connection::get_private(BusType::System).map_err(Error::ConnectDBus)?;
-            let systemd_resolved = SystemdResolved {
-                dbus_connection,
-                interface_link: None,
-            };
+        let dbus_connection =
+            Connection::get_private(BusType::System).map_err(Error::ConnectDBus)?;
+        let systemd_resolved = SystemdResolved {
+            dbus_connection,
+            interface_link: None,
+        };
 
-            systemd_resolved.ensure_resolved_exists()?;
-            if !super::network_manager::is_nm_managing_via_resolved(
-                &systemd_resolved.dbus_connection,
-            ) {
-                Self::ensure_resolv_conf_is_resolved_symlink()?;
-            }
-            Ok(systemd_resolved)
-        })();
-
-        match &result {
-            Ok(_) | Err(Error::NoSystemdResolved(_)) => (),
-            Err(error) => {
-                log::error!("systemd-resolved is not being used because: {}", error);
-            }
+        systemd_resolved.ensure_resolved_exists()?;
+        if !super::network_manager::is_nm_managing_via_resolved(&systemd_resolved.dbus_connection) {
+            Self::ensure_resolv_conf_is_resolved_symlink()?;
         }
-
-
-        result
+        Ok(systemd_resolved)
     }
 
     fn ensure_resolved_exists(&self) -> Result<()> {
diff --git a/talpid-core/src/dns/mod.rs b/talpid-core/src/dns/mod.rs
index 60d4a3d571..57872f9818 100644
--- a/talpid-core/src/dns/mod.rs
+++ b/talpid-core/src/dns/mod.rs
@@ -8,6 +8,9 @@ mod imp;
 #[path = "linux/mod.rs"]
 mod imp;
 
+#[cfg(target_os = "linux")]
+pub use imp::will_use_nm;
+
 #[cfg(windows)]
 #[path = "windows/mod.rs"]
 mod imp;
diff --git a/talpid-core/src/tunnel/wireguard/mod.rs b/talpid-core/src/tunnel/wireguard/mod.rs
index 30919c1b39..852e5b0996 100644
--- a/talpid-core/src/tunnel/wireguard/mod.rs
+++ b/talpid-core/src/tunnel/wireguard/mod.rs
@@ -65,6 +65,10 @@ lazy_static! {
     static ref FORCE_USERSPACE_WIREGUARD: bool = env::var("TALPID_FORCE_USERSPACE_WIREGUARD")
         .map(|v| v != "0")
         .unwrap_or(false);
+
+    static ref FORCE_NM_WIREGUARD: bool = env::var("TALPID_FORCE_NM_WIREGUARD")
+        .map(|v| v != "0")
+        .unwrap_or(false);
 }
 
 impl WireguardMonitor {
@@ -149,42 +153,30 @@ impl WireguardMonitor {
     ) -> Result<Box<dyn Tunnel>> {
         #[cfg(target_os = "linux")]
         if !*FORCE_USERSPACE_WIREGUARD {
-            match wireguard_kernel::NetworkManagerTunnel::new(
-                route_manager.runtime_handle(),
-                config,
-            ) {
-                Ok(tunnel) => {
+            if *FORCE_NM_WIREGUARD {
+                if let Ok(tunnel) = wireguard_kernel::NetworkManagerTunnel::new(
+                    route_manager.runtime_handle(),
+                    config,
+                ) {
                     log::debug!("Using NetworkManager to use kernel WireGuard implementation");
                     return Ok(Box::new(tunnel));
                 }
-                Err(err) => {
-                    if !err.should_use_userspace() {
-                        match wireguard_kernel::NetlinkTunnel::new(
-                            route_manager.runtime_handle(),
-                            config,
-                        ) {
-                            Ok(tunnel) => {
-                                log::debug!("Using kernel WireGuard implementation");
-                                return Ok(Box::new(tunnel));
-                            }
-                            Err(error) => {
-                                log::error!(
-                                    "{}",
-                                    error.display_chain_with_msg(
-                                        "Failed to setup kernel WireGuard device, falling back to userspace"
-                                    )
-                                );
-                            }
-                        };
+            } else if !crate::dns::will_use_nm() {
+                match wireguard_kernel::NetlinkTunnel::new(route_manager.runtime_handle(), config) {
+                    Ok(tunnel) => {
+                        log::debug!("Using kernel WireGuard implementation");
+                        return Ok(Box::new(tunnel));
                     }
-                    log::debug!(
-                        "{}",
-                        err.display_chain_with_msg(
-                            "Failed to create a WireGuard device via NetworkManager"
-                        )
-                    );
-                }
-            };
+                    Err(error) => {
+                        log::error!(
+                            "{}",
+                            error.display_chain_with_msg(
+                                "Failed to setup kernel WireGuard device, falling back to userspace"
+                            )
+                        );
+                    }
+                };
+            }
         } else {
             log::debug!("Using userspace WireGuard implementation");
         }
diff --git a/talpid-core/src/tunnel/wireguard/wireguard_kernel/mod.rs b/talpid-core/src/tunnel/wireguard/wireguard_kernel/mod.rs
index 284c93be13..6c3b86ee65 100644
--- a/talpid-core/src/tunnel/wireguard/wireguard_kernel/mod.rs
+++ b/talpid-core/src/tunnel/wireguard/wireguard_kernel/mod.rs
@@ -87,16 +87,6 @@ pub enum Error {
 
 const MULLVAD_INTERFACE_NAME: &str = "wg-mullvad";
 
-
-impl Error {
-    pub fn should_use_userspace(&self) -> bool {
-        match self {
-            Error::NetworkManager(nm_tunnel::Error::NMTooOld(_)) => true,
-            _ => false,
-        }
-    }
-}
-
 #[derive(Debug)]
 pub struct Handle {
     pub wg_handle: WireguardConnection,