From 1ece9472405f8413d5ce5240e7444a1a495f973e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20L=C3=B6nnhager?= Date: Wed, 6 Dec 2023 15:03:18 +0100 Subject: [PATCH 1/6] Cycle over all udp2tcp ports There are currently only two ports, so this only simplifies the logic but makes no practical difference --- mullvad-relay-selector/src/lib.rs | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/mullvad-relay-selector/src/lib.rs b/mullvad-relay-selector/src/lib.rs index 174454995bc8..c10d999afbe5 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 { - 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 { + const fn get_auto_obfuscator_retry_attempt(retry_attempt: u32) -> Option { 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), } } From 72ee5f1546d84200e2f1918c74540eb5df7ba612 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20L=C3=B6nnhager?= Date: Wed, 6 Dec 2023 16:52:16 +0100 Subject: [PATCH 2/6] Always alternate between random ports and port 53 when using WireGuard --- docs/relay-selector.md | 4 ++-- mullvad-relay-selector/src/lib.rs | 11 +++++------ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/docs/relay-selector.md b/docs/relay-selector.md index a4c4b8a249b1..7dca13765795 100644 --- a/docs/relay-selector.md +++ b/docs/relay-selector.md @@ -63,8 +63,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-relay-selector/src/lib.rs b/mullvad-relay-selector/src/lib.rs index c10d999afbe5..77bad9e8ab2e 100644 --- a/mullvad-relay-selector/src/lib.rs +++ b/mullvad-relay-selector/src/lib.rs @@ -1158,12 +1158,11 @@ impl RelaySelector { } const fn preferred_wireguard_port(retry_attempt: u32) -> Constraint { - // 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) } } From 9a4424d9599d323e0a2c154e1fca8ee0eb8e932d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20L=C3=B6nnhager?= Date: Wed, 6 Dec 2023 15:57:04 +0100 Subject: [PATCH 3/6] Select WireGuard as automatic tunnel protocol three times instead of two --- mullvad-relay-selector/src/lib.rs | 38 ++++++++++++------------------- 1 file changed, 14 insertions(+), 24 deletions(-) diff --git a/mullvad-relay-selector/src/lib.rs b/mullvad-relay-selector/src/lib.rs index 77bad9e8ab2e..e66e57414693 100644 --- a/mullvad-relay-selector/src/lib.rs +++ b/mullvad-relay-selector/src/lib.rs @@ -1135,17 +1135,10 @@ impl RelaySelector { pub const fn preferred_tunnel_constraints( retry_attempt: u32, ) -> (Constraint, 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, ), @@ -1875,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!( From 0fc6b4d44ec233894a931a006f274067d4bb201d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20L=C3=B6nnhager?= Date: Thu, 7 Dec 2023 17:26:35 +0100 Subject: [PATCH 4/6] Run device check on every third attempt instead of every other --- mullvad-daemon/src/device/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mullvad-daemon/src/device/mod.rs b/mullvad-daemon/src/device/mod.rs index 2668e995ee4e..188fa9a1d7ac 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 { From 9afdd496932303264ded2b4a76cf8b9e86a6b1af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20L=C3=B6nnhager?= Date: Thu, 7 Dec 2023 12:39:14 +0100 Subject: [PATCH 5/6] Update relay selector document --- docs/relay-selector.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/docs/relay-selector.md b/docs/relay-selector.md index 7dca13765795..6c49b8a47f5d 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 From 02d81d714f93faae40f7374f10c7675a4573401a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20L=C3=B6nnhager?= Date: Thu, 7 Dec 2023 11:52:15 +0100 Subject: [PATCH 6/6] Make Endpoint::from_socket_address const --- talpid-types/src/net/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/talpid-types/src/net/mod.rs b/talpid-types/src/net/mod.rs index cd8d5987a912..de9c5c765d11 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 } } }