Refactor code and improve tests for VPN filtering

Bug: 206482423
Test: atest ConnectivityServiceTest PermissionMonitorTest

Change-Id: Ic6ff7a3d7695ad6ce96764a9bab2c0a641ba2ba6
diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java
index 848901f..853a1a2 100644
--- a/service/src/com/android/server/ConnectivityService.java
+++ b/service/src/com/android/server/ConnectivityService.java
@@ -7760,10 +7760,6 @@
         // when the old rules are removed and the time when new rules are added. To fix this,
         // make eBPF support two allowlisted interfaces so here new rules can be added before the
         // old rules are being removed.
-
-        // Null iface given to onVpnUidRangesAdded/Removed is a wildcard to allow apps to receive
-        // packets on all interfaces. This is required to accept incoming traffic in Lockdown mode
-        // by overriding the Lockdown blocking rule.
         if (wasFiltering) {
             mPermissionMonitor.onVpnUidRangesRemoved(oldIface, ranges, vpnAppUid);
         }
@@ -8089,12 +8085,14 @@
      * Returns whether we need to set interface filtering rule or not
      */
     private boolean requiresVpnAllowRule(NetworkAgentInfo nai, LinkProperties lp,
-            String filterIface) {
-        // Only filter if lp has an interface.
-        if (lp == null || lp.getInterfaceName() == null) return false;
-        // Before T, allow rules are only needed if VPN isolation is enabled.
-        // T and After T, allow rules are needed for all VPNs.
-        return filterIface != null || (nai.isVPN() && SdkLevel.isAtLeastT());
+            String isolationIface) {
+        // Allow rules are always needed if VPN isolation is enabled.
+        if (isolationIface != null) return true;
+
+        // On T and above, allow rules are needed for all VPNs. Allow rule with null iface is a
+        // wildcard to allow apps to receive packets on all interfaces. This is required to accept
+        // incoming traffic in Lockdown mode by overriding the Lockdown blocking rule.
+        return SdkLevel.isAtLeastT() && nai.isVPN() && lp != null && lp.getInterfaceName() != null;
     }
 
     private static UidRangeParcel[] toUidRangeStableParcels(final @NonNull Set<UidRange> ranges) {
@@ -8237,10 +8235,6 @@
             // above, where the addition of new ranges happens before the removal of old ranges.
             // TODO Fix this window by computing an accurate diff on Set<UidRange>, so the old range
             // to be removed will never overlap with the new range to be added.
-
-            // Null iface given to onVpnUidRangesAdded/Removed is a wildcard to allow apps to
-            // receive packets on all interfaces. This is required to accept incoming traffic in
-            // Lockdown mode by overriding the Lockdown blocking rule.
             if (wasFiltering && !prevRanges.isEmpty()) {
                 mPermissionMonitor.onVpnUidRangesRemoved(oldIface, prevRanges,
                         prevNc.getOwnerUid());
diff --git a/service/src/com/android/server/connectivity/PermissionMonitor.java b/service/src/com/android/server/connectivity/PermissionMonitor.java
index 10db088..dedeb38 100755
--- a/service/src/com/android/server/connectivity/PermissionMonitor.java
+++ b/service/src/com/android/server/connectivity/PermissionMonitor.java
@@ -681,8 +681,12 @@
     }
 
     private synchronized void updateLockdownUid(int uid, boolean add) {
-        if (UidRange.containsUid(mVpnLockdownUidRanges.getSet(), uid)
-                && !hasRestrictedNetworksPermission(uid)) {
+        // Apps that can use restricted networks can always bypass VPNs.
+        if (hasRestrictedNetworksPermission(uid)) {
+            return;
+        }
+
+        if (UidRange.containsUid(mVpnLockdownUidRanges.getSet(), uid)) {
             updateLockdownUidRule(uid, add);
         }
     }
@@ -1252,7 +1256,7 @@
         pw.println("Lockdown filtering rules:");
         pw.increaseIndent();
         for (final UidRange range : mVpnLockdownUidRanges.getSet()) {
-            pw.println("UIDs: " + range.toString());
+            pw.println("UIDs: " + range);
         }
         pw.decreaseIndent();
 
diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
index b0b0750..900ee5a 100644
--- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
@@ -9515,34 +9515,28 @@
 
     @Test @IgnoreUpTo(Build.VERSION_CODES.S_V2)
     public void testLockdownSetFirewallUidRule() throws Exception {
-        // For ConnectivityService#setAlwaysOnVpnPackage.
-        mServiceContext.setPermission(
-                Manifest.permission.CONTROL_ALWAYS_ON_VPN, PERMISSION_GRANTED);
-        // Needed to call Vpn#setAlwaysOnPackage.
-        mServiceContext.setPermission(Manifest.permission.CONTROL_VPN, PERMISSION_GRANTED);
-        // Needed to call Vpn#isAlwaysOnPackageSupported.
-        mServiceContext.setPermission(NETWORK_SETTINGS, PERMISSION_GRANTED);
-
+        final Set<Range<Integer>> lockdownRange = UidRange.toIntRanges(Set.of(PRIMARY_UIDRANGE));
         // Enable Lockdown
-        final ArrayList<String> allowList = new ArrayList<>();
-        mVpnManagerService.setAlwaysOnVpnPackage(PRIMARY_USER, ALWAYS_ON_PACKAGE,
-                true /* lockdown */, allowList);
+        mCm.setRequireVpnForUids(true /* requireVpn */, lockdownRange);
         waitForIdle();
 
         // Lockdown rule is set to apps uids
+        verify(mBpfNetMaps, times(3)).updateUidLockdownRule(anyInt(), eq(true) /* add */);
         verify(mBpfNetMaps).updateUidLockdownRule(APP1_UID, true /* add */);
         verify(mBpfNetMaps).updateUidLockdownRule(APP2_UID, true /* add */);
+        verify(mBpfNetMaps).updateUidLockdownRule(VPN_UID, true /* add */);
 
         reset(mBpfNetMaps);
 
         // Disable lockdown
-        mVpnManagerService.setAlwaysOnVpnPackage(PRIMARY_USER, null, false /* lockdown */,
-                allowList);
+        mCm.setRequireVpnForUids(false /* requireVPN */, lockdownRange);
         waitForIdle();
 
         // Lockdown rule is removed from apps uids
+        verify(mBpfNetMaps, times(3)).updateUidLockdownRule(anyInt(), eq(false) /* add */);
         verify(mBpfNetMaps).updateUidLockdownRule(APP1_UID, false /* add */);
         verify(mBpfNetMaps).updateUidLockdownRule(APP2_UID, false /* add */);
+        verify(mBpfNetMaps).updateUidLockdownRule(VPN_UID, false /* add */);
 
         // Interface rules are not changed by Lockdown mode enable/disable
         verify(mBpfNetMaps, never()).addUidInterfaceRules(any(), any());
@@ -10523,27 +10517,28 @@
         assertNull(mService.mPermissionMonitor.getVpnInterfaceUidRanges("tun0"));
     }
 
-    @Test
-    public void testLegacyVpnInterfaceFilteringRule() throws Exception {
-        LinkProperties lp = new LinkProperties();
-        lp.setInterfaceName("tun0");
-        lp.addRoute(new RouteInfo(new IpPrefix(Inet6Address.ANY, 0), null));
-        lp.addRoute(new RouteInfo(new IpPrefix(Inet4Address.ANY, 0), null));
+    private void checkInterfaceFilteringRuleWithNullInterface(final LinkProperties lp,
+            final int uid) throws Exception {
         // The uid range needs to cover the test app so the network is visible to it.
         final Set<UidRange> vpnRange = Collections.singleton(PRIMARY_UIDRANGE);
-        mMockVpn.establish(lp, Process.SYSTEM_UID, vpnRange);
-        assertVpnUidRangesUpdated(true, vpnRange, Process.SYSTEM_UID);
+        mMockVpn.establish(lp, uid, vpnRange);
+        assertVpnUidRangesUpdated(true, vpnRange, uid);
 
         if (SdkLevel.isAtLeastT()) {
-            // On T and above, A connected Legacy VPN should have interface rules with null
-            // interface. Null Interface is a wildcard and this accepts traffic from all the
-            // interfaces. There are two expected invocations, one during the VPN initial
+            // On T and above, VPN should have rules for null interface. Null Interface is a
+            // wildcard and this accepts traffic from all the interfaces.
+            // There are two expected invocations, one during the VPN initial
             // connection, one during the VPN LinkProperties update.
             ArgumentCaptor<int[]> uidCaptor = ArgumentCaptor.forClass(int[].class);
             verify(mBpfNetMaps, times(2)).addUidInterfaceRules(
                     eq(null) /* iface */, uidCaptor.capture());
-            assertContainsExactly(uidCaptor.getAllValues().get(0), APP1_UID, APP2_UID, VPN_UID);
-            assertContainsExactly(uidCaptor.getAllValues().get(1), APP1_UID, APP2_UID, VPN_UID);
+            if (uid == VPN_UID) {
+                assertContainsExactly(uidCaptor.getAllValues().get(0), APP1_UID, APP2_UID);
+                assertContainsExactly(uidCaptor.getAllValues().get(1), APP1_UID, APP2_UID);
+            } else {
+                assertContainsExactly(uidCaptor.getAllValues().get(0), APP1_UID, APP2_UID, VPN_UID);
+                assertContainsExactly(uidCaptor.getAllValues().get(1), APP1_UID, APP2_UID, VPN_UID);
+            }
             assertEquals(mService.mPermissionMonitor.getVpnInterfaceUidRanges(null /* iface */),
                     vpnRange);
 
@@ -10552,50 +10547,37 @@
 
             // Disconnected VPN should have interface rules removed
             verify(mBpfNetMaps).removeUidInterfaceRules(uidCaptor.capture());
-            assertContainsExactly(uidCaptor.getValue(), APP1_UID, APP2_UID, VPN_UID);
+            if (uid == VPN_UID) {
+                assertContainsExactly(uidCaptor.getValue(), APP1_UID, APP2_UID);
+            } else {
+                assertContainsExactly(uidCaptor.getValue(), APP1_UID, APP2_UID, VPN_UID);
+            }
             assertNull(mService.mPermissionMonitor.getVpnInterfaceUidRanges(null /* iface */));
         } else {
-            // Before T, Legacy VPN should not have interface rules.
+            // Before T, rules are not configured for null interface.
             verify(mBpfNetMaps, never()).addUidInterfaceRules(any(), any());
         }
     }
 
     @Test
+    public void testLegacyVpnInterfaceFilteringRule() throws Exception {
+        LinkProperties lp = new LinkProperties();
+        lp.setInterfaceName("tun0");
+        lp.addRoute(new RouteInfo(new IpPrefix(Inet6Address.ANY, 0), null));
+        lp.addRoute(new RouteInfo(new IpPrefix(Inet4Address.ANY, 0), null));
+        // Legacy VPN should have interface filtering with null interface.
+        checkInterfaceFilteringRuleWithNullInterface(lp, Process.SYSTEM_UID);
+    }
+
+    @Test
     public void testLocalIpv4OnlyVpnInterfaceFilteringRule() throws Exception {
         LinkProperties lp = new LinkProperties();
         lp.setInterfaceName("tun0");
         lp.addRoute(new RouteInfo(new IpPrefix("192.0.2.0/24"), null, "tun0"));
         lp.addRoute(new RouteInfo(new IpPrefix(Inet6Address.ANY, 0), RTN_UNREACHABLE));
-        // The uid range needs to cover the test app so the network is visible to it.
-        final Set<UidRange> vpnRange = Collections.singleton(PRIMARY_UIDRANGE);
-        mMockVpn.establish(lp, Process.SYSTEM_UID, vpnRange);
-        assertVpnUidRangesUpdated(true, vpnRange, Process.SYSTEM_UID);
-
-        if (SdkLevel.isAtLeastT()) {
-            // IPv6 unreachable route should not be misinterpreted as a default route
-            // On T and above, A connected VPN that does not provide a default route should have
-            // interface rules with null interface. Null Interface is a wildcard and this accepts
-            // traffic from all the interfaces. There are two expected invocations, one during the
-            // VPN initial connection, one during the VPN LinkProperties update.
-            ArgumentCaptor<int[]> uidCaptor = ArgumentCaptor.forClass(int[].class);
-            verify(mBpfNetMaps, times(2)).addUidInterfaceRules(
-                    eq(null) /* iface */, uidCaptor.capture());
-            assertContainsExactly(uidCaptor.getAllValues().get(0), APP1_UID, APP2_UID, VPN_UID);
-            assertContainsExactly(uidCaptor.getAllValues().get(1), APP1_UID, APP2_UID, VPN_UID);
-            assertEquals(mService.mPermissionMonitor.getVpnInterfaceUidRanges(null /* iface */),
-                    vpnRange);
-
-            mMockVpn.disconnect();
-            waitForIdle();
-
-            // Disconnected VPN should have interface rules removed
-            verify(mBpfNetMaps).removeUidInterfaceRules(uidCaptor.capture());
-            assertContainsExactly(uidCaptor.getValue(), APP1_UID, APP2_UID, VPN_UID);
-            assertNull(mService.mPermissionMonitor.getVpnInterfaceUidRanges(null /* iface */));
-        } else {
-            // Before T, VPN with IPv6 unreachable route should not have interface rules.
-            verify(mBpfNetMaps, never()).addUidInterfaceRules(any(), any());
-        }
+        // VPN that does not provide a default route should have interface filtering with null
+        // interface.
+        checkInterfaceFilteringRuleWithNullInterface(lp, VPN_UID);
     }
 
     @Test
diff --git a/tests/unit/java/com/android/server/connectivity/PermissionMonitorTest.java b/tests/unit/java/com/android/server/connectivity/PermissionMonitorTest.java
index 7716522..354e79a 100644
--- a/tests/unit/java/com/android/server/connectivity/PermissionMonitorTest.java
+++ b/tests/unit/java/com/android/server/connectivity/PermissionMonitorTest.java
@@ -695,7 +695,8 @@
         mNetdMonitor.expectNetworkPerm(PERMISSION_SYSTEM, new UserHandle[]{MOCK_USER1},
                 SYSTEM_APPID1);
 
-        final List<PackageInfo> pkgs = List.of(buildPackageInfo(SYSTEM_PACKAGE1, SYSTEM_APP_UID21,
+        final List<PackageInfo> pkgs = List.of(
+                buildPackageInfo(SYSTEM_PACKAGE1, SYSTEM_APP_UID21,
                         CONNECTIVITY_USE_RESTRICTED_NETWORKS),
                 buildPackageInfo(SYSTEM_PACKAGE2, SYSTEM_APP_UID21, CHANGE_NETWORK_STATE));
         doReturn(pkgs).when(mPackageManager).getInstalledPackagesAsUser(eq(GET_PERMISSIONS),
@@ -761,9 +762,10 @@
                 MOCK_APPID1);
     }
 
-    private void doTestuidFilteringDuringVpnConnectDisconnectAndUidUpdates(@Nullable String ifName)
+    private void doTestUidFilteringDuringVpnConnectDisconnectAndUidUpdates(@Nullable String ifName)
             throws Exception {
-        doReturn(List.of(buildPackageInfo(SYSTEM_PACKAGE1, SYSTEM_APP_UID11, CHANGE_NETWORK_STATE,
+        doReturn(List.of(
+                buildPackageInfo(SYSTEM_PACKAGE1, SYSTEM_APP_UID11, CHANGE_NETWORK_STATE,
                         CONNECTIVITY_USE_RESTRICTED_NETWORKS),
                 buildPackageInfo(MOCK_PACKAGE1, MOCK_UID11),
                 buildPackageInfo(MOCK_PACKAGE2, MOCK_UID12),
@@ -771,7 +773,7 @@
                 .when(mPackageManager).getInstalledPackagesAsUser(eq(GET_PERMISSIONS), anyInt());
         buildAndMockPackageInfoWithPermissions(MOCK_PACKAGE1, MOCK_UID11);
         mPermissionMonitor.startMonitoring();
-        // Every app on user 0 except MOCK_UID12 are under VPN.
+        // Every app on user 0 except MOCK_UID12 is subject to the VPN.
         final Set<UidRange> vpnRange1 = Set.of(
                 new UidRange(0, MOCK_UID12 - 1),
                 new UidRange(MOCK_UID12 + 1, UserHandle.PER_USER_RANGE - 1));
@@ -808,18 +810,19 @@
 
     @Test
     public void testUidFilteringDuringVpnConnectDisconnectAndUidUpdates() throws Exception {
-        doTestuidFilteringDuringVpnConnectDisconnectAndUidUpdates("tun0");
+        doTestUidFilteringDuringVpnConnectDisconnectAndUidUpdates("tun0");
     }
 
     @Test
     public void testUidFilteringDuringVpnConnectDisconnectAndUidUpdatesWithWildcard()
             throws Exception {
-        doTestuidFilteringDuringVpnConnectDisconnectAndUidUpdates(null /* ifName */);
+        doTestUidFilteringDuringVpnConnectDisconnectAndUidUpdates(null /* ifName */);
     }
 
     private void doTestUidFilteringDuringPackageInstallAndUninstall(@Nullable String ifName) throws
             Exception {
-        doReturn(List.of(buildPackageInfo(SYSTEM_PACKAGE1, SYSTEM_APP_UID11, CHANGE_NETWORK_STATE,
+        doReturn(List.of(
+                buildPackageInfo(SYSTEM_PACKAGE1, SYSTEM_APP_UID11, CHANGE_NETWORK_STATE,
                         NETWORK_STACK, CONNECTIVITY_USE_RESTRICTED_NETWORKS),
                 buildPackageInfo(SYSTEM_PACKAGE2, VPN_UID)))
                 .when(mPackageManager).getInstalledPackagesAsUser(eq(GET_PERMISSIONS), anyInt());
@@ -854,125 +857,140 @@
 
     @Test
     public void testLockdownUidFilteringWithLockdownEnableDisable() {
-        doReturn(List.of(buildPackageInfo(SYSTEM_PACKAGE1, SYSTEM_APP_UID11, CHANGE_NETWORK_STATE,
+        doReturn(List.of(
+                buildPackageInfo(SYSTEM_PACKAGE1, SYSTEM_APP_UID11, CHANGE_NETWORK_STATE,
                         CONNECTIVITY_USE_RESTRICTED_NETWORKS),
                 buildPackageInfo(MOCK_PACKAGE1, MOCK_UID11),
                 buildPackageInfo(MOCK_PACKAGE2, MOCK_UID12),
                 buildPackageInfo(SYSTEM_PACKAGE2, VPN_UID)))
                 .when(mPackageManager).getInstalledPackagesAsUser(eq(GET_PERMISSIONS), anyInt());
         mPermissionMonitor.startMonitoring();
-        // Every app on user 0 except MOCK_UID12 are under VPN.
-        final UidRange[] vpnRange1 = {
+        // Every app on user 0 except MOCK_UID12 is subject to the VPN.
+        final UidRange[] lockdownRange = {
                 new UidRange(0, MOCK_UID12 - 1),
                 new UidRange(MOCK_UID12 + 1, UserHandle.PER_USER_RANGE - 1)
         };
 
-        // Add Lockdown uid range, expect a rule to be set up for user app MOCK_UID11
-        mPermissionMonitor.updateVpnLockdownUidRanges(true /* add */, vpnRange1);
+        // Add Lockdown uid range, expect a rule to be set up for MOCK_UID11 and VPN_UID
+        mPermissionMonitor.updateVpnLockdownUidRanges(true /* add */, lockdownRange);
+        verify(mBpfNetMaps, times(2)).updateUidLockdownRule(anyInt(), eq(true) /* add */);
         verify(mBpfNetMaps).updateUidLockdownRule(MOCK_UID11, true /* add */);
-        assertEquals(mPermissionMonitor.getVpnLockdownUidRanges(), Set.of(vpnRange1));
+        verify(mBpfNetMaps).updateUidLockdownRule(VPN_UID, true /* add */);
+        assertEquals(mPermissionMonitor.getVpnLockdownUidRanges(), Set.of(lockdownRange));
 
         reset(mBpfNetMaps);
 
         // Remove Lockdown uid range, expect rules to be torn down
-        mPermissionMonitor.updateVpnLockdownUidRanges(false /* false */, vpnRange1);
+        mPermissionMonitor.updateVpnLockdownUidRanges(false /* add */, lockdownRange);
+        verify(mBpfNetMaps, times(2)).updateUidLockdownRule(anyInt(), eq(false) /* add */);
         verify(mBpfNetMaps).updateUidLockdownRule(MOCK_UID11, false /* add */);
+        verify(mBpfNetMaps).updateUidLockdownRule(VPN_UID, false /* add */);
         assertTrue(mPermissionMonitor.getVpnLockdownUidRanges().isEmpty());
     }
 
     @Test
     public void testLockdownUidFilteringWithLockdownEnableDisableWithMultiAdd() {
-        doReturn(List.of(buildPackageInfo(SYSTEM_PACKAGE1, SYSTEM_APP_UID11, CHANGE_NETWORK_STATE,
+        doReturn(List.of(
+                buildPackageInfo(SYSTEM_PACKAGE1, SYSTEM_APP_UID11, CHANGE_NETWORK_STATE,
                         CONNECTIVITY_USE_RESTRICTED_NETWORKS),
                 buildPackageInfo(MOCK_PACKAGE1, MOCK_UID11),
                 buildPackageInfo(SYSTEM_PACKAGE2, VPN_UID)))
                 .when(mPackageManager).getInstalledPackagesAsUser(eq(GET_PERMISSIONS), anyInt());
         mPermissionMonitor.startMonitoring();
-        // MOCK_UID11 is under VPN.
+        // MOCK_UID11 is subject to the VPN.
         final UidRange range = new UidRange(MOCK_UID11, MOCK_UID11);
-        final UidRange[] vpnRange = {range};
+        final UidRange[] lockdownRange = {range};
 
         // Add Lockdown uid range at 1st time, expect a rule to be set up
-        mPermissionMonitor.updateVpnLockdownUidRanges(true /* add */, vpnRange);
+        mPermissionMonitor.updateVpnLockdownUidRanges(true /* add */, lockdownRange);
+        verify(mBpfNetMaps).updateUidLockdownRule(anyInt(), eq(true) /* add */);
         verify(mBpfNetMaps).updateUidLockdownRule(MOCK_UID11, true /* add */);
-        assertEquals(mPermissionMonitor.getVpnLockdownUidRanges(), Set.of(vpnRange));
+        assertEquals(mPermissionMonitor.getVpnLockdownUidRanges(), Set.of(lockdownRange));
 
         reset(mBpfNetMaps);
 
         // Add Lockdown uid range at 2nd time, expect a rule not to be set up because the uid
         // already has the rule
-        mPermissionMonitor.updateVpnLockdownUidRanges(true /* add */, vpnRange);
+        mPermissionMonitor.updateVpnLockdownUidRanges(true /* add */, lockdownRange);
         verify(mBpfNetMaps, never()).updateUidLockdownRule(anyInt(),  anyBoolean());
-        assertEquals(mPermissionMonitor.getVpnLockdownUidRanges(), Set.of(vpnRange));
+        assertEquals(mPermissionMonitor.getVpnLockdownUidRanges(), Set.of(lockdownRange));
 
         reset(mBpfNetMaps);
 
         // Remove Lockdown uid range at 1st time, expect a rule not to be torn down because we added
         // the range 2 times.
-        mPermissionMonitor.updateVpnLockdownUidRanges(false /* false */, vpnRange);
+        mPermissionMonitor.updateVpnLockdownUidRanges(false /* add */, lockdownRange);
         verify(mBpfNetMaps, never()).updateUidLockdownRule(anyInt(),  anyBoolean());
-        assertEquals(mPermissionMonitor.getVpnLockdownUidRanges(), Set.of(vpnRange));
+        assertEquals(mPermissionMonitor.getVpnLockdownUidRanges(), Set.of(lockdownRange));
 
         reset(mBpfNetMaps);
 
         // Remove Lockdown uid range at 2nd time, expect a rule to be torn down because we added
         // twice and we removed twice.
-        mPermissionMonitor.updateVpnLockdownUidRanges(false /* false */, vpnRange);
+        mPermissionMonitor.updateVpnLockdownUidRanges(false /* add */, lockdownRange);
+        verify(mBpfNetMaps).updateUidLockdownRule(anyInt(), eq(false) /* add */);
         verify(mBpfNetMaps).updateUidLockdownRule(MOCK_UID11, false /* add */);
         assertTrue(mPermissionMonitor.getVpnLockdownUidRanges().isEmpty());
     }
 
     @Test
     public void testLockdownUidFilteringWithLockdownEnableDisableWithDuplicates() {
-        doReturn(List.of(buildPackageInfo(SYSTEM_PACKAGE1, SYSTEM_APP_UID11, CHANGE_NETWORK_STATE,
+        doReturn(List.of(
+                buildPackageInfo(SYSTEM_PACKAGE1, SYSTEM_APP_UID11, CHANGE_NETWORK_STATE,
                         CONNECTIVITY_USE_RESTRICTED_NETWORKS),
                 buildPackageInfo(MOCK_PACKAGE1, MOCK_UID11),
                 buildPackageInfo(SYSTEM_PACKAGE2, VPN_UID)))
                 .when(mPackageManager).getInstalledPackagesAsUser(eq(GET_PERMISSIONS), anyInt());
         mPermissionMonitor.startMonitoring();
-        // MOCK_UID11 is under VPN.
+        // MOCK_UID11 is subject to the VPN.
         final UidRange range = new UidRange(MOCK_UID11, MOCK_UID11);
-        final UidRange[] vpnRangeDuplicates = {range, range};
-        final UidRange[] vpnRange = {range};
+        final UidRange[] lockdownRangeDuplicates = {range, range};
+        final UidRange[] lockdownRange = {range};
 
         // Add Lockdown uid ranges which contains duplicated uid ranges
-        mPermissionMonitor.updateVpnLockdownUidRanges(true /* add */, vpnRangeDuplicates);
+        mPermissionMonitor.updateVpnLockdownUidRanges(true /* add */, lockdownRangeDuplicates);
+        verify(mBpfNetMaps).updateUidLockdownRule(anyInt(), eq(true) /* add */);
         verify(mBpfNetMaps).updateUidLockdownRule(MOCK_UID11, true /* add */);
-        assertEquals(mPermissionMonitor.getVpnLockdownUidRanges(), Set.of(vpnRange));
+        assertEquals(mPermissionMonitor.getVpnLockdownUidRanges(), Set.of(lockdownRange));
 
         reset(mBpfNetMaps);
 
         // Remove Lockdown uid range at 1st time, expect a rule not to be torn down because uid
         // ranges we added contains duplicated uid ranges.
-        mPermissionMonitor.updateVpnLockdownUidRanges(false /* false */, vpnRange);
+        mPermissionMonitor.updateVpnLockdownUidRanges(false /* add */, lockdownRange);
         verify(mBpfNetMaps, never()).updateUidLockdownRule(anyInt(), anyBoolean());
-        assertEquals(mPermissionMonitor.getVpnLockdownUidRanges(), Set.of(vpnRange));
+        assertEquals(mPermissionMonitor.getVpnLockdownUidRanges(), Set.of(lockdownRange));
 
         reset(mBpfNetMaps);
 
         // Remove Lockdown uid range at 2nd time, expect a rule to be torn down.
-        mPermissionMonitor.updateVpnLockdownUidRanges(false /* false */, vpnRange);
+        mPermissionMonitor.updateVpnLockdownUidRanges(false /* add */, lockdownRange);
+        verify(mBpfNetMaps).updateUidLockdownRule(anyInt(), eq(false) /* add */);
         verify(mBpfNetMaps).updateUidLockdownRule(MOCK_UID11, false /* add */);
         assertTrue(mPermissionMonitor.getVpnLockdownUidRanges().isEmpty());
     }
 
     @Test
     public void testLockdownUidFilteringWithInstallAndUnInstall() {
-        doReturn(List.of(buildPackageInfo(SYSTEM_PACKAGE1, SYSTEM_APP_UID11, CHANGE_NETWORK_STATE,
+        doReturn(List.of(
+                buildPackageInfo(SYSTEM_PACKAGE1, SYSTEM_APP_UID11, CHANGE_NETWORK_STATE,
                         NETWORK_STACK, CONNECTIVITY_USE_RESTRICTED_NETWORKS),
                 buildPackageInfo(SYSTEM_PACKAGE2, VPN_UID)))
                 .when(mPackageManager).getInstalledPackagesAsUser(eq(GET_PERMISSIONS), anyInt());
         doReturn(List.of(MOCK_USER1, MOCK_USER2)).when(mUserManager).getUserHandles(eq(true));
 
         mPermissionMonitor.startMonitoring();
-        final UidRange[] vpnRange = {
+        final UidRange[] lockdownRange = {
                 UidRange.createForUser(MOCK_USER1),
                 UidRange.createForUser(MOCK_USER2)
         };
-        mPermissionMonitor.updateVpnLockdownUidRanges(true /* add */, vpnRange);
+        mPermissionMonitor.updateVpnLockdownUidRanges(true /* add */, lockdownRange);
+
+        reset(mBpfNetMaps);
 
         // Installing package should add Lockdown rules
         addPackageForUsers(new UserHandle[]{MOCK_USER1, MOCK_USER2}, MOCK_PACKAGE1, MOCK_APPID1);
+        verify(mBpfNetMaps, times(2)).updateUidLockdownRule(anyInt(), eq(true) /* add */);
         verify(mBpfNetMaps).updateUidLockdownRule(MOCK_UID11, true /* add */);
         verify(mBpfNetMaps).updateUidLockdownRule(MOCK_UID21, true /* add */);
 
@@ -980,8 +998,8 @@
 
         // Uninstalling package should remove Lockdown rules
         mPermissionMonitor.onPackageRemoved(MOCK_PACKAGE1, MOCK_UID11);
+        verify(mBpfNetMaps).updateUidLockdownRule(anyInt(), eq(false) /* add */);
         verify(mBpfNetMaps).updateUidLockdownRule(MOCK_UID11, false /* add */);
-        verify(mBpfNetMaps, never()).updateUidLockdownRule(MOCK_UID21, false /* add */);
     }
 
     // Normal package add/remove operations will trigger multiple intent for uids corresponding to
@@ -1305,7 +1323,8 @@
     public void testOnExternalApplicationsAvailable() throws Exception {
         // Initial the permission state. MOCK_PACKAGE1 and MOCK_PACKAGE2 are installed on external
         // and have different uids. There has no permission for both uids.
-        doReturn(List.of(buildPackageInfo(MOCK_PACKAGE1, MOCK_UID11),
+        doReturn(List.of(
+                buildPackageInfo(MOCK_PACKAGE1, MOCK_UID11),
                 buildPackageInfo(MOCK_PACKAGE2, MOCK_UID12)))
                 .when(mPackageManager).getInstalledPackagesAsUser(eq(GET_PERMISSIONS), anyInt());
         mPermissionMonitor.startMonitoring();
@@ -1363,7 +1382,8 @@
             throws Exception {
         // Initial the permission state. MOCK_PACKAGE1 and MOCK_PACKAGE2 are installed on external
         // storage and shared on MOCK_UID11. There has no permission for MOCK_UID11.
-        doReturn(List.of(buildPackageInfo(MOCK_PACKAGE1, MOCK_UID11),
+        doReturn(List.of(
+                buildPackageInfo(MOCK_PACKAGE1, MOCK_UID11),
                 buildPackageInfo(MOCK_PACKAGE2, MOCK_UID11)))
                 .when(mPackageManager).getInstalledPackagesAsUser(eq(GET_PERMISSIONS), anyInt());
         mPermissionMonitor.startMonitoring();
@@ -1389,7 +1409,8 @@
         // Initial the permission state. MOCK_PACKAGE1 is installed on external storage and
         // MOCK_PACKAGE2 is installed on device. These two packages are shared on MOCK_UID11.
         // MOCK_UID11 has NETWORK and INTERNET permissions.
-        doReturn(List.of(buildPackageInfo(MOCK_PACKAGE1, MOCK_UID11),
+        doReturn(List.of(
+                buildPackageInfo(MOCK_PACKAGE1, MOCK_UID11),
                 buildPackageInfo(MOCK_PACKAGE2, MOCK_UID11, CHANGE_NETWORK_STATE, INTERNET)))
                 .when(mPackageManager).getInstalledPackagesAsUser(eq(GET_PERMISSIONS), anyInt());
         mPermissionMonitor.startMonitoring();