Merge changes Ic6ff7a3d,Iff9b9792
* changes:
Refactor code and improve tests for VPN filtering
Remove LOCKDOWN from FirewallChain IntDef
diff --git a/framework/src/android/net/ConnectivityManager.java b/framework/src/android/net/ConnectivityManager.java
index 1b0578f..39cd7f3 100644
--- a/framework/src/android/net/ConnectivityManager.java
+++ b/framework/src/android/net/ConnectivityManager.java
@@ -983,16 +983,6 @@
public static final int FIREWALL_CHAIN_LOW_POWER_STANDBY = 5;
/**
- * Firewall chain used for lockdown VPN.
- * Denylist of apps that cannot receive incoming packets except on loopback because they are
- * subject to an always-on VPN which is not currently connected.
- *
- * @see #BLOCKED_REASON_LOCKDOWN_VPN
- * @hide
- */
- public static final int FIREWALL_CHAIN_LOCKDOWN_VPN = 6;
-
- /**
* Firewall chain used for OEM-specific application restrictions.
* Denylist of apps that will not have network access due to OEM-specific restrictions.
* @hide
@@ -1024,7 +1014,6 @@
FIREWALL_CHAIN_POWERSAVE,
FIREWALL_CHAIN_RESTRICTED,
FIREWALL_CHAIN_LOW_POWER_STANDBY,
- FIREWALL_CHAIN_LOCKDOWN_VPN,
FIREWALL_CHAIN_OEM_DENY_1,
FIREWALL_CHAIN_OEM_DENY_2,
FIREWALL_CHAIN_OEM_DENY_3
diff --git a/service/jni/com_android_server_BpfNetMaps.cpp b/service/jni/com_android_server_BpfNetMaps.cpp
index bc70c93..2780044 100644
--- a/service/jni/com_android_server_BpfNetMaps.cpp
+++ b/service/jni/com_android_server_BpfNetMaps.cpp
@@ -151,6 +151,12 @@
return (jint)status.code();
}
+static jint native_updateUidLockdownRule(JNIEnv* env, jobject self, jint uid, jboolean add) {
+ Status status = mTc.updateUidLockdownRule(uid, add);
+ CHECK_LOG(status);
+ return (jint)status.code();
+}
+
static jint native_swapActiveStatsMap(JNIEnv* env, jobject self) {
Status status = mTc.swapActiveStatsMap();
CHECK_LOG(status);
@@ -203,6 +209,8 @@
(void*)native_addUidInterfaceRules},
{"native_removeUidInterfaceRules", "([I)I",
(void*)native_removeUidInterfaceRules},
+ {"native_updateUidLockdownRule", "(IZ)I",
+ (void*)native_updateUidLockdownRule},
{"native_swapActiveStatsMap", "()I",
(void*)native_swapActiveStatsMap},
{"native_setPermissionForUids", "(I[I)V",
diff --git a/service/native/TrafficController.cpp b/service/native/TrafficController.cpp
index bce3bb5..adc1925 100644
--- a/service/native/TrafficController.cpp
+++ b/service/native/TrafficController.cpp
@@ -340,8 +340,6 @@
return ALLOWLIST;
case LOW_POWER_STANDBY:
return ALLOWLIST;
- case LOCKDOWN:
- return DENYLIST;
case OEM_DENY_1:
return DENYLIST;
case OEM_DENY_2:
@@ -373,9 +371,6 @@
case LOW_POWER_STANDBY:
res = updateOwnerMapEntry(LOW_POWER_STANDBY_MATCH, uid, rule, type);
break;
- case LOCKDOWN:
- res = updateOwnerMapEntry(LOCKDOWN_VPN_MATCH, uid, rule, type);
- break;
case OEM_DENY_1:
res = updateOwnerMapEntry(OEM_DENY_1_MATCH, uid, rule, type);
break;
@@ -447,6 +442,18 @@
return netdutils::status::ok;
}
+Status TrafficController::updateUidLockdownRule(const uid_t uid, const bool add) {
+ std::lock_guard guard(mMutex);
+
+ netdutils::Status result = add ? addRule(uid, LOCKDOWN_VPN_MATCH)
+ : removeRule(uid, LOCKDOWN_VPN_MATCH);
+ if (!isOk(result)) {
+ ALOGW("%s Lockdown rule failed(%d): uid=%d",
+ (add ? "add": "remove"), result.code(), uid);
+ }
+ return result;
+}
+
int TrafficController::replaceUidOwnerMap(const std::string& name, bool isAllowlist __unused,
const std::vector<int32_t>& uids) {
// FirewallRule rule = isAllowlist ? ALLOW : DENY;
diff --git a/service/native/TrafficControllerTest.cpp b/service/native/TrafficControllerTest.cpp
index f84a910..b77c465 100644
--- a/service/native/TrafficControllerTest.cpp
+++ b/service/native/TrafficControllerTest.cpp
@@ -218,7 +218,7 @@
checkEachUidValue(uids, match);
}
- void expectUidOwnerMapValues(const std::vector<uint32_t>& appUids, uint8_t expectedRule,
+ void expectUidOwnerMapValues(const std::vector<uint32_t>& appUids, uint32_t expectedRule,
uint32_t expectedIif) {
for (uint32_t uid : appUids) {
Result<UidOwnerValue> value = mFakeUidOwnerMap.readValue(uid);
@@ -407,7 +407,6 @@
checkUidOwnerRuleForChain(POWERSAVE, POWERSAVE_MATCH);
checkUidOwnerRuleForChain(RESTRICTED, RESTRICTED_MATCH);
checkUidOwnerRuleForChain(LOW_POWER_STANDBY, LOW_POWER_STANDBY_MATCH);
- checkUidOwnerRuleForChain(LOCKDOWN, LOCKDOWN_VPN_MATCH);
checkUidOwnerRuleForChain(OEM_DENY_1, OEM_DENY_1_MATCH);
checkUidOwnerRuleForChain(OEM_DENY_2, OEM_DENY_2_MATCH);
checkUidOwnerRuleForChain(OEM_DENY_3, OEM_DENY_3_MATCH);
@@ -539,6 +538,21 @@
expectMapEmpty(mFakeUidOwnerMap);
}
+TEST_F(TrafficControllerTest, TestUpdateUidLockdownRule) {
+ // Add Lockdown rules
+ ASSERT_TRUE(isOk(mTc.updateUidLockdownRule(1000, true /* add */)));
+ ASSERT_TRUE(isOk(mTc.updateUidLockdownRule(1001, true /* add */)));
+ expectUidOwnerMapValues({1000, 1001}, LOCKDOWN_VPN_MATCH, 0);
+
+ // Remove one of Lockdown rules
+ ASSERT_TRUE(isOk(mTc.updateUidLockdownRule(1000, false /* add */)));
+ expectUidOwnerMapValues({1001}, LOCKDOWN_VPN_MATCH, 0);
+
+ // Remove remaining Lockdown rule
+ ASSERT_TRUE(isOk(mTc.updateUidLockdownRule(1001, false /* add */)));
+ expectMapEmpty(mFakeUidOwnerMap);
+}
+
TEST_F(TrafficControllerTest, TestUidInterfaceFilteringRulesCoexistWithExistingMatches) {
// Set up existing PENALTY_BOX_MATCH rules
ASSERT_TRUE(isOk(updateUidOwnerMaps({1000, 1001, 10012}, PENALTY_BOX_MATCH,
@@ -885,7 +899,6 @@
{POWERSAVE, ALLOWLIST},
{RESTRICTED, ALLOWLIST},
{LOW_POWER_STANDBY, ALLOWLIST},
- {LOCKDOWN, DENYLIST},
{OEM_DENY_1, DENYLIST},
{OEM_DENY_2, DENYLIST},
{OEM_DENY_3, DENYLIST},
diff --git a/service/native/include/Common.h b/service/native/include/Common.h
index 2427aa9..3f28991 100644
--- a/service/native/include/Common.h
+++ b/service/native/include/Common.h
@@ -35,7 +35,6 @@
POWERSAVE = 3,
RESTRICTED = 4,
LOW_POWER_STANDBY = 5,
- LOCKDOWN = 6,
OEM_DENY_1 = 7,
OEM_DENY_2 = 8,
OEM_DENY_3 = 9,
diff --git a/service/native/include/TrafficController.h b/service/native/include/TrafficController.h
index c019ce7..c921ff2 100644
--- a/service/native/include/TrafficController.h
+++ b/service/native/include/TrafficController.h
@@ -71,6 +71,8 @@
EXCLUDES(mMutex);
netdutils::Status removeUidInterfaceRules(const std::vector<int32_t>& uids) EXCLUDES(mMutex);
+ netdutils::Status updateUidLockdownRule(const uid_t uid, const bool add) EXCLUDES(mMutex);
+
netdutils::Status updateUidOwnerMap(const uint32_t uid,
UidOwnerMatchType matchType, IptOp op) EXCLUDES(mMutex);
diff --git a/service/src/com/android/server/BpfNetMaps.java b/service/src/com/android/server/BpfNetMaps.java
index c006bc6..151d0e3 100644
--- a/service/src/com/android/server/BpfNetMaps.java
+++ b/service/src/com/android/server/BpfNetMaps.java
@@ -216,6 +216,19 @@
}
/**
+ * Update lockdown rule for uid
+ *
+ * @param uid target uid to add/remove the rule
+ * @param add {@code true} to add the rule, {@code false} to remove the rule.
+ * @throws ServiceSpecificException in case of failure, with an error code indicating the
+ * cause of the failure.
+ */
+ public void updateUidLockdownRule(final int uid, final boolean add) {
+ final int err = native_updateUidLockdownRule(uid, add);
+ maybeThrow(err, "Unable to update lockdown rule");
+ }
+
+ /**
* Request netd to change the current active network stats map.
*
* @throws ServiceSpecificException in case of failure, with an error code indicating the
@@ -271,6 +284,7 @@
private native int native_setUidRule(int childChain, int uid, int firewallRule);
private native int native_addUidInterfaceRules(String ifName, int[] uids);
private native int native_removeUidInterfaceRules(int[] uids);
+ private native int native_updateUidLockdownRule(int uid, boolean add);
private native int native_swapActiveStatsMap();
private native void native_setPermissionForUids(int permissions, int[] uids);
private native void native_dump(FileDescriptor fd, boolean verbose);
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 e4a2c20..dedeb38 100755
--- a/service/src/com/android/server/connectivity/PermissionMonitor.java
+++ b/service/src/com/android/server/connectivity/PermissionMonitor.java
@@ -23,9 +23,6 @@
import static android.Manifest.permission.UPDATE_DEVICE_STATS;
import static android.content.pm.PackageInfo.REQUESTED_PERMISSION_GRANTED;
import static android.content.pm.PackageManager.GET_PERMISSIONS;
-import static android.net.ConnectivityManager.FIREWALL_CHAIN_LOCKDOWN_VPN;
-import static android.net.ConnectivityManager.FIREWALL_RULE_ALLOW;
-import static android.net.ConnectivityManager.FIREWALL_RULE_DENY;
import static android.net.ConnectivitySettingsManager.UIDS_ALLOWED_ON_RESTRICTED_NETWORKS;
import static android.net.INetd.PERMISSION_INTERNET;
import static android.net.INetd.PERMISSION_NETWORK;
@@ -684,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);
}
}
@@ -1079,11 +1080,7 @@
private void updateLockdownUidRule(int uid, boolean add) {
try {
- if (add) {
- mBpfNetMaps.setUidRule(FIREWALL_CHAIN_LOCKDOWN_VPN, uid, FIREWALL_RULE_DENY);
- } else {
- mBpfNetMaps.setUidRule(FIREWALL_CHAIN_LOCKDOWN_VPN, uid, FIREWALL_RULE_ALLOW);
- }
+ mBpfNetMaps.updateUidLockdownRule(uid, add);
} catch (ServiceSpecificException e) {
loge("Failed to " + (add ? "add" : "remove") + " Lockdown rule: " + e);
}
@@ -1259,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 e84df16..900ee5a 100644
--- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
@@ -52,7 +52,6 @@
import static android.net.ConnectivityManager.EXTRA_NETWORK_INFO;
import static android.net.ConnectivityManager.EXTRA_NETWORK_TYPE;
import static android.net.ConnectivityManager.FIREWALL_CHAIN_DOZABLE;
-import static android.net.ConnectivityManager.FIREWALL_CHAIN_LOCKDOWN_VPN;
import static android.net.ConnectivityManager.FIREWALL_CHAIN_LOW_POWER_STANDBY;
import static android.net.ConnectivityManager.FIREWALL_CHAIN_OEM_DENY_1;
import static android.net.ConnectivityManager.FIREWALL_CHAIN_OEM_DENY_2;
@@ -9516,38 +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).setUidRule(
- eq(FIREWALL_CHAIN_LOCKDOWN_VPN), eq(APP1_UID), eq(FIREWALL_RULE_DENY));
- verify(mBpfNetMaps).setUidRule(
- eq(FIREWALL_CHAIN_LOCKDOWN_VPN), eq(APP2_UID), eq(FIREWALL_RULE_DENY));
+ 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).setUidRule(
- eq(FIREWALL_CHAIN_LOCKDOWN_VPN), eq(APP1_UID), eq(FIREWALL_RULE_ALLOW));
- verify(mBpfNetMaps).setUidRule(
- eq(FIREWALL_CHAIN_LOCKDOWN_VPN), eq(APP2_UID), eq(FIREWALL_RULE_ALLOW));
+ 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());
@@ -10528,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);
@@ -10557,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 ecd17ba..354e79a 100644
--- a/tests/unit/java/com/android/server/connectivity/PermissionMonitorTest.java
+++ b/tests/unit/java/com/android/server/connectivity/PermissionMonitorTest.java
@@ -30,9 +30,6 @@
import static android.content.pm.PackageInfo.REQUESTED_PERMISSION_REQUIRED;
import static android.content.pm.PackageManager.GET_PERMISSIONS;
import static android.content.pm.PackageManager.MATCH_ANY_USER;
-import static android.net.ConnectivityManager.FIREWALL_CHAIN_LOCKDOWN_VPN;
-import static android.net.ConnectivityManager.FIREWALL_RULE_ALLOW;
-import static android.net.ConnectivityManager.FIREWALL_RULE_DENY;
import static android.net.ConnectivitySettingsManager.UIDS_ALLOWED_ON_RESTRICTED_NETWORKS;
import static android.net.INetd.PERMISSION_INTERNET;
import static android.net.INetd.PERMISSION_NETWORK;
@@ -698,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),
@@ -764,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),
@@ -774,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));
@@ -811,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());
@@ -857,155 +857,149 @@
@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);
- verify(mBpfNetMaps)
- .setUidRule(
- eq(FIREWALL_CHAIN_LOCKDOWN_VPN), eq(MOCK_UID11),
- eq(FIREWALL_RULE_DENY));
- assertEquals(mPermissionMonitor.getVpnLockdownUidRanges(), Set.of(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 */);
+ 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);
- verify(mBpfNetMaps)
- .setUidRule(eq(FIREWALL_CHAIN_LOCKDOWN_VPN), eq(MOCK_UID11),
- eq(FIREWALL_RULE_ALLOW));
+ 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);
- verify(mBpfNetMaps)
- .setUidRule(eq(FIREWALL_CHAIN_LOCKDOWN_VPN), eq(MOCK_UID11),
- eq(FIREWALL_RULE_DENY));
- assertEquals(mPermissionMonitor.getVpnLockdownUidRanges(), Set.of(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(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);
- verify(mBpfNetMaps, never()).setUidRule(anyInt(), anyInt(), anyInt());
- assertEquals(mPermissionMonitor.getVpnLockdownUidRanges(), Set.of(vpnRange));
+ mPermissionMonitor.updateVpnLockdownUidRanges(true /* add */, lockdownRange);
+ verify(mBpfNetMaps, never()).updateUidLockdownRule(anyInt(), anyBoolean());
+ 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);
- verify(mBpfNetMaps, never()).setUidRule(anyInt(), anyInt(), anyInt());
- assertEquals(mPermissionMonitor.getVpnLockdownUidRanges(), Set.of(vpnRange));
+ mPermissionMonitor.updateVpnLockdownUidRanges(false /* add */, lockdownRange);
+ verify(mBpfNetMaps, never()).updateUidLockdownRule(anyInt(), anyBoolean());
+ 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);
- verify(mBpfNetMaps)
- .setUidRule(eq(FIREWALL_CHAIN_LOCKDOWN_VPN), eq(MOCK_UID11),
- eq(FIREWALL_RULE_ALLOW));
+ 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);
- verify(mBpfNetMaps)
- .setUidRule(eq(FIREWALL_CHAIN_LOCKDOWN_VPN), eq(MOCK_UID11),
- eq(FIREWALL_RULE_DENY));
- assertEquals(mPermissionMonitor.getVpnLockdownUidRanges(), Set.of(vpnRange));
+ mPermissionMonitor.updateVpnLockdownUidRanges(true /* add */, lockdownRangeDuplicates);
+ verify(mBpfNetMaps).updateUidLockdownRule(anyInt(), eq(true) /* add */);
+ verify(mBpfNetMaps).updateUidLockdownRule(MOCK_UID11, true /* add */);
+ 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);
- verify(mBpfNetMaps, never()).setUidRule(anyInt(), anyInt(), anyInt());
- assertEquals(mPermissionMonitor.getVpnLockdownUidRanges(), Set.of(vpnRange));
+ mPermissionMonitor.updateVpnLockdownUidRanges(false /* add */, lockdownRange);
+ verify(mBpfNetMaps, never()).updateUidLockdownRule(anyInt(), anyBoolean());
+ 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);
- verify(mBpfNetMaps)
- .setUidRule(eq(FIREWALL_CHAIN_LOCKDOWN_VPN), eq(MOCK_UID11),
- eq(FIREWALL_RULE_ALLOW));
+ 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)
- .setUidRule(eq(FIREWALL_CHAIN_LOCKDOWN_VPN), eq(MOCK_UID11),
- eq(FIREWALL_RULE_DENY));
- verify(mBpfNetMaps)
- .setUidRule(eq(FIREWALL_CHAIN_LOCKDOWN_VPN), eq(MOCK_UID21),
- eq(FIREWALL_RULE_DENY));
+ verify(mBpfNetMaps, times(2)).updateUidLockdownRule(anyInt(), eq(true) /* add */);
+ verify(mBpfNetMaps).updateUidLockdownRule(MOCK_UID11, true /* add */);
+ verify(mBpfNetMaps).updateUidLockdownRule(MOCK_UID21, true /* add */);
reset(mBpfNetMaps);
// Uninstalling package should remove Lockdown rules
mPermissionMonitor.onPackageRemoved(MOCK_PACKAGE1, MOCK_UID11);
- verify(mBpfNetMaps)
- .setUidRule(eq(FIREWALL_CHAIN_LOCKDOWN_VPN), eq(MOCK_UID11),
- eq(FIREWALL_RULE_ALLOW));
- verify(mBpfNetMaps, never())
- .setUidRule(eq(FIREWALL_CHAIN_LOCKDOWN_VPN), eq(MOCK_UID21),
- eq(FIREWALL_RULE_ALLOW));
+ verify(mBpfNetMaps).updateUidLockdownRule(anyInt(), eq(false) /* add */);
+ verify(mBpfNetMaps).updateUidLockdownRule(MOCK_UID11, false /* add */);
}
// Normal package add/remove operations will trigger multiple intent for uids corresponding to
@@ -1329,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();
@@ -1387,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();
@@ -1413,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();