diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index abe1259abf..52f0aa3f25 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -5889,6 +5889,11 @@ public class ConnectivityService extends IConnectivityManager.Stub wakeupModifyInterface(iface, nai, false); } nai.networkMonitor().notifyNetworkDisconnected(); + if (!nai.getCaptivePortalDelegateUids().isEmpty()) { + final Set oldDelegateBypassUids = getAllCaptivePortalDelegateUids(); + nai.clearCaptivePortalDelegateUids(); + updateAllVpnForDelegateUid(oldDelegateBypassUids, getAllCaptivePortalDelegateUids()); + } mNetworkAgentInfos.remove(nai); if (mDeps.isShortNetworkSuspensionEnforced(mContext)) { // Remove the message to disconnect the network if it's in suspended state for too long, @@ -6819,6 +6824,21 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + private Set getAllCaptivePortalDelegateUids() { + final Set uids = new ArraySet<>(); + forEachNetworkAgentInfo(nai -> uids.addAll(nai.getCaptivePortalDelegateUids())); + return uids; + } + + private void updateAllVpnForDelegateUid(Set oldDelegateBypassUids, + Set newDelegateBypassUids) { + forEachNetworkAgentInfo(vpnNai -> { + if (!vpnNai.isVPN()) return; + updateVpnFiltering(vpnNai.linkProperties, vpnNai.linkProperties, vpnNai, + oldDelegateBypassUids, newDelegateBypassUids); + }); + } + public class CaptivePortalImpl extends ICaptivePortal.Stub implements IBinder.DeathRecipient { private final Network mNetwork; // Binder object to track the lifetime of the setDelegateUid caller for cleanup purposes. @@ -6872,7 +6892,22 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + private int updateDelegateUid(@NonNull NetworkAgentInfo nai, int uid) { + if (nai.isCurrentUidDelegate(this, uid)) { + return 0; + } + // TODO: consider using exceptions instead of errnos. + final int errno = nai.removeCaptivePortalDelegateUid(this); + if (errno != 0) return errno; + + // If uid == INVALID_UID, we are done. + if (uid == INVALID_UID) return 0; + + return nai.setCaptivePortalDelegateUid(this, uid); + } + private int handleSetDelegateUid(int uid, @NonNull final IBinder callerBinder) { + ensureRunningOnConnectivityServiceThread(); if (mDelegateUidCaller == null) { mDelegateUidCaller = callerBinder; try { @@ -6884,7 +6919,6 @@ public class ConnectivityService extends IConnectivityManager.Stub return ENOTCONN; } } - final NetworkAgentInfo nai = getNetworkAgentInfoForNetwork(mNetwork); if (nai == null) return ENOENT; // network does not exist anymore. if (nai.isDestroyed()) return ENOENT; // network has already been destroyed. @@ -6892,16 +6926,13 @@ public class ConnectivityService extends IConnectivityManager.Stub // TODO: consider allowing the uid to bypass VPN on all networks before V. if (!mDeps.isAtLeastV()) return EOPNOTSUPP; - // Check whether there has already been a delegate UID configured, if so, perform - // cleanup and disallow bypassing VPN for that UID if no other caller is delegating - // this UID. - // TODO: consider using exceptions instead of errnos. - final int errno = nai.removeCaptivePortalDelegateUid(this); - if (errno != 0) return errno; - - // If uid == INVALID_UID, we are done. - if (uid == INVALID_UID) return 0; - return nai.setCaptivePortalDelegateUid(this, uid); + final Set oldDelegateBypassUids = getAllCaptivePortalDelegateUids(); + int ret = updateDelegateUid(nai, uid); + // updateDelegateUid() updates mCaptivePortalDelegateUids even if it returns non-zero + // value. Therefore, we need to call updateAllVpnForDelegateUid regardless of the + // returned value. + updateAllVpnForDelegateUid(oldDelegateBypassUids, getAllCaptivePortalDelegateUids()); + return ret; } @Override @@ -10189,7 +10220,10 @@ public class ConnectivityService extends IConnectivityManager.Stub // 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); + if (networkAgent.isVPN()) { + Set delegateBypassUids = getAllCaptivePortalDelegateUids(); + updateVpnFiltering(newLp, oldLp, networkAgent, delegateBypassUids, delegateBypassUids); + } updateIngressToVpnAddressFiltering(newLp, oldLp, networkAgent); @@ -10783,7 +10817,8 @@ public class ConnectivityService extends IConnectivityManager.Stub } private void updateVpnFiltering(@NonNull LinkProperties newLp, @Nullable LinkProperties oldLp, - @NonNull NetworkAgentInfo nai) { + @NonNull NetworkAgentInfo nai, Set oldDelegateBypassUids, + Set newDelegateBypassUids) { final String oldIface = getVpnIsolationInterface(nai, nai.networkCapabilities, oldLp); final String newIface = getVpnIsolationInterface(nai, nai.networkCapabilities, newLp); final boolean wasFiltering = requiresVpnAllowRule(nai, oldLp, oldIface); @@ -10794,7 +10829,8 @@ public class ConnectivityService extends IConnectivityManager.Stub return; } - if (Objects.equals(oldIface, newIface) && (wasFiltering == needsFiltering)) { + if (Objects.equals(oldIface, newIface) && (wasFiltering == needsFiltering) + && oldDelegateBypassUids.equals(newDelegateBypassUids)) { // Nothing changed. return; } @@ -10810,10 +10846,12 @@ public class ConnectivityService extends IConnectivityManager.Stub // make eBPF support two allowlisted interfaces so here new rules can be added before the // old rules are being removed. if (wasFiltering) { - mPermissionMonitor.onVpnUidRangesRemoved(oldIface, ranges, vpnAppUid); + mPermissionMonitor.onVpnUidRangesRemoved(oldIface, ranges, vpnAppUid, + oldDelegateBypassUids); } if (needsFiltering) { - mPermissionMonitor.onVpnUidRangesAdded(newIface, ranges, vpnAppUid); + mPermissionMonitor.onVpnUidRangesAdded(newIface, ranges, vpnAppUid, + newDelegateBypassUids); } } @@ -11565,10 +11603,11 @@ public class ConnectivityService extends IConnectivityManager.Stub // to be removed will never overlap with the new range to be added. if (wasFiltering && !prevRanges.isEmpty()) { mPermissionMonitor.onVpnUidRangesRemoved(oldIface, prevRanges, - prevNc.getOwnerUid()); + prevNc.getOwnerUid(), getAllCaptivePortalDelegateUids()); } if (shouldFilter && !newRanges.isEmpty()) { - mPermissionMonitor.onVpnUidRangesAdded(newIface, newRanges, newNc.getOwnerUid()); + mPermissionMonitor.onVpnUidRangesAdded(newIface, newRanges, newNc.getOwnerUid(), + getAllCaptivePortalDelegateUids()); } } catch (Exception e) { // Never crash! diff --git a/service/src/com/android/server/connectivity/NetworkAgentInfo.java b/service/src/com/android/server/connectivity/NetworkAgentInfo.java index 253ff9ed81..329f03c374 100644 --- a/service/src/com/android/server/connectivity/NetworkAgentInfo.java +++ b/service/src/com/android/server/connectivity/NetworkAgentInfo.java @@ -25,6 +25,7 @@ import static android.net.NetworkCapabilities.TRANSPORT_ETHERNET; import static android.net.NetworkCapabilities.TRANSPORT_TEST; import static android.net.NetworkCapabilities.TRANSPORT_WIFI; import static android.net.NetworkCapabilities.transportNamesOf; +import static android.os.Process.INVALID_UID; import static android.system.OsConstants.EEXIST; import static android.system.OsConstants.EIO; import static android.system.OsConstants.ENOENT; @@ -93,6 +94,7 @@ import java.util.Arrays; import java.util.List; import java.util.NoSuchElementException; import java.util.Objects; +import java.util.Set; import java.util.SortedSet; import java.util.TreeSet; import java.util.function.Consumer; @@ -1653,6 +1655,27 @@ public class NetworkAgentInfo implements NetworkRanker.Scoreable { } } + /** + * Get the delegate UIDs of the apps that are allowed to perform network traffic for captive. + */ + public Set getCaptivePortalDelegateUids() { + return new ArraySet<>(mCaptivePortalDelegateUids.values()); + } + + /** + * Clear all the delegate UIDs. + */ + public void clearCaptivePortalDelegateUids() { + mCaptivePortalDelegateUids.clear(); + } + + /** + * Check if the given uid is the delegate uid of the given caller. + */ + public boolean isCurrentUidDelegate(@NonNull CaptivePortalImpl caller, int uid) { + return uid == mCaptivePortalDelegateUids.getOrDefault(caller, INVALID_UID); + } + /** * Set the delegate UID of the app that is allowed to perform network traffic for captive * portal login, and configure the netd bypass rule with this delegated UID. diff --git a/service/src/com/android/server/connectivity/PermissionMonitor.java b/service/src/com/android/server/connectivity/PermissionMonitor.java index d4d947d75f..692567865f 100755 --- a/service/src/com/android/server/connectivity/PermissionMonitor.java +++ b/service/src/com/android/server/connectivity/PermissionMonitor.java @@ -1076,14 +1076,14 @@ public class PermissionMonitor { * @param vpnAppUid The uid of the VPN app */ public synchronized void onVpnUidRangesAdded(@Nullable String iface, Set rangesToAdd, - int vpnAppUid) { + int vpnAppUid, Set delegatedBypassUids) { // Calculate the list of new app uids under the VPN due to the new UID ranges and update // Netd about them. Because mAllApps only contains appIds instead of uids, the result might // be an overestimation if an app is not installed on the user on which the VPN is running, // but that's safe: if an app is not installed, it cannot receive any packets, so dropping // packets to that UID is fine. final Set changedUids = intersectUids(rangesToAdd, mAllApps); - removeBypassingUids(changedUids, vpnAppUid); + removeBypassingUids(changedUids, vpnAppUid, delegatedBypassUids); updateVpnUidsInterfaceRules(iface, changedUids, true /* add */); if (mVpnInterfaceUidRanges.containsKey(iface)) { mVpnInterfaceUidRanges.get(iface).addAll(rangesToAdd); @@ -1101,11 +1101,11 @@ public class PermissionMonitor { * @param vpnAppUid The uid of the VPN app */ public synchronized void onVpnUidRangesRemoved(@Nullable String iface, - Set rangesToRemove, int vpnAppUid) { + Set rangesToRemove, int vpnAppUid, Set delegatedBypassUids) { // Calculate the list of app uids that are no longer under the VPN due to the removed UID // ranges and update Netd about them. final Set changedUids = intersectUids(rangesToRemove, mAllApps); - removeBypassingUids(changedUids, vpnAppUid); + removeBypassingUids(changedUids, vpnAppUid, delegatedBypassUids); updateVpnUidsInterfaceRules(iface, changedUids, false /* add */); Set existingRanges = mVpnInterfaceUidRanges.getOrDefault(iface, null); if (existingRanges == null) { @@ -1201,8 +1201,10 @@ public class PermissionMonitor { * @param uids The list of uids to operate on * @param vpnAppUid The uid of the VPN app */ - private void removeBypassingUids(Set uids, int vpnAppUid) { + private void removeBypassingUids(Set uids, int vpnAppUid, + Set delegateBypassUids) { uids.remove(vpnAppUid); + uids.removeAll(delegateBypassUids); uids.removeIf(this::hasRestrictedNetworksPermission); } diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index fdfcc51337..25b564a138 100644 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -5072,68 +5072,142 @@ public class ConnectivityServiceTest { @Test @IgnoreUpTo(Build.VERSION_CODES.UPSIDE_DOWN_CAKE) - public void testCaptivePortalApp_SetDelegateUid() throws Exception { - final TestNetworkCallback captivePortalCallback = new TestNetworkCallback(); - final NetworkRequest captivePortalRequest = new NetworkRequest.Builder() - .addCapability(NET_CAPABILITY_CAPTIVE_PORTAL).build(); - mCm.registerNetworkCallback(captivePortalRequest, captivePortalCallback); + public void testCaptivePortalApp_SetDelegateUidWithVpn() throws Exception { + LinkProperties lp = new LinkProperties(); + InOrder inOrder = inOrder(mMockNetd, mBpfNetMaps); + lp.setInterfaceName("tun0"); + lp.addRoute(new RouteInfo(new IpPrefix(Inet4Address.ANY, 0), null)); + 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 vpnRange = Collections.singleton(PRIMARY_UIDRANGE); + mMockVpn.establish(lp, VPN_UID, vpnRange); + assertVpnUidRangesUpdated(true, vpnRange, VPN_UID); + // A connected VPN should have interface rules set up. There are two expected invocations, + // one during the VPN initial connection, one during the VPN LinkProperties update. + ArgumentCaptor uidCaptor = ArgumentCaptor.forClass(int[].class); + inOrder.verify(mBpfNetMaps, times(2)).addUidInterfaceRules(eq("tun0"), uidCaptor.capture()); + assertContainsExactly(uidCaptor.getAllValues().get(0), APP1_UID, APP2_UID); + assertContainsExactly(uidCaptor.getAllValues().get(1), APP1_UID, APP2_UID); + + // bring up Wi-Fi with captive portal + final TestNetworkCallback wifiCaptivePortalCallback = new TestNetworkCallback(); + mCm.registerNetworkCallback( + new NetworkRequest.Builder() + .addCapability(NET_CAPABILITY_CAPTIVE_PORTAL) + .addTransportType(TRANSPORT_WIFI) + .build(), + wifiCaptivePortalCallback); mWiFiAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI); mWiFiAgent.connectWithCaptivePortal(TEST_REDIRECT_URL, false); - captivePortalCallback.expectAvailableCallbacksUnvalidated(mWiFiAgent); - - final Intent signInIntent = startCaptivePortalApp(mWiFiAgent); - final CaptivePortal captivePortal = signInIntent + wifiCaptivePortalCallback.expectAvailableCallbacksUnvalidated(mWiFiAgent); + mCm.unregisterNetworkCallback(wifiCaptivePortalCallback); + final Intent wifiSignInIntent = startCaptivePortalApp(mWiFiAgent); + final CaptivePortal wificaptivePortal = wifiSignInIntent .getParcelableExtra(ConnectivityManager.EXTRA_CAPTIVE_PORTAL); - InOrder inOrder = inOrder(mMockNetd); + // bring up ethernet with captive portal + final TestNetworkCallback ethernetCaptivePortalCallback = new TestNetworkCallback(); + mCm.registerNetworkCallback( + new NetworkRequest.Builder() + .addCapability(NET_CAPABILITY_CAPTIVE_PORTAL) + .addTransportType(TRANSPORT_ETHERNET) + .build(), + ethernetCaptivePortalCallback); + mEthernetAgent = new TestNetworkAgentWrapper(TRANSPORT_ETHERNET); + mEthernetAgent.connectWithCaptivePortal(TEST_REDIRECT_URL, false); + ethernetCaptivePortalCallback.expectAvailableCallbacksUnvalidated(mEthernetAgent); + mCm.unregisterNetworkCallback(ethernetCaptivePortalCallback); + final Intent ethernetSignInIntent = startCaptivePortalApp(mEthernetAgent); + final CaptivePortal ethernetCaptivePortal = ethernetSignInIntent + .getParcelableExtra(ConnectivityManager.EXTRA_CAPTIVE_PORTAL); - // Add the UID and check that it's added to the bypass list. - FakeOutcomeReceiver or = new FakeOutcomeReceiver(); - captivePortal.setDelegateUid(APP2_UID, r -> r.run(), or); + // Set delegate UID + FakeOutcomeReceiver or = new FakeOutcomeReceiver<>(); + wificaptivePortal.setDelegateUid(APP1_UID, Runnable::run, or); or.awaitOutcome(); - inOrder.verify(mMockNetd).networkAllowBypassVpnOnNetwork(true, APP2_UID, - mWiFiAgent.getNetwork().netId); - - // Remove the UID and check that it's removed from the list. - or = new FakeOutcomeReceiver(); - captivePortal.setDelegateUid(Process.INVALID_UID, r -> r.run(), or); - or.awaitOutcome(); - inOrder.verify(mMockNetd).networkAllowBypassVpnOnNetwork(false, APP2_UID, - mWiFiAgent.getNetwork().netId); - - // Add the UID again. - or = new FakeOutcomeReceiver(); - captivePortal.setDelegateUid(APP2_UID, r -> r.run(), or); - or.awaitOutcome(); - inOrder.verify(mMockNetd).networkAllowBypassVpnOnNetwork(true, APP2_UID, - mWiFiAgent.getNetwork().netId); - - // Add the UID again. Nothing should change. - or = new FakeOutcomeReceiver(); - captivePortal.setDelegateUid(APP2_UID, r -> r.run(), or); - or.awaitOutcome(); - // BUG: the code should not call into netd. - inOrder.verify(mMockNetd).networkAllowBypassVpnOnNetwork(false, APP2_UID, - mWiFiAgent.getNetwork().netId); - inOrder.verify(mMockNetd).networkAllowBypassVpnOnNetwork(true, APP2_UID, - mWiFiAgent.getNetwork().netId); - - // Add another UID again. The old UID should be removed and the new one added. - or = new FakeOutcomeReceiver(); - captivePortal.setDelegateUid(APP1_UID, r -> r.run(), or); - or.awaitOutcome(); - inOrder.verify(mMockNetd).networkAllowBypassVpnOnNetwork(false, APP2_UID, - mWiFiAgent.getNetwork().netId); inOrder.verify(mMockNetd).networkAllowBypassVpnOnNetwork(true, APP1_UID, mWiFiAgent.getNetwork().netId); + uidCaptor = ArgumentCaptor.forClass(int[].class); + inOrder.verify(mBpfNetMaps, times(1)).removeUidInterfaceRules(uidCaptor.capture()); + assertContainsExactly(uidCaptor.getValue(), APP1_UID, APP2_UID); + uidCaptor = ArgumentCaptor.forClass(int[].class); + inOrder.verify(mBpfNetMaps, times(1)).addUidInterfaceRules(eq("tun0"), uidCaptor.capture()); + assertContainsExactly(uidCaptor.getValue(), APP2_UID); + // Add and remove delegate UID from another client for same UID should not trigger any VPN + // rule update. + or = new FakeOutcomeReceiver<>(); + ethernetCaptivePortal.setDelegateUid(APP1_UID, Runnable::run, or); + or.awaitOutcome(); + inOrder.verify(mMockNetd).networkAllowBypassVpnOnNetwork(true, APP1_UID, + mEthernetAgent.getNetwork().netId); + inOrder.verify(mBpfNetMaps, never()).addUidInterfaceRules(any(), any()); + inOrder.verify(mBpfNetMaps, never()).removeUidInterfaceRules(any()); + + or = new FakeOutcomeReceiver<>(); + ethernetCaptivePortal.setDelegateUid(INVALID_UID, Runnable::run, or); + or.awaitOutcome(); + inOrder.verify(mMockNetd).networkAllowBypassVpnOnNetwork(false, APP1_UID, + mEthernetAgent.getNetwork().netId); + inOrder.verify(mBpfNetMaps, never()).addUidInterfaceRules(any(), any()); + inOrder.verify(mBpfNetMaps, never()).removeUidInterfaceRules(any()); + + // Disconnect VPN + mMockVpn.disconnect(); + waitForIdle(); + uidCaptor = ArgumentCaptor.forClass(int[].class); + inOrder.verify(mBpfNetMaps, times(1)).removeUidInterfaceRules(uidCaptor.capture()); + assertContainsExactly(uidCaptor.getValue(), APP2_UID); + + // Reconnect VPN + mMockVpn.establish(lp, VPN_UID, vpnRange); + assertVpnUidRangesUpdated(true, vpnRange, VPN_UID); + uidCaptor = ArgumentCaptor.forClass(int[].class); + inOrder.verify(mBpfNetMaps, times(2)).addUidInterfaceRules(eq("tun0"), uidCaptor.capture()); + assertContainsExactly(uidCaptor.getAllValues().get(0), APP2_UID); + assertContainsExactly(uidCaptor.getAllValues().get(1), APP2_UID); + + // Remove delegate UID + or = new FakeOutcomeReceiver<>(); + wificaptivePortal.setDelegateUid(Process.INVALID_UID, Runnable::run, or); + or.awaitOutcome(); + inOrder.verify(mMockNetd).networkAllowBypassVpnOnNetwork(false, APP1_UID, + mWiFiAgent.getNetwork().netId); + inOrder.verify(mBpfNetMaps, times(1)).removeUidInterfaceRules(uidCaptor.capture()); + assertContainsExactly(uidCaptor.getValue(), APP2_UID); + uidCaptor = ArgumentCaptor.forClass(int[].class); + inOrder.verify(mBpfNetMaps, times(1)).addUidInterfaceRules(eq("tun0"), uidCaptor.capture()); + assertContainsExactly(uidCaptor.getValue(), APP1_UID, APP2_UID); + + // Add back delegate UID + or = new FakeOutcomeReceiver<>(); + wificaptivePortal.setDelegateUid(APP1_UID, Runnable::run, or); + or.awaitOutcome(); + inOrder.verify(mMockNetd).networkAllowBypassVpnOnNetwork(true, APP1_UID, + mWiFiAgent.getNetwork().netId); + uidCaptor = ArgumentCaptor.forClass(int[].class); + inOrder.verify(mBpfNetMaps, times(1)).removeUidInterfaceRules(uidCaptor.capture()); + assertContainsExactly(uidCaptor.getValue(), APP1_UID, APP2_UID); + uidCaptor = ArgumentCaptor.forClass(int[].class); + inOrder.verify(mBpfNetMaps, times(1)).addUidInterfaceRules(eq("tun0"), uidCaptor.capture()); + assertContainsExactly(uidCaptor.getValue(), APP2_UID); + + // Wi-Fi network go away should trigger VPN rule being cleaned up. mWiFiAgent.disconnect(); + waitForIdle(); + uidCaptor = ArgumentCaptor.forClass(int[].class); + inOrder.verify(mBpfNetMaps, times(1)).removeUidInterfaceRules(uidCaptor.capture()); + assertContainsExactly(uidCaptor.getValue(), APP2_UID); + uidCaptor = ArgumentCaptor.forClass(int[].class); + inOrder.verify(mBpfNetMaps, times(1)).addUidInterfaceRules(eq("tun0"), uidCaptor.capture()); + assertContainsExactly(uidCaptor.getValue(), APP1_UID, APP2_UID); - // TODO: check VPN interface filtering rules, see - // testFullyRoutedVpnResultsInInterfaceFilteringRules + mMockVpn.disconnect(); + waitForIdle(); } + @Test public void testAvoidOrIgnoreCaptivePortals() throws Exception { final TestNetworkCallback captivePortalCallback = new TestNetworkCallback(); diff --git a/tests/unit/java/com/android/server/connectivity/PermissionMonitorTest.java b/tests/unit/java/com/android/server/connectivity/PermissionMonitorTest.java index a0ead9afbc..83269bdb05 100644 --- a/tests/unit/java/com/android/server/connectivity/PermissionMonitorTest.java +++ b/tests/unit/java/com/android/server/connectivity/PermissionMonitorTest.java @@ -949,7 +949,7 @@ public class PermissionMonitorTest { final Set vpnRange2 = Set.of(new UidRange(MOCK_UID12, MOCK_UID12)); // When VPN is connected, expect a rule to be set up for user app MOCK_UID11 - mPermissionMonitor.onVpnUidRangesAdded(ifName, vpnRange1, VPN_UID); + mPermissionMonitor.onVpnUidRangesAdded(ifName, vpnRange1, VPN_UID, Set.of()); verify(mBpfNetMaps).addUidInterfaceRules(eq(ifName), aryEq(new int[]{MOCK_UID11})); reset(mBpfNetMaps); @@ -964,15 +964,15 @@ public class PermissionMonitorTest { // During VPN uid update (vpnRange1 -> vpnRange2), ConnectivityService first deletes the // old UID rules then adds the new ones. Expect netd to be updated - mPermissionMonitor.onVpnUidRangesRemoved(ifName, vpnRange1, VPN_UID); + mPermissionMonitor.onVpnUidRangesRemoved(ifName, vpnRange1, VPN_UID, Set.of()); verify(mBpfNetMaps).removeUidInterfaceRules(aryEq(new int[] {MOCK_UID11})); - mPermissionMonitor.onVpnUidRangesAdded(ifName, vpnRange2, VPN_UID); + mPermissionMonitor.onVpnUidRangesAdded(ifName, vpnRange2, VPN_UID, Set.of()); verify(mBpfNetMaps).addUidInterfaceRules(eq(ifName), aryEq(new int[]{MOCK_UID12})); reset(mBpfNetMaps); // When VPN is disconnected, expect rules to be torn down - mPermissionMonitor.onVpnUidRangesRemoved(ifName, vpnRange2, VPN_UID); + mPermissionMonitor.onVpnUidRangesRemoved(ifName, vpnRange2, VPN_UID, Set.of()); verify(mBpfNetMaps).removeUidInterfaceRules(aryEq(new int[] {MOCK_UID12})); } @@ -1000,7 +1000,7 @@ public class PermissionMonitorTest { onUserAddedWithInstalledPackageList(MOCK_USER2, pkgs); final Set vpnRange = Set.of(UidRange.createForUser(MOCK_USER1), UidRange.createForUser(MOCK_USER2)); - mPermissionMonitor.onVpnUidRangesAdded(ifName, vpnRange, VPN_UID); + mPermissionMonitor.onVpnUidRangesAdded(ifName, vpnRange, VPN_UID, Set.of()); // Newly-installed package should have uid rules added addPackageForUsers(new UserHandle[]{MOCK_USER1, MOCK_USER2}, MOCK_PACKAGE1, MOCK_APPID1); @@ -1025,6 +1025,36 @@ public class PermissionMonitorTest { doTestUidFilteringDuringPackageInstallAndUninstall(null /* ifName */); } + @Test + public void testUidFilteringWithDelegatedBypassUids() throws Exception { + final String ifName = "tun0"; + final int delegatedUid = MOCK_UID12; + final List pkgs = List.of( + buildPackageInfo(SYSTEM_PACKAGE1, SYSTEM_APP_UID11, CHANGE_NETWORK_STATE, + CONNECTIVITY_USE_RESTRICTED_NETWORKS), + buildPackageInfo(MOCK_PACKAGE1, MOCK_UID11), + buildPackageInfo(MOCK_PACKAGE2, delegatedUid), + buildPackageInfo(SYSTEM_PACKAGE2, VPN_UID)); + initialize(); + onUserAddedWithInstalledPackageList(MOCK_USER1, pkgs); + + final Set vpnRange = Set.of(UidRange.createForUser(MOCK_USER1)); + + // When VPN is connected, expect a rule to be set up for user app MOCK_UID11, but not for + // delegatedUid. + final Set delegatedUids = Set.of(delegatedUid); + mPermissionMonitor.onVpnUidRangesAdded(ifName, vpnRange, VPN_UID, delegatedUids); + verify(mBpfNetMaps).addUidInterfaceRules(eq(ifName), aryEq(new int[]{MOCK_UID11})); + verify(mBpfNetMaps, never()).addUidInterfaceRules(eq(ifName), + aryEq(new int[]{delegatedUid})); + + // When VPN is disconnected, expect rules to be torn down for MOCK_UID11 and still no rule + // for delegatedUid. + mPermissionMonitor.onVpnUidRangesRemoved(ifName, vpnRange, VPN_UID, delegatedUids); + verify(mBpfNetMaps).removeUidInterfaceRules(aryEq(new int[]{MOCK_UID11})); + verify(mBpfNetMaps, never()).removeUidInterfaceRules(aryEq(new int[]{delegatedUid})); + } + @Test @EnableCompatChanges(RESTRICT_LOCAL_NETWORK) public void testLockdownUidFilteringWithLockdownEnableDisable() { diff --git a/tests/unit/java/com/android/server/connectivityservice/CSCaptivePortalAppTest.kt b/tests/unit/java/com/android/server/connectivityservice/CSCaptivePortalAppTest.kt index b66fcc9dac..74555ccf61 100644 --- a/tests/unit/java/com/android/server/connectivityservice/CSCaptivePortalAppTest.kt +++ b/tests/unit/java/com/android/server/connectivityservice/CSCaptivePortalAppTest.kt @@ -30,17 +30,30 @@ import android.net.NetworkRequest import android.net.NetworkStack import android.os.Build import android.os.Bundle +import android.os.ConditionVariable +import android.os.OutcomeReceiver +import android.os.ServiceSpecificException import androidx.test.filters.SmallTest +import com.android.testutils.DevSdkIgnoreRule import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo import com.android.testutils.DevSdkIgnoreRunner import com.android.testutils.TestableNetworkCallback +import kotlin.test.DefaultAsserter.assertTrue import kotlin.test.assertEquals +import kotlin.test.assertTrue +import kotlin.test.fail +import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.mockito.ArgumentMatchers.anyInt +import org.mockito.Mockito.inOrder import org.mockito.Mockito.never import org.mockito.Mockito.verify +private const val APP1_UID = 10001 +private const val APP2_UID = 10002 +private const val TIMEOUT_MS = 2_000L + @DevSdkIgnoreRunner.MonitorThreadLeak @RunWith(DevSdkIgnoreRunner::class) @SmallTest @@ -48,7 +61,113 @@ import org.mockito.Mockito.verify class CSCaptivePortalAppTest : CSTest() { private val WIFI_IFACE = "wifi0" private val TEST_REDIRECT_URL = "http://example.com/firstPath" - private val TIMEOUT_MS = 2_000L + + @get:Rule val ignoreRule = DevSdkIgnoreRule() + + private class FakeOutcomeReceiver : OutcomeReceiver { + private val mCv = ConditionVariable() + private var mError: E? = null + + override fun onResult(result: R) { + mCv.open() + } + + override fun onError(error: E) { + mError = error + mCv.open() + } + + fun awaitOutcome() { + assertTrue( + "OutcomeReceiver did not receive outcome after $TIMEOUT_MS ms", + mCv.block(TIMEOUT_MS) + ) + if (mError != null) { + fail("OutcomeReceiver got: " + mError!!.message) + } + } + } + + /** + * Helper extension function to reduce boilerplate in the test. + */ + private fun CaptivePortal.setDelegateUidAndAwait(uid: Int) { + val or = FakeOutcomeReceiver() + this.setDelegateUid(uid, CSTestExecutor, or) + or.awaitOutcome() + } + + @Test + @IgnoreUpTo(Build.VERSION_CODES.UPSIDE_DOWN_CAKE) + fun testCaptivePortalApp_SetDelegateUid() { + val captivePortalCallback = TestableNetworkCallback() + val captivePortalRequest = NetworkRequest.Builder() + .addCapability(NET_CAPABILITY_CAPTIVE_PORTAL).build() + cm.registerNetworkCallback(captivePortalRequest, captivePortalCallback) + val wifiAgent = Agent(WIFI_IFACE, TRANSPORT_WIFI, NET_CAPABILITY_INTERNET) + wifiAgent.connectWithCaptivePortal(TEST_REDIRECT_URL) + captivePortalCallback.expectAvailableCallbacksUnvalidated(wifiAgent) + + val signInIntent = startCaptivePortalApp(wifiAgent) + val captivePortal = signInIntent.getParcelableExtra( + EXTRA_CAPTIVE_PORTAL, + CaptivePortal::class.java + )!! + + val inOrder = inOrder(netd) + + // Add the UID and check that it's added to the bypass list. + captivePortal.setDelegateUidAndAwait(APP2_UID) + inOrder.verify(netd).networkAllowBypassVpnOnNetwork( + true /* allow */, + APP2_UID, + wifiAgent.network.netId + ) + + // Remove the UID and check that it's removed from the list. + captivePortal.setDelegateUidAndAwait(android.os.Process.INVALID_UID) + inOrder.verify(netd).networkAllowBypassVpnOnNetwork( + false /* allow */, + APP2_UID, + wifiAgent.network.netId + ) + + // Add the UID again. + captivePortal.setDelegateUidAndAwait(APP2_UID) + inOrder.verify(netd).networkAllowBypassVpnOnNetwork( + true /* allow */, + APP2_UID, + wifiAgent.network.netId + ) + + // Add the UID again. Nothing should change. + captivePortal.setDelegateUidAndAwait(APP2_UID) + inOrder.verify(netd, never()).networkAllowBypassVpnOnNetwork( + false /* allow */, + APP2_UID, + wifiAgent.network.netId + ) + inOrder.verify(netd, never()).networkAllowBypassVpnOnNetwork( + true /* allow */, + APP2_UID, + wifiAgent.network.netId + ) + + // Add another UID again. The old UID should be removed and the new one added. + captivePortal.setDelegateUidAndAwait(APP1_UID) + inOrder.verify(netd).networkAllowBypassVpnOnNetwork( + false /* allow */, + APP2_UID, + wifiAgent.network.netId + ) + inOrder.verify(netd).networkAllowBypassVpnOnNetwork( + true /* allow */, + APP1_UID, + wifiAgent.network.netId + ) + + wifiAgent.disconnect() + } @Test fun testCaptivePortalApp_Reevaluate_Nopermission() {