From 439ee735e7036c5e87e80dcd0565c94a4bf8e911 Mon Sep 17 00:00:00 2001 From: grifmo <4677520+grifmo@users.noreply.github.com> Date: Sat, 10 Oct 2026 09:58:43 +0200 Subject: [PATCH] Keep the tunnel state honest: reconcile, retry, detect drops The switcher now records which profile the routes ask for and reconciles it with the tunnel that is actually up. Every trigger (app switch, retry, tunnel drop, refreshed config, end of a connection test) just signals one reconciler, and the UI only shows a profile as active once it is up. - Mullvad refresh kept the relay's bare hostname as Endpoint, which does not resolve. Refresh now keeps the existing endpoint, generated configs use .relays.mullvad.net, and broken configs are repaired when the service starts. Refreshed configs are applied to a running tunnel. - A failed start no longer marks the profile active. The status chip and overlay show "not connected" and the start is retried after 5/10/20/40/60 s. - A tunnel taken down externally (another VPN app took the slot) is detected. UniVPN shows the warning and reconnects on the next app switch instead of fighting the other app for the slot. - Restart and wake-up look back 3 days for the foreground app instead of 10 s, and the last requested profile survives a process restart. - "Default (keep current tunnel)" now keeps the current tunnel. - App switches during a connection test wait for it instead of being dropped after 2 s. - Test connection requires bytes from the peer (a completed handshake) before trusting the external-IP check. - Refresh updates the profile instead of INSERT OR REPLACE, so a profile deleted mid-refresh stays deleted and its metadata isn't cascaded away. Fixes #1, fixes #2, fixes #3, fixes #4, fixes #5, fixes #7, fixes #10, fixes #11 Co-Authored-By: Claude Opus 5.5 --- .../univpn/app/data/ProfileRefreshDaoTest.kt | 81 ++++++ .../univpn/app/service/ServiceRecoveryTest.kt | 9 +- .../app/data/db/GeneratedProfileMetaDao.kt | 3 + .../com/univpn/app/data/db/VpnProfileDao.kt | 4 + .../univpn/app/provider/FreshnessChecker.kt | 60 ++-- .../univpn/app/provider/MullvadConnector.kt | 2 +- .../univpn/app/provider/MullvadEndpoint.kt | 21 ++ .../univpn/app/service/DebugOverlayService.kt | 2 + .../com/univpn/app/service/RouteDecision.kt | 20 ++ .../univpn/app/service/VpnSwitcherService.kt | 260 ++++++++++++------ .../kotlin/com/univpn/app/tunnel/WgConfig.kt | 21 ++ .../app/tunnel/WireGuardTunnelManager.kt | 69 +++-- .../com/univpn/app/ui/TvMainActivity.kt | 19 +- .../app/provider/MullvadEndpointTest.kt | 54 ++++ .../univpn/app/service/RouteDecisionTest.kt | 36 +++ .../com/univpn/app/tunnel/WgConfigTest.kt | 28 ++ 16 files changed, 549 insertions(+), 140 deletions(-) create mode 100644 univpn/src/androidTest/kotlin/com/univpn/app/data/ProfileRefreshDaoTest.kt create mode 100644 univpn/src/main/kotlin/com/univpn/app/provider/MullvadEndpoint.kt create mode 100644 univpn/src/main/kotlin/com/univpn/app/service/RouteDecision.kt create mode 100644 univpn/src/main/kotlin/com/univpn/app/tunnel/WgConfig.kt create mode 100644 univpn/src/test/kotlin/com/univpn/app/provider/MullvadEndpointTest.kt create mode 100644 univpn/src/test/kotlin/com/univpn/app/service/RouteDecisionTest.kt create mode 100644 univpn/src/test/kotlin/com/univpn/app/tunnel/WgConfigTest.kt diff --git a/univpn/src/androidTest/kotlin/com/univpn/app/data/ProfileRefreshDaoTest.kt b/univpn/src/androidTest/kotlin/com/univpn/app/data/ProfileRefreshDaoTest.kt new file mode 100644 index 0000000..53da262 --- /dev/null +++ b/univpn/src/androidTest/kotlin/com/univpn/app/data/ProfileRefreshDaoTest.kt @@ -0,0 +1,81 @@ +package com.univpn.app.data + +import androidx.room.Room +import androidx.room.withTransaction +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.univpn.app.data.db.AppDatabase +import com.univpn.app.data.model.GeneratedProfileMeta +import com.univpn.app.data.model.TunnelType +import com.univpn.app.data.model.VpnProfile +import kotlinx.coroutines.runBlocking +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith + +/** The write pattern FreshnessChecker uses for a refreshed config (#11). */ +@RunWith(AndroidJUnit4::class) +class ProfileRefreshDaoTest { + + private lateinit var db: AppDatabase + + private val profile = VpnProfile("p1", "Mullvad — Tirana", TunnelType.WIREGUARD, "old config") + private val meta = GeneratedProfileMeta( + profileId = "p1", accountId = "a1", providerId = "mullvad", serverId = "al-tia-wg-001|pk", + serverName = "al-tia-wg-001", serverCountry = "Albania", serverCity = "Tirana", + generatedAt = 1L, publicKey = "pub" + ) + + @Before + fun setUp() { + db = Room.inMemoryDatabaseBuilder(ApplicationProvider.getApplicationContext(), AppDatabase::class.java) + .allowMainThreadQueries() + .build() + } + + @After + fun tearDown() { db.close() } + + private suspend fun refresh(newConfig: String): Boolean = db.withTransaction { + if (db.vpnProfileDao().update(profile.copy(configContent = newConfig)) == 0) false + else { + db.generatedProfileMetaDao().upsert(meta.copy(generatedAt = 2L)) + true + } + } + + @Test + fun refresh_updatesExistingProfileAndMeta() = runBlocking { + db.vpnProfileDao().insert(profile) + db.generatedProfileMetaDao().upsert(meta) + + assertEquals(true, refresh("new config")) + assertEquals("new config", db.vpnProfileDao().getById("p1")?.configContent) + assertEquals(2L, db.generatedProfileMetaDao().getByProfileId("p1")?.generatedAt) + } + + @Test + fun refresh_doesNotRecreateDeletedProfile() = runBlocking { + db.vpnProfileDao().insert(profile) + db.generatedProfileMetaDao().upsert(meta) + db.vpnProfileDao().delete(profile) // user deletes it while the refresh request is in flight + + assertEquals(false, refresh("new config")) + assertNull(db.vpnProfileDao().getById("p1")) + assertNull(db.generatedProfileMetaDao().getByProfileId("p1")) + } + + @Test + fun update_keepsMetadataRow() = runBlocking { + // INSERT OR REPLACE deletes the parent row first, which cascades to the metadata. + // UPDATE must leave it in place. + db.vpnProfileDao().insert(profile) + db.generatedProfileMetaDao().upsert(meta) + db.vpnProfileDao().update(profile.copy(configContent = "edited")) + assertNotNull(db.generatedProfileMetaDao().getByProfileId("p1")) + } +} diff --git a/univpn/src/androidTest/kotlin/com/univpn/app/service/ServiceRecoveryTest.kt b/univpn/src/androidTest/kotlin/com/univpn/app/service/ServiceRecoveryTest.kt index df1482a..9f08577 100644 --- a/univpn/src/androidTest/kotlin/com/univpn/app/service/ServiceRecoveryTest.kt +++ b/univpn/src/androidTest/kotlin/com/univpn/app/service/ServiceRecoveryTest.kt @@ -191,12 +191,11 @@ class ServiceRecoveryTest { } @Test - fun serviceDead_unmappedUserApp_implicitNoVpn_tunnelDropped() = runBlocking { - // User app with no DB entry and not a system app → route == null, not system. - // onAppForeground falls through to targetProfile = null → tunnel stopped. + fun serviceDead_unmappedUserApp_keepsCurrentTunnel() = runBlocking { + // An app with no route ("Default (keep current tunnel)", or installed after the + // seeder last ran) keeps whatever tunnel is up instead of dropping it (#5). val route = db.appRouteDao().getByPackage("com.some.newapp") assertNull(route) - // No route + not system app → targetProfile = null → tunnel will be dropped. - // This is the implicit NoVPN behaviour for unmapped user apps. + assertEquals(RouteDecision.Keep, decideRoute(route)) } } diff --git a/univpn/src/main/kotlin/com/univpn/app/data/db/GeneratedProfileMetaDao.kt b/univpn/src/main/kotlin/com/univpn/app/data/db/GeneratedProfileMetaDao.kt index 74a0f2a..186427c 100644 --- a/univpn/src/main/kotlin/com/univpn/app/data/db/GeneratedProfileMetaDao.kt +++ b/univpn/src/main/kotlin/com/univpn/app/data/db/GeneratedProfileMetaDao.kt @@ -8,6 +8,9 @@ interface GeneratedProfileMetaDao { @Query("SELECT * FROM generated_profile_meta WHERE profileId = :profileId") suspend fun getByProfileId(profileId: String): GeneratedProfileMeta? + @Query("SELECT * FROM generated_profile_meta WHERE providerId = :providerId") + suspend fun getByProvider(providerId: String): List + @Insert(onConflict = OnConflictStrategy.REPLACE) suspend fun upsert(meta: GeneratedProfileMeta) } diff --git a/univpn/src/main/kotlin/com/univpn/app/data/db/VpnProfileDao.kt b/univpn/src/main/kotlin/com/univpn/app/data/db/VpnProfileDao.kt index 728d839..b1dcced 100644 --- a/univpn/src/main/kotlin/com/univpn/app/data/db/VpnProfileDao.kt +++ b/univpn/src/main/kotlin/com/univpn/app/data/db/VpnProfileDao.kt @@ -15,6 +15,10 @@ interface VpnProfileDao { @Insert(onConflict = OnConflictStrategy.REPLACE) suspend fun insert(profile: VpnProfile) + /** Returns the number of rows updated: 0 if the profile no longer exists. */ + @Update + suspend fun update(profile: VpnProfile): Int + @Delete suspend fun delete(profile: VpnProfile) } diff --git a/univpn/src/main/kotlin/com/univpn/app/provider/FreshnessChecker.kt b/univpn/src/main/kotlin/com/univpn/app/provider/FreshnessChecker.kt index 80e5e14..0927187 100644 --- a/univpn/src/main/kotlin/com/univpn/app/provider/FreshnessChecker.kt +++ b/univpn/src/main/kotlin/com/univpn/app/provider/FreshnessChecker.kt @@ -2,35 +2,37 @@ package com.univpn.app.provider import android.content.Context import android.util.Log +import androidx.room.withTransaction import com.univpn.app.data.db.AppDatabase -import com.univpn.app.data.model.GeneratedProfileMeta import com.univpn.app.data.model.VpnProfile -import com.univpn.app.data.model.TunnelType -import java.util.UUID +import com.univpn.app.tunnel.WgConfig private const val REFRESH_AGE_MS = 7L * 24 * 60 * 60 * 1000 // 7 days private const val TAG = "FreshnessChecker" class FreshnessChecker(private val context: Context) { - suspend fun checkAndRefreshIfStale(profile: VpnProfile) { + /** Returns true if [profile]'s config was replaced, so a running tunnel should be restarted. */ + suspend fun checkAndRefreshIfStale(profile: VpnProfile): Boolean { val db = AppDatabase.get(context) - val meta = db.generatedProfileMetaDao().getByProfileId(profile.id) ?: return + val meta = db.generatedProfileMetaDao().getByProfileId(profile.id) ?: return false val age = System.currentTimeMillis() - meta.generatedAt - if (age < REFRESH_AGE_MS) return + if (age < REFRESH_AGE_MS) return false - val connector = ProviderRegistry.get(meta.providerId) ?: return + val connector = ProviderRegistry.get(meta.providerId) ?: return false val credStore = CredentialStore(context) val credentials = credStore.load(meta.accountId) ?: run { Log.w(TAG, "No credentials for ${meta.providerId} account ${meta.accountId}") - return + return false } + // Keep the endpoint the profile already uses (the relay's IP for Mullvad). The server id + // only holds the relay-list hostname, which doesn't resolve on its own. val server = VpnServer( id = meta.serverId, country = meta.serverCountry, city = meta.serverCity, - hostname = meta.serverId.substringBefore("|"), + hostname = WgConfig.endpointHost(profile.configContent) ?: meta.serverId.substringBefore("|"), serverName = meta.serverName, ) @@ -42,16 +44,38 @@ class FreshnessChecker(private val context: Context) { } else Pair(meta.publicKey.ifEmpty { null }, null) val result = connector.generateConfig(server, credentials, wgPub, wgPriv) - if (result is ConnectorResult.Success) { - val (newConfig, newPubKey) = result.value - val updatedProfile = profile.copy(configContent = newConfig) - db.vpnProfileDao().insert(updatedProfile) // REPLACE strategy - db.generatedProfileMetaDao().upsert( - meta.copy(generatedAt = System.currentTimeMillis(), publicKey = newPubKey) - ) - Log.i(TAG, "Silently refreshed config for ${profile.name}") - } else { + if (result !is ConnectorResult.Success) { Log.w(TAG, "Config refresh failed for ${profile.name}: $result") + return false + } + val (newConfig, newPubKey) = result.value + // UPDATE rather than INSERT OR REPLACE: a profile deleted while the request was in + // flight stays deleted, and the metadata isn't cascade-deleted and re-inserted. + val updated = db.withTransaction { + if (db.vpnProfileDao().update(profile.copy(configContent = newConfig)) == 0) { + false + } else { + db.generatedProfileMetaDao().upsert( + meta.copy(generatedAt = System.currentTimeMillis(), publicKey = newPubKey) + ) + true + } + } + if (updated) Log.i(TAG, "Silently refreshed config for ${profile.name}") + else Log.i(TAG, "${profile.name} was deleted during refresh; not recreating it") + return updated + } + + /** Fixes Mullvad configs whose Endpoint was written as a bare, unresolvable relay hostname. */ + suspend fun repairMullvadEndpoints() { + val db = AppDatabase.get(context) + for (meta in db.generatedProfileMetaDao().getByProvider("mullvad")) { + val profile = db.vpnProfileDao().getById(meta.profileId) ?: continue + val repaired = MullvadEndpoint.repair(profile.configContent) + if (repaired != profile.configContent) { + db.vpnProfileDao().update(profile.copy(configContent = repaired)) + Log.i(TAG, "Repaired Mullvad endpoint for ${profile.name}") + } } } } diff --git a/univpn/src/main/kotlin/com/univpn/app/provider/MullvadConnector.kt b/univpn/src/main/kotlin/com/univpn/app/provider/MullvadConnector.kt index 3851330..f59ece5 100644 --- a/univpn/src/main/kotlin/com/univpn/app/provider/MullvadConnector.kt +++ b/univpn/src/main/kotlin/com/univpn/app/provider/MullvadConnector.kt @@ -129,7 +129,7 @@ class MullvadConnector : VpnProviderConnector { appendLine() appendLine("[Peer]") appendLine("PublicKey = $serverPubKey") - appendLine("Endpoint = ${server.hostname}:51820") + appendLine("Endpoint = ${MullvadEndpoint.resolvable(server.hostname)}:51820") appendLine("AllowedIPs = 0.0.0.0/0, ::/0") } Log.i(TAG, "Mullvad config generated for ${server.city}") diff --git a/univpn/src/main/kotlin/com/univpn/app/provider/MullvadEndpoint.kt b/univpn/src/main/kotlin/com/univpn/app/provider/MullvadEndpoint.kt new file mode 100644 index 0000000..c75b59f --- /dev/null +++ b/univpn/src/main/kotlin/com/univpn/app/provider/MullvadEndpoint.kt @@ -0,0 +1,21 @@ +package com.univpn.app.provider + +/** + * Mullvad relay-list hostnames such as "se-got-wg-001" don't resolve in DNS on their own; + * only "se-got-wg-001.relays.mullvad.net" does. Config refreshes before 0.9.2 wrote the bare + * name into Endpoint, so these helpers both build and repair endpoints. + */ +object MullvadEndpoint { + private const val RELAY_DOMAIN = ".relays.mullvad.net" + + // Endpoint = :. IPv4 and FQDNs contain '.', IPv6 is bracketed, so neither matches. + private val BARE_ENDPOINT = Regex("""(?im)^(\s*Endpoint\s*=\s*)([A-Za-z0-9-]+)(:\d+)\s*$""") + + /** Returns [host] unchanged if it's an IP or FQDN, otherwise the relay's resolvable name. */ + fun resolvable(host: String): String = + if ('.' in host || ':' in host) host else host + RELAY_DOMAIN + + /** Rewrites a bare relay hostname in the Endpoint line; returns [config] unchanged otherwise. */ + fun repair(config: String): String = + BARE_ENDPOINT.replace(config) { m -> m.groupValues[1] + resolvable(m.groupValues[2]) + m.groupValues[3] } +} diff --git a/univpn/src/main/kotlin/com/univpn/app/service/DebugOverlayService.kt b/univpn/src/main/kotlin/com/univpn/app/service/DebugOverlayService.kt index a30e9c2..cff4d50 100644 --- a/univpn/src/main/kotlin/com/univpn/app/service/DebugOverlayService.kt +++ b/univpn/src/main/kotlin/com/univpn/app/service/DebugOverlayService.kt @@ -124,7 +124,9 @@ class DebugOverlayService : Service() { } } + val failed = VpnSwitcherService.failedProfileFlow.value val profileLine = when { + failed != null -> "⚠ $failed — NOT CONNECTED" DebugState.profileType.isNotEmpty() -> "● ${DebugState.profileName} (${DebugState.profileType})" DebugState.profileName != "None" -> "● ${DebugState.profileName}" else -> "○ No VPN" diff --git a/univpn/src/main/kotlin/com/univpn/app/service/RouteDecision.kt b/univpn/src/main/kotlin/com/univpn/app/service/RouteDecision.kt new file mode 100644 index 0000000..780b265 --- /dev/null +++ b/univpn/src/main/kotlin/com/univpn/app/service/RouteDecision.kt @@ -0,0 +1,20 @@ +package com.univpn.app.service + +import com.univpn.app.data.model.AppRoute + +/** What a foreground app asks of the tunnel. */ +sealed class RouteDecision { + /** Leave the tunnel as it is (Passthrough, or no route = "Default (keep current tunnel)"). */ + object Keep : RouteDecision() + + /** Run this profile; null means No VPN. */ + data class Use(val profileId: String?) : RouteDecision() +} + +fun decideRoute(route: AppRoute?): RouteDecision = when { + route == null || route.isPassthrough -> RouteDecision.Keep + else -> RouteDecision.Use(route.profileId) +} + +/** Delay before retrying a failed tunnel start: 5 s, 10 s, 20 s, 40 s, then every 60 s. */ +fun retryDelayMs(attempt: Int): Long = minOf(60_000L, 5_000L shl minOf(attempt, 4)) diff --git a/univpn/src/main/kotlin/com/univpn/app/service/VpnSwitcherService.kt b/univpn/src/main/kotlin/com/univpn/app/service/VpnSwitcherService.kt index e02cc15..38b5515 100644 --- a/univpn/src/main/kotlin/com/univpn/app/service/VpnSwitcherService.kt +++ b/univpn/src/main/kotlin/com/univpn/app/service/VpnSwitcherService.kt @@ -10,24 +10,23 @@ import android.content.BroadcastReceiver import android.content.Context import android.content.Intent import android.content.IntentFilter -import android.content.pm.ApplicationInfo -import android.content.pm.PackageManager import android.net.VpnService import android.os.Build import android.util.Log import androidx.core.content.ContextCompat import com.univpn.app.R import com.univpn.app.data.db.AppDatabase -import com.univpn.app.data.db.AppRouteDao -import com.univpn.app.data.db.VpnProfileDao import com.univpn.app.data.model.VpnProfile import com.univpn.app.provider.FreshnessChecker import com.univpn.app.receiver.ToggleOverlayReceiver import com.univpn.app.tunnel.TunnelManager import com.univpn.app.tunnel.WireGuardTunnelManager +import com.wireguard.android.backend.BackendException import kotlinx.coroutines.* +import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import java.net.HttpURLConnection import java.net.URL import kotlin.coroutines.coroutineContext @@ -39,15 +38,31 @@ sealed class TestResult { object ServiceUnavailable : TestResult() } +/** + * Keeps the WireGuard tunnel matching the foreground app's route. + * + * Foreground changes, failed starts, unexpected tunnel drops and config refreshes all just + * send a signal; a single reconciler compares the profile the routes ask for ([desiredProfileId]) + * with the tunnel actually running ([activeProfile]) and fixes any difference. The UI only ever + * shows a profile as active once its tunnel is up. + */ class VpnSwitcherService : VpnService() { private val scope = CoroutineScope(SupervisorJob() + Dispatchers.Default) private val tunnelMutex = Mutex() - private val wgManager = WireGuardTunnelManager() + private val wgManager = WireGuardTunnelManager(onUnexpectedDown = ::onTunnelDropped) private var activeTunnelManager: TunnelManager? = null - private var currentProfile: VpnProfile? = null + private var activeProfile: VpnProfile? = null // tunnel that is actually up + private var desiredProfileId: String? = null // what the routes ask for; null = no VPN + @Volatile private var foregroundPkg: String? = null + private val reconcileSignal = Channel(Channel.CONFLATED) + private var retryJob: Job? = null + private var retryAttempt = 0 + // Set when another app took the VPN slot; cleared by the next foreground change. + @Volatile private var holdUntilForegroundChange = false private val freshnessChecker by lazy { FreshnessChecker(applicationContext) } private var watcherJob: Job? = null + private var reconcilerJob: Job? = null // Re-checks the foreground app when the Shield wakes from standby. // ACTION_SCREEN_ON cannot be declared in the manifest — dynamic only. @@ -57,9 +72,8 @@ class VpnSwitcherService : VpnService() { Log.i(TAG, "Screen on — re-checking foreground app") scope.launch { val usm = getSystemService(USAGE_STATS_SERVICE) as UsageStatsManager - val db = AppDatabase.get(this@VpnSwitcherService) - val pkg = getCurrentForegroundPackage(usm) ?: return@launch - onAppForeground(pkg, db.appRouteDao(), db.vpnProfileDao()) + getCurrentForegroundPackage(usm)?.let { foregroundPkg = it } + requestReconcile() } // Restart the overlay if it was enabled before standby. if (DebugOverlayService.wasEnabled(context)) { @@ -71,14 +85,24 @@ class VpnSwitcherService : VpnService() { override fun onCreate() { super.onCreate() instance = this + // Restore the profile the routes last asked for, so a process restart while on the + // home screen (Passthrough) brings the same tunnel back. + desiredProfileId = prefs().getString(PREF_DESIRED_PROFILE, null) registerReceiver(screenOnReceiver, IntentFilter(Intent.ACTION_SCREEN_ON)) } override fun onStartCommand(intent: Intent?, flags: Int, startId: Int): Int { startForeground(NOTIF_ID, buildNotification()) Log.i(TAG, "Service started") - // Guard against duplicate watchers from multiple startService() calls + // Guard against duplicate workers from multiple startService() calls // (e.g. TvMainActivity + START_STICKY restart + BootReceiver). + if (reconcilerJob?.isActive != true) { + reconcilerJob = scope.launch { + runCatching { freshnessChecker.repairMullvadEndpoints() } + .onFailure { Log.e(TAG, "Mullvad endpoint repair failed: ${it.message}") } + for (signal in reconcileSignal) reconcile() + } + } if (watcherJob?.isActive != true) { watcherJob = scope.launch { runForegroundWatcher() } } @@ -99,39 +123,47 @@ class VpnSwitcherService : VpnService() { override fun onBind(intent: Intent?) = null + private fun prefs() = getSharedPreferences("univpn_prefs", MODE_PRIVATE) + + private fun requestReconcile() { + reconcileSignal.trySend(Unit) + } + // ── Test connection ─────────────────────────────────────────────────────── suspend fun testProfile(profile: VpnProfile): TestResult { val acquired = withTimeoutOrNull(3_000L) { tunnelMutex.lock() } if (acquired == null) return TestResult.Error("Another tunnel operation is in progress") - val previousProfile = currentProfile val testManager = WireGuardTunnelManager() try { runCatching { activeTunnelManager?.stop() } + activeTunnelManager = null + activeProfile = null testManager.start(profile, this) + // Received bytes prove the peer answered (a handshake completed), so the IP check + // below can't pass on traffic that bypassed the tunnel. + var handshake = false var externalIp: String? = null repeat(10) { attempt -> if (externalIp != null) return@repeat delay(if (attempt == 0) 2_000L else 1_500L) - externalIp = tryFetchExternalIp() + if (!handshake) handshake = testManager.receivedBytes() > 0 + if (handshake) externalIp = tryFetchExternalIp() } - return if (externalIp != null) TestResult.Success(externalIp!!) else TestResult.NoHandshake + return when { + externalIp != null -> TestResult.Success(externalIp!!) + handshake -> TestResult.Error("Handshake completed, but no internet access through the tunnel") + else -> TestResult.NoHandshake + } } catch (e: Exception) { return TestResult.Error(e.message ?: "Unknown error") } finally { runCatching { testManager.stop() } - if (previousProfile != null) { - runCatching { wgManager.start(previousProfile, this) } - .onSuccess { activeTunnelManager = wgManager } - .onFailure { activeTunnelManager = null } - } else { - activeTunnelManager = null - } - currentProfile = previousProfile tunnelMutex.unlock() + requestReconcile() // bring back the tunnel the foreground app needs } } @@ -151,14 +183,12 @@ class VpnSwitcherService : VpnService() { private suspend fun runForegroundWatcher() { val usm = getSystemService(USAGE_STATS_SERVICE) as UsageStatsManager - val db = AppDatabase.get(this) var lastPollTime = System.currentTimeMillis() - 2_000L var pollCount = 0 var currentPkg = getCurrentForegroundPackage(usm) ?: "" - if (currentPkg.isNotEmpty()) { - onAppForeground(currentPkg, db.appRouteDao(), db.vpnProfileDao()) - } + if (currentPkg.isNotEmpty()) foregroundPkg = currentPkg + requestReconcile() while (coroutineContext.isActive) { try { @@ -175,13 +205,15 @@ class VpnSwitcherService : VpnService() { if (pkg != currentPkg) { currentPkg = pkg Log.i(TAG, "FOREGROUND_CHANGE [poll #$pollCount]: $pkg") - onAppForeground(pkg, db.appRouteDao(), db.vpnProfileDao()) + foregroundPkg = pkg + holdUntilForegroundChange = false + requestReconcile() } } } if (pollCount % 30 == 0) { - Log.d(TAG, "heartbeat: poll #$pollCount, current=$currentPkg profile=${currentProfile?.name}") + Log.d(TAG, "heartbeat: poll #$pollCount, current=$currentPkg profile=${activeProfile?.name}") } } catch (e: SecurityException) { Log.e(TAG, "PACKAGE_USAGE_STATS revoked: ${e.message}") @@ -192,63 +224,126 @@ class VpnSwitcherService : VpnService() { Log.i(TAG, "Watcher stopped") } - private fun isSystemApp(packageName: String): Boolean = try { - val flags = packageManager.getApplicationInfo(packageName, 0).flags - (flags and ApplicationInfo.FLAG_SYSTEM) != 0 && - (flags and ApplicationInfo.FLAG_UPDATED_SYSTEM_APP) == 0 - } catch (e: PackageManager.NameNotFoundException) { true } - - private suspend fun onAppForeground(packageName: String, routeDao: AppRouteDao, profileDao: VpnProfileDao) { - if (packageName == this.packageName) return // management UI never affects the tunnel - if (packageName in ALWAYS_PASSTHROUGH) return - val route = routeDao.getByPackage(packageName) - if (route?.isPassthrough == true) return - if (route == null && isSystemApp(packageName)) return - - val targetProfile = route?.profileId?.let { profileDao.getById(it) } - if (targetProfile?.id == currentProfile?.id) return - - switchingFlow.value = true - val acquired = withTimeoutOrNull(2_000L) { tunnelMutex.lock() } - if (acquired == null) { - Log.w(TAG, "TunnelMutex acquisition timed out for $packageName") - switchingFlow.value = false - return + // ── Reconciler ──────────────────────────────────────────────────────────── + + private suspend fun reconcile() { + if (holdUntilForegroundChange) return + val db = AppDatabase.get(this) + val pkg = foregroundPkg + if (pkg != null && pkg != packageName && pkg !in ALWAYS_PASSTHROUGH) { + val decision = decideRoute(db.appRouteDao().getByPackage(pkg)) + if (decision is RouteDecision.Use && decision.profileId != desiredProfileId) { + desiredProfileId = decision.profileId + prefs().edit().putString(PREF_DESIRED_PROFILE, desiredProfileId).apply() + } } - try { - if (currentProfile != null) { - runCatching { activeTunnelManager?.stop() } - .onFailure { Log.e(TAG, "Error stopping tunnel: ${it.message}") } + + // Re-read so edits and refreshes are picked up; a deleted profile means no VPN. + val desired = desiredProfileId?.let { db.vpnProfileDao().getById(it) } + + tunnelMutex.withLock { + val active = activeProfile + if (desired?.id == active?.id && desired?.configContent == active?.configContent) { + // Nothing to switch, but a failed profile may no longer be wanted (e.g. a No VPN app). + if (desired == null && failedProfileFlow.value != null) { + cancelRetry() + publishState(active = null, failed = null) + } + return } - if (targetProfile != null) { - runCatching { wgManager.start(targetProfile, this) } - .onSuccess { - activeTunnelManager = wgManager - scope.launch(Dispatchers.IO) { - freshnessChecker.checkAndRefreshIfStale(targetProfile) - } - } - .onFailure { - Log.e(TAG, "Failed to start tunnel for ${targetProfile.name}: ${it.message}") - activeTunnelManager = null + + switchingFlow.value = true + try { + if (activeTunnelManager != null) { + runCatching { activeTunnelManager?.stop() } + .onFailure { Log.e(TAG, "Error stopping tunnel: ${it.message}") } + } + activeTunnelManager = null + activeProfile = null + + if (desired == null) { + cancelRetry() + publishState(active = null, failed = null) + Log.i(TAG, "No VPN for $pkg") + return + } + + try { + wgManager.start(desired, this) + activeTunnelManager = wgManager + activeProfile = desired + cancelRetry() + publishState(active = desired, failed = null) + Log.i(TAG, "Switched to profile=${desired.name} for $pkg") + scope.launch(Dispatchers.IO) { + if (freshnessChecker.checkAndRefreshIfStale(desired)) requestReconcile() } - } else { + } catch (e: Exception) { + Log.e(TAG, "Failed to start tunnel for ${desired.name}: ${(e as? BackendException)?.reason ?: e.message}") + publishState(active = null, failed = desired) + scheduleRetry(e) + } + } finally { + switchingFlow.value = false + } + } + } + + private fun scheduleRetry(cause: Exception) { + retryJob?.cancel() + if ((cause as? BackendException)?.reason == BackendException.Reason.VPN_NOT_AUTHORIZED) { + // Another VPN app holds the VPN slot. Retrying would fight it; the next foreground + // change or opening UniVPN (which asks for consent again) tries once more. + Log.w(TAG, "VPN permission lost — not retrying automatically") + return + } + val delayMs = retryDelayMs(retryAttempt++) + Log.i(TAG, "Retrying in ${delayMs / 1000}s (attempt $retryAttempt)") + retryJob = scope.launch { + delay(delayMs) + requestReconcile() + } + } + + private fun cancelRetry() { + retryJob?.cancel() + retryJob = null + retryAttempt = 0 + } + + /** + * Called by the WireGuard backend when the tunnel goes down without us stopping it, usually + * because another VPN app took the VPN slot. Once consent is given, Android lets either app + * take the slot back at any time, so reconnecting here would fight the other app endlessly. + * Instead flag the profile as not connected and wait for the next app switch. + */ + private fun onTunnelDropped() { + scope.launch { + tunnelMutex.withLock { + val dropped = activeProfile ?: return@launch + runCatching { wgManager.stop() } activeTunnelManager = null + activeProfile = null + holdUntilForegroundChange = true + cancelRetry() + publishState(active = null, failed = dropped) + Log.w(TAG, "${dropped.name} was taken down externally — waiting for the next app switch") } - currentProfile = targetProfile - DebugState.profileName = targetProfile?.name ?: "None" - DebugState.profileType = targetProfile?.type?.name ?: "" - profileFlow.value = targetProfile?.name - Log.i(TAG, "Switched to profile=${targetProfile?.name} for $packageName") - } finally { - tunnelMutex.unlock() - switchingFlow.value = false } } + private fun publishState(active: VpnProfile?, failed: VpnProfile?) { + DebugState.profileName = active?.name ?: "None" + DebugState.profileType = active?.type?.name ?: "" + profileFlow.value = active?.name + failedProfileFlow.value = failed?.name + } + private fun getCurrentForegroundPackage(usm: UsageStatsManager): String? { val now = System.currentTimeMillis() - val events = usm.queryEvents(now - 10_000L, now) + // Look back far enough to find an app that has been open for hours (e.g. after a + // process restart or waking from standby), not just the last few seconds. + val events = usm.queryEvents(now - FOREGROUND_LOOKBACK_MS, now) val event = UsageEvents.Event() var lastForeground: String? = null while (events.hasNextEvent()) { @@ -260,27 +355,18 @@ class VpnSwitcherService : VpnService() { return lastForeground } - override fun onRevoke() { - Log.i(TAG, "VPN revoked (another VPN took over)") - scope.launch { - runCatching { activeTunnelManager?.stop() } - activeTunnelManager = null - currentProfile = null - DebugState.profileName = "None" - DebugState.profileType = "" - profileFlow.value = null - } - super.onRevoke() - } - companion object { private const val TAG = "UniVPN" private const val CHANNEL_ID = "univpn_service" private const val NOTIF_ID = 1 + private const val PREF_DESIRED_PROFILE = "desired_profile_id" + private const val FOREGROUND_LOOKBACK_MS = 3L * 24 * 60 * 60 * 1000 // 3 days @Volatile var instance: VpnSwitcherService? = null val switchingFlow = MutableStateFlow(false) val profileFlow = MutableStateFlow(null) + /** Name of the profile that should be running but isn't (start failed or tunnel dropped). */ + val failedProfileFlow = MutableStateFlow(null) private val ALWAYS_PASSTHROUGH = setOf( "com.android.systemui", diff --git a/univpn/src/main/kotlin/com/univpn/app/tunnel/WgConfig.kt b/univpn/src/main/kotlin/com/univpn/app/tunnel/WgConfig.kt new file mode 100644 index 0000000..406a567 --- /dev/null +++ b/univpn/src/main/kotlin/com/univpn/app/tunnel/WgConfig.kt @@ -0,0 +1,21 @@ +package com.univpn.app.tunnel + +/** Helpers for reading WireGuard config text. No Android dependencies, so they're unit-testable. */ +object WgConfig { + + /** Host of the first peer's Endpoint, without brackets or port; null if there is none. */ + fun endpointHost(configContent: String): String? { + for (line in configContent.lines()) { + val trimmed = line.trim() + if (trimmed.startsWith("Endpoint", ignoreCase = true)) { + val value = trimmed.substringAfter("=").trim() + return if (value.startsWith("[")) { + value.substringAfter("[").substringBefore("]") + } else { + value.substringBeforeLast(":") + } + } + } + return null + } +} diff --git a/univpn/src/main/kotlin/com/univpn/app/tunnel/WireGuardTunnelManager.kt b/univpn/src/main/kotlin/com/univpn/app/tunnel/WireGuardTunnelManager.kt index 55e3aac..3399594 100644 --- a/univpn/src/main/kotlin/com/univpn/app/tunnel/WireGuardTunnelManager.kt +++ b/univpn/src/main/kotlin/com/univpn/app/tunnel/WireGuardTunnelManager.kt @@ -10,21 +10,35 @@ import com.wireguard.config.Config import kotlinx.coroutines.* import kotlin.coroutines.coroutineContext -class WireGuardTunnelManager : TunnelManager { +/** + * @param onUnexpectedDown called when the backend takes the tunnel down without us asking, + * e.g. when another VPN app takes over and GoBackend's VpnService is destroyed. + */ +class WireGuardTunnelManager( + private val onUnexpectedDown: (() -> Unit)? = null +) : TunnelManager { private var backend: GoBackend? = null private var activeTunnel: WgTunnel? = null private var pollScope: CoroutineScope? = null + @Volatile private var stopping = false override suspend fun start(profile: VpnProfile, service: VpnService) { withContext(Dispatchers.IO) { - val be = GoBackend(service).also { backend = it } + val be = GoBackend(service) val config = Config.parse(profile.configContent.reader().buffered()) - val tunnel = WgTunnel(profile.name).also { activeTunnel = it } + val tunnel = WgTunnel(profile.name) { t -> + if (!stopping && t === activeTunnel) { + Log.w(TAG, "WireGuard ${profile.name} went down unexpectedly") + onUnexpectedDown?.invoke() + } + } be.setState(tunnel, Tunnel.State.UP, config) + backend = be + activeTunnel = tunnel Log.i(TAG, "WireGuard tunnel up: ${profile.name}") - val peerHost = parsePeerEndpointHost(profile.configContent) + val peerHost = WgConfig.endpointHost(profile.configContent) pollScope?.cancel() pollScope = CoroutineScope(SupervisorJob() + Dispatchers.IO).also { scope -> @@ -35,14 +49,19 @@ class WireGuardTunnelManager : TunnelManager { override suspend fun stop() { withContext(Dispatchers.IO) { - pollScope?.cancel() - pollScope = null - activeTunnel?.let { t -> - runCatching { backend?.setState(t, Tunnel.State.DOWN, null) } - .onFailure { Log.e(TAG, "Error stopping tunnel: ${it.message}") } + stopping = true + try { + pollScope?.cancel() + pollScope = null + activeTunnel?.let { t -> + runCatching { backend?.setState(t, Tunnel.State.DOWN, null) } + .onFailure { Log.e(TAG, "Error stopping tunnel: ${it.message}") } + } + activeTunnel = null + backend = null + } finally { + stopping = false } - activeTunnel = null - backend = null DebugState.latencyMs = -1 DebugState.latencyFlow.value = -1 DebugState.txBytes = 0L @@ -50,6 +69,13 @@ class WireGuardTunnelManager : TunnelManager { } } + /** Bytes received from the peer. Non-zero means a handshake completed. */ + fun receivedBytes(): Long { + val be = backend ?: return 0L + val t = activeTunnel ?: return 0L + return runCatching { be.getStatistics(t).totalRx() }.getOrDefault(0L) + } + private suspend fun pollStats(be: GoBackend, tunnel: WgTunnel, peerHost: String?) { var baselineTx = 0L var baselineRx = 0L @@ -88,25 +114,14 @@ class WireGuardTunnelManager : TunnelManager { ?.groupValues?.get(1)?.toFloatOrNull()?.toInt() }.getOrNull() - private fun parsePeerEndpointHost(configContent: String): String? { - for (line in configContent.lines()) { - val trimmed = line.trim() - if (trimmed.startsWith("Endpoint", ignoreCase = true)) { - val value = trimmed.substringAfter("=").trim() - return if (value.startsWith("[")) { - value.substringAfter("[").substringBefore("]") - } else { - value.substringBeforeLast(":") - } - } - } - return null - } - - private class WgTunnel(private val name: String) : Tunnel { + private class WgTunnel( + private val name: String, + private val onDown: (WgTunnel) -> Unit + ) : Tunnel { override fun getName() = name override fun onStateChange(newState: Tunnel.State) { Log.i(TAG, "WireGuard $name → $newState") + if (newState == Tunnel.State.DOWN) onDown(this) } } diff --git a/univpn/src/main/kotlin/com/univpn/app/ui/TvMainActivity.kt b/univpn/src/main/kotlin/com/univpn/app/ui/TvMainActivity.kt index e73d463..74f6a05 100644 --- a/univpn/src/main/kotlin/com/univpn/app/ui/TvMainActivity.kt +++ b/univpn/src/main/kotlin/com/univpn/app/ui/TvMainActivity.kt @@ -76,10 +76,18 @@ class TvMainActivity : FragmentActivity() { combine( VpnSwitcherService.switchingFlow, VpnSwitcherService.profileFlow, + VpnSwitcherService.failedProfileFlow, DebugState.latencyFlow, - ) { switching, profileName, latencyMs -> Triple(switching, profileName, latencyMs) } - .collect { (switching, profileName, latencyMs) -> + ) { switching, profileName, failedName, latencyMs -> ChipState(switching, profileName, failedName, latencyMs) } + .collect { (switching, profileName, failedName, latencyMs) -> when { + // A failed profile stays flagged while retries run, so the warning + // doesn't flicker back to "Switching…" on every attempt. + failedName != null -> { + switchingChip.text = "⚠ $failedName — not connected" + switchingChip.setTextColor(ContextCompat.getColor(this@TvMainActivity, R.color.latency_bad)) + switchingChip.visibility = View.VISIBLE + } switching -> { switchingChip.text = "⟳ Switching…" switchingChip.setTextColor(ContextCompat.getColor(this@TvMainActivity, R.color.latency_good)) @@ -168,6 +176,13 @@ class TvMainActivity : FragmentActivity() { return mode == AppOpsManager.MODE_ALLOWED } + private data class ChipState( + val switching: Boolean, + val profileName: String?, + val failedName: String?, + val latencyMs: Int, + ) + companion object { private const val TAG = "UniVPN_TV" private const val REQUEST_VPN_PERMISSION = 100 diff --git a/univpn/src/test/kotlin/com/univpn/app/provider/MullvadEndpointTest.kt b/univpn/src/test/kotlin/com/univpn/app/provider/MullvadEndpointTest.kt new file mode 100644 index 0000000..285ad71 --- /dev/null +++ b/univpn/src/test/kotlin/com/univpn/app/provider/MullvadEndpointTest.kt @@ -0,0 +1,54 @@ +package com.univpn.app.provider + +import org.junit.Assert.assertEquals +import org.junit.Test + +class MullvadEndpointTest { + + private fun config(endpoint: String) = """ + [Interface] + PrivateKey = aaaa + Address = 10.64.1.2/32 + + [Peer] + PublicKey = bbbb + Endpoint = $endpoint + AllowedIPs = 0.0.0.0/0, ::/0 + """.trimIndent() + + @Test + fun resolvable_bareRelayHostname_getsRelayDomain() { + assertEquals("al-tia-wg-001.relays.mullvad.net", MullvadEndpoint.resolvable("al-tia-wg-001")) + } + + @Test + fun resolvable_ipAndFqdnAndIpv6_unchanged() { + assertEquals("103.124.165.2", MullvadEndpoint.resolvable("103.124.165.2")) + assertEquals("se-got-wg-001.relays.mullvad.net", MullvadEndpoint.resolvable("se-got-wg-001.relays.mullvad.net")) + assertEquals("2a03:1b20::1", MullvadEndpoint.resolvable("2a03:1b20::1")) + } + + @Test + fun repair_rewritesBareHostnameFromBrokenRefresh() { + assertEquals( + config("al-tia-wg-001.relays.mullvad.net:51820"), + MullvadEndpoint.repair(config("al-tia-wg-001:51820")) + ) + } + + @Test + fun repair_leavesWorkingEndpointsAlone() { + for (ok in listOf("103.124.165.2:51820", "[2a03:1b20::1]:51820", "se-got-wg-001.relays.mullvad.net:51820")) { + assertEquals(config(ok), MullvadEndpoint.repair(config(ok))) + } + } + + @Test + fun repair_handlesCrlfConfigs() { + val broken = config("al-tia-wg-001:51820").replace("\n", "\r\n") + assertEquals( + config("al-tia-wg-001.relays.mullvad.net:51820").replace("\n", "\r\n"), + MullvadEndpoint.repair(broken) + ) + } +} diff --git a/univpn/src/test/kotlin/com/univpn/app/service/RouteDecisionTest.kt b/univpn/src/test/kotlin/com/univpn/app/service/RouteDecisionTest.kt new file mode 100644 index 0000000..19be1a8 --- /dev/null +++ b/univpn/src/test/kotlin/com/univpn/app/service/RouteDecisionTest.kt @@ -0,0 +1,36 @@ +package com.univpn.app.service + +import com.univpn.app.data.model.AppRoute +import org.junit.Assert.assertEquals +import org.junit.Test + +class RouteDecisionTest { + + @Test + fun noRoute_keepsCurrentTunnel() { + // "Default (keep current tunnel)" deletes the route; it must not drop the tunnel (#5). + assertEquals(RouteDecision.Keep, decideRoute(null)) + } + + @Test + fun passthrough_keepsCurrentTunnel() { + assertEquals(RouteDecision.Keep, decideRoute(AppRoute("com.example.launcher", null, isPassthrough = true))) + } + + @Test + fun explicitNoVpn_usesNoProfile() { + assertEquals(RouteDecision.Use(null), decideRoute(AppRoute("com.example.tv", null, isPassthrough = false))) + } + + @Test + fun profileRoute_usesThatProfile() { + assertEquals(RouteDecision.Use("p1"), decideRoute(AppRoute("com.bbc.iplayer", "p1", isPassthrough = false))) + } + + @Test + fun retryDelay_backsOffAndCapsAtOneMinute() { + assertEquals(listOf(5_000L, 10_000L, 20_000L, 40_000L, 60_000L, 60_000L, 60_000L), + (0..6).map { retryDelayMs(it) }) + assertEquals(60_000L, retryDelayMs(1_000)) + } +} diff --git a/univpn/src/test/kotlin/com/univpn/app/tunnel/WgConfigTest.kt b/univpn/src/test/kotlin/com/univpn/app/tunnel/WgConfigTest.kt new file mode 100644 index 0000000..0d17777 --- /dev/null +++ b/univpn/src/test/kotlin/com/univpn/app/tunnel/WgConfigTest.kt @@ -0,0 +1,28 @@ +package com.univpn.app.tunnel + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +class WgConfigTest { + + @Test + fun endpointHost_ipv4() { + assertEquals("103.124.165.2", WgConfig.endpointHost("[Peer]\nEndpoint = 103.124.165.2:51820")) + } + + @Test + fun endpointHost_ipv6Bracketed() { + assertEquals("2a03:1b20::1", WgConfig.endpointHost("[Peer]\nEndpoint = [2a03:1b20::1]:51820")) + } + + @Test + fun endpointHost_hostnameAndLooseSpacing() { + assertEquals("vpn.example.org", WgConfig.endpointHost("[Peer]\n endpoint=vpn.example.org:443 ")) + } + + @Test + fun endpointHost_none() { + assertNull(WgConfig.endpointHost("[Interface]\nPrivateKey = x")) + } +}