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();