From a992009c71f1dcd16083b864bda6dd7a7fb80bc9 Mon Sep 17 00:00:00 2001 From: Zane Schepke Date: Sat, 30 Nov 2024 18:32:50 -0500 Subject: [PATCH] fix: restart on ping bugs Fixes bug where restart on ping could kill itself or not start correctly given certain settings combinations. This change also makes auto tunneling and all or nothing service as this intuitively makes the most sense with the way the global settings are presented. This change also makes it so users can toggle tunnel on untrusted wifi without location permissions because location permissions are only required when they go to add trusted ssids. --- .../autotunnel/AutoTunnelService.kt | 110 ++++++------------ .../foreground/autotunnel/AutoTunnelState.kt | 21 +++- .../ui/screens/main/MainViewModel.kt | 2 +- .../settings/autotunnel/AutoTunnelScreen.kt | 34 +++--- app/src/main/res/values/strings.xml | 2 +- 5 files changed, 69 insertions(+), 100 deletions(-) diff --git a/app/src/main/java/com/zaneschepke/wireguardautotunnel/service/foreground/autotunnel/AutoTunnelService.kt b/app/src/main/java/com/zaneschepke/wireguardautotunnel/service/foreground/autotunnel/AutoTunnelService.kt index b4028c6..07156c4 100644 --- a/app/src/main/java/com/zaneschepke/wireguardautotunnel/service/foreground/autotunnel/AutoTunnelService.kt +++ b/app/src/main/java/com/zaneschepke/wireguardautotunnel/service/foreground/autotunnel/AutoTunnelService.kt @@ -9,7 +9,6 @@ import androidx.lifecycle.LifecycleService import androidx.lifecycle.lifecycleScope import com.wireguard.android.util.RootShell import com.zaneschepke.wireguardautotunnel.R -import com.zaneschepke.wireguardautotunnel.data.domain.Settings import com.zaneschepke.wireguardautotunnel.data.domain.TunnelConfig import com.zaneschepke.wireguardautotunnel.data.repository.AppDataRepository import com.zaneschepke.wireguardautotunnel.module.AppShell @@ -23,7 +22,6 @@ import com.zaneschepke.wireguardautotunnel.service.network.NetworkStatus import com.zaneschepke.wireguardautotunnel.service.network.WifiService import com.zaneschepke.wireguardautotunnel.service.notification.NotificationService import com.zaneschepke.wireguardautotunnel.service.tunnel.TunnelService -import com.zaneschepke.wireguardautotunnel.service.tunnel.TunnelState import com.zaneschepke.wireguardautotunnel.util.Constants import com.zaneschepke.wireguardautotunnel.util.extensions.cancelWithMessage import com.zaneschepke.wireguardautotunnel.util.extensions.getCurrentWifiName @@ -35,6 +33,7 @@ import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.Job import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.collect import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.update @@ -42,6 +41,7 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import timber.log.Timber import java.net.InetAddress +import java.util.concurrent.atomic.AtomicBoolean import javax.inject.Inject import javax.inject.Provider @@ -86,11 +86,9 @@ class AutoTunnelService : LifecycleService() { private var wakeLock: PowerManager.WakeLock? = null - private var wifiJob: Job? = null - private var mobileDataJob: Job? = null - private var ethernetJob: Job? = null + private val pingTunnelRestartActive = AtomicBoolean(false) + private var pingJob: Job? = null - private var networkEventJob: Job? = null override fun onCreate() { super.onCreate() @@ -123,6 +121,8 @@ class AutoTunnelService : LifecycleService() { } startSettingsJob() startVpnStateJob() + startNetworkJobs() + startPingStateJob() }.onFailure { Timber.e(it) } @@ -138,7 +138,6 @@ class AutoTunnelService : LifecycleService() { } override fun onDestroy() { - cancelAndResetNetworkJobs() cancelAndResetPingJob() serviceManager.autoTunnelService = CompletableDeferred() super.onDestroy() @@ -203,6 +202,16 @@ class AutoTunnelService : LifecycleService() { handleNetworkEventChanges() } + private fun startPingStateJob() = lifecycleScope.launch { + autoTunnelStateFlow.collect { + if (it.isPingEnabled()) { + pingJob.onNotRunning { pingJob = startPingJob() } + } else { + if (!pingTunnelRestartActive.get()) cancelAndResetPingJob() + } + } + } + private suspend fun watchForMobileDataConnectivityChanges() { withContext(ioDispatcher) { Timber.i("Starting mobile data watcher") @@ -232,8 +241,8 @@ class AutoTunnelService : LifecycleService() { Timber.i("Starting ping watcher") runCatching { do { - val vpnState = tunnelService.get().vpnState.value - if (vpnState.status == TunnelState.UP) { + val vpnState = autoTunnelStateFlow.value.vpnState + if (vpnState.status.isUp() && !autoTunnelStateFlow.value.isNoConnectivity()) { if (vpnState.tunnelConfig != null) { val config = TunnelConfig.configFromWgQuick(vpnState.tunnelConfig.wgQuick) val results = if (vpnState.tunnelConfig.pingIp != null) { @@ -249,7 +258,9 @@ class AutoTunnelService : LifecycleService() { if (results.contains(false)) { Timber.i("Restarting VPN for ping failure") val cooldown = vpnState.tunnelConfig.pingCooldown + pingTunnelRestartActive.set(true) tunnelService.get().bounceTunnel() + pingTunnelRestartActive.set(false) delay(cooldown ?: Constants.PING_COOLDOWN) continue } @@ -267,14 +278,10 @@ class AutoTunnelService : LifecycleService() { Timber.i("Starting settings watcher") withContext(ioDispatcher) { appDataRepository.settings.getSettingsFlow().combine( - // ignore isActive changes to allow manual tunnel overrides - appDataRepository.tunnels.getTunnelConfigsFlow().distinctUntilChanged { old, new -> - old.map { it.isActive } != new.map { it.isActive } - }, + appDataRepository.tunnels.getTunnelConfigsFlow(), ) { settings, tunnels -> Pair(settings, tunnels) }.collect { pair -> - manageJobsBySettings(pair.first) autoTunnelStateFlow.update { it.copy( settings = pair.first, @@ -292,53 +299,16 @@ class AutoTunnelService : LifecycleService() { autoTunnelStateFlow.update { it.copy(vpnState = state) } - // TODO think about this - // What happens if we change the pinger setting while vpn is active? - state.tunnelConfig?.let { - val settings = appDataRepository.settings.getSettings() - if (it.isPingEnabled && !settings.isPingEnabled) { - pingJob.onNotRunning { pingJob = startPingJob() } - } - if (!it.isPingEnabled && !settings.isPingEnabled) { - cancelAndResetPingJob() - } - } - } - } - } - - private fun manageJobsBySettings(settings: Settings) { - with(settings) { - if (isPingEnabled) { - pingJob.onNotRunning { pingJob = startPingJob() } - } else { - cancelAndResetPingJob() - } - if (isTunnelOnWifiEnabled || isTunnelOnEthernetEnabled || isTunnelOnMobileDataEnabled) { - startNetworkJobs() - } else { - cancelAndResetNetworkJobs() } } } private fun startNetworkJobs() { - wifiJob.onNotRunning { - Timber.i("Wifi job starting") - wifiJob = startWifiJob() - } - ethernetJob.onNotRunning { - ethernetJob = startEthernetJob() - Timber.i("Ethernet job starting") - } - mobileDataJob.onNotRunning { - mobileDataJob = startMobileDataJob() - Timber.i("Mobile data job starting") - } - networkEventJob.onNotRunning { - Timber.i("Network event job starting") - networkEventJob = startNetworkEventJob() - } + Timber.i("Starting all network state jobs..") + startWifiJob() + startEthernetJob() + startMobileDataJob() + startNetworkEventJob() } private fun cancelAndResetPingJob() { @@ -346,17 +316,6 @@ class AutoTunnelService : LifecycleService() { pingJob = null } - private fun cancelAndResetNetworkJobs() { - networkEventJob?.cancelWithMessage("Network event job canceled") - wifiJob?.cancelWithMessage("Wifi job canceled") - ethernetJob?.cancelWithMessage("Ethernet job canceled") - mobileDataJob?.cancelWithMessage("Mobile data job canceled") - networkEventJob = null - wifiJob = null - ethernetJob = null - mobileDataJob = null - } - private fun emitEthernetConnected(connected: Boolean) { autoTunnelStateFlow.update { it.copy( @@ -459,19 +418,16 @@ class AutoTunnelService : LifecycleService() { private suspend fun handleNetworkEventChanges() { withContext(ioDispatcher) { Timber.i("Starting auto-tunnel network event watcher") - // allow manual overrides + // ignore vpnState emits to allow manual overrides autoTunnelStateFlow.distinctUntilChanged { old, new -> - old.copy(vpnState = new.vpnState) == new + old.copy(vpnState = new.vpnState) == new || old.tunnels.map { it.isActive } != new.tunnels.map { it.isActive } }.collect { watcherState -> when (val event = watcherState.asAutoTunnelEvent()) { - is AutoTunnelEvent.Start -> { - Timber.d("Start tunnel ${event.tunnelConfig?.name}") - tunnelService.get().startTunnel(event.tunnelConfig ?: appDataRepository.getPrimaryOrFirstTunnel()) - } - is AutoTunnelEvent.Stop -> { - Timber.d("Stop tunnel") - tunnelService.get().stopTunnel() - } + is AutoTunnelEvent.Start -> tunnelService.get().startTunnel( + event.tunnelConfig + ?: appDataRepository.getPrimaryOrFirstTunnel(), + ) + is AutoTunnelEvent.Stop -> tunnelService.get().stopTunnel() AutoTunnelEvent.DoNothing -> Timber.i("Auto-tunneling: no condition met") } } diff --git a/app/src/main/java/com/zaneschepke/wireguardautotunnel/service/foreground/autotunnel/AutoTunnelState.kt b/app/src/main/java/com/zaneschepke/wireguardautotunnel/service/foreground/autotunnel/AutoTunnelState.kt index de9b103..f9fa3c3 100644 --- a/app/src/main/java/com/zaneschepke/wireguardautotunnel/service/foreground/autotunnel/AutoTunnelState.kt +++ b/app/src/main/java/com/zaneschepke/wireguardautotunnel/service/foreground/autotunnel/AutoTunnelState.kt @@ -51,11 +51,11 @@ data class AutoTunnelState( return isEthernetConnected && settings.isTunnelOnEthernetEnabled && vpnState.status.isDown() } - private fun stopOnEthernet() : Boolean { + private fun stopOnEthernet(): Boolean { return isEthernetConnected && !settings.isTunnelOnEthernetEnabled && vpnState.status.isUp() } - private fun isNoConnectivity(): Boolean { + fun isNoConnectivity(): Boolean { return !isEthernetConnected && !isWifiConnected && !isMobileDataConnected } @@ -96,7 +96,9 @@ data class AutoTunnelState( val vpnTunnel = vpnState.tunnelConfig return if (preferred != null && vpnTunnel != null) { preferred.id == vpnTunnel.id - } else true + } else { + true + } } fun asAutoTunnelEvent(): AutoTunnelEvent { @@ -120,14 +122,23 @@ data class AutoTunnelState( private fun isCurrentSSIDTrusted(): Boolean { return if (settings.isWildcardsEnabled) { settings.trustedNetworkSSIDs.isMatchingToWildcardList(currentNetworkSSID) - } else settings.trustedNetworkSSIDs.contains(currentNetworkSSID) + } else { + settings.trustedNetworkSSIDs.contains(currentNetworkSSID) + } } private fun getTunnelWithMatchingTunnelNetwork(): TunnelConfig? { return tunnels.firstOrNull { if (settings.isWildcardsEnabled) { it.tunnelNetworks.isMatchingToWildcardList(currentNetworkSSID) - } else it.tunnelNetworks.contains(currentNetworkSSID) + } else { + it.tunnelNetworks.contains(currentNetworkSSID) + } } } + + fun isPingEnabled(): Boolean { + return settings.isPingEnabled || + (vpnState.status.isUp() && vpnState.tunnelConfig != null && tunnels.first { it.id == vpnState.tunnelConfig.id }.isPingEnabled) + } } diff --git a/app/src/main/java/com/zaneschepke/wireguardautotunnel/ui/screens/main/MainViewModel.kt b/app/src/main/java/com/zaneschepke/wireguardautotunnel/ui/screens/main/MainViewModel.kt index f000325..d5d0bc3 100644 --- a/app/src/main/java/com/zaneschepke/wireguardautotunnel/ui/screens/main/MainViewModel.kt +++ b/app/src/main/java/com/zaneschepke/wireguardautotunnel/ui/screens/main/MainViewModel.kt @@ -252,7 +252,7 @@ constructor( fun onCopyTunnel(tunnel: TunnelConfig) = viewModelScope.launch { saveTunnel( - TunnelConfig(name = makeTunnelNameUnique(tunnel.name), wgQuick = tunnel.wgQuick, amQuick = tunnel.amQuick) + TunnelConfig(name = makeTunnelNameUnique(tunnel.name), wgQuick = tunnel.wgQuick, amQuick = tunnel.amQuick), ) } diff --git a/app/src/main/java/com/zaneschepke/wireguardautotunnel/ui/screens/settings/autotunnel/AutoTunnelScreen.kt b/app/src/main/java/com/zaneschepke/wireguardautotunnel/ui/screens/settings/autotunnel/AutoTunnelScreen.kt index 6a74127..b84e82c 100644 --- a/app/src/main/java/com/zaneschepke/wireguardautotunnel/ui/screens/settings/autotunnel/AutoTunnelScreen.kt +++ b/app/src/main/java/com/zaneschepke/wireguardautotunnel/ui/screens/settings/autotunnel/AutoTunnelScreen.kt @@ -72,16 +72,18 @@ fun AutoTunnelScreen(uiState: AppUiState, viewModel: AutoTunnelViewModel = hiltV isBackgroundLocationGranted = fineLocationState.status.isGranted } - fun onAutoTunnelWifiChecked() { - if (uiState.settings.isTunnelOnWifiEnabled) viewModel.onToggleTunnelOnWifi().also { return } - when (false) { - isBackgroundLocationGranted -> showLocationDialog = true - fineLocationState.status.isGranted -> showLocationDialog = true - context.isLocationServicesEnabled() -> - showLocationServicesAlertDialog = true - else -> { - viewModel.onToggleTunnelOnWifi() + fun isWifiNameReadable(): Boolean { + return when { + !isBackgroundLocationGranted || + !fineLocationState.status.isGranted -> { + showLocationDialog = true + false } + !context.isLocationServicesEnabled() -> { + showLocationServicesAlertDialog = true + false + } + else -> true } } @@ -118,14 +120,14 @@ fun AutoTunnelScreen(uiState: AppUiState, viewModel: AutoTunnelViewModel = hiltV topBar = { TopNavBar(stringResource(R.string.auto_tunneling)) }, - ) { + ) { padding -> Column( horizontalAlignment = Alignment.Start, verticalArrangement = Arrangement.spacedBy(24.dp.scaledHeight(), Alignment.Top), modifier = Modifier .fillMaxSize() - .padding(it) + .padding(padding) .padding(top = 24.dp.scaledHeight()) .padding(horizontal = 24.dp.scaledWidth()), ) { @@ -148,14 +150,12 @@ fun AutoTunnelScreen(uiState: AppUiState, viewModel: AutoTunnelViewModel = hiltV enabled = !uiState.settings.isAlwaysOnVpnEnabled, checked = uiState.settings.isTunnelOnWifiEnabled, onClick = { - if (uiState.settings.isWifiNameByShellEnabled) viewModel.onToggleTunnelOnWifi().also { return@ScaledSwitch } - onAutoTunnelWifiChecked() + viewModel.onToggleTunnelOnWifi() }, ) }, onClick = { - if (uiState.settings.isWifiNameByShellEnabled) viewModel.onToggleTunnelOnWifi().also { return@SelectionItem } - onAutoTunnelWifiChecked() + viewModel.onToggleTunnelOnWifi() }, ), SelectionItem( @@ -251,7 +251,9 @@ fun AutoTunnelScreen(uiState: AppUiState, viewModel: AutoTunnelViewModel = hiltV uiState.settings.trustedNetworkSSIDs, onDelete = viewModel::onDeleteTrustedSSID, currentText = currentText, - onSave = viewModel::onSaveTrustedSSID, + onSave = { ssid -> + if (uiState.settings.isWifiNameByShellEnabled || isWifiNameReadable()) viewModel.onSaveTrustedSSID(ssid) + }, onValueChange = { currentText = it }, supporting = { if (uiState.settings.isWildcardsEnabled) { diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 299aa92..a6a33e0 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -133,7 +133,7 @@ Feature requires at least one tunnel Allow all the time location permission and/or precise location is required for this feature. Please see app settings - to make sure these permissions are enabled. + to make sure these permissions are enabled Root shell accepted Set custom ping ip (optional, defaults to peers)