diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 7861bcda64..3418af4672 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -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 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); - 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) if (networkAgent.isVPN()) { Set 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 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 oldDelegateBypassUids, - Set newDelegateBypassUids) { + Set 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! diff --git a/service/src/com/android/server/connectivity/PermissionMonitor.java b/service/src/com/android/server/connectivity/PermissionMonitor.java index 692567865f..fa301d2d23 100755 --- a/service/src/com/android/server/connectivity/PermissionMonitor.java +++ b/service/src/com/android/server/connectivity/PermissionMonitor.java @@ -1084,6 +1084,7 @@ public class PermissionMonitor { // packets to that UID is fine. final Set 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 changedUids = intersectUids(rangesToRemove, mAllApps); removeBypassingUids(changedUids, vpnAppUid, delegatedBypassUids); + removeVpnLockdownUids(iface, changedUids); updateVpnUidsInterfaceRules(iface, changedUids, false /* add */); Set 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 rangesToModify, int vpnAppUid, Set delegatedBypassUids, + boolean add) { + if (iface != null) { + Set uidsToModify = intersectUids(rangesToModify, mAllApps); + removeBypassingUids(uidsToModify, vpnAppUid, delegatedBypassUids); + Set 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 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.