diff --git a/src/frontends/android/app/src/main/java/org/strongswan/android/logic/connectivity/HealthCheckManager.kt b/src/frontends/android/app/src/main/java/org/strongswan/android/logic/connectivity/HealthCheckManager.kt index 39a592a15..62b8465ac 100644 --- a/src/frontends/android/app/src/main/java/org/strongswan/android/logic/connectivity/HealthCheckManager.kt +++ b/src/frontends/android/app/src/main/java/org/strongswan/android/logic/connectivity/HealthCheckManager.kt @@ -25,6 +25,8 @@ import android.net.ConnectivityManager import android.net.Network import android.net.NetworkCapabilities import android.net.NetworkRequest +import android.os.BatteryManager +import android.os.Build import android.os.Handler import android.os.IBinder import android.os.Looper @@ -116,6 +118,9 @@ class HealthCheckManager private constructor( private val cycleMutex = Mutex() private var config = HealthConfig() private var running = false + /* last state seen by onStateChanged(): VpnStateService also notifies on + * error/IMC/rebind changes, which must not restart a pending cycle */ + private var lastState: VpnStateService.State? = null private var service: VpnStateService? = null private var cycleJob: Job? = null private var confirmJob: Job? = null @@ -230,6 +235,7 @@ class HealthCheckManager private constructor( { } service = null + lastState = null resetCounters() } @@ -239,6 +245,8 @@ class HealthCheckManager private constructor( if (config.enabled) { start() + /* re-arm with the new settings even if the state is unchanged */ + lastState = null onStateChanged() } else @@ -265,6 +273,8 @@ class HealthCheckManager private constructor( if (updated.enabled) { start() + /* re-arm with the new settings even if the state is unchanged */ + lastState = null onStateChanged() } else @@ -284,12 +294,17 @@ class HealthCheckManager private constructor( { return } - when (service?.state) + val state = service?.state + val previous = lastState + lastState = state + when (state) { VpnStateService.State.CONNECTED -> { - /* let the fresh tunnel settle before the first probe */ - scheduleCycle(INITIAL_DELAY_MS) + if (previous != VpnStateService.State.CONNECTED) + { /* let the fresh tunnel settle before the first probe */ + scheduleCycle(INITIAL_DELAY_MS) + } } VpnStateService.State.DISABLED -> { @@ -345,29 +360,27 @@ class HealthCheckManager private constructor( } val gateway = svc.profile?.gateway val targets = config.targets - val checkServer = config.serverCheckMode != ServerCheckMode.OFF + val mode = config.serverCheckMode val timeout = config.timeoutMs - val results = coroutineScope { - val serverDeferred = if (checkServer) - { - async { ConnectivityProbe.probe(ProbeTarget.SERVER, timeout, context, gateway) } - } - else - { - null - } - val external = targets.map { target -> + val externalResults = coroutineScope { + targets.map { target -> async { ConnectivityProbe.probe(target, timeout, context, gateway) } - } - (listOfNotNull(serverDeferred) + external).map { it.await() } + }.map { it.await() } } - - val serverResult = results.firstOrNull { it.target == ProbeTarget.SERVER } - val externalResults = results.filter { it.target != ProbeTarget.SERVER } val externalOk = externalResults.count { it.ok } val externalTotal = externalResults.size - val serverDown = serverResult?.let { !it.ok } + /* the direct server ping (a fork of /system/bin/ping plus a DNS lookup + * outside the tunnel) only runs when its result can matter: always in + * ALWAYS mode, in WITH_TUNNEL mode only once the tunnel looks dead */ + val serverDown = if (shouldProbeServer(mode, externalOk, externalTotal)) + { + !ConnectivityProbe.probe(ProbeTarget.SERVER, timeout, context, gateway).ok + } + else + { + null + } val decision = decide(config, serverDown, externalOk, externalTotal, serverAttempts, tunnelAttempts) val healthy = serverDown != true && (externalTotal == 0 || externalOk == externalTotal) @@ -523,9 +536,10 @@ class HealthCheckManager private constructor( powerState = PowerState( screenOn = pm?.isInteractive ?: true, powerSaveMode = pm?.isPowerSaveMode ?: false, - batteryLow = false, + batteryLow = isBatteryLowNow(), ) - /* BATTERY_LOW/OKAY are sticky, so registration immediately reflects the current state */ + /* BATTERY_LOW/OKAY are not sticky: the initial state comes from the + * sticky ACTION_BATTERY_CHANGED, later changes from the broadcasts */ val filter = IntentFilter().apply { addAction(Intent.ACTION_SCREEN_ON) addAction(Intent.ACTION_SCREEN_OFF) @@ -543,6 +557,29 @@ class HealthCheckManager private constructor( } } + private fun isBatteryLowNow(): Boolean + { + return try + { + val sticky = context.registerReceiver(null, IntentFilter(Intent.ACTION_BATTERY_CHANGED)) + ?: return false + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) + { + sticky.getBooleanExtra(BatteryManager.EXTRA_BATTERY_LOW, false) + } + else + { /* the platform's default low-battery threshold */ + val level = sticky.getIntExtra(BatteryManager.EXTRA_LEVEL, -1) + val scale = sticky.getIntExtra(BatteryManager.EXTRA_SCALE, -1) + level >= 0 && scale > 0 && level * 100 / scale <= 15 + } + } + catch (_: Exception) + { + false + } + } + private fun unregisterPowerReceiver() { try @@ -710,6 +747,16 @@ class HealthCheckManager private constructor( * @param serverAttempts reconnect decisions already taken for the server rule * @param tunnelAttempts reconnect decisions already taken for the tunnel rules */ + /** Whether the server probe can influence [decide] this cycle. */ + @JvmStatic + fun shouldProbeServer(mode: ServerCheckMode, externalOk: Int, externalTotal: Int): Boolean = + when (mode) + { + ServerCheckMode.OFF -> false + ServerCheckMode.ALWAYS -> true + ServerCheckMode.WITH_TUNNEL -> externalTotal > 0 && externalOk == 0 + } + fun decide( config: HealthConfig, serverDown: Boolean?, diff --git a/src/frontends/android/app/src/test/java/org/strongswan/android/logic/connectivity/HealthCheckManagerTest.kt b/src/frontends/android/app/src/test/java/org/strongswan/android/logic/connectivity/HealthCheckManagerTest.kt index 620de0fb3..0eaf912dc 100644 --- a/src/frontends/android/app/src/test/java/org/strongswan/android/logic/connectivity/HealthCheckManagerTest.kt +++ b/src/frontends/android/app/src/test/java/org/strongswan/android/logic/connectivity/HealthCheckManagerTest.kt @@ -230,4 +230,16 @@ class HealthCheckManagerTest assertThat(ConnectivityProbe.parsePingRtt("time<1 ms")).isEqualTo(1L) assertThat(ConnectivityProbe.parsePingRtt("no time here")).isNull() } + + @Test + fun serverIsOnlyPingedWhenItCanMatter() + { + assertThat(HealthCheckManager.shouldProbeServer(ServerCheckMode.OFF, 0, 3)).isFalse() + assertThat(HealthCheckManager.shouldProbeServer(ServerCheckMode.ALWAYS, 3, 3)).isTrue() + /* WITH_TUNNEL: a working tunnel makes the server result irrelevant */ + assertThat(HealthCheckManager.shouldProbeServer(ServerCheckMode.WITH_TUNNEL, 3, 3)).isFalse() + assertThat(HealthCheckManager.shouldProbeServer(ServerCheckMode.WITH_TUNNEL, 1, 3)).isFalse() + assertThat(HealthCheckManager.shouldProbeServer(ServerCheckMode.WITH_TUNNEL, 0, 3)).isTrue() + assertThat(HealthCheckManager.shouldProbeServer(ServerCheckMode.WITH_TUNNEL, 0, 0)).isFalse() + } }