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
This commit is contained in:
Oliver Scott
parent
8e92e094e9
commit
0e19223594
2 changed files with 66 additions and 6 deletions
|
|
@ -7270,7 +7270,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
|||
forEachNetworkAgentInfo(vpnNai -> {
|
||||
if (!vpnNai.isVPN()) return;
|
||||
updateVpnFiltering(vpnNai.linkProperties, vpnNai.linkProperties, vpnNai,
|
||||
oldDelegateBypassUids, newDelegateBypassUids);
|
||||
oldDelegateBypassUids, newDelegateBypassUids, false);
|
||||
});
|
||||
}
|
||||
|
||||
|
|
@ -8470,6 +8470,12 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
|||
final boolean curMetered = nai.networkCapabilities.isMetered();
|
||||
maybeNotifyNetworkBlocked(nai, curMetered, curMetered,
|
||||
mVpnBlockedUidRanges, newVpnBlockedUidRanges);
|
||||
|
||||
if (nai.isVPN()) {
|
||||
Set<Integer> delegateBypassUids = getAllCaptivePortalDelegateUids();
|
||||
updateVpnFiltering(nai.linkProperties, nai.linkProperties, nai, delegateBypassUids,
|
||||
delegateBypassUids, true /* force */);
|
||||
}
|
||||
});
|
||||
|
||||
mVpnBlockedUidRanges = newVpnBlockedUidRanges;
|
||||
|
|
@ -10708,13 +10714,19 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
|||
// the LinkProperties for the network are accurate.
|
||||
networkAgent.clatd.fixupLinkProperties(oldLp, newLp);
|
||||
|
||||
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)
|
||||
if (networkAgent.isVPN()) {
|
||||
Set<Integer> delegateBypassUids = getAllCaptivePortalDelegateUids();
|
||||
updateVpnFiltering(newLp, oldLp, networkAgent, delegateBypassUids, delegateBypassUids);
|
||||
updateVpnFiltering(newLp, oldLp, networkAgent, delegateBypassUids, delegateBypassUids,
|
||||
shouldForceUpdateVpnFiltering);
|
||||
}
|
||||
|
||||
updateIngressToVpnAddressFiltering(newLp, oldLp, networkAgent);
|
||||
|
|
@ -10961,7 +10973,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
|||
}
|
||||
}
|
||||
|
||||
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<>(
|
||||
|
|
@ -10995,6 +11008,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
|||
loge("Exception removing interface: " + e);
|
||||
}
|
||||
}
|
||||
return !(interfaceDiff.added.isEmpty() && interfaceDiff.removed.isEmpty());
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -11310,7 +11324,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
|||
|
||||
private void updateVpnFiltering(@NonNull LinkProperties newLp, @Nullable LinkProperties oldLp,
|
||||
@NonNull NetworkAgentInfo nai, Set<Integer> oldDelegateBypassUids,
|
||||
Set<Integer> newDelegateBypassUids) {
|
||||
Set<Integer> newDelegateBypassUids, 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);
|
||||
|
|
@ -11321,7 +11335,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
|||
return;
|
||||
}
|
||||
|
||||
if (Objects.equals(oldIface, newIface) && (wasFiltering == needsFiltering)
|
||||
if (!force && Objects.equals(oldIface, newIface) && (wasFiltering == needsFiltering)
|
||||
&& oldDelegateBypassUids.equals(newDelegateBypassUids)) {
|
||||
// Nothing changed.
|
||||
return;
|
||||
|
|
@ -11340,10 +11354,14 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
|||
if (wasFiltering) {
|
||||
mPermissionMonitor.onVpnUidRangesRemoved(oldIface, ranges, vpnAppUid,
|
||||
oldDelegateBypassUids);
|
||||
mPermissionMonitor.updateVpnLockdownUidInterfaceRules(oldLp.getInterfaceName(), ranges,
|
||||
vpnAppUid, oldDelegateBypassUids, false /* add */);
|
||||
}
|
||||
if (needsFiltering) {
|
||||
mPermissionMonitor.onVpnUidRangesAdded(newIface, ranges, vpnAppUid,
|
||||
newDelegateBypassUids);
|
||||
mPermissionMonitor.updateVpnLockdownUidInterfaceRules(newLp.getInterfaceName(), ranges,
|
||||
vpnAppUid, newDelegateBypassUids, true /* add */);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -12099,10 +12117,16 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
|||
if (wasFiltering && !prevRanges.isEmpty()) {
|
||||
mPermissionMonitor.onVpnUidRangesRemoved(oldIface, prevRanges,
|
||||
prevNc.getOwnerUid(), getAllCaptivePortalDelegateUids());
|
||||
mPermissionMonitor.updateVpnLockdownUidInterfaceRules(
|
||||
nai.linkProperties.getInterfaceName(), prevRanges, prevNc.getOwnerUid(),
|
||||
getAllCaptivePortalDelegateUids(), false /* add */);
|
||||
}
|
||||
if (shouldFilter && !newRanges.isEmpty()) {
|
||||
mPermissionMonitor.onVpnUidRangesAdded(newIface, newRanges, newNc.getOwnerUid(),
|
||||
getAllCaptivePortalDelegateUids());
|
||||
mPermissionMonitor.updateVpnLockdownUidInterfaceRules(
|
||||
nai.linkProperties.getInterfaceName(), newRanges, newNc.getOwnerUid(),
|
||||
getAllCaptivePortalDelegateUids(), true /* add */);
|
||||
}
|
||||
} catch (Exception e) {
|
||||
// Never crash!
|
||||
|
|
|
|||
|
|
@ -1084,6 +1084,7 @@ public class PermissionMonitor {
|
|||
// packets to that UID is fine.
|
||||
final Set<Integer> changedUids = intersectUids(rangesToAdd, mAllApps);
|
||||
removeBypassingUids(changedUids, vpnAppUid, delegatedBypassUids);
|
||||
removeVpnLockdownUids(iface, changedUids);
|
||||
updateVpnUidsInterfaceRules(iface, changedUids, true /* add */);
|
||||
if (mVpnInterfaceUidRanges.containsKey(iface)) {
|
||||
mVpnInterfaceUidRanges.get(iface).addAll(rangesToAdd);
|
||||
|
|
@ -1106,6 +1107,7 @@ public class PermissionMonitor {
|
|||
// ranges and update Netd about them.
|
||||
final Set<Integer> changedUids = intersectUids(rangesToRemove, mAllApps);
|
||||
removeBypassingUids(changedUids, vpnAppUid, delegatedBypassUids);
|
||||
removeVpnLockdownUids(iface, changedUids);
|
||||
updateVpnUidsInterfaceRules(iface, changedUids, false /* add */);
|
||||
Set<UidRange> existingRanges = mVpnInterfaceUidRanges.getOrDefault(iface, null);
|
||||
if (existingRanges == null) {
|
||||
|
|
@ -1118,6 +1120,28 @@ public class PermissionMonitor {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 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, Set<Integer> delegatedBypassUids,
|
||||
boolean add) {
|
||||
if (iface != null) {
|
||||
Set<Integer> uidsToModify = intersectUids(rangesToModify, mAllApps);
|
||||
removeBypassingUids(uidsToModify, vpnAppUid, delegatedBypassUids);
|
||||
Set<Integer> vpnLockdownUids = intersectUids(mVpnLockdownUidRanges.getSet(), mAllApps);
|
||||
uidsToModify.retainAll(vpnLockdownUids);
|
||||
updateVpnUidsInterfaceRules(iface, uidsToModify, add);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Called when UID ranges under VPN Lockdown are updated
|
||||
*
|
||||
|
|
@ -1208,6 +1232,18 @@ public class PermissionMonitor {
|
|||
uids.removeIf(this::hasRestrictedNetworksPermission);
|
||||
}
|
||||
|
||||
/**
|
||||
* 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.
|
||||
|
|
|
|||
Loading…
Reference in a new issue