From 40dd14ca16e0c5dc658350947b5160677284f8c1 Mon Sep 17 00:00:00 2001 From: Zane Schepke Date: Thu, 19 Dec 2024 01:07:25 -0500 Subject: [PATCH] fix: auto tunnel bug fixes * add debounce to prevent rapid state changes * fix get wifi from shell bug * ignore default emitted state values --- .../wireguardautotunnel/module/AppModule.kt | 2 +- .../autotunnel/AutoTunnelService.kt | 47 +++++++++++++------ .../autotunnel/model/AutoTunnelState.kt | 2 - .../service/network/BaseNetworkService.kt | 14 +++--- .../service/tunnel/WireGuardTunnel.kt | 8 ++-- 5 files changed, 44 insertions(+), 29 deletions(-) diff --git a/app/src/main/java/com/zaneschepke/wireguardautotunnel/module/AppModule.kt b/app/src/main/java/com/zaneschepke/wireguardautotunnel/module/AppModule.kt index 246b7fa..2c5f0a4 100644 --- a/app/src/main/java/com/zaneschepke/wireguardautotunnel/module/AppModule.kt +++ b/app/src/main/java/com/zaneschepke/wireguardautotunnel/module/AppModule.kt @@ -2,9 +2,9 @@ package com.zaneschepke.wireguardautotunnel.module import android.content.Context import com.zaneschepke.logcatter.LogReader +import com.zaneschepke.logcatter.LogcatReader import com.zaneschepke.wireguardautotunnel.service.notification.NotificationService import com.zaneschepke.wireguardautotunnel.service.notification.WireGuardNotification -import com.zaneschepke.logcatter.LogcatReader import dagger.Module import dagger.Provides import dagger.hilt.InstallIn 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 d8c1b1b..c127f6f 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 @@ -37,13 +37,16 @@ import com.zaneschepke.wireguardautotunnel.util.extensions.onNotRunning import dagger.hilt.android.AndroidEntryPoint import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.Job import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.debounce import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.filterNot +import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch import kotlinx.coroutines.withContext @@ -89,7 +92,9 @@ class AutoTunnelService : LifecycleService() { @MainImmediateDispatcher lateinit var mainImmediateDispatcher: CoroutineDispatcher - private val autoTunnelStateFlow = MutableStateFlow(AutoTunnelState()) + private val defaultState = AutoTunnelState() + + private val autoTunnelStateFlow = MutableStateFlow(defaultState) private var wakeLock: PowerManager.WakeLock? = null @@ -183,6 +188,7 @@ class AutoTunnelService : LifecycleService() { private fun startPingStateJob() = lifecycleScope.launch { autoTunnelStateFlow.collect { + if (it == defaultState) return@collect if (it.isPingEnabled()) { pingJob.onNotRunning { pingJob = startPingJob() } } else { @@ -237,7 +243,7 @@ class AutoTunnelService : LifecycleService() { AutoTunnelState(tunnelService.get().vpnState.value, networkState, double.first, double.second) }.collect { state -> autoTunnelStateFlow.update { - it.copy(state.vpnState, state.networkState, state.settings, state.tunnels) + it.copy(vpnState = state.vpnState, networkState = state.networkState, settings = state.settings, tunnels = state.tunnels) } } } @@ -247,6 +253,7 @@ class AutoTunnelService : LifecycleService() { pingJob = null } + @OptIn(FlowPreview::class) private fun combineNetworkEventsJob(): Flow { return combine( wifiService.networkStatus, @@ -263,14 +270,15 @@ class AutoTunnelService : LifecycleService() { is NetworkStatus.Unavailable -> null }, ) - }.distinctUntilChanged().filterNot { it.isWifiConnected && it.wifiName == null } + }.distinctUntilChanged().filterNot { it.isWifiConnected && it.wifiName == null }.debounce(500L) } private fun combineSettings(): Flow> { return combine( appDataRepository.get().settings.getSettingsFlow(), - appDataRepository.get().tunnels.getTunnelConfigsFlow().distinctUntilChanged { old, new -> - old.map { it.isActive } != new.map { it.isActive } + appDataRepository.get().tunnels.getTunnelConfigsFlow().map { tunnels -> + // isActive is ignored for equality checks so user can manually toggle off tunnel with auto-tunnel + tunnels.map { it.copy(isActive = false) } }, ) { settings, tunnels -> Pair(settings, tunnels) @@ -279,22 +287,31 @@ class AutoTunnelService : LifecycleService() { private suspend fun getWifiSSID(networkCapabilities: NetworkCapabilities): String? { return withContext(ioDispatcher) { - with(autoTunnelStateFlow.value.settings) { - if (isWifiNameByShellEnabled) return@withContext rootShell.get().getCurrentWifiName() - wifiService.getNetworkName(networkCapabilities) - }.also { - if (it?.contains(Constants.UNREADABLE_SSID) == true) { - Timber.w("SSID unreadable: missing permissions") - } else { - Timber.i("Detected valid SSID") - } + val settings = settings() + if (settings.isWifiNameByShellEnabled) return@withContext rootShell.get().getCurrentWifiName() + wifiService.getNetworkName(networkCapabilities) + }.also { + if (it?.contains(Constants.UNREADABLE_SSID) == true) { + Timber.w("SSID unreadable: missing permissions") + } else { + Timber.i("Detected valid SSID") } } } + private suspend fun settings(): Settings { + return if (autoTunnelStateFlow.value == defaultState) { + appDataRepository.get().settings.getSettings() + } else { + autoTunnelStateFlow.value.settings + } + } + + @OptIn(FlowPreview::class) private fun startAutoTunnelJob() = lifecycleScope.launch(ioDispatcher) { Timber.i("Starting auto-tunnel network event watcher") - autoTunnelStateFlow.collect { watcherState -> + autoTunnelStateFlow.debounce(1000L).collect { watcherState -> + if (watcherState == defaultState) return@collect Timber.d("New auto tunnel state emitted") when (val event = watcherState.asAutoTunnelEvent()) { is AutoTunnelEvent.Start -> tunnelService.get().startTunnel( diff --git a/app/src/main/java/com/zaneschepke/wireguardautotunnel/service/foreground/autotunnel/model/AutoTunnelState.kt b/app/src/main/java/com/zaneschepke/wireguardautotunnel/service/foreground/autotunnel/model/AutoTunnelState.kt index d76569d..e85ff56 100644 --- a/app/src/main/java/com/zaneschepke/wireguardautotunnel/service/foreground/autotunnel/model/AutoTunnelState.kt +++ b/app/src/main/java/com/zaneschepke/wireguardautotunnel/service/foreground/autotunnel/model/AutoTunnelState.kt @@ -5,7 +5,6 @@ import com.zaneschepke.wireguardautotunnel.data.domain.TunnelConfig import com.zaneschepke.wireguardautotunnel.service.tunnel.VpnState import com.zaneschepke.wireguardautotunnel.util.extensions.TunnelConfigs import com.zaneschepke.wireguardautotunnel.util.extensions.isMatchingToWildcardList -import timber.log.Timber data class AutoTunnelState( val vpnState: VpnState = VpnState(), @@ -82,7 +81,6 @@ data class AutoTunnelState( } private fun startOnUntrustedWifi(): Boolean { - Timber.d("Is tunnel on wifi enabled ${settings.isTunnelOnWifiEnabled}") return isWifiActive() && settings.isTunnelOnWifiEnabled && vpnState.status.isDown() && !isCurrentSSIDTrusted() } diff --git a/app/src/main/java/com/zaneschepke/wireguardautotunnel/service/network/BaseNetworkService.kt b/app/src/main/java/com/zaneschepke/wireguardautotunnel/service/network/BaseNetworkService.kt index d71fe63..4c87665 100644 --- a/app/src/main/java/com/zaneschepke/wireguardautotunnel/service/network/BaseNetworkService.kt +++ b/app/src/main/java/com/zaneschepke/wireguardautotunnel/service/network/BaseNetworkService.kt @@ -11,8 +11,8 @@ import kotlinx.coroutines.channels.awaitClose import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.callbackFlow import kotlinx.coroutines.flow.catch -import kotlinx.coroutines.flow.conflate import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.onStart import timber.log.Timber abstract class BaseNetworkService>( @@ -25,7 +25,7 @@ abstract class BaseNetworkService>( val wifiManager = context.applicationContext.getSystemService(Context.WIFI_SERVICE) as WifiManager - fun checkHasCapability(networkCapability: Int): Boolean { + private fun checkHasCapability(networkCapability: Int): Boolean { val network = connectivityManager.activeNetwork val networkCapabilities = connectivityManager.getNetworkCapabilities(network) return networkCapabilities?.hasTransport(networkCapability) == true @@ -33,9 +33,6 @@ abstract class BaseNetworkService>( override val networkStatus = callbackFlow { - if (!checkHasCapability(networkCapability)) { - trySend(NetworkStatus.Unavailable()) - } val networkStatusCallback = when (Build.VERSION.SDK_INT) { in Build.VERSION_CODES.S..Int.MAX_VALUE -> { @@ -92,10 +89,13 @@ abstract class BaseNetworkService>( connectivityManager.registerNetworkCallback(request, networkStatusCallback) awaitClose { connectivityManager.unregisterNetworkCallback(networkStatusCallback) } + }.onStart { + // needed for services that are not yet available as it will impact later combine flows if we don't emit + emit(NetworkStatus.Unavailable()) }.catch { Timber.e(it) - // conflate for backpressure - }.conflate() + emit(NetworkStatus.Unavailable()) + } } inline fun Flow.map( diff --git a/app/src/main/java/com/zaneschepke/wireguardautotunnel/service/tunnel/WireGuardTunnel.kt b/app/src/main/java/com/zaneschepke/wireguardautotunnel/service/tunnel/WireGuardTunnel.kt index 47818b3..c4becca 100644 --- a/app/src/main/java/com/zaneschepke/wireguardautotunnel/service/tunnel/WireGuardTunnel.kt +++ b/app/src/main/java/com/zaneschepke/wireguardautotunnel/service/tunnel/WireGuardTunnel.kt @@ -2,6 +2,7 @@ package com.zaneschepke.wireguardautotunnel.service.tunnel import com.wireguard.android.backend.Backend import com.wireguard.android.backend.Tunnel.State +import com.zaneschepke.wireguardautotunnel.R import com.zaneschepke.wireguardautotunnel.data.domain.TunnelConfig import com.zaneschepke.wireguardautotunnel.data.repository.AppDataRepository import com.zaneschepke.wireguardautotunnel.data.repository.TunnelConfigRepository @@ -9,7 +10,9 @@ import com.zaneschepke.wireguardautotunnel.module.ApplicationScope import com.zaneschepke.wireguardautotunnel.module.IoDispatcher import com.zaneschepke.wireguardautotunnel.module.Kernel import com.zaneschepke.wireguardautotunnel.service.foreground.ServiceManager +import com.zaneschepke.wireguardautotunnel.service.notification.NotificationAction import com.zaneschepke.wireguardautotunnel.service.notification.NotificationService +import com.zaneschepke.wireguardautotunnel.service.notification.NotificationService.Companion.VPN_NOTIFICATION_ID import com.zaneschepke.wireguardautotunnel.service.notification.WireGuardNotification import com.zaneschepke.wireguardautotunnel.service.tunnel.statistics.AmneziaStatistics import com.zaneschepke.wireguardautotunnel.service.tunnel.statistics.TunnelStatistics @@ -29,9 +32,6 @@ import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext import org.amnezia.awg.backend.Tunnel -import com.zaneschepke.wireguardautotunnel.R -import com.zaneschepke.wireguardautotunnel.service.notification.NotificationAction -import com.zaneschepke.wireguardautotunnel.service.notification.NotificationService.Companion.VPN_NOTIFICATION_ID import timber.log.Timber import javax.inject.Inject import javax.inject.Provider @@ -102,7 +102,7 @@ constructor( else -> throw NotImplementedError() } }.onFailure { - // TODO add better error message and comms to user + // TODO add better error message to user, especially for kernel as exceptions contain no details Timber.e(it) } }