Merge branch 'fix-device-validity-check' into main

4 files changed, 35 insertions, 52 deletions

diff --git a/docs/relay-selector.md b/docs/relay-selector.md
index a4c4b8a249..6c49b8a47f 100644
--- a/docs/relay-selector.md
+++ b/docs/relay-selector.md
@@ -49,9 +49,13 @@ Endpoints may be filtered by:
 Whilst all user selected constraints are always honored, when the user hasn't selected any specific
 constraints, following default ones will take effect:
 
-- If no tunnel protocol is specified, the first two connection attempts will use WireGuard, over a
-  random port at first and then port 53. From the third attempt onwards, OpenVPN will be used,
-  alternating between UDP on any port and TCP on port 443.
+- If no tunnel protocol is specified, the first three connection attempts will use WireGuard. All
+  remaining attempts will use OpenVPN. If no specific constraints are set:
+  - The first two attempts will connect to a Wireguard server, first on a random port, and then port
+    53.
+  - The third attempt will connect to a Wireguard server on port 80 with _udp2tcp_.
+  - Remaining attempts will connect to OpenVPN servers, first over UDP on two random ports, and then
+    over TCP on port 443. Remaining attempts alternate between TCP and UDP on random ports.
 
 - If the tunnel protocol is specified as WireGuard and obfuscation mode is set to _Auto_:
   - First two attempts will be used without _udp2tcp_, using a random port on first attempt, and
@@ -63,8 +67,8 @@ constraints, following default ones will take effect:
   _udp2tcp_ all of the time.
 
   If obfuscation is turned _off_, WireGuard connections will first alternate between using
-  a random port and port 53, with 2 attempts each, e.g. first attempt using port 22151, second
-  26107, third attempt and fourth attempt using port 53, and then back to random ports.
+  a random port and port 53, e.g. first attempt using port 22151, second 53, third
+  26107, fourth attempt using port 53, and so on.
 
   If the user has specified a specific port for either _udp2tcp_ or WireGuard, it will override the
   port selection, but it will not change the connection type described above (WireGuard or WireGuard
diff --git a/mullvad-daemon/src/device/mod.rs b/mullvad-daemon/src/device/mod.rs
index 2668e995ee..188fa9a1d7 100644
--- a/mullvad-daemon/src/device/mod.rs
+++ b/mullvad-daemon/src/device/mod.rs
@@ -46,7 +46,7 @@ const LOGOUT_TIMEOUT: Duration = Duration::from_secs(2);
 
 /// Validate the current device once for every `WG_DEVICE_CHECK_THRESHOLD` attempt to set up
 /// a WireGuard tunnel.
-const WG_DEVICE_CHECK_THRESHOLD: usize = 2;
+const WG_DEVICE_CHECK_THRESHOLD: usize = 3;
 
 #[derive(err_derive::Error, Debug, Clone)]
 pub enum Error {
diff --git a/mullvad-relay-selector/src/lib.rs b/mullvad-relay-selector/src/lib.rs
index 174454995b..e66e574146 100644
--- a/mullvad-relay-selector/src/lib.rs
+++ b/mullvad-relay-selector/src/lib.rs
@@ -1051,30 +1051,20 @@ impl RelaySelector {
         endpoint: &MullvadWireguardEndpoint,
         retry_attempt: u32,
     ) -> Option<SelectedObfuscator> {
-        if !self.should_use_auto_obfuscator(retry_attempt) {
-            return None;
-        }
-        // TODO FIX: The third obfuscator entry will never be chosen
-        // Because get_auto_obfuscator_retry_attempt() returns [0, 1]
-        // And the udp2tcp endpoints are defined in a vector with entries [0, 1, 2]
+        let obfuscation_attempt = Self::get_auto_obfuscator_retry_attempt(retry_attempt)?;
         self.get_udp2tcp_obfuscator(
             &obfuscation_settings.udp2tcp,
             relay,
             endpoint,
-            self.get_auto_obfuscator_retry_attempt(retry_attempt)
-                .unwrap(),
+            obfuscation_attempt,
         )
     }
 
-    fn should_use_auto_obfuscator(&self, retry_attempt: u32) -> bool {
-        self.get_auto_obfuscator_retry_attempt(retry_attempt)
-            .is_some()
-    }
-
-    fn get_auto_obfuscator_retry_attempt(&self, retry_attempt: u32) -> Option<u32> {
+    const fn get_auto_obfuscator_retry_attempt(retry_attempt: u32) -> Option<u32> {
         match retry_attempt % 4 {
             0 | 1 => None,
-            filtered_retry => Some(filtered_retry - 2),
+            // when the retry attempt is 2-3, 6-7, 10-11 ... obfuscation will be used
+            filtered_retry => Some(retry_attempt / 4 + filtered_retry - 2),
         }
     }
 
@@ -1145,17 +1135,10 @@ impl RelaySelector {
     pub const fn preferred_tunnel_constraints(
         retry_attempt: u32,
     ) -> (Constraint<u16>, TransportProtocol, TunnelType) {
-        // Try out WireGuard in the first two connection attempts, first with any port,
-        // afterwards on port 53. Afterwards, connect through OpenVPN alternating between UDP
-        // on any port twice and TCP on port 443 once.
+        // Use WireGuard on the first three attempts, then OpenVPN
         match retry_attempt {
-            0 => (
-                Constraint::Any,
-                TransportProtocol::Udp,
-                TunnelType::Wireguard,
-            ),
-            1 => (
-                Constraint::Only(53),
+            0..=2 => (
+                Self::preferred_wireguard_port(retry_attempt),
                 TransportProtocol::Udp,
                 TunnelType::Wireguard,
             ),
@@ -1168,12 +1151,11 @@ impl RelaySelector {
     }
 
     const fn preferred_wireguard_port(retry_attempt: u32) -> Constraint<u16> {
-        // This ensures that if after the first 2 failed attempts the daemon does not
-        // connect, then afterwards 2 of each 4 successive attempts will try to connect
-        // on port 53.
-        match retry_attempt % 4 {
-            0 | 1 => Constraint::Any,
-            _ => Constraint::Only(53),
+        // Alternate between using a random port and port 53
+        if retry_attempt % 2 == 0 {
+            Constraint::Any
+        } else {
+            Constraint::Only(53)
         }
     }
 
@@ -1886,23 +1868,20 @@ mod test {
             protocol: TransportProtocol::Udp,
             port: Constraint::Any,
         });
-        #[cfg(all(unix, not(target_os = "android")))]
-        {
-            let preferred = relay_selector.preferred_constraints(
-                &relay_constraints,
-                BridgeState::On,
-                0,
-                &CustomListsSettings::default(),
-            );
-            assert_eq!(
-                preferred.tunnel_protocol,
-                Constraint::Only(TunnelType::Wireguard)
-            );
-        }
         let preferred = relay_selector.preferred_constraints(
             &relay_constraints,
             BridgeState::On,
-            2,
+            0,
+            &CustomListsSettings::default(),
+        );
+        assert_eq!(
+            preferred.tunnel_protocol,
+            Constraint::Only(TunnelType::Wireguard)
+        );
+        let preferred = relay_selector.preferred_constraints(
+            &relay_constraints,
+            BridgeState::On,
+            3,
             &CustomListsSettings::default(),
         );
         assert_eq!(
diff --git a/talpid-types/src/net/mod.rs b/talpid-types/src/net/mod.rs
index cd8d5987a9..de9c5c765d 100644
--- a/talpid-types/src/net/mod.rs
+++ b/talpid-types/src/net/mod.rs
@@ -262,7 +262,7 @@ impl Endpoint {
         }
     }
 
-    pub fn from_socket_address(address: SocketAddr, protocol: TransportProtocol) -> Self {
+    pub const fn from_socket_address(address: SocketAddr, protocol: TransportProtocol) -> Self {
         Endpoint { address, protocol }
     }
 }