Merge branch 'wintun-alias-fix'

12 files changed, 143 insertions, 60 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 93fa7ade1d..236800d178 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -49,6 +49,9 @@ Line wrap the file at 100 chars.                                              Th
 - Ignore failure to  add IPv6 split-tunneling routing rules when they fail due to IPv6 being
   unavailable.
 
+#### Windows
+- Fix failure when Wintun adapter name conflicts with that of a non-Wintun adapter.
+
 
 ## [2021.1] - 2021-02-10
 This release is for desktop only.
diff --git a/talpid-core/src/tunnel/openvpn/mod.rs b/talpid-core/src/tunnel/openvpn/mod.rs
index 8f6766cf37..7fdde6cdf0 100644
--- a/talpid-core/src/tunnel/openvpn/mod.rs
+++ b/talpid-core/src/tunnel/openvpn/mod.rs
@@ -30,7 +30,12 @@ use std::{
 #[cfg(target_os = "linux")]
 use std::{collections::HashSet, net::IpAddr};
 #[cfg(windows)]
-use std::{ffi::OsStr, os::windows::ffi::OsStrExt, sync::Mutex, time::Instant};
+use std::{
+    ffi::{OsStr, OsString},
+    os::windows::ffi::OsStrExt,
+    sync::Mutex,
+    time::Instant,
+};
 use talpid_types::net::openvpn;
 #[cfg(any(windows, target_os = "linux"))]
 use talpid_types::ErrorExt;
@@ -48,7 +53,7 @@ use winapi::shared::{
         GetUnicastIpAddressTable, MIB_UNICASTIPADDRESS_ROW, MIB_UNICASTIPADDRESS_TABLE,
     },
     nldef::{IpDadStatePreferred, IpDadStateTentative, NL_DAD_STATE},
-    winerror::{ERROR_FILE_NOT_FOUND, NO_ERROR},
+    winerror::NO_ERROR,
     ws2def::AF_UNSPEC,
 };
 #[cfg(windows)]
@@ -132,7 +137,12 @@ pub enum Error {
     #[error(display = "Failed to create Wintun adapter")]
     WintunError(#[error(source)] io::Error),
 
-    /// cannot create a wintun interface
+    /// cannot determine adapter name
+    #[cfg(windows)]
+    #[error(display = "Failed to determine alias of Wintun adapter")]
+    WintunFindAlias(#[error(source)] io::Error),
+
+    /// cannot delete wintun interface
     #[cfg(windows)]
     #[error(display = "Failed to delete existing Wintun adapter")]
     WintunDeleteExistingError(#[error(source)] io::Error),
@@ -354,36 +364,18 @@ impl OpenVpnMonitor<OpenVpnCommand> {
 
         let proxy_monitor = Self::start_proxy(&params.proxy, &proxy_resources)?;
 
-        let cmd = Self::create_openvpn_cmd(
-            params,
-            user_pass_file.as_ref(),
-            match proxy_auth_file {
-                Some(ref file) => Some(file.as_ref()),
-                _ => None,
-            },
-            resource_dir,
-            &proxy_monitor,
-        )?;
-
-        let plugin_path = Self::get_plugin_path(resource_dir)?;
-
         #[cfg(windows)]
         let wintun_adapter = {
             let dll = get_wintun_dll(resource_dir)?;
 
             {
-                match windows::WintunAdapter::open(dll.clone(), &*ADAPTER_ALIAS, &*ADAPTER_POOL) {
-                    Ok(adapter) => {
-                        // Delete existing adapter in case it has residual config
-                        adapter
-                            .delete(false)
-                            .map_err(Error::WintunDeleteExistingError)?;
-                    }
-                    Err(error) => {
-                        if error.raw_os_error() != Some(ERROR_FILE_NOT_FOUND as i32) {
-                            return Err(Error::WintunError(error));
-                        }
-                    }
+                if let Ok(adapter) =
+                    windows::WintunAdapter::open(dll.clone(), &*ADAPTER_ALIAS, &*ADAPTER_POOL)
+                {
+                    // Delete existing adapter in case it has residual config
+                    adapter
+                        .delete(false)
+                        .map_err(Error::WintunDeleteExistingError)?;
                 }
             }
 
@@ -434,6 +426,26 @@ impl OpenVpnMonitor<OpenVpnCommand> {
             adapter
         };
 
+        #[cfg(windows)]
+        let adapter_alias = wintun_adapter
+            .adapter()
+            .name()
+            .map_err(Error::WintunFindAlias)?;
+        #[cfg(windows)]
+        log::debug!("Adapter alias: {}", adapter_alias.to_string_lossy());
+
+        let cmd = Self::create_openvpn_cmd(
+            params,
+            user_pass_file.as_ref(),
+            proxy_auth_file.as_ref().map(AsRef::as_ref),
+            resource_dir,
+            &proxy_monitor,
+            #[cfg(windows)]
+            adapter_alias.to_os_string(),
+        )?;
+
+        let plugin_path = Self::get_plugin_path(resource_dir)?;
+
         Self::new_internal(
             cmd,
             on_openvpn_event,
@@ -738,6 +750,7 @@ impl<C: OpenVpnBuilder + 'static> OpenVpnMonitor<C> {
         proxy_auth_file: Option<&Path>,
         resource_dir: &Path,
         proxy_monitor: &Option<Box<dyn ProxyMonitor>>,
+        #[cfg(windows)] alias: OsString,
     ) -> Result<OpenVpnCommand> {
         let mut cmd = OpenVpnCommand::new(Self::get_openvpn_bin(resource_dir)?);
         if let Some(config) = Self::get_config_path(resource_dir) {
@@ -752,7 +765,7 @@ impl<C: OpenVpnBuilder + 'static> OpenVpnMonitor<C> {
             .ca(resource_dir.join("ca.crt"));
         #[cfg(windows)]
         {
-            cmd.tunnel_alias(Some(ADAPTER_ALIAS.to_os_string()));
+            cmd.tunnel_alias(Some(alias));
             cmd.windows_driver(Some(crate::process::openvpn::WindowsDriver::Wintun));
         }
         if let Some(proxy_settings) = params.proxy.clone().take() {
diff --git a/talpid-core/src/tunnel/openvpn/windows.rs b/talpid-core/src/tunnel/openvpn/windows.rs
index f87efc3967..ae150a1117 100644
--- a/talpid-core/src/tunnel/openvpn/windows.rs
+++ b/talpid-core/src/tunnel/openvpn/windows.rs
@@ -7,7 +7,7 @@ use std::{
     sync::Arc,
 };
 use talpid_types::ErrorExt;
-use widestring::U16CStr;
+use widestring::{U16CStr, U16CString};
 use winapi::{
     shared::{
         guiddef::GUID,
@@ -23,6 +23,9 @@ use winapi::{
 use winreg::{enums::HKEY_LOCAL_MACHINE, RegKey};
 
 
+/// Longest possible adapter name (in characters), including null terminator
+const MAX_ADAPTER_NAME: usize = 128;
+
 type WintunOpenAdapterFn =
     unsafe extern "stdcall" fn(pool: *const u16, name: *const u16) -> RawHandle;
 
@@ -41,6 +44,9 @@ type WintunDeleteAdapterFn = unsafe extern "stdcall" fn(
     reboot_required: *mut BOOL,
 ) -> BOOL;
 
+type WintunGetAdapterNameFn =
+    unsafe extern "stdcall" fn(adapter: RawHandle, name: *mut u16) -> BOOL;
+
 
 pub struct WintunDll {
     handle: HINSTANCE,
@@ -48,6 +54,7 @@ pub struct WintunDll {
     func_create: WintunCreateAdapterFn,
     func_free: WintunFreeAdapterFn,
     func_delete: WintunDeleteAdapterFn,
+    func_get_adapter_name: WintunGetAdapterNameFn,
 }
 
 unsafe impl Send for WintunDll {}
@@ -72,6 +79,10 @@ impl TemporaryWintunAdapter {
             WintunAdapter::create(dll_handle, pool, name, requested_guid)?;
         Ok((TemporaryWintunAdapter { adapter }, reboot_required))
     }
+
+    pub fn adapter(&self) -> &WintunAdapter {
+        &self.adapter
+    }
 }
 
 impl Drop for TemporaryWintunAdapter {
@@ -129,6 +140,10 @@ impl WintunAdapter {
                 .delete_adapter(self.handle, force_close_sessions)
         }
     }
+
+    pub fn name(&self) -> io::Result<U16CString> {
+        unsafe { self.dll_handle.get_adapter_name(self.handle) }
+    }
 }
 
 impl Drop for WintunAdapter {
@@ -157,32 +172,45 @@ impl WintunDll {
             return Err(io::Error::last_os_error());
         }
 
+        Self::new_inner(handle, Self::get_proc_address)
+    }
+
+    fn new_inner(
+        handle: HMODULE,
+        get_proc_fn: unsafe fn(HMODULE, &CStr) -> io::Result<FARPROC>,
+    ) -> io::Result<Self> {
         Ok(WintunDll {
             handle,
             func_open: unsafe {
-                std::mem::transmute(Self::get_proc_address(
+                std::mem::transmute(get_proc_fn(
                     handle,
                     CStr::from_bytes_with_nul(b"WintunOpenAdapter\0").unwrap(),
                 )?)
             },
             func_create: unsafe {
-                std::mem::transmute(Self::get_proc_address(
+                std::mem::transmute(get_proc_fn(
                     handle,
                     CStr::from_bytes_with_nul(b"WintunCreateAdapter\0").unwrap(),
                 )?)
             },
             func_delete: unsafe {
-                std::mem::transmute(Self::get_proc_address(
+                std::mem::transmute(get_proc_fn(
                     handle,
                     CStr::from_bytes_with_nul(b"WintunDeleteAdapter\0").unwrap(),
                 )?)
             },
             func_free: unsafe {
-                std::mem::transmute(Self::get_proc_address(
+                std::mem::transmute(get_proc_fn(
                     handle,
                     CStr::from_bytes_with_nul(b"WintunFreeAdapter\0").unwrap(),
                 )?)
             },
+            func_get_adapter_name: unsafe {
+                std::mem::transmute(get_proc_fn(
+                    handle,
+                    CStr::from_bytes_with_nul(b"WintunGetAdapterName\0").unwrap(),
+                )?)
+            },
         })
     }
 
@@ -239,6 +267,16 @@ impl WintunDll {
     pub unsafe fn free_adapter(&self, adapter: RawHandle) {
         (self.func_free)(adapter);
     }
+
+    pub unsafe fn get_adapter_name(&self, adapter: RawHandle) -> io::Result<U16CString> {
+        let mut alias_buffer = vec![0u16; MAX_ADAPTER_NAME];
+        let result = (self.func_get_adapter_name)(adapter, alias_buffer.as_mut_ptr());
+        if result == 0 {
+            return Err(io::Error::last_os_error());
+        }
+        Ok(U16CString::from_vec_with_nul(alias_buffer)
+            .map_err(|_| io::Error::new(io::ErrorKind::Other, "missing null terminator"))?)
+    }
 }
 
 impl Drop for WintunDll {
@@ -291,3 +329,17 @@ pub fn find_adapter_registry_key(find_guid: &str, permissions: REGSAM) -> io::Re
 
     Err(io::Error::new(io::ErrorKind::NotFound, "device not found"))
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn get_proc_fn(_handle: HMODULE, _symbol: &CStr) -> io::Result<FARPROC> {
+        Ok(std::ptr::null_mut())
+    }
+
+    #[test]
+    fn test_wintun_imports() {
+        WintunDll::new_inner(ptr::null_mut(), get_proc_fn).unwrap();
+    }
+}
diff --git a/talpid-core/src/tunnel/wireguard/mod.rs b/talpid-core/src/tunnel/wireguard/mod.rs
index 5e9cd6704e..9fe6bec4e7 100644
--- a/talpid-core/src/tunnel/wireguard/mod.rs
+++ b/talpid-core/src/tunnel/wireguard/mod.rs
@@ -389,6 +389,11 @@ pub enum TunnelError {
     #[error(display = "Invalid tunnel interface name")]
     InterfaceNameError(#[error(source)] std::ffi::NulError),
 
+    /// Failed to convert adapter alias to UTF-8.
+    #[cfg(target_os = "windows")]
+    #[error(display = "Failed to convert adapter alias")]
+    InvalidAlias,
+
     /// Failed to set ip addresses on tunnel interface.
     #[cfg(target_os = "windows")]
     #[error(display = "Failed to set IP addresses on WireGuard interface")]
diff --git a/talpid-core/src/tunnel/wireguard/wireguard_go.rs b/talpid-core/src/tunnel/wireguard/wireguard_go.rs
index 58b77ab47a..2751fbdfbf 100644
--- a/talpid-core/src/tunnel/wireguard/wireguard_go.rs
+++ b/talpid-core/src/tunnel/wireguard/wireguard_go.rs
@@ -133,12 +133,15 @@ impl WgGoTunnel {
                 .iter()
                 .any(|config| config.allowed_ips.iter().any(|ip| ip.is_ipv6()));
 
+        let mut alias_ptr = std::ptr::null_mut();
+
         let handle = unsafe {
             wgTurnOn(
                 cstr_iface_name.as_ptr(),
                 config.mtu as i64,
                 wait_on_ipv6 as u8,
                 wg_config_str.as_ptr(),
+                &mut alias_ptr,
                 Some(logging_callback),
                 logging_context.0 as *mut libc::c_void,
             )
@@ -148,13 +151,25 @@ impl WgGoTunnel {
             return Err(TunnelError::FatalStartWireguardError);
         }
 
-        if !add_device_ip_addresses(&iface_name, &config.tunnel.addresses) {
+        let actual_iface_name = {
+            let actual_iface_name_c = unsafe { CStr::from_ptr(alias_ptr) };
+            let actual_iface_name = actual_iface_name_c
+                .to_str()
+                .map_err(|_| TunnelError::InvalidAlias)?
+                .to_string();
+            unsafe { wgFreePtr(alias_ptr as *mut c_void) };
+            actual_iface_name
+        };
+
+        log::debug!("Adapter alias: {}", actual_iface_name);
+
+        if !add_device_ip_addresses(&actual_iface_name, &config.tunnel.addresses) {
             // Todo: what kind of clean-up is required?
             return Err(TunnelError::SetIpAddressesError);
         }
 
         Ok(WgGoTunnel {
-            interface_name: iface_name.clone(),
+            interface_name: actual_iface_name,
             handle: Some(handle),
             _logging_context: logging_context,
         })
@@ -360,6 +375,7 @@ extern "C" {
         mtu: i64,
         wait_on_ipv6: u8,
         settings: *const i8,
+        iface_name_out: *const *mut std::os::raw::c_char,
         logging_callback: Option<LoggingCallback>,
         logging_context: *mut libc::c_void,
     ) -> i32;
diff --git a/windows/libshared/src/libshared/network/interfaceutils.cpp b/windows/libshared/src/libshared/network/interfaceutils.cpp
index b5b65d3d61..28263f383b 100644
--- a/windows/libshared/src/libshared/network/interfaceutils.cpp
+++ b/windows/libshared/src/libshared/network/interfaceutils.cpp
@@ -5,18 +5,6 @@
 #include <libcommon/error.h>
 #include <libcommon/string.h>
 
-namespace
-{
-
-// Interface description substrings found for virtual adapters.
-const wchar_t *TUNNEL_INTERFACE_DESCS[] = {
-	L"WireGuard",
-	L"Wintun",
-	L"Mullvad"
-};
-
-} // anonymous namespace
-
 namespace shared::network
 {
 
diff --git a/wireguard/libwg/interfacewatcher/interfacewatcher_windows.go b/wireguard/libwg/interfacewatcher/interfacewatcher_windows.go
index 8242ab0ff5..fcb53b8584 100644
--- a/wireguard/libwg/interfacewatcher/interfacewatcher_windows.go
+++ b/wireguard/libwg/interfacewatcher/interfacewatcher_windows.go
@@ -1,7 +1,7 @@
 /* SPDX-License-Identifier: MIT
  *
  * Copyright (C) 2019 WireGuard LLC. All Rights Reserved.
- * Copyright (C) 2020 Mullvad VPN AB. All Rights Reserved.
+ * Copyright (C) 2021 Mullvad VPN AB. All Rights Reserved.
  */
 
 package interfacewatcher
diff --git a/wireguard/libwg/libwg.go b/wireguard/libwg/libwg.go
index 153ddd94e9..521dee38df 100644
--- a/wireguard/libwg/libwg.go
+++ b/wireguard/libwg/libwg.go
@@ -1,7 +1,7 @@
 /* SPDX-License-Identifier: Apache-2.0
  *
  * Copyright (C) 2017-2019 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
- * Copyright (C) 2020 Mullvad VPN AB. All Rights Reserved.
+ * Copyright (C) 2021 Mullvad VPN AB. All Rights Reserved.
  */
 
 package main
diff --git a/wireguard/libwg/libwg_android.go b/wireguard/libwg/libwg_android.go
index 6ba61c660c..d26f824065 100644
--- a/wireguard/libwg/libwg_android.go
+++ b/wireguard/libwg/libwg_android.go
@@ -1,7 +1,7 @@
 /* SPDX-License-Identifier: Apache-2.0
  *
  * Copyright (C) 2017-2019 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
- * Copyright (C) 2020 Mullvad VPN AB. All Rights Reserved.
+ * Copyright (C) 2021 Mullvad VPN AB. All Rights Reserved.
  */
 
 package main
diff --git a/wireguard/libwg/libwg_default.go b/wireguard/libwg/libwg_default.go
index 0cceee2ff8..6b553a86b7 100644
--- a/wireguard/libwg/libwg_default.go
+++ b/wireguard/libwg/libwg_default.go
@@ -4,7 +4,7 @@
 /* SPDX-License-Identifier: Apache-2.0
  *
  * Copyright (C) 2017-2019 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
- * Copyright (C) 2020 Mullvad VPN AB. All Rights Reserved.
+ * Copyright (C) 2021 Mullvad VPN AB. All Rights Reserved.
  */
 
 package main
diff --git a/wireguard/libwg/libwg_windows.go b/wireguard/libwg/libwg_windows.go
index 0718caeb6c..3bbaee7b6a 100644
--- a/wireguard/libwg/libwg_windows.go
+++ b/wireguard/libwg/libwg_windows.go
@@ -1,13 +1,15 @@
 /* SPDX-License-Identifier: Apache-2.0
  *
  * Copyright (C) 2017-2019 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
- * Copyright (C) 2020 Mullvad VPN AB. All Rights Reserved.
+ * Copyright (C) 2021 Mullvad VPN AB. All Rights Reserved.
  */
 
 package main
 
+// #include <stdlib.h>
+import "C"
+
 import (
-	"C"
 	"bufio"
 	"fmt"
 	"strings"
@@ -64,8 +66,11 @@ func createInterfaceWatcherEvents(waitOnIpv6 bool, tunLuid uint64) []interfacewa
 }
 
 //export wgTurnOn
-func wgTurnOn(cIfaceName *C.char, mtu int, waitOnIpv6 bool, cSettings *C.char, logSink LogSink, logContext LogContext) int32 {
+func wgTurnOn(cIfaceName *C.char, mtu int, waitOnIpv6 bool, cSettings *C.char, cIfaceNameOut **C.char, logSink LogSink, logContext LogContext) int32 {
 	logger := logging.NewLogger(logSink, logContext)
+	if cIfaceNameOut != nil {
+		*cIfaceNameOut = nil
+	}
 
 	if cIfaceName == nil {
 		logger.Error.Println("cIfaceName is null")
@@ -109,13 +114,10 @@ func wgTurnOn(cIfaceName *C.char, mtu int, waitOnIpv6 bool, cSettings *C.char, l
 		logger.Error.Println("Failed to determine name of wintun adapter")
 		return ERROR_GENERAL_FAILURE
 	}
-
 	if actualInterfaceName != ifaceName {
 		// WireGuard picked a different name for the adapter than the one we expected.
 		// This indicates there is already an adapter with the name we intended to use.
-		nativeTun.Close()
-		logger.Error.Println("Failed to create adapter with specific name")
-		return ERROR_GENERAL_FAILURE
+		logger.Debug.Println("Failed to create adapter with specific name")
 	}
 
 	device := device.NewDevice(wintun, logger)
@@ -154,6 +156,10 @@ func wgTurnOn(cIfaceName *C.char, mtu int, waitOnIpv6 bool, cSettings *C.char, l
 		return ERROR_GENERAL_FAILURE
 	}
 
+	if cIfaceNameOut != nil {
+		*cIfaceNameOut = C.CString(actualInterfaceName)
+	}
+
 	return handle
 }
 
diff --git a/wireguard/libwg/logging/logging.go b/wireguard/libwg/logging/logging.go
index 643419bb46..8d831379fb 100644
--- a/wireguard/libwg/logging/logging.go
+++ b/wireguard/libwg/logging/logging.go
@@ -1,7 +1,7 @@
 /* SPDX-License-Identifier: Apache-2.0
  *
  * Copyright (C) 2017-2019 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
- * Copyright (C) 2020 Mullvad VPN AB. All Rights Reserved.
+ * Copyright (C) 2021 Mullvad VPN AB. All Rights Reserved.
  */
 
 package logging