Always drop non-VPN ingress in lockdown mode
When "Block connections without VPN" is specified, incoming traffic
from non-VPN interfaces should be blocked regardless of the
determination made by ConnectivityService#getVpnIsolationInterface.
Outgoing traffic to non-VPN interfaces is already blocked in this case.
(Loopback is excluded as usual.)
Test: `adb shell dumpsys connectivity trafficcontroller` will now show
the tunnel interface for uids affected by lockdown when
getVpnIsolationInterface returns null (wildcard), to block non-VPN
ingress to such uids. This will return to 0 (wildcard) when lockdown
is toggled back off.
Also includes squashed change:
Author: Tommy Webb <tommy@calyxinstitute.org>
Date: Mon May 1 16:52:28 2023 -0400
fixup! Always drop non-VPN ingress in lockdown mode
For lockdown purposes, force an update of VPN filtering whenever the
interface names for a VPN have changed, to ensure that the BPF owner
map uses the most up-to-date interface for ingress filtering.
Issue: calyxos#1651
Change-Id: Ia0c75a723134023906134597b395653c7a570686
Co-authored-by: Tommy Webb <tommy@calyxinstitute.org>
Issue: calyxos#1255
Bug: 206482423
Change-Id: Id7954816566cb06bf2e9869ea98b20678835df9d
Signed-off-by: Dmitrii <bankersenator@gmail.com>
Signed-off-by: Jis G Jacob <studiokeys@blissroms.org>
diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java
index 9027fca..d5c015f 100755
--- a/service/src/com/android/server/ConnectivityService.java
+++ b/service/src/com/android/server/ConnectivityService.java
@@ -7382,6 +7382,10 @@
final boolean curMetered = nai.networkCapabilities.isMetered();
maybeNotifyNetworkBlocked(nai, curMetered, curMetered,
mVpnBlockedUidRanges, newVpnBlockedUidRanges);
+
+ if (nai.isVPN()) {
+ updateVpnFiltering(nai.linkProperties, nai.linkProperties, nai, true /* force */);
+ }
}
mVpnBlockedUidRanges = newVpnBlockedUidRanges;
@@ -9056,11 +9060,16 @@
// the LinkProperties for the network are accurate.
networkAgent.clatd.fixupLinkProperties(oldLp, newLp);
- updateInterfaces(newLp, oldLp, netId, networkAgent);
+ final boolean anyIfaceChanges =
+ updateInterfaces(newLp, oldLp, netId, networkAgent);
+
+ // If any interface names changed, ensure VPN filtering is updated so that ingress filtering
+ // uses the most up-to-date allowed interface.
+ final boolean shouldForceUpdateVpnFiltering = networkAgent.isVPN() && anyIfaceChanges;
// update filtering rules, need to happen after the interface update so netd knows about the
// new interface (the interface name -> index map becomes initialized)
- updateVpnFiltering(newLp, oldLp, networkAgent);
+ updateVpnFiltering(newLp, oldLp, networkAgent, shouldForceUpdateVpnFiltering);
updateMtu(newLp, oldLp);
// TODO - figure out what to do for clat
@@ -9211,7 +9220,8 @@
}
}
- private void updateInterfaces(final @NonNull LinkProperties newLp,
+ /** Return whether there were any added or removed interface names. */
+ private boolean updateInterfaces(final @NonNull LinkProperties newLp,
final @Nullable LinkProperties oldLp, final int netId,
final @NonNull NetworkAgentInfo nai) {
final CompareResult<String> interfaceDiff = new CompareResult<>(
@@ -9238,6 +9248,7 @@
loge("Exception removing interface: " + e);
}
}
+ return !(interfaceDiff.added.isEmpty() && interfaceDiff.removed.isEmpty());
}
/**
@@ -9319,7 +9330,7 @@
}
private void updateVpnFiltering(@NonNull LinkProperties newLp, @Nullable LinkProperties oldLp,
- @NonNull NetworkAgentInfo nai) {
+ @NonNull NetworkAgentInfo nai, boolean force) {
final String oldIface = getVpnIsolationInterface(nai, nai.networkCapabilities, oldLp);
final String newIface = getVpnIsolationInterface(nai, nai.networkCapabilities, newLp);
final boolean wasFiltering = requiresVpnAllowRule(nai, oldLp, oldIface);
@@ -9330,7 +9341,7 @@
return;
}
- if (Objects.equals(oldIface, newIface) && (wasFiltering == needsFiltering)) {
+ if (!force && Objects.equals(oldIface, newIface) && (wasFiltering == needsFiltering)) {
// Nothing changed.
return;
}
@@ -9347,9 +9358,13 @@
// old rules are being removed.
if (wasFiltering) {
mPermissionMonitor.onVpnUidRangesRemoved(oldIface, ranges, vpnAppUid);
+ mPermissionMonitor.updateVpnLockdownUidInterfaceRules(oldLp.getInterfaceName(), ranges,
+ vpnAppUid, false /* add */);
}
if (needsFiltering) {
mPermissionMonitor.onVpnUidRangesAdded(newIface, ranges, vpnAppUid);
+ mPermissionMonitor.updateVpnLockdownUidInterfaceRules(newLp.getInterfaceName(), ranges,
+ vpnAppUid, true /* add */);
}
}
@@ -9988,9 +10003,15 @@
if (wasFiltering && !prevRanges.isEmpty()) {
mPermissionMonitor.onVpnUidRangesRemoved(oldIface, prevRanges,
prevNc.getOwnerUid());
+ mPermissionMonitor.updateVpnLockdownUidInterfaceRules(
+ nai.linkProperties.getInterfaceName(), prevRanges, prevNc.getOwnerUid(),
+ false /* add */);
}
if (shouldFilter && !newRanges.isEmpty()) {
mPermissionMonitor.onVpnUidRangesAdded(newIface, newRanges, newNc.getOwnerUid());
+ mPermissionMonitor.updateVpnLockdownUidInterfaceRules(
+ nai.linkProperties.getInterfaceName(), newRanges, newNc.getOwnerUid(),
+ true /* add */);
}
} catch (Exception e) {
// Never crash!
diff --git a/service/src/com/android/server/connectivity/PermissionMonitor.java b/service/src/com/android/server/connectivity/PermissionMonitor.java
index beaa174..b5c8f18 100755
--- a/service/src/com/android/server/connectivity/PermissionMonitor.java
+++ b/service/src/com/android/server/connectivity/PermissionMonitor.java
@@ -969,6 +969,7 @@
// packets to that UID is fine.
final Set<Integer> changedUids = intersectUids(rangesToAdd, mAllApps);
removeBypassingUids(changedUids, vpnAppUid);
+ removeVpnLockdownUids(iface, changedUids);
updateVpnUidsInterfaceRules(iface, changedUids, true /* add */);
if (mVpnInterfaceUidRanges.containsKey(iface)) {
mVpnInterfaceUidRanges.get(iface).addAll(rangesToAdd);
@@ -991,6 +992,7 @@
// ranges and update Netd about them.
final Set<Integer> changedUids = intersectUids(rangesToRemove, mAllApps);
removeBypassingUids(changedUids, vpnAppUid);
+ removeVpnLockdownUids(iface, changedUids);
updateVpnUidsInterfaceRules(iface, changedUids, false /* add */);
Set<UidRange> existingRanges = mVpnInterfaceUidRanges.getOrDefault(iface, null);
if (existingRanges == null) {
@@ -1004,6 +1006,27 @@
}
/**
+ * Called when a set of UID ranges are added/removed from an active VPN network and when
+ * UID ranges under VPN Lockdown are updated
+ *
+ * @param iface The VPN network's interface name. Null iface indicates that the interface is not
+ * available.
+ * @param rangesToModify Existing UID ranges to be modified on the VPN network
+ * @param add {@code true} to add the UID rules, {@code false} to remove them.
+ * @param vpnAppUid The uid of the VPN app
+ */
+ public synchronized void updateVpnLockdownUidInterfaceRules(@Nullable String iface,
+ Set<UidRange> rangesToModify, int vpnAppUid, boolean add) {
+ if (iface != null) {
+ Set<Integer> uidsToModify = intersectUids(rangesToModify, mAllApps);
+ removeBypassingUids(uidsToModify, vpnAppUid);
+ Set<Integer> vpnLockdownUids = intersectUids(mVpnLockdownUidRanges.getSet(), mAllApps);
+ uidsToModify.retainAll(vpnLockdownUids);
+ updateVpnUidsInterfaceRules(iface, uidsToModify, add);
+ }
+ }
+
+ /**
* Called when UID ranges under VPN Lockdown are updated
*
* @param add {@code true} if the uids are to be added to the Lockdown, {@code false} if they
@@ -1092,6 +1115,18 @@
}
/**
+ * Remove all apps which are under VPN Lockdown from the list of uids
+ *
+ * @param iface The interface name of the active VPN connection
+ * @param uids The list of uids to operate on
+ */
+ private void removeVpnLockdownUids(@Nullable String iface, Set<Integer> uids) {
+ if (iface == null) {
+ uids.removeAll(intersectUids(mVpnLockdownUidRanges.getSet(), mAllApps));
+ }
+ }
+
+ /**
* Update netd about the list of uids that are under an active VPN connection which they cannot
* bypass.
*