Improve other-VPN interaction error checks

Author: pimterry

app/src/main/java/tech/httptoolkit/android/Constants.kt

@@ -7,6 +7,7 @@ object IntentExtras {
     const val PROXY_CONFIG_EXTRA = "tech.httptoolkit.android.PROXY_CONFIG"
     const val UNINTERCEPTED_APPS_EXTRA = "tech.httptoolkit.android.UNINTERCEPTED_APPS"
     const val INTERCEPTED_PORTS_EXTRA = "tech.httptoolkit.android.INTERCEPTED_PORTS"
+    const val VPN_FAILED_EXTRA = "tech.httptoolkit.android.VPN_FAILED"
 }
 
 object Constants {

app/src/main/java/tech/httptoolkit/android/ProxyVpnService.kt

@@ -8,6 +8,7 @@ import android.net.ProxyInfo
 import android.net.VpnService
 import android.os.Build
 import android.os.ParcelFileDescriptor
+import android.os.SystemClock
 import android.util.Log
 import androidx.core.app.NotificationCompat
 import androidx.localbroadcastmanager.content.LocalBroadcastManager
@@ -29,6 +30,10 @@ const val STOP_VPN_ACTION = "tech.httptoolkit.android.STOP_VPN_ACTION"
 const val VPN_STARTED_BROADCAST = "tech.httptoolkit.android.VPN_STARTED_BROADCAST"
 const val VPN_STOPPED_BROADCAST = "tech.httptoolkit.android.VPN_STOPPED_BROADCAST"
 
+// If our VPN is revoked this quickly after starting, we assume another VPN took it from us,
+// rather than the user having deliberately turned us off:
+private const val VPN_TAKEOVER_THRESHOLD_MS = 1000
+
 private var currentService: ProxyVpnService? = null
 fun isVpnActive(): Boolean {
     return if (currentService == null)
@@ -51,6 +56,8 @@ class ProxyVpnService : VpnService(), IProtectSocket {
         private set
 
     private var vpnInterface: ParcelFileDescriptor? = null
+
+    private var vpnStartedAt = 0L
     private var vpnRunnable: ProxyVpnRunnable? = null
 
     override fun onCreate() {
@@ -89,7 +96,7 @@ class ProxyVpnService : VpnService(), IProtectSocket {
                 return Service.START_REDELIVER_INTENT
             } else {
                 // We failed to start somehow - cleanup
-                stopVpn()
+                stopVpn(failed = true)
             }
         } else if (intent.action == STOP_VPN_ACTION) {
             stopVpn()
@@ -103,7 +110,8 @@ class ProxyVpnService : VpnService(), IProtectSocket {
     override fun onRevoke() {
         super.onRevoke()
         Log.i(TAG, "onRevoke called")
-        stopVpn()
+        val runtime = SystemClock.elapsedRealtime() - vpnStartedAt
+        stopVpn(failed = runtime < VPN_TAKEOVER_THRESHOLD_MS)
     }
 
     private fun showServiceNotification() {
@@ -232,6 +240,7 @@ class ProxyVpnService : VpnService(), IProtectSocket {
             return false
         } else {
             this.vpnInterface = vpnInterface
+            this.vpnStartedAt = SystemClock.elapsedRealtime()
         }
 
         app.lastProxy = proxyConfig
@@ -281,8 +290,8 @@ class ProxyVpnService : VpnService(), IProtectSocket {
         return startVpn(proxyConfig, uninterceptedApps, interceptedPorts)
     }
 
-    private fun stopVpn() {
-        Log.i(TAG, "VPN stopping...")
+    private fun stopVpn(failed: Boolean = false) {
+        Log.i(TAG, if (failed) "VPN stopping after failure..." else "VPN stopping...")
 
         if (vpnRunnable != null) {
             vpnRunnable!!.stop()
@@ -297,7 +306,9 @@ class ProxyVpnService : VpnService(), IProtectSocket {
         }
 
         stopForeground(true)
-        localBroadcastManager!!.sendBroadcast(Intent(VPN_STOPPED_BROADCAST))
+        localBroadcastManager!!.sendBroadcast(Intent(VPN_STOPPED_BROADCAST).apply {
+            putExtra(IntentExtras.VPN_FAILED_EXTRA, failed)
+        })
         stopSelf()
 
         currentService = null

app/src/main/java/tech/httptoolkit/android/main/MainActivity.kt

@@ -75,6 +75,10 @@ class MainActivity : ComponentActivity(), CoroutineScope by MainScope() {
             } else if (intent.action == VPN_STOPPED_BROADCAST) {
                 mainState = ConnectionState.DISCONNECTED
                 currentProxyConfig = null
+
+                if (intent.getBooleanExtra(IntentExtras.VPN_FAILED_EXTRA, false)) {
+                    showActiveVpnFailureAlert()
+                }
             }
         }
     }
@@ -88,10 +92,6 @@ class MainActivity : ComponentActivity(), CoroutineScope by MainScope() {
     private var interceptedAppCount: Int by mutableIntStateOf(0)
     private var interceptedPorts: Set<Int> by mutableStateOf(emptySet())
 
-    // Used to track extremely fast VPN setup failures, indicating setup issues (rather than
-    // manual user cancellation). Doesn't matter that it's not properly persistent.
-    private var lastPauseTime = -1L
-
     val pickAppsContract = registerForActivityResult(ActivityResultContracts.StartActivityForResult()) { result ->
         if (result.resultCode == RESULT_OK) {
             Log.i(TAG, "Pick apps result: OK")
@@ -260,7 +260,6 @@ class MainActivity : ComponentActivity(), CoroutineScope by MainScope() {
     override fun onPause() {
         super.onPause()
         Log.d(TAG, "onPause")
-        this.lastPauseTime = System.currentTimeMillis()
     }
 
     override fun onDestroy() {
@@ -548,18 +547,6 @@ class MainActivity : ComponentActivity(), CoroutineScope by MainScope() {
                 Log.i(TAG, "Notifications OK, starting VPN...")
                 startVpn()
             }
-        } else if (
-            requestCode == START_VPN_REQUEST &&
-            System.currentTimeMillis() - lastPauseTime < 200 && // On Pixel 4a it takes < 50ms
-            resultCode == RESULT_CANCELED
-        ) {
-            // If another always-on VPN is active, VPN start requests fail instantly as cancelled.
-            // We can't check that the VPN is always-on, but given an instant failure that's
-            // the likely cause, so we warn about it:
-            showActiveVpnFailureAlert()
-
-            // Then go back to the disconnected state:
-            mainState = ConnectionState.DISCONNECTED
         } else if (
             requestCode == INSTALL_CERT_REQUEST &&
             Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q // Required for promptToManuallyInstallCert
@@ -1016,15 +1003,18 @@ class MainActivity : ComponentActivity(), CoroutineScope by MainScope() {
     }
 
     private fun showActiveVpnFailureAlert() {
+        // The VPN service can report failures while we're in the background:
+        if (isFinishing || isDestroyed) return
+
         MaterialAlertDialogBuilder(this)
             .setTitle("VPN setup failed")
             .setIcon(R.drawable.ic_exclamation_triangle)
             .setMessage(
                 "HTTP Toolkit could not be configured as a VPN on your device." +
                 "\n\n" +
-                "This usually means you have an always-on VPN configured, which blocks " +
-                "installation of other VPNs. To activate HTTP Toolkit you'll need to " +
-                "deactivate that VPN first."
+                "This usually means another VPN is active, which blocks HTTP Toolkit's VPN. To " +
+                "activate HTTP Toolkit you'll need to deactivate that VPN first, including any " +
+                "always-on or auto-reconnect options."
             )
             .setNegativeButton("Cancel") { _, _ -> }
             .setPositiveButton("Open VPN Settings") { _, _ ->