From 6c024cb2d964b1e20a80403fab19adcb81fb7ca0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20G=C3=B6ransson?= Date: Wed, 12 Feb 2025 09:23:21 +0100 Subject: [PATCH 01/10] Clear dnsServers on establish and closeTun Since we know the dnsServers are invalid after having invoked `closeTun` & `establish` this prevents the daemon from using DNS servers that are no longer valid. --- .../net/mullvad/talpid/ConnectivityListener.kt | 9 ++++++--- .../net/mullvad/talpid/TalpidVpnService.kt | 17 +++++++++++++++-- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/ConnectivityListener.kt b/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/ConnectivityListener.kt index fdee5039ade6..1bc25293bbad 100644 --- a/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/ConnectivityListener.kt +++ b/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/ConnectivityListener.kt @@ -14,6 +14,7 @@ import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.merge import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.scan import kotlinx.coroutines.flow.stateIn @@ -23,7 +24,10 @@ import net.mullvad.talpid.util.RawNetworkState import net.mullvad.talpid.util.defaultRawNetworkStateFlow import net.mullvad.talpid.util.networkEvents -class ConnectivityListener(private val connectivityManager: ConnectivityManager) { +class ConnectivityListener( + private val connectivityManager: ConnectivityManager, + private val resetDnsFlow: Flow, +) { private lateinit var _isConnected: StateFlow // Used by JNI val isConnected @@ -44,8 +48,7 @@ class ConnectivityListener(private val connectivityManager: ConnectivityManager) // the default network may fail if the network on Android 11 // https://issuetracker.google.com/issues/175055271?pli=1 _currentNetworkState = - connectivityManager - .defaultRawNetworkStateFlow() + merge(connectivityManager.defaultRawNetworkStateFlow(), resetDnsFlow.map { null }) .map { it?.toNetworkState() } .onEach { notifyDefaultNetworkChange(it) } .stateIn(scope, SharingStarted.Eagerly, null) diff --git a/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt b/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt index a143df61322e..0e40caf73673 100644 --- a/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt +++ b/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt @@ -15,6 +15,9 @@ import java.net.Inet4Address import java.net.Inet6Address import java.net.InetAddress import kotlin.properties.Delegates.observable +import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.flow.receiveAsFlow +import kotlinx.coroutines.runBlocking import net.mullvad.mullvadvpn.lib.common.util.establishSafe import net.mullvad.mullvadvpn.lib.common.util.prepareVpnSafe import net.mullvad.mullvadvpn.lib.model.PrepareError @@ -42,6 +45,7 @@ open class TalpidVpnService : LifecycleVpnService() { } } + private val resetDnsChannel = Channel() private var currentTunConfig: TunConfig? = null // Used by JNI @@ -50,7 +54,11 @@ open class TalpidVpnService : LifecycleVpnService() { @CallSuper override fun onCreate() { super.onCreate() - connectivityListener = ConnectivityListener(getSystemService()!!) + connectivityListener = + ConnectivityListener( + getSystemService()!!, + resetDnsChannel.receiveAsFlow(), + ) connectivityListener.register(lifecycleScope) } @@ -71,7 +79,11 @@ open class TalpidVpnService : LifecycleVpnService() { synchronized(this) { openTunImpl(config) } // Used by JNI - fun closeTun(): Unit = synchronized(this) { activeTunStatus = null } + fun closeTun(): Unit = + synchronized(this) { + runBlocking { resetDnsChannel.send(Unit) } + activeTunStatus = null + } // Used by JNI fun bypass(socket: Int): Boolean = protect(socket) @@ -123,6 +135,7 @@ open class TalpidVpnService : LifecycleVpnService() { builder.addDnsServer(FALLBACK_DUMMY_DNS_SERVER) } + runBlocking { resetDnsChannel.send(Unit) } val vpnInterfaceFd = builder .establishSafe() From 38cc5d779952b5800a903db86772f429b342486d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20G=C3=B6ransson?= Date: Wed, 12 Feb 2025 09:30:03 +0100 Subject: [PATCH 02/10] Wait for routes after invoking establish (openTun) --- talpid-routing/src/unix/android.rs | 48 +++++++++++++------ talpid-routing/src/unix/mod.rs | 19 ++++++-- talpid-tunnel/src/tun_provider/android/mod.rs | 23 ++++++++- talpid-tunnel/src/tun_provider/mod.rs | 13 +++++ talpid-wireguard/src/lib.rs | 13 +++-- talpid-wireguard/src/wireguard_go/mod.rs | 33 +++++++++++-- 6 files changed, 118 insertions(+), 31 deletions(-) diff --git a/talpid-routing/src/unix/android.rs b/talpid-routing/src/unix/android.rs index 137e69c1deb5..f8a345f88f85 100644 --- a/talpid-routing/src/unix/android.rs +++ b/talpid-routing/src/unix/android.rs @@ -1,5 +1,5 @@ use std::collections::HashSet; -use std::ops::{ControlFlow, Not}; +use std::ops::{ControlFlow}; use std::sync::Mutex; use futures::channel::mpsc::{self, UnboundedReceiver, UnboundedSender}; @@ -51,7 +51,7 @@ pub struct RouteManagerImpl { last_state: Option, /// Clients waiting on response to [RouteManagerCommand::WaitForRoutes]. - waiting_for_routes: Vec>, + waiting_for_routes: Vec<(oneshot::Sender<()>, Vec)>, } impl RouteManagerImpl { @@ -64,7 +64,7 @@ impl RouteManagerImpl { // Try to poll for the current network state at startup. // This will most likely be null, but it covers the edge case where a NetworkState - // update has been emitted before we anyone starts to listen for route updates some + // update has been emitted before anyone starts to listen for route updates some // time in the future (when connecting). let last_state = match current_network_state(android_context) { Ok(initial_state) => initial_state, @@ -105,12 +105,17 @@ impl RouteManagerImpl { // update the last known NetworkState self.last_state = network_state; - if has_routes(self.last_state.as_ref()) { - // notify waiting clients that routes exist - for client in self.waiting_for_routes.drain(..) { + // notify waiting clients that routes exist + let mut unused_routes: Vec<(oneshot::Sender<()>, Vec)> = Vec::new(); + let ret = for (client, expected_routes) in self.waiting_for_routes.drain(..) { + if has_routes(self.last_state.as_ref(), expected_routes.clone()) { let _ = client.send(()); + } else { + unused_routes.push((client, expected_routes)); } - } + }; + self.waiting_for_routes = unused_routes; + ret } } } @@ -126,31 +131,44 @@ impl RouteManagerImpl { let _ = tx.send(()); return ControlFlow::Break(()); } - RouteManagerCommand::WaitForRoutes(response_tx) => { + RouteManagerCommand::WaitForRoutes(response_tx, expected_routes) => { // check if routes have already been configured on the Android system. // otherwise, register a listener for network state changes. // routes may come in at any moment in the future. - if has_routes(self.last_state.as_ref()) { + if has_routes(self.last_state.as_ref(), expected_routes.clone()) { let _ = response_tx.send(()); } else { - self.waiting_for_routes.push(response_tx); + self.waiting_for_routes.push((response_tx, expected_routes)); } } + RouteManagerCommand::ClearRoutes(tx) => { + self.clear_routes(); + let _ = tx.send(()); + } } ControlFlow::Continue(()) } + + pub fn clear_routes(&mut self) { + self.last_state = None; + } } -/// Check whether the [NetworkState] contains any routes. +/// Check whether the [NetworkState] contains expected routes. /// -/// Since we are the ones telling Android what routes to set, we make the assumption that: -/// If any routes exist whatsoever, they are the the routes we specified. -fn has_routes(state: Option<&NetworkState>) -> bool { +/// Matches the routes reported from Android and checks if all the routes we expect to be there is +/// present. +fn has_routes(state: Option<&NetworkState>, expected_routes: Vec) -> bool { let Some(network_state) = state else { return false; }; - configured_routes(network_state).is_empty().not() + + let routes = configured_routes(network_state); + if routes.is_empty() { + return false; + } + routes.is_superset(&HashSet::from_iter(expected_routes)) } fn configured_routes(state: &NetworkState) -> HashSet { diff --git a/talpid-routing/src/unix/mod.rs b/talpid-routing/src/unix/mod.rs index 5aedc9626ea9..5fd33bafb080 100644 --- a/talpid-routing/src/unix/mod.rs +++ b/talpid-routing/src/unix/mod.rs @@ -37,6 +37,8 @@ mod imp; #[path = "android.rs"] mod imp; +#[cfg(target_os = "android")] +use crate::Route; #[cfg(any(target_os = "macos", target_os = "linux"))] pub use imp::Error as PlatformError; @@ -103,7 +105,8 @@ pub(crate) enum RouteManagerCommand { #[cfg(target_os = "android")] #[derive(Debug)] pub(crate) enum RouteManagerCommand { - WaitForRoutes(oneshot::Sender<()>), + ClearRoutes(oneshot::Sender<()>), + WaitForRoutes(oneshot::Sender<()>, Vec), Shutdown(oneshot::Sender<()>), } @@ -215,7 +218,7 @@ impl RouteManagerHandle { /// This function is guaranteed to *not* wait for longer than 2 seconds. /// Please, see the implementation of this function for further details. #[cfg(target_os = "android")] - pub async fn wait_for_routes(&self) -> Result<(), Error> { + pub async fn wait_for_routes(&self, expect_routes: Vec) -> Result<(), Error> { use std::time::Duration; use tokio::time::timeout; /// Maximum time to wait for routes to come up. The expected mean time is low (~200 ms), but @@ -224,7 +227,7 @@ impl RouteManagerHandle { let (result_tx, result_rx) = oneshot::channel(); self.tx - .unbounded_send(RouteManagerCommand::WaitForRoutes(result_tx)) + .unbounded_send(RouteManagerCommand::WaitForRoutes(result_tx, expect_routes)) .map_err(|_| Error::RouteManagerDown)?; timeout(WAIT_FOR_ROUTES_TIMEOUT, result_rx) @@ -247,6 +250,16 @@ impl RouteManagerHandle { Ok(()) } + /// xD + pub async fn clear_android_routes(&self) -> Result<(), Error> { + let (result_tx, result_rx) = oneshot::channel(); + self.tx + .unbounded_send(RouteManagerCommand::ClearRoutes(result_tx)) + .map_err(|_| Error::RouteManagerDown)?; + let _ = result_rx.await; + Ok(()) + } + /// Listen for non-tunnel default route changes. #[cfg(target_os = "macos")] pub async fn default_route_listener( diff --git a/talpid-tunnel/src/tun_provider/android/mod.rs b/talpid-tunnel/src/tun_provider/android/mod.rs index f285b4a64ca1..fae8d62a368f 100644 --- a/talpid-tunnel/src/tun_provider/android/mod.rs +++ b/talpid-tunnel/src/tun_provider/android/mod.rs @@ -16,6 +16,7 @@ use std::{ os::unix::io::{AsRawFd, RawFd}, sync::Arc, }; +use talpid_routing::Route; use talpid_types::net::{ALLOWED_LAN_MULTICAST_NETS, ALLOWED_LAN_NETS}; use talpid_types::{android::AndroidContext, ErrorExt}; @@ -188,6 +189,14 @@ impl AndroidTunProvider { } } + pub fn real_routes(&self) -> Vec { + self.config + .real_routes() + .iter() + .map(|ip_network| Route::new(ip_network.clone())) + .collect() + } + fn call_method( &self, name: &'static str, @@ -221,7 +230,7 @@ impl AndroidTunProvider { /// Configuration to use for VpnService #[derive(Clone, Debug, Eq, PartialEq, IntoJava)] #[jnix(class_name = "net.mullvad.talpid.model.TunConfig")] -struct VpnServiceConfig { +pub struct VpnServiceConfig { /// IP addresses for the tunnel interface. pub addresses: Vec, @@ -318,7 +327,7 @@ impl VpnServiceConfig { #[derive(Clone, Debug, Eq, PartialEq, IntoJava)] #[jnix(package = "net.mullvad.talpid.model")] -struct InetNetwork { +pub struct InetNetwork { address: IpAddr, prefix: i16, } @@ -332,6 +341,16 @@ impl From for InetNetwork { } } +impl From<&InetNetwork> for IpNetwork { + fn from(inet_network: &InetNetwork) -> Self { + IpNetwork::new( + inet_network.address, + inet_network.prefix.to_be_bytes().last().unwrap().clone(), + ) + .unwrap() + } +} + /// Handle to a tunnel device on Android. pub struct VpnServiceTun { tunnel: RawFd, diff --git a/talpid-tunnel/src/tun_provider/mod.rs b/talpid-tunnel/src/tun_provider/mod.rs index 1bf4e1abb483..9ce767aa8856 100644 --- a/talpid-tunnel/src/tun_provider/mod.rs +++ b/talpid-tunnel/src/tun_provider/mod.rs @@ -1,3 +1,5 @@ +#[cfg(target_os = "android")] +use crate::tun_provider::imp::VpnServiceConfig; use cfg_if::cfg_if; use ipnetwork::IpNetwork; use std::{ @@ -73,6 +75,17 @@ impl TunConfig { } servers } + + /// Routes to configure for the tunnel. + #[cfg(target_os = "android")] + pub fn real_routes(&self) -> Vec { + VpnServiceConfig::new(self.clone()) + .routes + .clone() + .iter() + .map(|x| IpNetwork::from(x)) + .collect() + } } /// Return a tunnel configuration that routes all traffic inside the tunnel. diff --git a/talpid-wireguard/src/lib.rs b/talpid-wireguard/src/lib.rs index fe1a848e9a74..05be428b6eaf 100644 --- a/talpid-wireguard/src/lib.rs +++ b/talpid-wireguard/src/lib.rs @@ -28,6 +28,8 @@ use talpid_tunnel::{ tun_provider::TunProvider, EventHook, TunnelArgs, TunnelEvent, TunnelMetadata, }; +#[cfg(target_os = "android")] +use talpid_routing::RouteManagerHandle; #[cfg(daita)] use talpid_tunnel_config_client::DaitaSettings; use talpid_types::{ @@ -434,6 +436,7 @@ impl WireguardMonitor { &config, log_path, args.tun_provider.clone(), + args.route_manager, // In case we should negotiate an ephemeral peer, we should specify via AllowedIPs // that we only allows traffic to/from the gateway. This is only needed on Android // since we lack a firewall there. @@ -465,13 +468,6 @@ impl WireguardMonitor { .on_event(TunnelEvent::InterfaceUp(metadata.clone(), allowed_traffic)) .await; - // Wait for routes to come up - args.route_manager - .wait_for_routes() - .await - .map_err(Error::SetupRoutingError) - .map_err(CloseMsg::SetupError)?; - if should_negotiate_ephemeral_peer { let ephemeral_obfs_sender = close_obfs_sender.clone(); @@ -743,6 +739,7 @@ impl WireguardMonitor { config: &Config, log_path: Option<&Path>, #[cfg(unix)] tun_provider: Arc>, + #[cfg(target_os = "android")] route_manager: RouteManagerHandle, #[cfg(windows)] setup_done_tx: mpsc::Sender>, #[cfg(windows)] route_manager: talpid_routing::RouteManagerHandle, #[cfg(target_os = "android")] gateway_only: bool, @@ -780,6 +777,7 @@ impl WireguardMonitor { exit_peer, log_path, tun_provider, + route_manager, routes, cancel_receiver, ) @@ -791,6 +789,7 @@ impl WireguardMonitor { &config, log_path, tun_provider, + route_manager, routes, cancel_receiver, ) diff --git a/talpid-wireguard/src/wireguard_go/mod.rs b/talpid-wireguard/src/wireguard_go/mod.rs index 813490899a6e..a8ce7d0aacb3 100644 --- a/talpid-wireguard/src/wireguard_go/mod.rs +++ b/talpid-wireguard/src/wireguard_go/mod.rs @@ -2,7 +2,7 @@ use super::config; use super::{ stats::{Stats, StatsMap}, - Config, Tunnel, TunnelError, + CloseMsg, Config, Error, Tunnel, TunnelError, }; #[cfg(target_os = "linux")] use crate::config::MULLVAD_INTERFACE_NAME; @@ -22,6 +22,7 @@ use std::{ path::{Path, PathBuf}, pin::Pin, }; +use talpid_routing::{RouteManagerHandle}; #[cfg(target_os = "android")] use talpid_tunnel::tun_provider::Error as TunProviderError; #[cfg(not(target_os = "windows"))] @@ -116,6 +117,7 @@ impl WgGoTunnel { let cancel_receiver = state.cancel_receiver.clone(); let tun_provider = Arc::clone(&state.tun_provider); let routes = config.get_tunnel_destinations(); + let route_manager = &state.route_manager.clone(); match self { WgGoTunnel::Multihop(state) if !config.is_multihop() => { @@ -124,6 +126,7 @@ impl WgGoTunnel { config, log_path.as_deref(), tun_provider, + route_manager.clone(), routes, cancel_receiver, ) @@ -136,6 +139,7 @@ impl WgGoTunnel { &config.exit_peer.clone().unwrap().clone(), log_path.as_deref(), tun_provider, + route_manager.clone(), routes, cancel_receiver, ) @@ -143,15 +147,12 @@ impl WgGoTunnel { } WgGoTunnel::Singlehop(mut state) => { state.set_config(config.clone())?; - // HACK: Check if the tunnel is working by sending a ping in the tunnel. let new_state = WgGoTunnel::Singlehop(state); - new_state.ensure_tunnel_is_running().await?; Ok(new_state) } WgGoTunnel::Multihop(mut state) => { state.set_config(config.clone())?; let new_state = WgGoTunnel::Multihop(state); - new_state.ensure_tunnel_is_running().await?; Ok(new_state) } } @@ -173,6 +174,8 @@ pub(crate) struct WgGoTunnelState { _logging_context: LoggingContext, #[cfg(target_os = "android")] tun_provider: Arc>, + #[cfg(target_os = "android")] + route_manager: RouteManagerHandle, #[cfg(daita)] config: Config, /// This is used to cancel the connectivity checks that occur when toggling multihop @@ -395,9 +398,12 @@ impl WgGoTunnel { config: &Config, log_path: Option<&Path>, tun_provider: Arc>, + route_manager: RouteManagerHandle, routes: impl Iterator, cancel_receiver: connectivity::CancelReceiver, ) -> Result { + let _ = route_manager.clear_android_routes().await; + let (mut tunnel_device, tunnel_fd) = Self::get_tunnel(Arc::clone(&tun_provider), config, routes)?; @@ -427,6 +433,7 @@ impl WgGoTunnel { _tunnel_device: tunnel_device, _logging_context: logging_context, tun_provider, + route_manager, #[cfg(daita)] config: config.clone(), cancel_receiver, @@ -443,9 +450,12 @@ impl WgGoTunnel { exit_peer: &PeerConfig, log_path: Option<&Path>, tun_provider: Arc>, + route_manager: RouteManagerHandle, routes: impl Iterator, cancel_receiver: connectivity::CancelReceiver, ) -> Result { + let _ = route_manager.clear_android_routes().await; + let (mut tunnel_device, tunnel_fd) = Self::get_tunnel(Arc::clone(&tun_provider), config, routes)?; @@ -491,6 +501,7 @@ impl WgGoTunnel { _tunnel_device: tunnel_device, _logging_context: logging_context, tun_provider, + route_manager, #[cfg(daita)] config: config.clone(), cancel_receiver: cancel_receiver.clone(), @@ -519,6 +530,20 @@ impl WgGoTunnel { /// traffic. This function blocks until the tunnel starts to serve traffic or until [connectivity::Check] times out. async fn ensure_tunnel_is_running(&self) -> Result<()> { let state = self.as_state(); + + let expected_routes = state.tun_provider.lock().unwrap().real_routes(); + + // TODO HANDLE UNWRAP + // Wait for routes to come up + state + .route_manager + .clone() + .wait_for_routes(expected_routes) + .await + .map_err(Error::SetupRoutingError) + .map_err(CloseMsg::SetupError) + .unwrap(); + let addr = state.config.ipv4_gateway; let cancel_receiver = state.cancel_receiver.clone(); let mut check = connectivity::Check::new(addr, 0, cancel_receiver) From 6adce8f908ddd64fa73188d7ff14324e7e20fc10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20G=C3=B6ransson?= Date: Wed, 12 Feb 2025 14:12:35 +0100 Subject: [PATCH 03/10] Remove open_tun_forced --- .../net/mullvad/talpid/TalpidVpnService.kt | 22 ++------ .../tunnel_state_machine/connected_state.rs | 39 ++------------ .../tunnel_state_machine/connecting_state.rs | 52 ++++--------------- talpid-routing/src/unix/mod.rs | 2 + talpid-tunnel/src/tun_provider/android/mod.rs | 6 --- 5 files changed, 18 insertions(+), 103 deletions(-) diff --git a/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt b/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt index 0e40caf73673..9a26d5d0d115 100644 --- a/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt +++ b/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt @@ -65,19 +65,12 @@ open class TalpidVpnService : LifecycleVpnService() { // Used by JNI fun openTun(config: TunConfig): CreateTunResult = synchronized(this) { - val tunStatus = activeTunStatus - - if (config == currentTunConfig && tunStatus != null && tunStatus.isOpen) { - tunStatus - } else { - openTunImpl(config) + createTun(config).merge().also { + currentTunConfig = config + activeTunStatus = it } } - // Used by JNI - fun openTunForced(config: TunConfig): CreateTunResult = - synchronized(this) { openTunImpl(config) } - // Used by JNI fun closeTun(): Unit = synchronized(this) { @@ -88,15 +81,6 @@ open class TalpidVpnService : LifecycleVpnService() { // Used by JNI fun bypass(socket: Int): Boolean = protect(socket) - private fun openTunImpl(config: TunConfig): CreateTunResult { - val newTunStatus = createTun(config).merge() - - currentTunConfig = config - activeTunStatus = newTunStatus - - return newTunStatus - } - private fun createTun( config: TunConfig ): Either = either { diff --git a/talpid-core/src/tunnel_state_machine/connected_state.rs b/talpid-core/src/tunnel_state_machine/connected_state.rs index 0ad41b049c14..c515e2b3831f 100644 --- a/talpid-core/src/tunnel_state_machine/connected_state.rs +++ b/talpid-core/src/tunnel_state_machine/connected_state.rs @@ -260,14 +260,7 @@ impl ConnectedState { let consequence = if shared_values.set_allow_lan(allow_lan) { #[cfg(target_os = "android")] { - if let Err(_err) = shared_values.restart_tunnel(false) { - self.disconnect( - shared_values, - AfterDisconnect::Block(ErrorStateCause::StartTunnelError), - ) - } else { - self.disconnect(shared_values, AfterDisconnect::Reconnect(0)) - } + self.disconnect(shared_values, AfterDisconnect::Reconnect(0)) } #[cfg(not(target_os = "android"))] { @@ -298,22 +291,7 @@ impl ConnectedState { let consequence = if shared_values.set_dns_config(servers) { #[cfg(target_os = "android")] { - if let Err(_err) = shared_values.restart_tunnel(false) { - match _err { - Error::InvalidDnsServers(ip_addrs) => self.disconnect( - shared_values, - AfterDisconnect::Block(ErrorStateCause::InvalidDnsServers( - ip_addrs, - )), - ), - _ => self.disconnect( - shared_values, - AfterDisconnect::Block(ErrorStateCause::StartTunnelError), - ), - } - } else { - self.disconnect(shared_values, AfterDisconnect::Reconnect(0)) - } + self.disconnect(shared_values, AfterDisconnect::Reconnect(0)) } #[cfg(not(target_os = "android"))] { @@ -385,17 +363,8 @@ impl ConnectedState { #[cfg(target_os = "android")] Some(TunnelCommand::SetExcludedApps(result_tx, paths)) => { if shared_values.set_excluded_paths(paths) { - if let Err(err) = shared_values.restart_tunnel(false) { - let _ = - result_tx.send(Err(crate::split_tunnel::Error::SetExcludedApps(err))); - self.disconnect( - shared_values, - AfterDisconnect::Block(ErrorStateCause::SplitTunnelError), - ) - } else { - let _ = result_tx.send(Ok(())); - self.disconnect(shared_values, AfterDisconnect::Reconnect(0)) - } + let _ = result_tx.send(Ok(())); + self.disconnect(shared_values, AfterDisconnect::Reconnect(0)) } else { let _ = result_tx.send(Ok(())); SameState(self) diff --git a/talpid-core/src/tunnel_state_machine/connecting_state.rs b/talpid-core/src/tunnel_state_machine/connecting_state.rs index 9060787536db..8dcf1b08b4a2 100644 --- a/talpid-core/src/tunnel_state_machine/connecting_state.rs +++ b/talpid-core/src/tunnel_state_machine/connecting_state.rs @@ -29,8 +29,6 @@ use crate::tunnel::{self, TunnelMonitor}; pub(crate) type TunnelCloseEvent = Fuse>>; -#[cfg(target_os = "android")] -const MAX_ATTEMPTS_WITH_SAME_TUN: u32 = 5; const MIN_TUNNEL_ALIVE_TIME: Duration = Duration::from_millis(1000); #[cfg(target_os = "windows")] const MAX_ATTEMPT_CREATE_TUN: u32 = 4; @@ -114,21 +112,12 @@ impl ConnectingState { ErrorStateCause::SetFirewallPolicyError(error), ) } else { + // This is magically shimmed in on the side on Android to prep the TunConfig + // with the right DNS servers. On Android DNS is part of creating the VPN + // interface and this call should be part of start_tunnel call instead #[cfg(target_os = "android")] - { - shared_values.prepare_tun_config(false); - if retry_attempt > 0 && retry_attempt % MAX_ATTEMPTS_WITH_SAME_TUN == 0 { - if let Err(error) = - { shared_values.tun_provider.lock().unwrap().open_tun_forced() } - { - log::error!( - "{}", - error.display_chain_with_msg("Failed to recreate tun device") - ); - } - } - } - + shared_values.prepare_tun_config(false); + let connecting_state = Self::start_tunnel( shared_values.runtime.clone(), tunnel_parameters, @@ -386,14 +375,7 @@ impl ConnectingState { let consequence = if shared_values.set_allow_lan(allow_lan) { #[cfg(target_os = "android")] { - if let Err(_err) = shared_values.restart_tunnel(false) { - self.disconnect( - shared_values, - AfterDisconnect::Block(ErrorStateCause::StartTunnelError), - ) - } else { - self.disconnect(shared_values, AfterDisconnect::Reconnect(0)) - } + self.disconnect(shared_values, AfterDisconnect::Reconnect(0)) } #[cfg(not(target_os = "android"))] self.reset_firewall(shared_values) @@ -427,14 +409,7 @@ impl ConnectingState { let consequence = if shared_values.set_dns_config(servers) { #[cfg(target_os = "android")] { - if let Err(_err) = shared_values.restart_tunnel(false) { - self.disconnect( - shared_values, - AfterDisconnect::Block(ErrorStateCause::StartTunnelError), - ) - } else { - self.disconnect(shared_values, AfterDisconnect::Reconnect(0)) - } + self.disconnect(shared_values, AfterDisconnect::Reconnect(0)) } #[cfg(not(target_os = "android"))] SameState(self) @@ -484,17 +459,8 @@ impl ConnectingState { #[cfg(target_os = "android")] Some(TunnelCommand::SetExcludedApps(result_tx, paths)) => { if shared_values.set_excluded_paths(paths) { - if let Err(err) = shared_values.restart_tunnel(false) { - let _ = - result_tx.send(Err(crate::split_tunnel::Error::SetExcludedApps(err))); - self.disconnect( - shared_values, - AfterDisconnect::Block(ErrorStateCause::SplitTunnelError), - ) - } else { - let _ = result_tx.send(Ok(())); - self.disconnect(shared_values, AfterDisconnect::Reconnect(0)) - } + let _ = result_tx.send(Ok(())); + self.disconnect(shared_values, AfterDisconnect::Reconnect(0)) } else { let _ = result_tx.send(Ok(())); SameState(self) diff --git a/talpid-routing/src/unix/mod.rs b/talpid-routing/src/unix/mod.rs index 5fd33bafb080..dd8f9108e171 100644 --- a/talpid-routing/src/unix/mod.rs +++ b/talpid-routing/src/unix/mod.rs @@ -251,6 +251,8 @@ impl RouteManagerHandle { } /// xD + /// + #[cfg(target_os = "android")] pub async fn clear_android_routes(&self) -> Result<(), Error> { let (result_tx, result_rx) = oneshot::channel(); self.tx diff --git a/talpid-tunnel/src/tun_provider/android/mod.rs b/talpid-tunnel/src/tun_provider/android/mod.rs index fae8d62a368f..6ac99c62f744 100644 --- a/talpid-tunnel/src/tun_provider/android/mod.rs +++ b/talpid-tunnel/src/tun_provider/android/mod.rs @@ -98,12 +98,6 @@ impl AndroidTunProvider { self.open_tun_inner("openTun") } - /// Open a tunnel with the current configuration. - /// Force recreation even if the tunnel config hasn't changed. - pub fn open_tun_forced(&mut self) -> Result { - self.open_tun_inner("openTunForced") - } - /// Open a tunnel with the current configuration. fn open_tun_inner(&mut self, get_tun_func_name: &'static str) -> Result { let tun_fd = self.open_tun_fd(get_tun_func_name)?; From 9e62b83d42091c0090a1e83ea79a7d1dc7b600a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20G=C3=B6ransson?= Date: Fri, 14 Feb 2025 11:35:38 +0100 Subject: [PATCH 04/10] Only recreate android tunnel when necessary --- .../tunnel_state_machine/connected_state.rs | 2 - .../tunnel_state_machine/connecting_state.rs | 1 + talpid-tunnel/src/tun_provider/android/mod.rs | 77 ++++++++++------ talpid-wireguard/src/lib.rs | 26 +++++- talpid-wireguard/src/wireguard_go/mod.rs | 88 +++++++++++-------- 5 files changed, 126 insertions(+), 68 deletions(-) diff --git a/talpid-core/src/tunnel_state_machine/connected_state.rs b/talpid-core/src/tunnel_state_machine/connected_state.rs index c515e2b3831f..05024554a377 100644 --- a/talpid-core/src/tunnel_state_machine/connected_state.rs +++ b/talpid-core/src/tunnel_state_machine/connected_state.rs @@ -2,8 +2,6 @@ use futures::channel::{mpsc, oneshot}; use futures::stream::Fuse; use futures::StreamExt; -#[cfg(target_os = "android")] -use talpid_tunnel::tun_provider::Error; use talpid_types::net::{AllowedClients, AllowedEndpoint, TunnelParameters}; use talpid_types::tunnel::{ErrorStateCause, FirewallPolicyError}; use talpid_types::{BoxedError, ErrorExt}; diff --git a/talpid-core/src/tunnel_state_machine/connecting_state.rs b/talpid-core/src/tunnel_state_machine/connecting_state.rs index 8dcf1b08b4a2..994a1154dbbf 100644 --- a/talpid-core/src/tunnel_state_machine/connecting_state.rs +++ b/talpid-core/src/tunnel_state_machine/connecting_state.rs @@ -36,6 +36,7 @@ const MAX_ATTEMPT_CREATE_TUN: u32 = 4; const INITIAL_ALLOWED_TUNNEL_TRAFFIC: AllowedTunnelTraffic = AllowedTunnelTraffic::None; /// The tunnel has been started, but it is not established/functional. +#[derive(Debug)] pub struct ConnectingState { tunnel_events: TunnelEventsReceiver, tunnel_parameters: TunnelParameters, diff --git a/talpid-tunnel/src/tun_provider/android/mod.rs b/talpid-tunnel/src/tun_provider/android/mod.rs index 6ac99c62f744..96e0b8fe57f1 100644 --- a/talpid-tunnel/src/tun_provider/android/mod.rs +++ b/talpid-tunnel/src/tun_provider/android/mod.rs @@ -66,6 +66,7 @@ pub struct AndroidTunProvider { class: GlobalRef, object: GlobalRef, config: TunConfig, + current_config: Option<(VpnServiceConfig, RawFd)>, } impl AndroidTunProvider { @@ -84,6 +85,7 @@ impl AndroidTunProvider { class: talpid_vpn_service_class, object: context.vpn_service, config, + current_config: None, } } @@ -94,45 +96,65 @@ impl AndroidTunProvider { } /// Open a tunnel with the current configuration. - pub fn open_tun(&mut self) -> Result { + pub fn open_tun(&mut self) -> Result<(VpnServiceTun, bool), Error> { self.open_tun_inner("openTun") } /// Open a tunnel with the current configuration. - fn open_tun_inner(&mut self, get_tun_func_name: &'static str) -> Result { - let tun_fd = self.open_tun_fd(get_tun_func_name)?; + fn open_tun_inner( + &mut self, + get_tun_func_name: &'static str, + ) -> Result<(VpnServiceTun, bool), Error> { + let (tun_fd, reuse) = self.open_tun_fd(get_tun_func_name)?; + log::debug!("DEBUG: Opening tun: {}", tun_fd); let jvm = unsafe { JavaVM::from_raw(self.jvm.get_java_vm_pointer()) } .map_err(Error::CloneJavaVm)?; - Ok(VpnServiceTun { - tunnel: tun_fd, - jvm, - class: self.class.clone(), - object: self.object.clone(), - }) + Ok(( + VpnServiceTun { + tunnel: tun_fd, + jvm, + class: self.class.clone(), + object: self.object.clone(), + }, + reuse, + )) } - fn open_tun_fd(&self, get_tun_func_name: &'static str) -> Result { + fn open_tun_fd(&mut self, get_tun_func_name: &'static str) -> Result<(RawFd, bool), Error> { let config = VpnServiceConfig::new(self.config.clone()); - let env = self.env()?; - let java_config = config.into_java(&env); - - let result = self.call_method( - get_tun_func_name, - "(Lnet/mullvad/talpid/model/TunConfig;)Lnet/mullvad/talpid/model/CreateTunResult;", - JavaType::Object("net/mullvad/talpid/model/CreateTunResult".to_owned()), - &[JValue::Object(java_config.as_obj())], - )?; - - match result { - JValue::Object(result) => CreateTunResult::from_java(&env, result).into(), - value => Err(Error::InvalidMethodResult( + // If we are recreating the same tunnel we return the same file descriptor to avoid calling + // open_tun in android since it may cause leaks. + if let Some(current_config) = &self.current_config { + if current_config.0 == config { + return Ok((current_config.1, false)); + } + } + let create_result = { + let env = self.env()?; + let java_config = config.clone().into_java(&env); + let result = self.call_method( get_tun_func_name, - format!("{:?}", value), - )), + "(Lnet/mullvad/talpid/model/TunConfig;)Lnet/mullvad/talpid/model/CreateTunResult;", + JavaType::Object("net/mullvad/talpid/model/CreateTunResult".to_owned()), + &[JValue::Object(java_config.as_obj())], + )?; + + match result { + JValue::Object(result) => CreateTunResult::from_java(&env, result).into(), + value => Err(Error::InvalidMethodResult( + get_tun_func_name, + format!("{:?}", value), + )), + } + .map(|raw_fd| (raw_fd, true)) + }; + if let Ok(create_result) = create_result { + self.current_config = Some((config, create_result.0)); } + create_result } /// Close currently active tunnel device. @@ -153,6 +175,9 @@ impl AndroidTunProvider { "{}", error.display_chain_with_msg("Failed to close the tunnel") ); + } else { + // Remove the cache of config + self.current_config = None; } } @@ -347,7 +372,7 @@ impl From<&InetNetwork> for IpNetwork { /// Handle to a tunnel device on Android. pub struct VpnServiceTun { - tunnel: RawFd, + pub tunnel: RawFd, jvm: JavaVM, class: GlobalRef, object: GlobalRef, diff --git a/talpid-wireguard/src/lib.rs b/talpid-wireguard/src/lib.rs index 05be428b6eaf..a9e0fb336492 100644 --- a/talpid-wireguard/src/lib.rs +++ b/talpid-wireguard/src/lib.rs @@ -425,7 +425,7 @@ impl WireguardMonitor { let should_negotiate_ephemeral_peer = config.quantum_resistant || config.daita; let (cancel_token, cancel_receiver) = connectivity::CancelToken::new(); - let connectivity_check = connectivity::Check::new( + let mut connectivity_check = connectivity::Check::new( config.ipv4_gateway, args.retry_attempt, cancel_receiver.clone(), @@ -498,6 +498,24 @@ impl WireguardMonitor { .await; } + match connectivity_check + .establish_connectivity(&tunnel.lock().await.as_ref().unwrap()) + .await + { + Ok(true) => Ok(()), + Ok(false) => { + log::warn!("Timeout while checking tunnel connection"); + Err(CloseMsg::PingErr) + } + Err(error) => { + log::error!( + "{}", + error.display_chain_with_msg("Failed to check tunnel connection") + ); + Err(CloseMsg::PingErr) + } + }?; + let metadata = Self::tunnel_metadata(&iface_name, &config); event_hook.on_event(TunnelEvent::Up(metadata)).await; @@ -745,7 +763,7 @@ impl WireguardMonitor { #[cfg(target_os = "android")] gateway_only: bool, #[cfg(target_os = "android")] cancel_receiver: connectivity::CancelReceiver, ) -> Result { - #[cfg(unix)] + #[cfg(all(unix, not(target_os = "android")))] let routes = config .get_tunnel_destinations() .flat_map(Self::replace_default_prefixes); @@ -778,7 +796,6 @@ impl WireguardMonitor { log_path, tun_provider, route_manager, - routes, cancel_receiver, ) .await @@ -790,7 +807,6 @@ impl WireguardMonitor { log_path, tun_provider, route_manager, - routes, cancel_receiver, ) .await @@ -812,6 +828,7 @@ impl WireguardMonitor { Err(_) => Ok(()), }; + log::debug!("Wait result : {:?}", wait_result); self.pinger_stop_sender.close(); self.runtime @@ -968,6 +985,7 @@ impl WireguardMonitor { } /// Replace default (0-prefix) routes with more specific routes. + #[cfg(all(unix, not(target_os = "android")))] fn replace_default_prefixes(network: ipnetwork::IpNetwork) -> Vec { #[cfg(windows)] if network.prefix() == 0 { diff --git a/talpid-wireguard/src/wireguard_go/mod.rs b/talpid-wireguard/src/wireguard_go/mod.rs index a8ce7d0aacb3..776787035e48 100644 --- a/talpid-wireguard/src/wireguard_go/mod.rs +++ b/talpid-wireguard/src/wireguard_go/mod.rs @@ -1,5 +1,6 @@ #[cfg(target_os = "android")] use super::config; +#[cfg(target_os = "android")] use super::{ stats::{Stats, StatsMap}, CloseMsg, Config, Error, Tunnel, TunnelError, @@ -9,7 +10,7 @@ use crate::config::MULLVAD_INTERFACE_NAME; #[cfg(target_os = "android")] use crate::connectivity; use crate::logging::{clean_up_logging, initialize_logging}; -#[cfg(unix)] +#[cfg(all(unix, not(target_os = "android")))] use ipnetwork::IpNetwork; #[cfg(daita)] use std::ffi::CString; @@ -22,7 +23,8 @@ use std::{ path::{Path, PathBuf}, pin::Pin, }; -use talpid_routing::{RouteManagerHandle}; +#[cfg(target_os = "android")] +use talpid_routing::RouteManagerHandle; #[cfg(target_os = "android")] use talpid_tunnel::tun_provider::Error as TunProviderError; #[cfg(not(target_os = "windows"))] @@ -116,7 +118,6 @@ impl WgGoTunnel { let log_path = state._logging_context.path.clone(); let cancel_receiver = state.cancel_receiver.clone(); let tun_provider = Arc::clone(&state.tun_provider); - let routes = config.get_tunnel_destinations(); let route_manager = &state.route_manager.clone(); match self { @@ -127,7 +128,6 @@ impl WgGoTunnel { log_path.as_deref(), tun_provider, route_manager.clone(), - routes, cancel_receiver, ) .await @@ -140,7 +140,6 @@ impl WgGoTunnel { log_path.as_deref(), tun_provider, route_manager.clone(), - routes, cancel_receiver, ) .await @@ -347,7 +346,41 @@ impl WgGoTunnel { } } - #[cfg(unix)] + #[cfg(target_os = "android")] + fn get_tunnel( + tun_provider: Arc>, + config: &Config, + ) -> Result<(Tun, RawFd, bool)> { + log::debug!("WgGoTunnel get_tunnel"); + let mut tun_provider = tun_provider.lock().unwrap(); + let mut last_error = None; + let tun_config = tun_provider.config_mut(); + + tun_config.addresses = config.tunnel.addresses.clone(); + tun_config.ipv4_gateway = config.ipv4_gateway; + tun_config.ipv6_gateway = config.ipv6_gateway; + tun_config.mtu = config.mtu; + tun_config.routes = vec!["0.0.0.0/0".parse().unwrap(), "::/0".parse().unwrap()]; + + for _ in 1..=MAX_PREPARE_TUN_ATTEMPTS { + let (tunnel_device, is_new_tunnel) = tun_provider + .open_tun() + .map_err(TunnelError::SetupTunnelDevice)?; + + match nix::unistd::dup(tunnel_device.as_raw_fd()) { + Ok(fd) => return Ok((tunnel_device, fd, is_new_tunnel)), + #[cfg(not(target_os = "macos"))] + Err(error @ nix::errno::Errno::EBADFD) => last_error = Some(error), + Err(error @ nix::errno::Errno::EBADF) => last_error = Some(error), + Err(error) => return Err(TunnelError::FdDuplicationError(error)), + } + } + + Err(TunnelError::FdDuplicationError( + last_error.expect("Should be collected in loop"), + )) + } + #[cfg(any(target_os = "linux", target_os = "macos"))] fn get_tunnel( tun_provider: Arc>, config: &Config, @@ -399,13 +432,14 @@ impl WgGoTunnel { log_path: Option<&Path>, tun_provider: Arc>, route_manager: RouteManagerHandle, - routes: impl Iterator, cancel_receiver: connectivity::CancelReceiver, ) -> Result { let _ = route_manager.clear_android_routes().await; - let (mut tunnel_device, tunnel_fd) = - Self::get_tunnel(Arc::clone(&tun_provider), config, routes)?; + let (mut tunnel_device, tunnel_fd, is_new_tunnel) = + Self::get_tunnel(Arc::clone(&tun_provider), config)?; + + log::debug!("DEBUG: get_tunnel"); let interface_name: String = tunnel_device .interface_name() @@ -439,8 +473,9 @@ impl WgGoTunnel { cancel_receiver, }); - // HACK: Check if the tunnel is working by sending a ping in the tunnel. - tunnel.ensure_tunnel_is_running().await?; + if is_new_tunnel { + tunnel.wait_for_routes().await?; + } Ok(tunnel) } @@ -451,13 +486,12 @@ impl WgGoTunnel { log_path: Option<&Path>, tun_provider: Arc>, route_manager: RouteManagerHandle, - routes: impl Iterator, cancel_receiver: connectivity::CancelReceiver, ) -> Result { let _ = route_manager.clear_android_routes().await; - let (mut tunnel_device, tunnel_fd) = - Self::get_tunnel(Arc::clone(&tun_provider), config, routes)?; + let (mut tunnel_device, tunnel_fd, is_new_tunnel) = + Self::get_tunnel(Arc::clone(&tun_provider), config)?; let interface_name: String = tunnel_device .interface_name() @@ -507,8 +541,9 @@ impl WgGoTunnel { cancel_receiver: cancel_receiver.clone(), }); - // HACK: Check if the tunnel is working by sending a ping in the tunnel. - tunnel.ensure_tunnel_is_running().await?; + if is_new_tunnel { + tunnel.wait_for_routes().await?; + } Ok(tunnel) } @@ -528,7 +563,7 @@ impl WgGoTunnel { /// There is a brief period of time between setting up a Wireguard-go tunnel and the tunnel being ready to serve /// traffic. This function blocks until the tunnel starts to serve traffic or until [connectivity::Check] times out. - async fn ensure_tunnel_is_running(&self) -> Result<()> { + async fn wait_for_routes(&self) -> Result<()> { let state = self.as_state(); let expected_routes = state.tun_provider.lock().unwrap().real_routes(); @@ -544,25 +579,6 @@ impl WgGoTunnel { .map_err(CloseMsg::SetupError) .unwrap(); - let addr = state.config.ipv4_gateway; - let cancel_receiver = state.cancel_receiver.clone(); - let mut check = connectivity::Check::new(addr, 0, cancel_receiver) - .map_err(|err| TunnelError::RecoverableStartWireguardError(Box::new(err)))?; - - // TODO: retry attempt? - - let connection_established = check - .establish_connectivity(self) - .await - .map_err(|e| TunnelError::RecoverableStartWireguardError(Box::new(e)))?; - - // Timed out - if !connection_established { - return Err(TunnelError::RecoverableStartWireguardError(Box::new( - super::Error::TimeoutError, - ))); - } - Ok(()) } } From f4451d5f63fc5126840aa20a969834e04a751bb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20G=C3=B6ransson?= Date: Fri, 14 Feb 2025 14:39:21 +0100 Subject: [PATCH 05/10] wip --- .../mullvad/talpid/ConnectivityListener.kt | 42 ++++++++++++------- .../net/mullvad/talpid/TalpidVpnService.kt | 10 ++++- talpid-core/src/tunnel/mod.rs | 3 ++ .../tunnel_state_machine/connecting_state.rs | 6 +++ .../src/tunnel_state_machine/error_state.rs | 3 ++ talpid-core/src/tunnel_state_machine/mod.rs | 2 + talpid-wireguard/src/connectivity/monitor.rs | 9 ++++ talpid-wireguard/src/lib.rs | 13 +++++- talpid-wireguard/src/wireguard_go/mod.rs | 6 +++ wireguard-go-rs/src/lib.rs | 2 + 10 files changed, 78 insertions(+), 18 deletions(-) diff --git a/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/ConnectivityListener.kt b/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/ConnectivityListener.kt index 1bc25293bbad..8d37528f042d 100644 --- a/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/ConnectivityListener.kt +++ b/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/ConnectivityListener.kt @@ -13,6 +13,7 @@ import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.drop import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.merge import kotlinx.coroutines.flow.onEach @@ -50,46 +51,55 @@ class ConnectivityListener( _currentNetworkState = merge(connectivityManager.defaultRawNetworkStateFlow(), resetDnsFlow.map { null }) .map { it?.toNetworkState() } - .onEach { notifyDefaultNetworkChange(it) } + .onEach { + Logger.d("NetworkState routes: ${it?.routes}") + notifyDefaultNetworkChange(it) + } .stateIn(scope, SharingStarted.Eagerly, null) + @Suppress("DEPRECATION") _isConnected = hasInternetCapability() .onEach { notifyConnectivityChange(it) } - .stateIn(scope, SharingStarted.Eagerly, false) + .stateIn( + scope, + SharingStarted.Eagerly, + true, // Assume we have internet until we know otherwise + ) } private fun LinkProperties.dnsServersWithoutFallback(): List = dnsServers.filter { it.hostAddress != TalpidVpnService.FALLBACK_DUMMY_DNS_SERVER } + private val nonVPNNetworksRequest = + NetworkRequest.Builder().addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN).build() + private fun hasInternetCapability(): Flow { - val request = - NetworkRequest.Builder() - .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) - .addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN) - .build() return connectivityManager - .networkEvents(request) - .scan(setOf()) { networks, event -> + .networkEvents(nonVPNNetworksRequest) + .scan(mapOf()) { networks, event -> when (event) { - is NetworkEvent.Available -> { - Logger.d("Network available ${event.network}") - (networks + event.network).also { - Logger.d("Number of networks: ${it.size}") - } - } is NetworkEvent.Lost -> { Logger.d("Network lost ${event.network}") + (networks - event.network).also { Logger.d("Number of networks: ${it.size}") } } + is NetworkEvent.CapabilitiesChanged -> { + Logger.d("Network capabilities changed ${event.network}") + (networks + (event.network to event.networkCapabilities)).also { + Logger.d("Number of networks: ${it.size}") + } + } else -> networks } } - .map { it.isNotEmpty() } .distinctUntilChanged() + .drop(1) + .map { it.any { it.value?.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) == true} } + .onEach { Logger.d("Do we have connectivity? $it") } } private fun RawNetworkState.toNetworkState(): NetworkState = diff --git a/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt b/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt index 9a26d5d0d115..53f746aac87b 100644 --- a/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt +++ b/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt @@ -41,6 +41,7 @@ open class TalpidVpnService : LifecycleVpnService() { } if (oldTunFd != null) { + Logger.d("Closing old tunFd $oldTunFd") ParcelFileDescriptor.adoptFd(oldTunFd).close() } } @@ -65,6 +66,7 @@ open class TalpidVpnService : LifecycleVpnService() { // Used by JNI fun openTun(config: TunConfig): CreateTunResult = synchronized(this) { + Logger.d("TalpidVpnService.openTun") createTun(config).merge().also { currentTunConfig = config activeTunStatus = it @@ -74,17 +76,22 @@ open class TalpidVpnService : LifecycleVpnService() { // Used by JNI fun closeTun(): Unit = synchronized(this) { + Logger.d("TalpidVpnService.closeTun") runBlocking { resetDnsChannel.send(Unit) } activeTunStatus = null } // Used by JNI - fun bypass(socket: Int): Boolean = protect(socket) + fun bypass(socket: Int): Boolean { + Logger.d("TalpidVpnService.bypass") + return protect(socket) + } private fun createTun( config: TunConfig ): Either = either { prepareVpnSafe().mapLeft { it.toCreateTunError() }.bind() + Logger.d("TalpidVpnService.createTun $config") val builder = Builder() builder.setMtu(config.mtu) @@ -126,6 +133,7 @@ open class TalpidVpnService : LifecycleVpnService() { .onLeft { Logger.w("Failed to establish tunnel $it") } .mapLeft { EstablishError } .bind() + Logger.d("Establish!") val tunFd = vpnInterfaceFd.detachFd() diff --git a/talpid-core/src/tunnel/mod.rs b/talpid-core/src/tunnel/mod.rs index 70d496b03196..165087f138d7 100644 --- a/talpid-core/src/tunnel/mod.rs +++ b/talpid-core/src/tunnel/mod.rs @@ -132,6 +132,8 @@ impl TunnelMonitor { log_dir: &Option, args: TunnelArgs<'_>, ) -> Result { + + log::debug!("DEBUG: TunnelMonitor::start"); Self::ensure_ipv6_can_be_used_if_enabled(tunnel_parameters)?; let log_file = Self::prepare_tunnel_log_file(tunnel_parameters, log_dir)?; @@ -182,6 +184,7 @@ impl TunnelMonitor { log: Option, args: TunnelArgs<'_>, ) -> Result { + log::debug!("DEBUG: start_wireguard_tunnel"); let monitor = talpid_wireguard::WireguardMonitor::start(params, log.as_deref(), args)?; Ok(TunnelMonitor { monitor: InternalTunnelMonitor::Wireguard(monitor), diff --git a/talpid-core/src/tunnel_state_machine/connecting_state.rs b/talpid-core/src/tunnel_state_machine/connecting_state.rs index 994a1154dbbf..6c226979a90f 100644 --- a/talpid-core/src/tunnel_state_machine/connecting_state.rs +++ b/talpid-core/src/tunnel_state_machine/connecting_state.rs @@ -52,6 +52,7 @@ impl ConnectingState { shared_values: &mut SharedTunnelStateValues, retry_attempt: u32, ) -> (Box, TunnelStateTransition) { + log::debug!("DEBUG: ENTERING CONNECTING STATE"); #[cfg(target_os = "macos")] if *LOCAL_DNS_RESOLVER { // Set system DNS to our local DNS resolver @@ -73,6 +74,7 @@ impl ConnectingState { } if shared_values.connectivity.is_offline() { + // FIXME: Temporary: Nudge route manager to update the default interface #[cfg(target_os = "macos")] { @@ -128,6 +130,7 @@ impl ConnectingState { &shared_values.route_manager, retry_attempt, ); + log::debug!("Connecting state {:?}", connecting_state); let params = connecting_state.tunnel_parameters.clone(); ( @@ -209,6 +212,7 @@ impl ConnectingState { route_manager: &RouteManagerHandle, retry_attempt: u32, ) -> Self { + log::debug!("DEBUG: ConnectingState::start_tunnel"); let (event_tx, event_rx) = mpsc::unbounded(); let event_hook = EventHook::new(event_tx); @@ -234,8 +238,10 @@ impl ConnectingState { route_manager, }; + log::debug!("DEBUG: ConnectingState::start_tunnel start"); let block_reason = match TunnelMonitor::start(&tunnel_parameters, &log_dir, args) { Ok(monitor) => { + log::debug!("DEBUG: ConnectingState::start_tunnel success"); let reason = Self::wait_for_tunnel_monitor(monitor, retry_attempt); log::debug!("Tunnel monitor exited with block reason: {:?}", reason); reason diff --git a/talpid-core/src/tunnel_state_machine/error_state.rs b/talpid-core/src/tunnel_state_machine/error_state.rs index 0cf392a0e572..b52e95ef9c2b 100644 --- a/talpid-core/src/tunnel_state_machine/error_state.rs +++ b/talpid-core/src/tunnel_state_machine/error_state.rs @@ -55,6 +55,7 @@ impl ErrorState { #[cfg(not(target_os = "android"))] let block_failure = Self::set_firewall_policy(shared_values).err(); + log::debug!("DEBUG: DID WE REALLY ENTER ERROR?!"); #[cfg(target_os = "android")] let block_failure = if shared_values.restart_tunnel(true).is_err() { Some(FirewallPolicyError::Generic) @@ -125,6 +126,7 @@ impl TunnelState for ErrorState { match runtime.block_on(commands.next()) { Some(TunnelCommand::AllowLan(allow_lan, complete_tx)) => { let consequence = if shared_values.set_allow_lan(allow_lan) { + log::debug!("DEBUG: Just allowing lan in error, don't mind me"); #[cfg(target_os = "android")] if let Err(_err) = shared_values.restart_tunnel(true) { NewState(Self::enter( @@ -217,6 +219,7 @@ impl TunnelState for ErrorState { #[cfg(target_os = "android")] Some(TunnelCommand::SetExcludedApps(result_tx, paths)) => { if shared_values.set_excluded_paths(paths) { + log::debug!("DEBUG: Just excluding apps error, don't mind me"); if let Err(err) = shared_values.restart_tunnel(true) { let _ = result_tx.send(Err(crate::split_tunnel::Error::SetExcludedApps(err))); diff --git a/talpid-core/src/tunnel_state_machine/mod.rs b/talpid-core/src/tunnel_state_machine/mod.rs index cae7f2384bf5..2e5556146437 100644 --- a/talpid-core/src/tunnel_state_machine/mod.rs +++ b/talpid-core/src/tunnel_state_machine/mod.rs @@ -347,6 +347,7 @@ impl TunnelStateMachine { ) .await; let connectivity = offline_monitor.connectivity().await; + log::debug!("DEBUG: Initial connectivity: {:?}", connectivity); let _ = initial_offline_state_tx.unbounded_send(connectivity); #[cfg(windows)] @@ -653,6 +654,7 @@ impl SharedTunnelStateValues { #[cfg(target_os = "android")] pub fn restart_tunnel(&self, blocking: bool) -> Result<(), talpid_tunnel::tun_provider::Error> { self.prepare_tun_config(blocking); + log::debug!("DEBUG: RESTART tunnel!"); match self.tun_provider.lock().unwrap().open_tun() { Ok(_tun) => Ok(()), diff --git a/talpid-wireguard/src/connectivity/monitor.rs b/talpid-wireguard/src/connectivity/monitor.rs index 1272b43f4db2..1f8a2b8784b5 100644 --- a/talpid-wireguard/src/connectivity/monitor.rs +++ b/talpid-wireguard/src/connectivity/monitor.rs @@ -34,6 +34,7 @@ impl Monitor { loop { if self.connectivity_check.should_shut_down() { + log::debug!("Should shut down!:w"); return Ok(()); } @@ -42,8 +43,10 @@ impl Monitor { last_check = now; if time_slept >= SUSPEND_TIMEOUT { + log::debug!("Maybe this?"); self.connectivity_check.reset(now).await; } else if !self.tunnel_exists_and_is_connected(&tunnel_handle).await? { + log::debug!("Did we hit this?"); return Ok(()); } @@ -55,16 +58,22 @@ impl Monitor { &mut self, tunnel_handle: &Weak>>, ) -> Result { + log::debug!("tunnel_exists_and_is_connected"); let Some(tunnel) = tunnel_handle.upgrade() else { + log::debug!("Tunnel closed!"); // Tunnel closed return Ok(false); }; + log::debug!("tunnel_exists_and_is_connected 2"); let lock = tunnel.lock().await; let Some(tunnel) = lock.as_ref() else { // Tunnel closed + log::debug!("Tunnel closed2!"); return Ok(false); }; + log::debug!("tunnel_exists_and_is_connected 3"); + log::debug!("Doing conncheck!"); self.connectivity_check .check_connectivity(Instant::now(), tunnel) .await diff --git a/talpid-wireguard/src/lib.rs b/talpid-wireguard/src/lib.rs index a9e0fb336492..ba92c64103a4 100644 --- a/talpid-wireguard/src/lib.rs +++ b/talpid-wireguard/src/lib.rs @@ -432,6 +432,8 @@ impl WireguardMonitor { ) .map_err(Error::ConnectivityMonitorError)?; + log::debug!("DEBUG: WireguardMonitor start"); + let tunnel = args.runtime.block_on(Self::open_wireguard_go_tunnel( &config, log_path, @@ -529,6 +531,8 @@ impl WireguardMonitor { ); } + log::debug!("Monitor failed?!"); + Err::(CloseMsg::PingErr) }; @@ -785,11 +789,14 @@ impl WireguardMonitor { // tunnel to where the ephemeral peer resides. // // Refer to `docs/architecture.md` for details on how to use multihop + PQ. + log::debug!("patching allowed ips"); #[cfg(target_os = "android")] let config = Self::patch_allowed_ips(config, gateway_only); #[cfg(target_os = "android")] let tunnel = if let Some(exit_peer) = &config.exit_peer { + + log::debug!("Starting multihop tunnel"); WgGoTunnel::start_multihop_tunnel( &config, exit_peer, @@ -801,6 +808,7 @@ impl WireguardMonitor { .await .map_err(Error::TunnelError)? } else { + log::debug!("Starting singlehop tunnel"); WgGoTunnel::start_tunnel( #[allow(clippy::needless_borrow)] &config, @@ -831,11 +839,13 @@ impl WireguardMonitor { log::debug!("Wait result : {:?}", wait_result); self.pinger_stop_sender.close(); + log::debug!("Wait result : {:?}", wait_result); self.runtime .block_on(self.event_hook.on_event(TunnelEvent::Down)); + log::debug!("About to stop tunnel: {:?}", wait_result); self.stop_tunnel(); - + log::debug!("Tunnel stopped: {:?}", wait_result); wait_result } @@ -843,6 +853,7 @@ impl WireguardMonitor { /// /// NOTE: will panic if called from within a tokio runtime. fn stop_tunnel(&mut self) { + log::debug!("Blocking lock"); match self.tunnel.blocking_lock().take() { Some(tunnel) => { if let Err(e) = tunnel.stop() { diff --git a/talpid-wireguard/src/wireguard_go/mod.rs b/talpid-wireguard/src/wireguard_go/mod.rs index 776787035e48..8510fc5d2d9f 100644 --- a/talpid-wireguard/src/wireguard_go/mod.rs +++ b/talpid-wireguard/src/wireguard_go/mod.rs @@ -145,11 +145,13 @@ impl WgGoTunnel { .await } WgGoTunnel::Singlehop(mut state) => { + log::debug!("Switching Singlehop WG conf"); state.set_config(config.clone())?; let new_state = WgGoTunnel::Singlehop(state); Ok(new_state) } WgGoTunnel::Multihop(mut state) => { + log::debug!("Switching Multihop WG conf"); state.set_config(config.clone())?; let new_state = WgGoTunnel::Multihop(state); Ok(new_state) @@ -434,6 +436,7 @@ impl WgGoTunnel { route_manager: RouteManagerHandle, cancel_receiver: connectivity::CancelReceiver, ) -> Result { + log::debug!("WgGoTunnel start_tunnel"); let _ = route_manager.clear_android_routes().await; let (mut tunnel_device, tunnel_fd, is_new_tunnel) = @@ -450,6 +453,7 @@ impl WgGoTunnel { let wg_config_str = config.to_userspace_format(); + log::debug!("DEBUG: get_tunnel 1"); let handle = wireguard_go_rs::Tunnel::turn_on( &wg_config_str, tunnel_fd, @@ -457,6 +461,7 @@ impl WgGoTunnel { logging_context.ordinal, ) .map_err(|e| TunnelError::FatalStartWireguardError(Box::new(e)))?; + log::debug!("DEBUG: get_tunnel 2"); Self::bypass_tunnel_sockets(&handle, &mut tunnel_device) .map_err(TunnelError::BypassError)?; @@ -477,6 +482,7 @@ impl WgGoTunnel { tunnel.wait_for_routes().await?; } + log::debug!("DEBUG: get_tunnel 4"); Ok(tunnel) } diff --git a/wireguard-go-rs/src/lib.rs b/wireguard-go-rs/src/lib.rs index 37756a7afbf6..96b28a426033 100644 --- a/wireguard-go-rs/src/lib.rs +++ b/wireguard-go-rs/src/lib.rs @@ -155,7 +155,9 @@ impl Tunnel { pub fn turn_off(self) -> Result<(), Error> { // we manually turn off the tunnel here, so wrap it in ManuallyDrop to prevent the Drop // impl from doing the same. + log::debug!("About to turn off"); let code = unsafe { ffi::wgTurnOff(self.handle) }; + log::debug!("Did we turn off? :["); let _ = ManuallyDrop::new(self); result_from_code(code) } From 677ac028c1a5be86ef4b85f7e75fe345e787b3c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20G=C3=B6ransson?= Date: Mon, 17 Feb 2025 20:31:37 +0100 Subject: [PATCH 06/10] Avoid Go crash --- talpid-wireguard/src/lib.rs | 20 +--------------- talpid-wireguard/src/wireguard_go/mod.rs | 29 ++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 19 deletions(-) diff --git a/talpid-wireguard/src/lib.rs b/talpid-wireguard/src/lib.rs index ba92c64103a4..260a09a7daf8 100644 --- a/talpid-wireguard/src/lib.rs +++ b/talpid-wireguard/src/lib.rs @@ -425,7 +425,7 @@ impl WireguardMonitor { let should_negotiate_ephemeral_peer = config.quantum_resistant || config.daita; let (cancel_token, cancel_receiver) = connectivity::CancelToken::new(); - let mut connectivity_check = connectivity::Check::new( + let connectivity_check = connectivity::Check::new( config.ipv4_gateway, args.retry_attempt, cancel_receiver.clone(), @@ -500,24 +500,6 @@ impl WireguardMonitor { .await; } - match connectivity_check - .establish_connectivity(&tunnel.lock().await.as_ref().unwrap()) - .await - { - Ok(true) => Ok(()), - Ok(false) => { - log::warn!("Timeout while checking tunnel connection"); - Err(CloseMsg::PingErr) - } - Err(error) => { - log::error!( - "{}", - error.display_chain_with_msg("Failed to check tunnel connection") - ); - Err(CloseMsg::PingErr) - } - }?; - let metadata = Self::tunnel_metadata(&iface_name, &config); event_hook.on_event(TunnelEvent::Up(metadata)).await; diff --git a/talpid-wireguard/src/wireguard_go/mod.rs b/talpid-wireguard/src/wireguard_go/mod.rs index 8510fc5d2d9f..bd37d1fc1657 100644 --- a/talpid-wireguard/src/wireguard_go/mod.rs +++ b/talpid-wireguard/src/wireguard_go/mod.rs @@ -482,7 +482,10 @@ impl WgGoTunnel { tunnel.wait_for_routes().await?; } + // This seemingly fixes the GO crash we see + tunnel.ensure_tunnel_is_running().await?; log::debug!("DEBUG: get_tunnel 4"); + Ok(tunnel) } @@ -551,6 +554,9 @@ impl WgGoTunnel { tunnel.wait_for_routes().await?; } + // This seemingly fixes the GO crash we see + tunnel.ensure_tunnel_is_running().await?; + Ok(tunnel) } @@ -585,6 +591,29 @@ impl WgGoTunnel { .map_err(CloseMsg::SetupError) .unwrap(); + Ok(()) + } + async fn ensure_tunnel_is_running(&self) -> Result<()> { + let state = self.as_state(); + let addr = state.config.ipv4_gateway; + let cancel_receiver = state.cancel_receiver.clone(); + let mut check = connectivity::Check::new(addr, 0, cancel_receiver) + .map_err(|err| TunnelError::RecoverableStartWireguardError(Box::new(err)))?; + + // TODO: retry attempt? + + let connection_established = check + .establish_connectivity(self) + .await + .map_err(|e| TunnelError::RecoverableStartWireguardError(Box::new(e)))?; + + // Timed out + if !connection_established { + return Err(TunnelError::RecoverableStartWireguardError(Box::new( + super::Error::TimeoutError, + ))); + } + Ok(()) } } From aa20fb95600074fc671b5e21bb5dfb7adecde94d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20G=C3=B6ransson?= Date: Mon, 17 Feb 2025 20:44:53 +0100 Subject: [PATCH 07/10] Remove logs --- .../kotlin/net/mullvad/talpid/TalpidVpnService.kt | 10 +--------- talpid-core/src/tunnel/mod.rs | 3 --- .../src/tunnel_state_machine/connecting_state.rs | 6 ------ talpid-core/src/tunnel_state_machine/error_state.rs | 3 --- talpid-core/src/tunnel_state_machine/mod.rs | 2 -- talpid-wireguard/src/connectivity/monitor.rs | 9 --------- talpid-wireguard/src/lib.rs | 12 ------------ talpid-wireguard/src/wireguard_go/mod.rs | 9 --------- wireguard-go-rs/src/lib.rs | 2 -- 9 files changed, 1 insertion(+), 55 deletions(-) diff --git a/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt b/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt index 53f746aac87b..9a26d5d0d115 100644 --- a/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt +++ b/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt @@ -41,7 +41,6 @@ open class TalpidVpnService : LifecycleVpnService() { } if (oldTunFd != null) { - Logger.d("Closing old tunFd $oldTunFd") ParcelFileDescriptor.adoptFd(oldTunFd).close() } } @@ -66,7 +65,6 @@ open class TalpidVpnService : LifecycleVpnService() { // Used by JNI fun openTun(config: TunConfig): CreateTunResult = synchronized(this) { - Logger.d("TalpidVpnService.openTun") createTun(config).merge().also { currentTunConfig = config activeTunStatus = it @@ -76,22 +74,17 @@ open class TalpidVpnService : LifecycleVpnService() { // Used by JNI fun closeTun(): Unit = synchronized(this) { - Logger.d("TalpidVpnService.closeTun") runBlocking { resetDnsChannel.send(Unit) } activeTunStatus = null } // Used by JNI - fun bypass(socket: Int): Boolean { - Logger.d("TalpidVpnService.bypass") - return protect(socket) - } + fun bypass(socket: Int): Boolean = protect(socket) private fun createTun( config: TunConfig ): Either = either { prepareVpnSafe().mapLeft { it.toCreateTunError() }.bind() - Logger.d("TalpidVpnService.createTun $config") val builder = Builder() builder.setMtu(config.mtu) @@ -133,7 +126,6 @@ open class TalpidVpnService : LifecycleVpnService() { .onLeft { Logger.w("Failed to establish tunnel $it") } .mapLeft { EstablishError } .bind() - Logger.d("Establish!") val tunFd = vpnInterfaceFd.detachFd() diff --git a/talpid-core/src/tunnel/mod.rs b/talpid-core/src/tunnel/mod.rs index 165087f138d7..70d496b03196 100644 --- a/talpid-core/src/tunnel/mod.rs +++ b/talpid-core/src/tunnel/mod.rs @@ -132,8 +132,6 @@ impl TunnelMonitor { log_dir: &Option, args: TunnelArgs<'_>, ) -> Result { - - log::debug!("DEBUG: TunnelMonitor::start"); Self::ensure_ipv6_can_be_used_if_enabled(tunnel_parameters)?; let log_file = Self::prepare_tunnel_log_file(tunnel_parameters, log_dir)?; @@ -184,7 +182,6 @@ impl TunnelMonitor { log: Option, args: TunnelArgs<'_>, ) -> Result { - log::debug!("DEBUG: start_wireguard_tunnel"); let monitor = talpid_wireguard::WireguardMonitor::start(params, log.as_deref(), args)?; Ok(TunnelMonitor { monitor: InternalTunnelMonitor::Wireguard(monitor), diff --git a/talpid-core/src/tunnel_state_machine/connecting_state.rs b/talpid-core/src/tunnel_state_machine/connecting_state.rs index 6c226979a90f..994a1154dbbf 100644 --- a/talpid-core/src/tunnel_state_machine/connecting_state.rs +++ b/talpid-core/src/tunnel_state_machine/connecting_state.rs @@ -52,7 +52,6 @@ impl ConnectingState { shared_values: &mut SharedTunnelStateValues, retry_attempt: u32, ) -> (Box, TunnelStateTransition) { - log::debug!("DEBUG: ENTERING CONNECTING STATE"); #[cfg(target_os = "macos")] if *LOCAL_DNS_RESOLVER { // Set system DNS to our local DNS resolver @@ -74,7 +73,6 @@ impl ConnectingState { } if shared_values.connectivity.is_offline() { - // FIXME: Temporary: Nudge route manager to update the default interface #[cfg(target_os = "macos")] { @@ -130,7 +128,6 @@ impl ConnectingState { &shared_values.route_manager, retry_attempt, ); - log::debug!("Connecting state {:?}", connecting_state); let params = connecting_state.tunnel_parameters.clone(); ( @@ -212,7 +209,6 @@ impl ConnectingState { route_manager: &RouteManagerHandle, retry_attempt: u32, ) -> Self { - log::debug!("DEBUG: ConnectingState::start_tunnel"); let (event_tx, event_rx) = mpsc::unbounded(); let event_hook = EventHook::new(event_tx); @@ -238,10 +234,8 @@ impl ConnectingState { route_manager, }; - log::debug!("DEBUG: ConnectingState::start_tunnel start"); let block_reason = match TunnelMonitor::start(&tunnel_parameters, &log_dir, args) { Ok(monitor) => { - log::debug!("DEBUG: ConnectingState::start_tunnel success"); let reason = Self::wait_for_tunnel_monitor(monitor, retry_attempt); log::debug!("Tunnel monitor exited with block reason: {:?}", reason); reason diff --git a/talpid-core/src/tunnel_state_machine/error_state.rs b/talpid-core/src/tunnel_state_machine/error_state.rs index b52e95ef9c2b..0cf392a0e572 100644 --- a/talpid-core/src/tunnel_state_machine/error_state.rs +++ b/talpid-core/src/tunnel_state_machine/error_state.rs @@ -55,7 +55,6 @@ impl ErrorState { #[cfg(not(target_os = "android"))] let block_failure = Self::set_firewall_policy(shared_values).err(); - log::debug!("DEBUG: DID WE REALLY ENTER ERROR?!"); #[cfg(target_os = "android")] let block_failure = if shared_values.restart_tunnel(true).is_err() { Some(FirewallPolicyError::Generic) @@ -126,7 +125,6 @@ impl TunnelState for ErrorState { match runtime.block_on(commands.next()) { Some(TunnelCommand::AllowLan(allow_lan, complete_tx)) => { let consequence = if shared_values.set_allow_lan(allow_lan) { - log::debug!("DEBUG: Just allowing lan in error, don't mind me"); #[cfg(target_os = "android")] if let Err(_err) = shared_values.restart_tunnel(true) { NewState(Self::enter( @@ -219,7 +217,6 @@ impl TunnelState for ErrorState { #[cfg(target_os = "android")] Some(TunnelCommand::SetExcludedApps(result_tx, paths)) => { if shared_values.set_excluded_paths(paths) { - log::debug!("DEBUG: Just excluding apps error, don't mind me"); if let Err(err) = shared_values.restart_tunnel(true) { let _ = result_tx.send(Err(crate::split_tunnel::Error::SetExcludedApps(err))); diff --git a/talpid-core/src/tunnel_state_machine/mod.rs b/talpid-core/src/tunnel_state_machine/mod.rs index 2e5556146437..cae7f2384bf5 100644 --- a/talpid-core/src/tunnel_state_machine/mod.rs +++ b/talpid-core/src/tunnel_state_machine/mod.rs @@ -347,7 +347,6 @@ impl TunnelStateMachine { ) .await; let connectivity = offline_monitor.connectivity().await; - log::debug!("DEBUG: Initial connectivity: {:?}", connectivity); let _ = initial_offline_state_tx.unbounded_send(connectivity); #[cfg(windows)] @@ -654,7 +653,6 @@ impl SharedTunnelStateValues { #[cfg(target_os = "android")] pub fn restart_tunnel(&self, blocking: bool) -> Result<(), talpid_tunnel::tun_provider::Error> { self.prepare_tun_config(blocking); - log::debug!("DEBUG: RESTART tunnel!"); match self.tun_provider.lock().unwrap().open_tun() { Ok(_tun) => Ok(()), diff --git a/talpid-wireguard/src/connectivity/monitor.rs b/talpid-wireguard/src/connectivity/monitor.rs index 1f8a2b8784b5..1272b43f4db2 100644 --- a/talpid-wireguard/src/connectivity/monitor.rs +++ b/talpid-wireguard/src/connectivity/monitor.rs @@ -34,7 +34,6 @@ impl Monitor { loop { if self.connectivity_check.should_shut_down() { - log::debug!("Should shut down!:w"); return Ok(()); } @@ -43,10 +42,8 @@ impl Monitor { last_check = now; if time_slept >= SUSPEND_TIMEOUT { - log::debug!("Maybe this?"); self.connectivity_check.reset(now).await; } else if !self.tunnel_exists_and_is_connected(&tunnel_handle).await? { - log::debug!("Did we hit this?"); return Ok(()); } @@ -58,22 +55,16 @@ impl Monitor { &mut self, tunnel_handle: &Weak>>, ) -> Result { - log::debug!("tunnel_exists_and_is_connected"); let Some(tunnel) = tunnel_handle.upgrade() else { - log::debug!("Tunnel closed!"); // Tunnel closed return Ok(false); }; - log::debug!("tunnel_exists_and_is_connected 2"); let lock = tunnel.lock().await; let Some(tunnel) = lock.as_ref() else { // Tunnel closed - log::debug!("Tunnel closed2!"); return Ok(false); }; - log::debug!("tunnel_exists_and_is_connected 3"); - log::debug!("Doing conncheck!"); self.connectivity_check .check_connectivity(Instant::now(), tunnel) .await diff --git a/talpid-wireguard/src/lib.rs b/talpid-wireguard/src/lib.rs index 260a09a7daf8..05b55c173796 100644 --- a/talpid-wireguard/src/lib.rs +++ b/talpid-wireguard/src/lib.rs @@ -432,8 +432,6 @@ impl WireguardMonitor { ) .map_err(Error::ConnectivityMonitorError)?; - log::debug!("DEBUG: WireguardMonitor start"); - let tunnel = args.runtime.block_on(Self::open_wireguard_go_tunnel( &config, log_path, @@ -513,8 +511,6 @@ impl WireguardMonitor { ); } - log::debug!("Monitor failed?!"); - Err::(CloseMsg::PingErr) }; @@ -777,8 +773,6 @@ impl WireguardMonitor { #[cfg(target_os = "android")] let tunnel = if let Some(exit_peer) = &config.exit_peer { - - log::debug!("Starting multihop tunnel"); WgGoTunnel::start_multihop_tunnel( &config, exit_peer, @@ -790,7 +784,6 @@ impl WireguardMonitor { .await .map_err(Error::TunnelError)? } else { - log::debug!("Starting singlehop tunnel"); WgGoTunnel::start_tunnel( #[allow(clippy::needless_borrow)] &config, @@ -818,16 +811,12 @@ impl WireguardMonitor { Err(_) => Ok(()), }; - log::debug!("Wait result : {:?}", wait_result); self.pinger_stop_sender.close(); - log::debug!("Wait result : {:?}", wait_result); self.runtime .block_on(self.event_hook.on_event(TunnelEvent::Down)); - log::debug!("About to stop tunnel: {:?}", wait_result); self.stop_tunnel(); - log::debug!("Tunnel stopped: {:?}", wait_result); wait_result } @@ -835,7 +824,6 @@ impl WireguardMonitor { /// /// NOTE: will panic if called from within a tokio runtime. fn stop_tunnel(&mut self) { - log::debug!("Blocking lock"); match self.tunnel.blocking_lock().take() { Some(tunnel) => { if let Err(e) = tunnel.stop() { diff --git a/talpid-wireguard/src/wireguard_go/mod.rs b/talpid-wireguard/src/wireguard_go/mod.rs index bd37d1fc1657..ac8ede6753f0 100644 --- a/talpid-wireguard/src/wireguard_go/mod.rs +++ b/talpid-wireguard/src/wireguard_go/mod.rs @@ -145,13 +145,11 @@ impl WgGoTunnel { .await } WgGoTunnel::Singlehop(mut state) => { - log::debug!("Switching Singlehop WG conf"); state.set_config(config.clone())?; let new_state = WgGoTunnel::Singlehop(state); Ok(new_state) } WgGoTunnel::Multihop(mut state) => { - log::debug!("Switching Multihop WG conf"); state.set_config(config.clone())?; let new_state = WgGoTunnel::Multihop(state); Ok(new_state) @@ -353,7 +351,6 @@ impl WgGoTunnel { tun_provider: Arc>, config: &Config, ) -> Result<(Tun, RawFd, bool)> { - log::debug!("WgGoTunnel get_tunnel"); let mut tun_provider = tun_provider.lock().unwrap(); let mut last_error = None; let tun_config = tun_provider.config_mut(); @@ -436,14 +433,11 @@ impl WgGoTunnel { route_manager: RouteManagerHandle, cancel_receiver: connectivity::CancelReceiver, ) -> Result { - log::debug!("WgGoTunnel start_tunnel"); let _ = route_manager.clear_android_routes().await; let (mut tunnel_device, tunnel_fd, is_new_tunnel) = Self::get_tunnel(Arc::clone(&tun_provider), config)?; - log::debug!("DEBUG: get_tunnel"); - let interface_name: String = tunnel_device .interface_name() .expect("Tunnel name trivially exists on Android"); @@ -453,7 +447,6 @@ impl WgGoTunnel { let wg_config_str = config.to_userspace_format(); - log::debug!("DEBUG: get_tunnel 1"); let handle = wireguard_go_rs::Tunnel::turn_on( &wg_config_str, tunnel_fd, @@ -461,7 +454,6 @@ impl WgGoTunnel { logging_context.ordinal, ) .map_err(|e| TunnelError::FatalStartWireguardError(Box::new(e)))?; - log::debug!("DEBUG: get_tunnel 2"); Self::bypass_tunnel_sockets(&handle, &mut tunnel_device) .map_err(TunnelError::BypassError)?; @@ -484,7 +476,6 @@ impl WgGoTunnel { // This seemingly fixes the GO crash we see tunnel.ensure_tunnel_is_running().await?; - log::debug!("DEBUG: get_tunnel 4"); Ok(tunnel) } diff --git a/wireguard-go-rs/src/lib.rs b/wireguard-go-rs/src/lib.rs index 96b28a426033..37756a7afbf6 100644 --- a/wireguard-go-rs/src/lib.rs +++ b/wireguard-go-rs/src/lib.rs @@ -155,9 +155,7 @@ impl Tunnel { pub fn turn_off(self) -> Result<(), Error> { // we manually turn off the tunnel here, so wrap it in ManuallyDrop to prevent the Drop // impl from doing the same. - log::debug!("About to turn off"); let code = unsafe { ffi::wgTurnOff(self.handle) }; - log::debug!("Did we turn off? :["); let _ = ManuallyDrop::new(self); result_from_code(code) } From 7e2fd0981aa67e3ec9710f7c193e8e16ef56b707 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20G=C3=B6ransson?= Date: Mon, 17 Feb 2025 20:47:14 +0100 Subject: [PATCH 08/10] Fix formatting --- .../main/kotlin/net/mullvad/talpid/ConnectivityListener.kt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/ConnectivityListener.kt b/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/ConnectivityListener.kt index 8d37528f042d..766be62ffaa7 100644 --- a/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/ConnectivityListener.kt +++ b/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/ConnectivityListener.kt @@ -98,7 +98,11 @@ class ConnectivityListener( } .distinctUntilChanged() .drop(1) - .map { it.any { it.value?.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) == true} } + .map { + it.any { + it.value?.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) == true + } + } .onEach { Logger.d("Do we have connectivity? $it") } } From e401796d584588e99a1e076e41afed55ff5996ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20G=C3=B6ransson?= Date: Tue, 18 Feb 2025 08:46:30 +0100 Subject: [PATCH 09/10] Clean up ConnectivityListener --- .../mullvad/talpid/ConnectivityListener.kt | 44 ++++++++++++------- .../net/mullvad/talpid/TalpidVpnService.kt | 14 ++---- .../talpid/util/ConnectivityManagerUtil.kt | 31 ++++++------- 3 files changed, 44 insertions(+), 45 deletions(-) diff --git a/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/ConnectivityListener.kt b/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/ConnectivityListener.kt index 766be62ffaa7..437a9b0ba65f 100644 --- a/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/ConnectivityListener.kt +++ b/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/ConnectivityListener.kt @@ -2,39 +2,38 @@ package net.mullvad.talpid import android.net.ConnectivityManager import android.net.LinkProperties -import android.net.Network import android.net.NetworkCapabilities import android.net.NetworkRequest import co.touchlab.kermit.Logger import java.net.InetAddress import kotlin.collections.ArrayList import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.distinctUntilChanged -import kotlinx.coroutines.flow.drop import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.merge import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.flow.scan import kotlinx.coroutines.flow.stateIn +import kotlinx.coroutines.runBlocking import net.mullvad.talpid.model.NetworkState import net.mullvad.talpid.util.NetworkEvent import net.mullvad.talpid.util.RawNetworkState import net.mullvad.talpid.util.defaultRawNetworkStateFlow import net.mullvad.talpid.util.networkEvents -class ConnectivityListener( - private val connectivityManager: ConnectivityManager, - private val resetDnsFlow: Flow, -) { +class ConnectivityListener(private val connectivityManager: ConnectivityManager) { private lateinit var _isConnected: StateFlow // Used by JNI val isConnected get() = _isConnected.value private lateinit var _currentNetworkState: StateFlow + private val resetNetworkState: Channel = Channel() // Used by JNI val currentDefaultNetworkState: NetworkState? @@ -49,7 +48,10 @@ class ConnectivityListener( // the default network may fail if the network on Android 11 // https://issuetracker.google.com/issues/175055271?pli=1 _currentNetworkState = - merge(connectivityManager.defaultRawNetworkStateFlow(), resetDnsFlow.map { null }) + merge( + connectivityManager.defaultRawNetworkStateFlow(), + resetNetworkState.receiveAsFlow().map { null }, + ) .map { it?.toNetworkState() } .onEach { Logger.d("NetworkState routes: ${it?.routes}") @@ -57,7 +59,6 @@ class ConnectivityListener( } .stateIn(scope, SharingStarted.Eagerly, null) - @Suppress("DEPRECATION") _isConnected = hasInternetCapability() .onEach { notifyConnectivityChange(it) } @@ -68,6 +69,15 @@ class ConnectivityListener( ) } + /** + * Invalidates the network state cache. E.g when the VPN is connected or disconnected, and we + * know the last known values not to be correct anymore. + */ + fun invalidateNetworkStateCache() { + // TODO remove runBlocking + runBlocking { resetNetworkState.send(Unit) } + } + private fun LinkProperties.dnsServersWithoutFallback(): List = dnsServers.filter { it.hostAddress != TalpidVpnService.FALLBACK_DUMMY_DNS_SERVER } @@ -75,10 +85,14 @@ class ConnectivityListener( NetworkRequest.Builder().addCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN).build() private fun hasInternetCapability(): Flow { - + @Suppress("DEPRECATION") return connectivityManager .networkEvents(nonVPNNetworksRequest) - .scan(mapOf()) { networks, event -> + .scan( + connectivityManager.allNetworks.associateWith { + connectivityManager.getNetworkCapabilities(it) + } + ) { networks, event -> when (event) { is NetworkEvent.Lost -> { Logger.d("Network lost ${event.network}") @@ -97,15 +111,13 @@ class ConnectivityListener( } } .distinctUntilChanged() - .drop(1) - .map { - it.any { - it.value?.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) == true - } - } + .map { it.any { it.value.hasInternetCapability() } } .onEach { Logger.d("Do we have connectivity? $it") } } + private fun NetworkCapabilities?.hasInternetCapability(): Boolean = + this?.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) == true + private fun RawNetworkState.toNetworkState(): NetworkState = NetworkState( network.networkHandle, diff --git a/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt b/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt index 9a26d5d0d115..1457ff35f441 100644 --- a/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt +++ b/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt @@ -15,9 +15,6 @@ import java.net.Inet4Address import java.net.Inet6Address import java.net.InetAddress import kotlin.properties.Delegates.observable -import kotlinx.coroutines.channels.Channel -import kotlinx.coroutines.flow.receiveAsFlow -import kotlinx.coroutines.runBlocking import net.mullvad.mullvadvpn.lib.common.util.establishSafe import net.mullvad.mullvadvpn.lib.common.util.prepareVpnSafe import net.mullvad.mullvadvpn.lib.model.PrepareError @@ -45,7 +42,6 @@ open class TalpidVpnService : LifecycleVpnService() { } } - private val resetDnsChannel = Channel() private var currentTunConfig: TunConfig? = null // Used by JNI @@ -54,11 +50,7 @@ open class TalpidVpnService : LifecycleVpnService() { @CallSuper override fun onCreate() { super.onCreate() - connectivityListener = - ConnectivityListener( - getSystemService()!!, - resetDnsChannel.receiveAsFlow(), - ) + connectivityListener = ConnectivityListener(getSystemService()!!) connectivityListener.register(lifecycleScope) } @@ -74,7 +66,7 @@ open class TalpidVpnService : LifecycleVpnService() { // Used by JNI fun closeTun(): Unit = synchronized(this) { - runBlocking { resetDnsChannel.send(Unit) } + connectivityListener.invalidateNetworkStateCache() activeTunStatus = null } @@ -119,7 +111,7 @@ open class TalpidVpnService : LifecycleVpnService() { builder.addDnsServer(FALLBACK_DUMMY_DNS_SERVER) } - runBlocking { resetDnsChannel.send(Unit) } + connectivityListener.invalidateNetworkStateCache() val vpnInterfaceFd = builder .establishSafe() diff --git a/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/util/ConnectivityManagerUtil.kt b/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/util/ConnectivityManagerUtil.kt index fddaa6fb8806..2f150cf67856 100644 --- a/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/util/ConnectivityManagerUtil.kt +++ b/android/lib/talpid/src/main/kotlin/net/mullvad/talpid/util/ConnectivityManagerUtil.kt @@ -109,24 +109,19 @@ fun ConnectivityManager.networkEvents(networkRequest: NetworkRequest): Flow = - defaultNetworkEvents() - .scan( - null as RawNetworkState?, - { state, event -> - return@scan when (event) { - is NetworkEvent.Available -> RawNetworkState(network = event.network) - is NetworkEvent.BlockedStatusChanged -> - state?.copy(blockedStatus = event.blocked) - is NetworkEvent.CapabilitiesChanged -> - state?.copy(networkCapabilities = event.networkCapabilities) - is NetworkEvent.LinkPropertiesChanged -> - state?.copy(linkProperties = event.linkProperties) - is NetworkEvent.Losing -> state?.copy(maxMsToLive = event.maxMsToLive) - is NetworkEvent.Lost -> null - NetworkEvent.Unavailable -> null - } - }, - ) + defaultNetworkEvents().scan(null as RawNetworkState?) { state, event -> state.reduce(event) } + +internal fun RawNetworkState?.reduce(event: NetworkEvent): RawNetworkState? = + when (event) { + is NetworkEvent.Available -> RawNetworkState(network = event.network) + is NetworkEvent.BlockedStatusChanged -> this?.copy(blockedStatus = event.blocked) + is NetworkEvent.CapabilitiesChanged -> + this?.copy(networkCapabilities = event.networkCapabilities) + is NetworkEvent.LinkPropertiesChanged -> this?.copy(linkProperties = event.linkProperties) + is NetworkEvent.Losing -> this?.copy(maxMsToLive = event.maxMsToLive) + is NetworkEvent.Lost -> null + NetworkEvent.Unavailable -> null + } sealed interface NetworkEvent { data class Available(val network: Network) : NetworkEvent From e1f9189b51f660869d4748da3d09f7e5bb9387ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20G=C3=B6ransson?= Date: Tue, 18 Feb 2025 08:52:23 +0100 Subject: [PATCH 10/10] Fix fmt --- talpid-core/src/tunnel_state_machine/connecting_state.rs | 2 +- talpid-routing/src/unix/android.rs | 2 +- talpid-routing/src/unix/mod.rs | 2 +- talpid-wireguard/src/wireguard_go/mod.rs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/talpid-core/src/tunnel_state_machine/connecting_state.rs b/talpid-core/src/tunnel_state_machine/connecting_state.rs index 994a1154dbbf..3b1f7dc76987 100644 --- a/talpid-core/src/tunnel_state_machine/connecting_state.rs +++ b/talpid-core/src/tunnel_state_machine/connecting_state.rs @@ -118,7 +118,7 @@ impl ConnectingState { // interface and this call should be part of start_tunnel call instead #[cfg(target_os = "android")] shared_values.prepare_tun_config(false); - + let connecting_state = Self::start_tunnel( shared_values.runtime.clone(), tunnel_parameters, diff --git a/talpid-routing/src/unix/android.rs b/talpid-routing/src/unix/android.rs index f8a345f88f85..9ade7a19f06c 100644 --- a/talpid-routing/src/unix/android.rs +++ b/talpid-routing/src/unix/android.rs @@ -1,5 +1,5 @@ use std::collections::HashSet; -use std::ops::{ControlFlow}; +use std::ops::ControlFlow; use std::sync::Mutex; use futures::channel::mpsc::{self, UnboundedReceiver, UnboundedSender}; diff --git a/talpid-routing/src/unix/mod.rs b/talpid-routing/src/unix/mod.rs index dd8f9108e171..63a68745e9e6 100644 --- a/talpid-routing/src/unix/mod.rs +++ b/talpid-routing/src/unix/mod.rs @@ -251,7 +251,7 @@ impl RouteManagerHandle { } /// xD - /// + /// #[cfg(target_os = "android")] pub async fn clear_android_routes(&self) -> Result<(), Error> { let (result_tx, result_rx) = oneshot::channel(); diff --git a/talpid-wireguard/src/wireguard_go/mod.rs b/talpid-wireguard/src/wireguard_go/mod.rs index ac8ede6753f0..d1b0495a60ba 100644 --- a/talpid-wireguard/src/wireguard_go/mod.rs +++ b/talpid-wireguard/src/wireguard_go/mod.rs @@ -476,7 +476,7 @@ impl WgGoTunnel { // This seemingly fixes the GO crash we see tunnel.ensure_tunnel_is_running().await?; - + Ok(tunnel) }