fix: auto tunnel bug fixes

* add debounce to prevent rapid state changes
* fix get wifi from shell bug
* ignore default emitted state values
This commit is contained in:
Zane Schepke 2024-12-19 01:07:25 -05:00
parent 048bcd4ad7
commit 40dd14ca16
5 changed files with 44 additions and 29 deletions

View File

@ -2,9 +2,9 @@ package com.zaneschepke.wireguardautotunnel.module
import android.content.Context import android.content.Context
import com.zaneschepke.logcatter.LogReader import com.zaneschepke.logcatter.LogReader
import com.zaneschepke.logcatter.LogcatReader
import com.zaneschepke.wireguardautotunnel.service.notification.NotificationService import com.zaneschepke.wireguardautotunnel.service.notification.NotificationService
import com.zaneschepke.wireguardautotunnel.service.notification.WireGuardNotification import com.zaneschepke.wireguardautotunnel.service.notification.WireGuardNotification
import com.zaneschepke.logcatter.LogcatReader
import dagger.Module import dagger.Module
import dagger.Provides import dagger.Provides
import dagger.hilt.InstallIn import dagger.hilt.InstallIn

View File

@ -37,13 +37,16 @@ import com.zaneschepke.wireguardautotunnel.util.extensions.onNotRunning
import dagger.hilt.android.AndroidEntryPoint import dagger.hilt.android.AndroidEntryPoint
import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.FlowPreview
import kotlinx.coroutines.Job import kotlinx.coroutines.Job
import kotlinx.coroutines.delay import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.debounce
import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.filterNot import kotlinx.coroutines.flow.filterNot
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.update import kotlinx.coroutines.flow.update
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext import kotlinx.coroutines.withContext
@ -89,7 +92,9 @@ class AutoTunnelService : LifecycleService() {
@MainImmediateDispatcher @MainImmediateDispatcher
lateinit var mainImmediateDispatcher: CoroutineDispatcher lateinit var mainImmediateDispatcher: CoroutineDispatcher
private val autoTunnelStateFlow = MutableStateFlow(AutoTunnelState()) private val defaultState = AutoTunnelState()
private val autoTunnelStateFlow = MutableStateFlow(defaultState)
private var wakeLock: PowerManager.WakeLock? = null private var wakeLock: PowerManager.WakeLock? = null
@ -183,6 +188,7 @@ class AutoTunnelService : LifecycleService() {
private fun startPingStateJob() = lifecycleScope.launch { private fun startPingStateJob() = lifecycleScope.launch {
autoTunnelStateFlow.collect { autoTunnelStateFlow.collect {
if (it == defaultState) return@collect
if (it.isPingEnabled()) { if (it.isPingEnabled()) {
pingJob.onNotRunning { pingJob = startPingJob() } pingJob.onNotRunning { pingJob = startPingJob() }
} else { } else {
@ -237,7 +243,7 @@ class AutoTunnelService : LifecycleService() {
AutoTunnelState(tunnelService.get().vpnState.value, networkState, double.first, double.second) AutoTunnelState(tunnelService.get().vpnState.value, networkState, double.first, double.second)
}.collect { state -> }.collect { state ->
autoTunnelStateFlow.update { 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 pingJob = null
} }
@OptIn(FlowPreview::class)
private fun combineNetworkEventsJob(): Flow<NetworkState> { private fun combineNetworkEventsJob(): Flow<NetworkState> {
return combine( return combine(
wifiService.networkStatus, wifiService.networkStatus,
@ -263,14 +270,15 @@ class AutoTunnelService : LifecycleService() {
is NetworkStatus.Unavailable -> null is NetworkStatus.Unavailable -> null
}, },
) )
}.distinctUntilChanged().filterNot { it.isWifiConnected && it.wifiName == null } }.distinctUntilChanged().filterNot { it.isWifiConnected && it.wifiName == null }.debounce(500L)
} }
private fun combineSettings(): Flow<Pair<Settings, TunnelConfigs>> { private fun combineSettings(): Flow<Pair<Settings, TunnelConfigs>> {
return combine( return combine(
appDataRepository.get().settings.getSettingsFlow(), appDataRepository.get().settings.getSettingsFlow(),
appDataRepository.get().tunnels.getTunnelConfigsFlow().distinctUntilChanged { old, new -> appDataRepository.get().tunnels.getTunnelConfigsFlow().map { tunnels ->
old.map { it.isActive } != new.map { it.isActive } // isActive is ignored for equality checks so user can manually toggle off tunnel with auto-tunnel
tunnels.map { it.copy(isActive = false) }
}, },
) { settings, tunnels -> ) { settings, tunnels ->
Pair(settings, tunnels) Pair(settings, tunnels)
@ -279,22 +287,31 @@ class AutoTunnelService : LifecycleService() {
private suspend fun getWifiSSID(networkCapabilities: NetworkCapabilities): String? { private suspend fun getWifiSSID(networkCapabilities: NetworkCapabilities): String? {
return withContext(ioDispatcher) { return withContext(ioDispatcher) {
with(autoTunnelStateFlow.value.settings) { val settings = settings()
if (isWifiNameByShellEnabled) return@withContext rootShell.get().getCurrentWifiName() if (settings.isWifiNameByShellEnabled) return@withContext rootShell.get().getCurrentWifiName()
wifiService.getNetworkName(networkCapabilities) wifiService.getNetworkName(networkCapabilities)
}.also { }.also {
if (it?.contains(Constants.UNREADABLE_SSID) == true) { if (it?.contains(Constants.UNREADABLE_SSID) == true) {
Timber.w("SSID unreadable: missing permissions") Timber.w("SSID unreadable: missing permissions")
} else { } else {
Timber.i("Detected valid SSID") 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) { private fun startAutoTunnelJob() = lifecycleScope.launch(ioDispatcher) {
Timber.i("Starting auto-tunnel network event watcher") 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") Timber.d("New auto tunnel state emitted")
when (val event = watcherState.asAutoTunnelEvent()) { when (val event = watcherState.asAutoTunnelEvent()) {
is AutoTunnelEvent.Start -> tunnelService.get().startTunnel( is AutoTunnelEvent.Start -> tunnelService.get().startTunnel(

View File

@ -5,7 +5,6 @@ import com.zaneschepke.wireguardautotunnel.data.domain.TunnelConfig
import com.zaneschepke.wireguardautotunnel.service.tunnel.VpnState import com.zaneschepke.wireguardautotunnel.service.tunnel.VpnState
import com.zaneschepke.wireguardautotunnel.util.extensions.TunnelConfigs import com.zaneschepke.wireguardautotunnel.util.extensions.TunnelConfigs
import com.zaneschepke.wireguardautotunnel.util.extensions.isMatchingToWildcardList import com.zaneschepke.wireguardautotunnel.util.extensions.isMatchingToWildcardList
import timber.log.Timber
data class AutoTunnelState( data class AutoTunnelState(
val vpnState: VpnState = VpnState(), val vpnState: VpnState = VpnState(),
@ -82,7 +81,6 @@ data class AutoTunnelState(
} }
private fun startOnUntrustedWifi(): Boolean { private fun startOnUntrustedWifi(): Boolean {
Timber.d("Is tunnel on wifi enabled ${settings.isTunnelOnWifiEnabled}")
return isWifiActive() && settings.isTunnelOnWifiEnabled && vpnState.status.isDown() && !isCurrentSSIDTrusted() return isWifiActive() && settings.isTunnelOnWifiEnabled && vpnState.status.isDown() && !isCurrentSSIDTrusted()
} }

View File

@ -11,8 +11,8 @@ import kotlinx.coroutines.channels.awaitClose
import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.callbackFlow import kotlinx.coroutines.flow.callbackFlow
import kotlinx.coroutines.flow.catch import kotlinx.coroutines.flow.catch
import kotlinx.coroutines.flow.conflate
import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.onStart
import timber.log.Timber import timber.log.Timber
abstract class BaseNetworkService<T : BaseNetworkService<T>>( abstract class BaseNetworkService<T : BaseNetworkService<T>>(
@ -25,7 +25,7 @@ abstract class BaseNetworkService<T : BaseNetworkService<T>>(
val wifiManager = val wifiManager =
context.applicationContext.getSystemService(Context.WIFI_SERVICE) as 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 network = connectivityManager.activeNetwork
val networkCapabilities = connectivityManager.getNetworkCapabilities(network) val networkCapabilities = connectivityManager.getNetworkCapabilities(network)
return networkCapabilities?.hasTransport(networkCapability) == true return networkCapabilities?.hasTransport(networkCapability) == true
@ -33,9 +33,6 @@ abstract class BaseNetworkService<T : BaseNetworkService<T>>(
override val networkStatus = override val networkStatus =
callbackFlow { callbackFlow {
if (!checkHasCapability(networkCapability)) {
trySend(NetworkStatus.Unavailable())
}
val networkStatusCallback = val networkStatusCallback =
when (Build.VERSION.SDK_INT) { when (Build.VERSION.SDK_INT) {
in Build.VERSION_CODES.S..Int.MAX_VALUE -> { in Build.VERSION_CODES.S..Int.MAX_VALUE -> {
@ -92,10 +89,13 @@ abstract class BaseNetworkService<T : BaseNetworkService<T>>(
connectivityManager.registerNetworkCallback(request, networkStatusCallback) connectivityManager.registerNetworkCallback(request, networkStatusCallback)
awaitClose { connectivityManager.unregisterNetworkCallback(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 { }.catch {
Timber.e(it) Timber.e(it)
// conflate for backpressure emit(NetworkStatus.Unavailable())
}.conflate() }
} }
inline fun <Result> Flow<NetworkStatus>.map( inline fun <Result> Flow<NetworkStatus>.map(

View File

@ -2,6 +2,7 @@ package com.zaneschepke.wireguardautotunnel.service.tunnel
import com.wireguard.android.backend.Backend import com.wireguard.android.backend.Backend
import com.wireguard.android.backend.Tunnel.State import com.wireguard.android.backend.Tunnel.State
import com.zaneschepke.wireguardautotunnel.R
import com.zaneschepke.wireguardautotunnel.data.domain.TunnelConfig import com.zaneschepke.wireguardautotunnel.data.domain.TunnelConfig
import com.zaneschepke.wireguardautotunnel.data.repository.AppDataRepository import com.zaneschepke.wireguardautotunnel.data.repository.AppDataRepository
import com.zaneschepke.wireguardautotunnel.data.repository.TunnelConfigRepository 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.IoDispatcher
import com.zaneschepke.wireguardautotunnel.module.Kernel import com.zaneschepke.wireguardautotunnel.module.Kernel
import com.zaneschepke.wireguardautotunnel.service.foreground.ServiceManager 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
import com.zaneschepke.wireguardautotunnel.service.notification.NotificationService.Companion.VPN_NOTIFICATION_ID
import com.zaneschepke.wireguardautotunnel.service.notification.WireGuardNotification import com.zaneschepke.wireguardautotunnel.service.notification.WireGuardNotification
import com.zaneschepke.wireguardautotunnel.service.tunnel.statistics.AmneziaStatistics import com.zaneschepke.wireguardautotunnel.service.tunnel.statistics.AmneziaStatistics
import com.zaneschepke.wireguardautotunnel.service.tunnel.statistics.TunnelStatistics 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.sync.withLock
import kotlinx.coroutines.withContext import kotlinx.coroutines.withContext
import org.amnezia.awg.backend.Tunnel 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 timber.log.Timber
import javax.inject.Inject import javax.inject.Inject
import javax.inject.Provider import javax.inject.Provider
@ -102,7 +102,7 @@ constructor(
else -> throw NotImplementedError() else -> throw NotImplementedError()
} }
}.onFailure { }.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) Timber.e(it)
} }
} }