Move tunnel interface metric update to talpid-core

9 files changed, 74 insertions, 110 deletions

diff --git a/talpid-core/src/firewall/windows.rs b/talpid-core/src/firewall/windows.rs
index 471e464c0a..db0060f25d 100644
--- a/talpid-core/src/firewall/windows.rs
+++ b/talpid-core/src/firewall/windows.rs
@@ -4,7 +4,6 @@ use std::{net::IpAddr, path::Path, ptr};
 
 use self::winfw::*;
 use super::{FirewallArguments, FirewallPolicy, InitialFirewallState};
-use crate::winnet;
 use talpid_types::{
     net::{AllowedEndpoint, AllowedTunnelTraffic, Endpoint},
     tunnel::FirewallPolicyError,
@@ -38,10 +37,6 @@ pub enum Error {
     /// Failure to reset firewall policies
     #[error(display = "Failed to reset firewall policies")]
     ResettingPolicy(#[error(source)] FirewallPolicyError),
-
-    /// Failure to set virtual adapter metric
-    #[error(display = "Unable to set virtual adapter metric")]
-    SetTunMetric(#[error(source)] crate::winnet::Error),
 }
 
 /// Timeout for acquiring the WFP transaction lock
@@ -230,15 +225,6 @@ impl Firewall {
             protocol: WinFwProt::from(endpoint.protocol),
         };
 
-        let metrics_set = winnet::ensure_best_metric_for_interface(&tunnel_metadata.interface)
-            .map_err(Error::SetTunMetric)?;
-
-        if metrics_set {
-            log::debug!("Network interface metrics were changed");
-        } else {
-            log::debug!("Network interface metrics were not changed");
-        }
-
         let v6_gateway_ptr = match &v6_gateway {
             Some(v6_ip) => v6_ip.as_ptr(),
             None => ptr::null(),
diff --git a/talpid-core/src/tunnel/mod.rs b/talpid-core/src/tunnel/mod.rs
index b4e6170220..8f07843f71 100644
--- a/talpid-core/src/tunnel/mod.rs
+++ b/talpid-core/src/tunnel/mod.rs
@@ -13,6 +13,9 @@ use talpid_types::net::{wireguard as wireguard_types, AllowedTunnelTraffic, Tunn
 #[cfg(target_os = "android")]
 pub use self::tun_provider::TunConfig;
 
+#[cfg(target_os = "windows")]
+mod windows;
+
 /// A module for all OpenVPN related tunnel management.
 #[cfg(not(target_os = "android"))]
 pub mod openvpn;
diff --git a/talpid-core/src/tunnel/openvpn/mod.rs b/talpid-core/src/tunnel/openvpn/mod.rs
index 9fdfb3e80b..9c09c935f8 100644
--- a/talpid-core/src/tunnel/openvpn/mod.rs
+++ b/talpid-core/src/tunnel/openvpn/mod.rs
@@ -197,7 +197,7 @@ trait WintunContext: Send + Sync {
     fn luid(&self) -> NET_LUID;
     fn ipv6(&self) -> bool;
     async fn wait_for_interfaces(&self) -> io::Result<()>;
-    fn disable_unused_features(&self) {}
+    fn prepare_interface(&self) {}
 }
 
 #[cfg(windows)]
@@ -236,8 +236,8 @@ impl WintunContext for WintunContextImpl {
         crate::windows::wait_for_interfaces(luid, true, self.wait_v6_interface).await
     }
 
-    fn disable_unused_features(&self) {
-        self.adapter.adapter().try_disable_unused_features();
+    fn prepare_interface(&self) {
+        self.adapter.adapter().prepare_interface();
     }
 }
 
@@ -464,7 +464,7 @@ impl<C: OpenVpnBuilder + Send + 'static> OpenVpnMonitor<C> {
         {
             log::debug!("Wait for IP interfaces");
             wintun.wait_for_interfaces().await?;
-            wintun.disable_unused_features();
+            wintun.prepare_interface();
         }
         cmd.start()
     }
diff --git a/talpid-core/src/tunnel/openvpn/wintun.rs b/talpid-core/src/tunnel/openvpn/wintun.rs
index 1746756db4..813d0de2e7 100644
--- a/talpid-core/src/tunnel/openvpn/wintun.rs
+++ b/talpid-core/src/tunnel/openvpn/wintun.rs
@@ -1,6 +1,4 @@
-use crate::windows::{
-    get_ip_interface_entry, set_ip_interface_entry, string_from_guid, AddressFamily,
-};
+use crate::windows::string_from_guid;
 use lazy_static::lazy_static;
 use std::{
     ffi::CStr,
@@ -18,8 +16,6 @@ use winapi::{
         ifdef::NET_LUID,
         minwindef::{BOOL, FARPROC, HINSTANCE, HMODULE},
         netioapi::ConvertInterfaceLuidToGuid,
-        nldef::RouterDiscoveryDisabled,
-        ntdef::FALSE,
         winerror::NO_ERROR,
     },
     um::{
@@ -186,25 +182,12 @@ impl WintunAdapter {
         Ok((adapter, restart_required))
     }
 
-    pub fn try_disable_unused_features(&self) {
-        // Disable DAD, DHCP, and router discovery
-        let luid = self.luid();
-        for family in &[AddressFamily::Ipv4, AddressFamily::Ipv6] {
-            if let Ok(mut row) = get_ip_interface_entry(*family, &luid) {
-                row.SitePrefixLength = 0;
-                row.RouterDiscoveryBehavior = RouterDiscoveryDisabled;
-                row.DadTransmits = 0;
-                row.ManagedAddressConfigurationSupported = FALSE;
-                row.OtherStatefulConfigurationSupported = FALSE;
-
-                if let Err(error) = set_ip_interface_entry(&row) {
-                    log::error!(
-                        "{} (family: {})",
-                        error.display_chain_with_msg("Failed to update Wintun interface"),
-                        family,
-                    );
-                }
-            }
+    pub fn prepare_interface(&self) {
+        if let Err(error) = crate::tunnel::windows::initialize_interfaces(self.luid(), None) {
+            log::error!(
+                "{}",
+                error.display_chain_with_msg("Failed to set tunnel interface metric"),
+            );
         }
     }
 
diff --git a/talpid-core/src/tunnel/windows.rs b/talpid-core/src/tunnel/windows.rs
new file mode 100644
index 0000000000..cf4871904f
--- /dev/null
+++ b/talpid-core/src/tunnel/windows.rs
@@ -0,0 +1,36 @@
+use crate::windows::{get_ip_interface_entry, set_ip_interface_entry, AddressFamily};
+use std::io;
+use winapi::shared::{
+    ifdef::NET_LUID, nldef::RouterDiscoveryDisabled, ntdef::FALSE, winerror::ERROR_NOT_FOUND,
+};
+
+/// Sets MTU, metric, and disables unnecessary features for the IP interfaces
+/// on the specified network interface (identified by `luid`).
+pub fn initialize_interfaces(luid: NET_LUID, mtu: Option<u32>) -> io::Result<()> {
+    for family in &[AddressFamily::Ipv4, AddressFamily::Ipv6] {
+        let mut row = match get_ip_interface_entry(*family, &luid) {
+            Ok(row) => row,
+            Err(error) if error.raw_os_error() == Some(ERROR_NOT_FOUND as i32) => continue,
+            Err(error) => return Err(error),
+        };
+
+        if let Some(mtu) = mtu {
+            row.NlMtu = mtu;
+        }
+
+        // Disable DAD, DHCP, and router discovery
+        row.SitePrefixLength = 0;
+        row.RouterDiscoveryBehavior = RouterDiscoveryDisabled;
+        row.DadTransmits = 0;
+        row.ManagedAddressConfigurationSupported = FALSE;
+        row.OtherStatefulConfigurationSupported = FALSE;
+
+        // Ensure lowest interface metric
+        row.Metric = 1;
+        row.UseAutomaticMetric = FALSE;
+
+        set_ip_interface_entry(&mut row)?;
+    }
+
+    Ok(())
+}
diff --git a/talpid-core/src/tunnel/wireguard/wireguard_go.rs b/talpid-core/src/tunnel/wireguard/wireguard_go.rs
index 4fbdcefcdd..5673406982 100644
--- a/talpid-core/src/tunnel/wireguard/wireguard_go.rs
+++ b/talpid-core/src/tunnel/wireguard/wireguard_go.rs
@@ -119,6 +119,8 @@ impl WgGoTunnel {
         log_path: Option<&Path>,
         mut done_tx: futures::channel::mpsc::Sender<std::result::Result<(), BoxedError>>,
     ) -> Result<Self> {
+        use talpid_types::ErrorExt;
+
         let route_callback_handle = winnet::add_default_route_change_callback(
             Some(WgGoTunnel::default_route_changed_callback),
             (),
@@ -171,9 +173,23 @@ impl WgGoTunnel {
                 Value: interface_luid,
             };
             log::debug!("Waiting for tunnel IP interfaces to arrive");
+
+            let prepare_interfaces = async move {
+                crate::windows::wait_for_interfaces(luid, true, has_ipv6).await?;
+
+                if let Err(error) = crate::tunnel::windows::initialize_interfaces(luid, None) {
+                    log::error!(
+                        "{}",
+                        error.display_chain_with_msg("Failed to set tunnel interface metric"),
+                    );
+                }
+
+                Ok(())
+            };
+
             let _ = done_tx
                 .send(
-                    crate::windows::wait_for_interfaces(luid, true, has_ipv6)
+                    prepare_interfaces
                         .await
                         .map_err(|error| BoxedError::new(TunnelError::SetupIpInterfaces(error))),
                 )
diff --git a/talpid-core/src/tunnel/wireguard/wireguard_nt.rs b/talpid-core/src/tunnel/wireguard/wireguard_nt.rs
index 0ae469144b..113f6534c0 100644
--- a/talpid-core/src/tunnel/wireguard/wireguard_nt.rs
+++ b/talpid-core/src/tunnel/wireguard/wireguard_nt.rs
@@ -31,8 +31,6 @@ use winapi::{
         in6addr::IN6_ADDR,
         inaddr::IN_ADDR,
         minwindef::{BOOL, FARPROC, HINSTANCE, HMODULE},
-        nldef::RouterDiscoveryDisabled,
-        ntdef::FALSE,
         winerror::ERROR_MORE_DATA,
         ws2def::{ADDRESS_FAMILY, AF_INET, AF_INET6},
         ws2ipdef::SOCKADDR_INET,
@@ -139,13 +137,9 @@ pub enum Error {
     #[error(display = "Failed to wait on tunnel IP interfaces")]
     IpInterfacesError(#[error(source)] io::Error),
 
-    /// Failed to set MTU on tunnel device
-    #[error(display = "Failed to set tunnel IPv4 interface MTU")]
-    SetTunnelIpv4MtuError(#[error(source)] io::Error),
-
-    /// Failed to set MTU on tunnel device
-    #[error(display = "Failed to set tunnel IPv6 interface MTU")]
-    SetTunnelIpv6MtuError(#[error(source)] io::Error),
+    /// Failed to set MTU and metric on tunnel device
+    #[error(display = "Failed to set tunnel interface MTU")]
+    SetTunnelMtuError(#[error(source)] io::Error),
 
     /// Failed to set the tunnel state to up
     #[error(display = "Failed to enable the tunnel adapter")]
@@ -483,10 +477,8 @@ async fn setup_ip_listener(
         .map_err(Error::IpInterfacesError)?;
     log::debug!("Waiting for tunnel IP interfaces: Done");
 
-    prepare_interface(&luid, AF_INET as u16, mtu).map_err(Error::SetTunnelIpv4MtuError)?;
-    if has_ipv6 {
-        prepare_interface(&luid, AF_INET6 as u16, mtu).map_err(Error::SetTunnelIpv6MtuError)?;
-    }
+    crate::tunnel::windows::initialize_interfaces(luid, Some(mtu))
+        .map_err(Error::SetTunnelMtuError)?;
 
     if let Some(device) = &*device.lock().unwrap() {
         device
@@ -939,19 +931,6 @@ unsafe fn deserialize_config(
     Ok((interface, peers))
 }
 
-fn prepare_interface(luid: &NET_LUID, family: u16, mtu: u32) -> io::Result<()> {
-    let family = windows::AddressFamily::try_from_af_family(family)
-        .map_err(|error| io::Error::new(io::ErrorKind::InvalidInput, error))?;
-    let mut iface = windows::get_ip_interface_entry(family, luid)?;
-    iface.SitePrefixLength = 0;
-    iface.NlMtu = mtu;
-    iface.RouterDiscoveryBehavior = RouterDiscoveryDisabled;
-    iface.DadTransmits = 0;
-    iface.ManagedAddressConfigurationSupported = FALSE;
-    iface.OtherStatefulConfigurationSupported = FALSE;
-    windows::set_ip_interface_entry(&iface)
-}
-
 impl Tunnel for WgNtTunnel {
     fn get_interface_name(&self) -> String {
         self.interface_name.clone()
diff --git a/talpid-core/src/windows/mod.rs b/talpid-core/src/windows/mod.rs
index 115d8f5397..ae15c9cb9c 100644
--- a/talpid-core/src/windows/mod.rs
+++ b/talpid-core/src/windows/mod.rs
@@ -194,8 +194,8 @@ pub fn get_ip_interface_entry(
 }
 
 /// Set the properties of an IP interface.
-pub fn set_ip_interface_entry(row: &MIB_IPINTERFACE_ROW) -> io::Result<()> {
-    let result = unsafe { SetIpInterfaceEntry(row as *const _ as *mut _) };
+pub fn set_ip_interface_entry(row: &mut MIB_IPINTERFACE_ROW) -> io::Result<()> {
+    let result = unsafe { SetIpInterfaceEntry(row as *mut _) };
     if result == NO_ERROR {
         Ok(())
     } else {
diff --git a/talpid-core/src/winnet.rs b/talpid-core/src/winnet.rs
index af13e5b4e7..50ae88b6da 100644
--- a/talpid-core/src/winnet.rs
+++ b/talpid-core/src/winnet.rs
@@ -12,10 +12,6 @@ use widestring::WideCString;
 /// Errors that this module may produce.
 #[derive(err_derive::Error, Debug)]
 pub enum Error {
-    /// Failed to set the metrics for a network interface.
-    #[error(display = "Failed to set the metrics for a network interface")]
-    MetricApplication,
-
     /// Supplied interface alias is invalid.
     #[error(display = "Supplied interface alias is invalid")]
     InvalidInterfaceAlias(#[error(source)] widestring::NulError<u16>),
@@ -53,34 +49,6 @@ fn logging_context() -> *const u8 {
     b"WinNet\0".as_ptr()
 }
 
-/// Returns true if metrics were changed, false otherwise
-pub fn ensure_best_metric_for_interface(interface_alias: &str) -> Result<bool, Error> {
-    let interface_alias_ws =
-        WideCString::from_str(interface_alias).map_err(Error::InvalidInterfaceAlias)?;
-
-    let metric_result = unsafe {
-        WinNet_EnsureBestMetric(
-            interface_alias_ws.as_ptr(),
-            Some(log_sink),
-            logging_context(),
-        )
-    };
-
-    match metric_result {
-        // Metrics didn't change
-        0 => Ok(false),
-        // Metrics changed
-        1 => Ok(true),
-        // Failure
-        2 => Err(Error::MetricApplication),
-        // Unexpected value
-        i => {
-            log::error!("Unexpected return code from WinNet_EnsureBestMetric: {}", i);
-            Err(Error::MetricApplication)
-        }
-    }
-}
-
 #[derive(Debug, Default, Clone, Copy)]
 #[allow(dead_code)]
 #[repr(u32)]
@@ -460,13 +428,6 @@ mod api {
         #[link_name = "WinNet_DeactivateRouteManager"]
         pub fn WinNet_DeactivateRouteManager();
 
-        #[link_name = "WinNet_EnsureBestMetric"]
-        pub fn WinNet_EnsureBestMetric(
-            tunnel_interface_alias: *const wchar_t,
-            sink: Option<LogSink>,
-            sink_context: *const u8,
-        ) -> u32;
-
         #[link_name = "WinNet_GetBestDefaultRoute"]
         pub fn WinNet_GetBestDefaultRoute(
             family: super::WinNetAddrFamily,