Handle external VPN revocation policy

app/src/main/java/eu/faircode/netguard/ServiceSinkhole.java

@@ -3291,34 +3291,33 @@ private void set(Intent intent) {
     public void onRevoke() {
         Log.i(TAG, "Revoke");
 
-        // Android does not tell us *why* onRevoke() fired. The system calls it
-        // identically for (a) a deliberate user disable from the system VPN
-        // settings, (b) another VPN app taking the single VPN slot (#331), and
-        // (c) an OS-initiated teardown/re-auth of the always-on slot — Doze,
-        // memory pressure, an OS update — which is the "always-on VPN turns
-        // itself off after 3-5 days" report (#526).
+        // Preserve the enabled state only while Android still designates us as
+        // the always-on VPN. In that case a transient system teardown should be
+        // recoverable. A deliberate revoke in system settings, or another VPN
+        // taking the slot, clears the always-on designation and must disable TC;
+        // otherwise the repeating watchdog would keep trying to reclaim the VPN.
         //
-        // We deliberately do NOT persist enabled=false here. Every genuine
-        // in-app disable path (the main toggle in ActivityMain, the quick-
-        // settings tile in ServiceTileMain, and the widget in WidgetAdmin)
-        // already sets enabled=false *itself* before stopping the service, and
-        // tearing down our own tunnel does not trigger onRevoke(). So a revoke
-        // reaching this method is always externally initiated. Clearing
-        // "enabled" here used to turn every such external teardown into a
-        // permanent self-disable with no recovery: the service is START_STICKY
-        // and onStartCommand() keys off "enabled", so once it is false every
-        // subsequent (re)start immediately stops again.
-        //
-        // Leaving "enabled" untouched lets the existing recovery mechanisms
-        // re-establish the tunnel once TC reacquires the VPN slot: the OS
-        // restarting the always-on service, ReceiverAutostart on boot, the
-        // watchdog alarm, and reopening ActivityMain (which calls start() when
-        // enabled). We do not add any retry loop here, so a second VPN that
-        // legitimately holds the slot is not thrashed — recovery is driven only
-        // by those OS-paced triggers. The remaining tradeoff is that a user who
-        // disables TC via the *system* VPN settings (rather than the in-app
-        // toggle) will see it come back on the next such trigger; the in-app
-        // toggle remains the supported way to turn TC off.
+        // Correctness in both directions depends on the OS reporting the
+        // always-on designation correctly at revoke time. On pre-Q devices the
+        // hidden setting may be unavailable, so unknown is biased toward keeping
+        // the existing state; only a positively-known false disables TC.
+        Boolean alwaysOn = null;
+        try {
+            if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q)
+                alwaysOn = isAlwaysOn();
+            else {
+                String alwaysOnPackage = Settings.Secure.getString(
+                        getContentResolver(), "always_on_vpn_app");
+                alwaysOn = (alwaysOnPackage == null) ? null : getPackageName().equals(alwaysOnPackage);
+            }
+        } catch (Throwable ex) {
+            Log.w(TAG, "Could not determine always-on VPN state", ex);
+        }
+
+        SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(this);
+        boolean enabled = resolveEnabledAfterRevoke(prefs.getBoolean("enabled", false), alwaysOn);
+        prefs.edit().putBoolean("enabled", enabled).apply();
+        Log.i(TAG, "Revoke always-on=" + alwaysOn + " enabled=" + enabled);
 
         // Feedback: the tunnel really did drop, so inform the user (mirrors
         // Android's own "VPN disconnected" notice). This does not disable TC.
@@ -3328,6 +3327,12 @@ public void onRevoke() {
         super.onRevoke();
     }
 
+    static boolean resolveEnabledAfterRevoke(boolean enabled, Boolean alwaysOn) {
+        if (alwaysOn == null)
+            return enabled;
+        return enabled && alwaysOn;
+    }
+
     @Override
     public void onDestroy() {
         synchronized (this) {

app/src/test/java/eu/faircode/netguard/VpnRevokePolicyTest.java

@@ -0,0 +1,36 @@
+package eu.faircode.netguard;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.RobolectricTestRunner;
+
+@RunWith(RobolectricTestRunner.class)
+public class VpnRevokePolicyTest {
+    @Test
+    public void alwaysOnRevokeKeepsEnabledStateForRecovery() {
+        assertTrue(ServiceSinkhole.resolveEnabledAfterRevoke(true, true));
+    }
+
+    @Test
+    public void ordinaryRevokeDisablesWatchdogRecovery() {
+        assertFalse(ServiceSinkhole.resolveEnabledAfterRevoke(true, false));
+    }
+
+    @Test
+    public void revokeNeverEnablesAnAlreadyDisabledService() {
+        assertFalse(ServiceSinkhole.resolveEnabledAfterRevoke(false, true));
+    }
+
+    @Test
+    public void unknownAlwaysOnStateKeepsEnabledStateForRecovery() {
+        assertTrue(ServiceSinkhole.resolveEnabledAfterRevoke(true, null));
+    }
+
+    @Test
+    public void unknownAlwaysOnStateNeverEnablesAnAlreadyDisabledService() {
+        assertFalse(ServiceSinkhole.resolveEnabledAfterRevoke(false, null));
+    }
+}