From 5ac0cc547c0c2448c068c80657cfc22a9abc7aba Mon Sep 17 00:00:00 2001
From: Robin Lee <rgl@google.com>
Date: Fri, 10 Mar 2017 16:19:54 +0000
Subject: [PATCH] Use Vpn rules (not firewall) for always-on VPN

Firewall rules don't work on 464xlat because they were created under
an assumption that there's only one address for the server and it's
ipv4, which doesn't go so well when we're on an ipv6-only network.

Bug: 33159037
Test: runtest -x net/java/com/android/server/connectivity/VpnTest.java
Change-Id: Id331526367fe13838874961da194b07bd50d4c97
---
 .../android/server/ConnectivityService.java   | 14 +----
 .../android/server/connectivity/VpnTest.java  | 57 +++++++++++++++++++
 2 files changed, 60 insertions(+), 11 deletions(-)

diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 5b029d1cc0..5467a34146 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -3702,17 +3702,9 @@ public class ConnectivityService extends IConnectivityManager.Stub
             existing.shutdown();
         }
 
-        try {
-            if (tracker != null) {
-                mNetd.setFirewallEnabled(true);
-                mNetd.setFirewallInterfaceRule("lo", true);
-                mLockdownTracker = tracker;
-                mLockdownTracker.init();
-            } else {
-                mNetd.setFirewallEnabled(false);
-            }
-        } catch (RemoteException e) {
-            // ignored; NMS lives inside system_server
+        if (tracker != null) {
+            mLockdownTracker = tracker;
+            mLockdownTracker.init();
         }
     }
 
diff --git a/tests/net/java/com/android/server/connectivity/VpnTest.java b/tests/net/java/com/android/server/connectivity/VpnTest.java
index efe6fec6fc..506d9e5043 100644
--- a/tests/net/java/com/android/server/connectivity/VpnTest.java
+++ b/tests/net/java/com/android/server/connectivity/VpnTest.java
@@ -27,6 +27,7 @@ import android.annotation.UserIdInt;
 import android.app.AppOpsManager;
 import android.app.NotificationManager;
 import android.content.Context;
+import android.content.Intent;
 import android.content.pm.ApplicationInfo;
 import android.content.pm.PackageManager;
 import android.content.pm.UserInfo;
@@ -42,6 +43,8 @@ import android.test.suitebuilder.annotation.SmallTest;
 import android.util.ArrayMap;
 import android.util.ArraySet;
 
+import com.android.internal.net.VpnConfig;
+
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Map;
@@ -101,8 +104,10 @@ public class VpnTest extends AndroidTestCase {
     @Override
     public void setUp() throws Exception {
         MockitoAnnotations.initMocks(this);
+
         when(mContext.getPackageManager()).thenReturn(mPackageManager);
         setMockedPackages(mPackages);
+
         when(mContext.getPackageName()).thenReturn(Vpn.class.getPackage().getName());
         when(mContext.getSystemService(eq(Context.USER_SERVICE))).thenReturn(mUserManager);
         when(mContext.getSystemService(eq(Context.APP_OPS_SERVICE))).thenReturn(mAppOps);
@@ -257,6 +262,58 @@ public class VpnTest extends AndroidTestCase {
         }));
     }
 
+    @SmallTest
+    public void testLockdownRuleRepeatability() throws Exception {
+        final Vpn vpn = createVpn(primaryUser.id);
+
+        // Given legacy lockdown is already enabled,
+        vpn.setLockdown(true);
+        verify(mNetService, times(1)).setAllowOnlyVpnForUids(
+                eq(true), aryEq(new UidRange[] {UidRange.createForUser(primaryUser.id)}));
+
+        // Enabling legacy lockdown twice should do nothing.
+        vpn.setLockdown(true);
+        verify(mNetService, times(1)).setAllowOnlyVpnForUids(anyBoolean(), any(UidRange[].class));
+
+        // And disabling should remove the rules exactly once.
+        vpn.setLockdown(false);
+        verify(mNetService, times(1)).setAllowOnlyVpnForUids(
+                eq(false), aryEq(new UidRange[] {UidRange.createForUser(primaryUser.id)}));
+
+        // Removing the lockdown again should have no effect.
+        vpn.setLockdown(false);
+        verify(mNetService, times(2)).setAllowOnlyVpnForUids(anyBoolean(), any(UidRange[].class));
+    }
+
+    @SmallTest
+    public void testLockdownRuleReversibility() throws Exception {
+        final Vpn vpn = createVpn(primaryUser.id);
+
+        final UidRange[] entireUser = {
+            UidRange.createForUser(primaryUser.id)
+        };
+        final UidRange[] exceptPkg0 = {
+            new UidRange(entireUser[0].start, entireUser[0].start + PKG_UIDS[0] - 1),
+            new UidRange(entireUser[0].start + PKG_UIDS[0] + 1, entireUser[0].stop)
+        };
+
+        final InOrder order = inOrder(mNetService);
+
+        // Given lockdown is enabled with no package (legacy VPN),
+        vpn.setLockdown(true);
+        order.verify(mNetService).setAllowOnlyVpnForUids(eq(true), aryEq(entireUser));
+
+        // When a new VPN package is set the rules should change to cover that package.
+        vpn.prepare(null, PKGS[0]);
+        order.verify(mNetService).setAllowOnlyVpnForUids(eq(false), aryEq(entireUser));
+        order.verify(mNetService).setAllowOnlyVpnForUids(eq(true), aryEq(exceptPkg0));
+
+        // When that VPN package is unset, everything should be undone again in reverse.
+        vpn.prepare(null, VpnConfig.LEGACY_VPN);
+        order.verify(mNetService).setAllowOnlyVpnForUids(eq(false), aryEq(exceptPkg0));
+        order.verify(mNetService).setAllowOnlyVpnForUids(eq(true), aryEq(entireUser));
+    }
+
     @SmallTest
     public void testNotificationShownForAlwaysOnApp() {
         final UserHandle userHandle = UserHandle.of(primaryUser.id);