Merge branch 'fix-obfs-mtu-adjustment'

3 files changed, 101 insertions, 82 deletions

diff --git a/talpid-core/src/tunnel/mod.rs b/talpid-core/src/tunnel/mod.rs
index ce860d0ed6..70693d60e7 100644
--- a/talpid-core/src/tunnel/mod.rs
+++ b/talpid-core/src/tunnel/mod.rs
@@ -12,14 +12,6 @@ use talpid_types::net::{wireguard as wireguard_types, TunnelParameters};
 const OPENVPN_LOG_FILENAME: &str = "openvpn.log";
 const WIREGUARD_LOG_FILENAME: &str = "wireguard.log";
 
-/// Set the MTU to the lowest possible whilst still allowing for IPv6 to help with wireless
-/// carriers that do a lot of encapsulation.
-const DEFAULT_MTU: u16 = if cfg!(target_os = "android") {
-    1280
-} else {
-    1380
-};
-
 /// Results from operations in the tunnel module.
 pub type Result<T> = std::result::Result<T, Error>;
 
@@ -38,10 +30,6 @@ pub enum Error {
     #[error("Failed to rotate tunnel log file")]
     RotateLogError(#[from] crate::logging::RotateLogError),
 
-    /// Failure to build Wireguard configuration.
-    #[error("Failed to configure Wireguard with the given parameters")]
-    WireguardConfigError(#[from] talpid_wireguard::config::Error),
-
     /// There was an error listening for events from the OpenVPN tunnel
     #[cfg(not(target_os = "android"))]
     #[error("Failed while listening for events from the OpenVPN tunnel")]
@@ -89,7 +77,7 @@ impl TunnelMonitor {
     /// on tunnel state changes.
     #[cfg_attr(any(target_os = "android", windows), allow(unused_variables))]
     pub fn start<L>(
-        tunnel_parameters: &mut TunnelParameters,
+        tunnel_parameters: &TunnelParameters,
         log_dir: &Option<path::PathBuf>,
         args: TunnelArgs<'_, L>,
     ) -> Result<Self>
@@ -116,7 +104,7 @@ impl TunnelMonitor {
             #[cfg(target_os = "android")]
             TunnelParameters::OpenVpn(_) => Err(Error::UnsupportedPlatform),
 
-            TunnelParameters::Wireguard(ref mut config) => {
+            TunnelParameters::Wireguard(config) => {
                 Self::start_wireguard_tunnel(config, log_file, args)
             }
         }
@@ -146,7 +134,7 @@ impl TunnelMonitor {
         #[cfg(not(any(target_os = "linux", target_os = "windows")))]
         params: &wireguard_types::TunnelParameters,
         #[cfg(any(target_os = "linux", target_os = "windows"))]
-        params: &mut wireguard_types::TunnelParameters,
+        params: &wireguard_types::TunnelParameters,
         log: Option<path::PathBuf>,
         args: TunnelArgs<'_, L>,
     ) -> Result<Self>
@@ -157,68 +145,12 @@ impl TunnelMonitor {
             + Clone
             + 'static,
     {
-        let default_mtu = DEFAULT_MTU;
-
-        // Detects the MTU of the device and sets the default tunnel MTU to that minus headers and
-        // the safety margin
-        #[cfg(any(target_os = "linux", target_os = "windows"))]
-        let default_mtu = args
-            .runtime
-            .block_on(
-                args.route_manager
-                    .get_mtu_for_route(params.connection.peer.endpoint.ip()),
-            )
-            .map(|mtu| Self::clamp_mtu(params, mtu))
-            .unwrap_or(default_mtu);
-
-        #[cfg(not(target_os = "android"))]
-        let detect_mtu = params.options.mtu.is_none();
-
-        let config = talpid_wireguard::config::Config::from_parameters(params, default_mtu)?;
-        let monitor = talpid_wireguard::WireguardMonitor::start(
-            config,
-            #[cfg(not(target_os = "android"))]
-            detect_mtu,
-            log.as_deref(),
-            args,
-        )?;
+        let monitor = talpid_wireguard::WireguardMonitor::start(params, log.as_deref(), args)?;
         Ok(TunnelMonitor {
             monitor: InternalTunnelMonitor::Wireguard(monitor),
         })
     }
 
-    /// Calculates and appropriate tunnel MTU based on the given peer MTU minus header sizes
-    #[cfg(any(target_os = "linux", target_os = "windows"))]
-    fn clamp_mtu(params: &wireguard_types::TunnelParameters, peer_mtu: u16) -> u16 {
-        use talpid_tunnel::{
-            IPV4_HEADER_SIZE, IPV6_HEADER_SIZE, MIN_IPV4_MTU, MIN_IPV6_MTU, WIREGUARD_HEADER_SIZE,
-        };
-        // Some users experience fragmentation issues even when we take the interface MTU and
-        // subtract the header sizes. This is likely due to some program that they use which does
-        // not change the interface MTU but adds its own header onto the outgoing packets. For this
-        // reason we subtract some extra bytes from our MTU in order to give other programs some
-        // safety margin.
-        const MTU_SAFETY_MARGIN: u16 = 60;
-
-        let total_header_size = WIREGUARD_HEADER_SIZE
-            + match params.connection.peer.endpoint.is_ipv6() {
-                false => IPV4_HEADER_SIZE,
-                true => IPV6_HEADER_SIZE,
-            };
-
-        // The largest peer MTU that we allow
-        let max_peer_mtu: u16 = 1500 - MTU_SAFETY_MARGIN - total_header_size;
-
-        let min_mtu = match params.generic_options.enable_ipv6 {
-            false => MIN_IPV4_MTU,
-            true => MIN_IPV6_MTU,
-        };
-
-        peer_mtu
-            .saturating_sub(total_header_size)
-            .clamp(min_mtu, max_peer_mtu)
-    }
-
     #[cfg(not(target_os = "android"))]
     async fn start_openvpn_tunnel<L>(
         config: &openvpn_types::TunnelParameters,
diff --git a/talpid-core/src/tunnel_state_machine/connecting_state.rs b/talpid-core/src/tunnel_state_machine/connecting_state.rs
index bc38401147..199aa7eaa7 100644
--- a/talpid-core/src/tunnel_state_machine/connecting_state.rs
+++ b/talpid-core/src/tunnel_state_machine/connecting_state.rs
@@ -230,7 +230,7 @@ impl ConnectingState {
         let (tunnel_close_tx, tunnel_close_rx) = oneshot::channel();
         let (tunnel_close_event_tx, tunnel_close_event_rx) = oneshot::channel();
 
-        let mut tunnel_parameters = parameters.clone();
+        let tunnel_parameters = parameters.clone();
 
         tokio::task::spawn_blocking(move || {
             let start = Instant::now();
@@ -245,7 +245,7 @@ impl ConnectingState {
                 route_manager,
             };
 
-            let block_reason = match TunnelMonitor::start(&mut tunnel_parameters, &log_dir, args) {
+            let block_reason = match TunnelMonitor::start(&tunnel_parameters, &log_dir, args) {
                 Ok(monitor) => {
                     let reason = Self::wait_for_tunnel_monitor(monitor, retry_attempt);
                     log::debug!("Tunnel monitor exited with block reason: {:?}", reason);
diff --git a/talpid-wireguard/src/lib.rs b/talpid-wireguard/src/lib.rs
index bc178a6c64..330f6f9c07 100644
--- a/talpid-wireguard/src/lib.rs
+++ b/talpid-wireguard/src/lib.rs
@@ -26,6 +26,7 @@ use talpid_routing::{self, RequiredRoute};
 use talpid_tunnel::tun_provider;
 use talpid_tunnel::{tun_provider::TunProvider, TunnelArgs, TunnelEvent, TunnelMetadata};
 
+use talpid_types::net::wireguard::TunnelParameters;
 use talpid_types::{
     net::{AllowedTunnelTraffic, Endpoint, TransportProtocol},
     BoxedError, ErrorExt,
@@ -67,6 +68,10 @@ pub enum Error {
     #[error("Tunnel timed out")]
     TimeoutError,
 
+    /// Invalid WireGuard configuration
+    #[error("Invalid WireGuard configuration")]
+    WireguardConfigError(#[from] crate::config::Error),
+
     /// An interaction with a tunnel failed
     #[error("Tunnel failed")]
     TunnelError(#[source] TunnelError),
@@ -155,14 +160,20 @@ impl WireguardMonitor {
             + Clone
             + 'static,
     >(
-        mut config: Config,
-        detect_mtu: bool,
+        params: &TunnelParameters,
         log_path: Option<&Path>,
         args: TunnelArgs<'_, F>,
     ) -> Result<WireguardMonitor> {
         let on_event = args.on_event.clone();
 
-        let endpoint_addrs: Vec<IpAddr> = config.peers().map(|peer| peer.endpoint.ip()).collect();
+        #[cfg(any(target_os = "windows", target_os = "linux"))]
+        let desired_mtu = args
+            .runtime
+            .block_on(get_desired_mtu(params, &args.route_manager));
+        #[cfg(target_os = "macos")]
+        let desired_mtu = get_desired_mtu(params);
+        let mut config = crate::config::Config::from_parameters(params, desired_mtu)
+            .map_err(Error::WireguardConfigError)?;
 
         let (close_obfs_sender, close_obfs_listener) = sync_mpsc::channel();
         // Start obfuscation server and patch the WireGuard config to point the endpoint to it.
@@ -172,10 +183,16 @@ impl WireguardMonitor {
                 &mut config,
                 close_obfs_sender.clone(),
             ))?;
-        if let Some(obfuscator) = obfuscator.as_ref() {
-            config.mtu = config.mtu.saturating_sub(obfuscator.packet_overhead());
+        // Don't adjust MTU if overridden by user
+        if params.options.mtu.is_none() {
+            if let Some(obfuscator) = obfuscator.as_ref() {
+                config.mtu = config.mtu.saturating_sub(obfuscator.packet_overhead());
+            }
+            config.mtu = clamp_mtu(params, config.mtu);
         }
 
+        let endpoint_addrs: Vec<IpAddr> = config.peers().map(|peer| peer.endpoint.ip()).collect();
+
         #[cfg(target_os = "windows")]
         let (setup_done_tx, setup_done_rx) = mpsc::channel(0);
         let tunnel = Self::open_tunnel(
@@ -217,6 +234,7 @@ impl WireguardMonitor {
         let moved_tunnel = monitor.tunnel.clone();
         let moved_close_obfs_sender = close_obfs_sender.clone();
         let moved_obfuscator = monitor.obfuscator.clone();
+        let detect_mtu = params.options.mtu.is_none();
         let tunnel_fut = async move {
             let tunnel = moved_tunnel;
             let close_obfs_sender: sync_mpsc::Sender<CloseMsg> = moved_close_obfs_sender;
@@ -373,10 +391,14 @@ impl WireguardMonitor {
             + Clone
             + 'static,
     >(
-        mut config: Config,
+        params: &TunnelParameters,
         log_path: Option<&Path>,
         args: TunnelArgs<'_, F>,
     ) -> Result<WireguardMonitor> {
+        let desired_mtu = get_desired_mtu(params);
+        let mut config = crate::config::Config::from_parameters(params, desired_mtu)
+            .map_err(Error::WireguardConfigError)?;
+
         let (close_obfs_sender, close_obfs_listener) = sync_mpsc::channel();
         // Start obfuscation server and patch the WireGuard config to point the endpoint to it.
         let obfuscator = args
@@ -386,8 +408,12 @@ impl WireguardMonitor {
                 close_obfs_sender.clone(),
                 args.tun_provider.clone(),
             ))?;
-        if let Some(obfuscator) = obfuscator.as_ref() {
-            config.mtu = config.mtu.saturating_sub(obfuscator.packet_overhead());
+        // Don't adjust MTU if overridden by user
+        if params.options.mtu.is_none() {
+            if let Some(obfuscator) = obfuscator.as_ref() {
+                config.mtu = config.mtu.saturating_sub(obfuscator.packet_overhead());
+            }
+            config.mtu = clamp_mtu(params, config.mtu);
         }
 
         let should_negotiate_ephemeral_peer = config.quantum_resistant || config.daita;
@@ -1059,3 +1085,64 @@ fn will_nm_manage_dns() -> bool {
         })
         .unwrap_or(false)
 }
+
+// Set the MTU to the lowest possible whilst still allowing for IPv6 to help with wireless
+// carriers that do a lot of encapsulation.
+const DEFAULT_MTU: u16 = if cfg!(target_os = "android") {
+    1280
+} else {
+    1380
+};
+
+#[cfg(any(target_os = "linux", target_os = "windows"))]
+async fn get_desired_mtu(
+    params: &TunnelParameters,
+    route_manager: &talpid_routing::RouteManagerHandle,
+) -> u16 {
+    match params.options.mtu {
+        Some(mtu) => mtu,
+        None => {
+            // Detect the MTU of the device
+            route_manager
+                .get_mtu_for_route(params.connection.peer.endpoint.ip())
+                .await
+                .unwrap_or(DEFAULT_MTU)
+        }
+    }
+}
+
+#[cfg(any(target_os = "macos", target_os = "android"))]
+fn get_desired_mtu(params: &TunnelParameters) -> u16 {
+    params.options.mtu.unwrap_or(DEFAULT_MTU)
+}
+
+/// Calculates and appropriate tunnel MTU based on the given peer MTU minus header sizes
+fn clamp_mtu(params: &TunnelParameters, peer_mtu: u16) -> u16 {
+    use talpid_tunnel::{
+        IPV4_HEADER_SIZE, IPV6_HEADER_SIZE, MIN_IPV4_MTU, MIN_IPV6_MTU, WIREGUARD_HEADER_SIZE,
+    };
+    // Some users experience fragmentation issues even when we take the interface MTU and
+    // subtract the header sizes. This is likely due to some program that they use which does
+    // not change the interface MTU but adds its own header onto the outgoing packets. For this
+    // reason we subtract some extra bytes from our MTU in order to give other programs some
+    // safety margin.
+    const MTU_SAFETY_MARGIN: u16 = 60;
+
+    let total_header_size = WIREGUARD_HEADER_SIZE
+        + match params.connection.peer.endpoint.is_ipv6() {
+            false => IPV4_HEADER_SIZE,
+            true => IPV6_HEADER_SIZE,
+        };
+
+    // The largest peer MTU that we allow
+    let max_peer_mtu: u16 = 1500 - MTU_SAFETY_MARGIN - total_header_size;
+
+    let min_mtu = match params.generic_options.enable_ipv6 {
+        false => MIN_IPV4_MTU,
+        true => MIN_IPV6_MTU,
+    };
+
+    peer_mtu
+        .saturating_sub(total_header_size)
+        .clamp(min_mtu, max_peer_mtu)
+}