From 7daab8cc60bad9b612614016fc5d7a9463056c01 Mon Sep 17 00:00:00 2001 From: Yuyang Huang Date: Sat, 26 Jul 2025 15:13:29 +0900 Subject: [PATCH] CCT VPN bypass for Android U and below Implements logic to allow CCT to bypass VPN on Android U (API 34) and older. On these Android versions, netd's VPN bypass APIs (networkSetProtectAllow/Deny) operate globally. Test: TH Flag: NONE legacy flag protected Change-Id: I3650c3823d65504046d74cd60485d9cc890937d8 --- .../android/server/ConnectivityService.java | 39 +++++++-- .../server/connectivity/NetworkAgentInfo.java | 14 ++++ .../server/ConnectivityServiceTest.java | 44 +++++++--- .../CSCaptivePortalAppTest.kt | 84 ++++++++++++++++--- .../CSNetdBypassRulesTests.kt | 8 +- 5 files changed, 157 insertions(+), 32 deletions(-) diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 52f0aa3f25..1fe49d8020 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -128,9 +128,9 @@ import static android.net.connectivity.ConnectivityCompatChanges.ENABLE_SELF_CER import static android.net.connectivity.ConnectivityCompatChanges.NETWORK_BLOCKED_WITHOUT_INTERNET_PERMISSION; import static android.os.Process.INVALID_UID; import static android.os.Process.VPN_UID; +import static android.system.OsConstants.EIO; import static android.system.OsConstants.ENOENT; import static android.system.OsConstants.ENOTCONN; -import static android.system.OsConstants.EOPNOTSUPP; import static android.system.OsConstants.ETH_P_ALL; import static android.system.OsConstants.IPPROTO_TCP; import static android.system.OsConstants.IPPROTO_UDP; @@ -5892,7 +5892,11 @@ public class ConnectivityService extends IConnectivityManager.Stub if (!nai.getCaptivePortalDelegateUids().isEmpty()) { final Set oldDelegateBypassUids = getAllCaptivePortalDelegateUids(); nai.clearCaptivePortalDelegateUids(); - updateAllVpnForDelegateUid(oldDelegateBypassUids, getAllCaptivePortalDelegateUids()); + final Set newDelegateBypassUids = getAllCaptivePortalDelegateUids(); + if (!mDeps.isAtLeastV()) { + updateGlobalAllowBypassVpn(oldDelegateBypassUids, newDelegateBypassUids); + } + updateAllVpnForDelegateUid(oldDelegateBypassUids, newDelegateBypassUids); } mNetworkAgentInfos.remove(nai); if (mDeps.isShortNetworkSuspensionEnforced(mContext)) { @@ -6839,6 +6843,26 @@ public class ConnectivityService extends IConnectivityManager.Stub }); } + private int updateGlobalAllowBypassVpn(@NonNull Set oldDelegateBypassUids, + @NonNull Set newDelegateBypassUids) { + // this method is for U- and V+ must use per network VPN bypass. + CompareResult diff = new CompareResult<>(oldDelegateBypassUids, + newDelegateBypassUids); + try { + for (Integer uid : diff.removed) { + mNetd.networkSetProtectDeny(uid); + } + for (Integer uid : diff.added) { + mNetd.networkSetProtectAllow(uid); + } + } catch (RemoteException e) { + return EIO; + } catch (ServiceSpecificException e) { + return e.errorCode; + } + return 0; + } + 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. @@ -6923,15 +6947,18 @@ public class ConnectivityService extends IConnectivityManager.Stub if (nai == null) return ENOENT; // network does not exist anymore. if (nai.isDestroyed()) return ENOENT; // network has already been destroyed. - // TODO: consider allowing the uid to bypass VPN on all networks before V. - if (!mDeps.isAtLeastV()) return EOPNOTSUPP; - 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()); + final Set newDelegateBypassUids = getAllCaptivePortalDelegateUids(); + if (!mDeps.isAtLeastV()) { + // Before V, we need to update protect VPN rules globally instead of per network. + // TODO: think about proper error handling. + ret = updateGlobalAllowBypassVpn(oldDelegateBypassUids, newDelegateBypassUids); + } + updateAllVpnForDelegateUid(oldDelegateBypassUids, newDelegateBypassUids); return ret; } diff --git a/service/src/com/android/server/connectivity/NetworkAgentInfo.java b/service/src/com/android/server/connectivity/NetworkAgentInfo.java index 329f03c374..275be3301b 100644 --- a/service/src/com/android/server/connectivity/NetworkAgentInfo.java +++ b/service/src/com/android/server/connectivity/NetworkAgentInfo.java @@ -1687,6 +1687,13 @@ public class NetworkAgentInfo implements NetworkRanker.Scoreable { */ public int setCaptivePortalDelegateUid(@NonNull final CaptivePortalImpl caller, int uid) { HandlerUtils.ensureRunningOnHandlerThread(mHandler); + if (!mConnServiceDeps.isAtLeastV()) { + // For Android U and below, only update the map and return. The connectivity service + // will handle the bypass rules configuration. + // per network bypass (setAllowBypassVpnOnNetwork) is only available on V+. + mCaptivePortalDelegateUids.put(caller, uid); + return 0; + } int errorCode = setAllowBypassVpnOnNetwork(true /* allow */, uid, network.netId); if (errorCode != 0 && errorCode != EEXIST) return errorCode; if (errorCode == EEXIST) { @@ -1723,6 +1730,13 @@ public class NetworkAgentInfo implements NetworkRanker.Scoreable { */ public int removeCaptivePortalDelegateUid(@NonNull final CaptivePortalImpl caller) { HandlerUtils.ensureRunningOnHandlerThread(mHandler); + if (!mConnServiceDeps.isAtLeastV()) { + // For Android U and below, only update the map and return. The connectivity service + // will handle the bypass rules configuration. + // per network bypass (setAllowBypassVpnOnNetwork) is only available on V+. + mCaptivePortalDelegateUids.remove(caller); + return 0; + } final Integer maybeDelegateUid = mCaptivePortalDelegateUids.remove(caller); if (maybeDelegateUid == null) return 0; if (mCaptivePortalDelegateUids.values().contains(maybeDelegateUid)) return 0; diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index 25b564a138..2dd42081d5 100644 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -5071,7 +5071,6 @@ public class ConnectivityServiceTest { } @Test - @IgnoreUpTo(Build.VERSION_CODES.UPSIDE_DOWN_CAKE) public void testCaptivePortalApp_SetDelegateUidWithVpn() throws Exception { LinkProperties lp = new LinkProperties(); InOrder inOrder = inOrder(mMockNetd, mBpfNetMaps); @@ -5126,8 +5125,12 @@ public class ConnectivityServiceTest { FakeOutcomeReceiver or = new FakeOutcomeReceiver<>(); wificaptivePortal.setDelegateUid(APP1_UID, Runnable::run, or); or.awaitOutcome(); - inOrder.verify(mMockNetd).networkAllowBypassVpnOnNetwork(true, APP1_UID, - mWiFiAgent.getNetwork().netId); + if (SdkLevel.isAtLeastV()) { + inOrder.verify(mMockNetd).networkAllowBypassVpnOnNetwork(true, APP1_UID, + mWiFiAgent.getNetwork().netId); + } else { + inOrder.verify(mMockNetd).networkSetProtectAllow(APP1_UID); + } uidCaptor = ArgumentCaptor.forClass(int[].class); inOrder.verify(mBpfNetMaps, times(1)).removeUidInterfaceRules(uidCaptor.capture()); assertContainsExactly(uidCaptor.getValue(), APP1_UID, APP2_UID); @@ -5140,16 +5143,24 @@ public class ConnectivityServiceTest { or = new FakeOutcomeReceiver<>(); ethernetCaptivePortal.setDelegateUid(APP1_UID, Runnable::run, or); or.awaitOutcome(); - inOrder.verify(mMockNetd).networkAllowBypassVpnOnNetwork(true, APP1_UID, - mEthernetAgent.getNetwork().netId); + if (SdkLevel.isAtLeastV()) { + inOrder.verify(mMockNetd).networkAllowBypassVpnOnNetwork(true, APP1_UID, + mEthernetAgent.getNetwork().netId); + } else { + inOrder.verify(mMockNetd, never()).networkSetProtectAllow(APP1_UID); + } 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); + if (SdkLevel.isAtLeastV()) { + inOrder.verify(mMockNetd).networkAllowBypassVpnOnNetwork(false, APP1_UID, + mEthernetAgent.getNetwork().netId); + } else { + inOrder.verify(mMockNetd, never()).networkSetProtectDeny(APP1_UID); + } inOrder.verify(mBpfNetMaps, never()).addUidInterfaceRules(any(), any()); inOrder.verify(mBpfNetMaps, never()).removeUidInterfaceRules(any()); @@ -5172,8 +5183,12 @@ public class ConnectivityServiceTest { or = new FakeOutcomeReceiver<>(); wificaptivePortal.setDelegateUid(Process.INVALID_UID, Runnable::run, or); or.awaitOutcome(); - inOrder.verify(mMockNetd).networkAllowBypassVpnOnNetwork(false, APP1_UID, - mWiFiAgent.getNetwork().netId); + if (SdkLevel.isAtLeastV()) { + inOrder.verify(mMockNetd).networkAllowBypassVpnOnNetwork(false, APP1_UID, + mWiFiAgent.getNetwork().netId); + } else { + inOrder.verify(mMockNetd).networkSetProtectDeny(APP1_UID); + } inOrder.verify(mBpfNetMaps, times(1)).removeUidInterfaceRules(uidCaptor.capture()); assertContainsExactly(uidCaptor.getValue(), APP2_UID); uidCaptor = ArgumentCaptor.forClass(int[].class); @@ -5184,8 +5199,12 @@ public class ConnectivityServiceTest { or = new FakeOutcomeReceiver<>(); wificaptivePortal.setDelegateUid(APP1_UID, Runnable::run, or); or.awaitOutcome(); - inOrder.verify(mMockNetd).networkAllowBypassVpnOnNetwork(true, APP1_UID, - mWiFiAgent.getNetwork().netId); + if (SdkLevel.isAtLeastV()) { + inOrder.verify(mMockNetd).networkAllowBypassVpnOnNetwork(true, APP1_UID, + mWiFiAgent.getNetwork().netId); + } else { + inOrder.verify(mMockNetd).networkSetProtectAllow(APP1_UID); + } uidCaptor = ArgumentCaptor.forClass(int[].class); inOrder.verify(mBpfNetMaps, times(1)).removeUidInterfaceRules(uidCaptor.capture()); assertContainsExactly(uidCaptor.getValue(), APP1_UID, APP2_UID); @@ -5196,6 +5215,9 @@ public class ConnectivityServiceTest { // Wi-Fi network go away should trigger VPN rule being cleaned up. mWiFiAgent.disconnect(); waitForIdle(); + if (!SdkLevel.isAtLeastV()) { + inOrder.verify(mMockNetd).networkSetProtectDeny(APP1_UID); + } uidCaptor = ArgumentCaptor.forClass(int[].class); inOrder.verify(mBpfNetMaps, times(1)).removeUidInterfaceRules(uidCaptor.capture()); assertContainsExactly(uidCaptor.getValue(), APP2_UID); diff --git a/tests/unit/java/com/android/server/connectivityservice/CSCaptivePortalAppTest.kt b/tests/unit/java/com/android/server/connectivityservice/CSCaptivePortalAppTest.kt index 74555ccf61..36094dfb5f 100644 --- a/tests/unit/java/com/android/server/connectivityservice/CSCaptivePortalAppTest.kt +++ b/tests/unit/java/com/android/server/connectivityservice/CSCaptivePortalAppTest.kt @@ -25,6 +25,7 @@ import android.net.ConnectivityManager.ACTION_CAPTIVE_PORTAL_SIGN_IN import android.net.ConnectivityManager.EXTRA_CAPTIVE_PORTAL import android.net.NetworkCapabilities.NET_CAPABILITY_CAPTIVE_PORTAL import android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET +import android.net.NetworkCapabilities.TRANSPORT_ETHERNET import android.net.NetworkCapabilities.TRANSPORT_WIFI import android.net.NetworkRequest import android.net.NetworkStack @@ -35,12 +36,12 @@ import android.os.OutcomeReceiver import android.os.ServiceSpecificException import androidx.test.filters.SmallTest import com.android.testutils.DevSdkIgnoreRule +import com.android.testutils.DevSdkIgnoreRule.IgnoreAfter 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 @@ -60,6 +61,7 @@ private const val TIMEOUT_MS = 2_000L @IgnoreUpTo(Build.VERSION_CODES.R) class CSCaptivePortalAppTest : CSTest() { private val WIFI_IFACE = "wifi0" + private val ETHERNET_IFACE = "eth0" private val TEST_REDIRECT_URL = "http://example.com/firstPath" @get:Rule val ignoreRule = DevSdkIgnoreRule() @@ -97,23 +99,43 @@ class CSCaptivePortalAppTest : CSTest() { or.awaitOutcome() } - @Test - @IgnoreUpTo(Build.VERSION_CODES.UPSIDE_DOWN_CAKE) - fun testCaptivePortalApp_SetDelegateUid() { + private fun connectToNetworkWithPortal( + iface: String, + transportType: Int + ): Pair { val captivePortalCallback = TestableNetworkCallback() val captivePortalRequest = NetworkRequest.Builder() - .addCapability(NET_CAPABILITY_CAPTIVE_PORTAL).build() + .addTransportType(transportType) + .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 agent = Agent(iface, transportType, NET_CAPABILITY_INTERNET) + agent.connectWithCaptivePortal(TEST_REDIRECT_URL) + captivePortalCallback.expectAvailableCallbacksUnvalidated(agent) - val signInIntent = startCaptivePortalApp(wifiAgent) - val captivePortal = signInIntent.getParcelableExtra( - EXTRA_CAPTIVE_PORTAL, - CaptivePortal::class.java + val signInIntent = startCaptivePortalApp(agent) + val captivePortal: CaptivePortal = signInIntent.getParcelableExtra( + EXTRA_CAPTIVE_PORTAL )!! + cm.unregisterNetworkCallback(captivePortalCallback) + + return Pair(agent, captivePortal) + } + + private fun connectToWifiWithPortal(iface: String = WIFI_IFACE): + Pair { + return connectToNetworkWithPortal(iface, TRANSPORT_WIFI) + } + + private fun connectToEthernetWithPortal(iface: String = ETHERNET_IFACE): + Pair { + return connectToNetworkWithPortal(iface, TRANSPORT_ETHERNET) + } + + @Test + @IgnoreUpTo(Build.VERSION_CODES.UPSIDE_DOWN_CAKE) + fun testCaptivePortalApp_SetDelegateUidForVAndAbove() { + val (wifiAgent, captivePortal) = connectToWifiWithPortal() val inOrder = inOrder(netd) // Add the UID and check that it's added to the bypass list. @@ -169,6 +191,44 @@ class CSCaptivePortalAppTest : CSTest() { wifiAgent.disconnect() } + @Test + @IgnoreAfter(Build.VERSION_CODES.UPSIDE_DOWN_CAKE) + fun testCaptivePortalApp_SetDelegateUidForUAndBelow() { + val (wifiAgent, captivePortal) = connectToWifiWithPortal(WIFI_IFACE) + val inOrder = inOrder(netd) + + // Add the UID and check that it's added to the bypass list. + captivePortal.setDelegateUidAndAwait(APP2_UID) + inOrder.verify(netd).networkSetProtectAllow(APP2_UID) + + // Remove the UID and check that it's removed from the list. + captivePortal.setDelegateUidAndAwait(android.os.Process.INVALID_UID) + inOrder.verify(netd).networkSetProtectDeny(APP2_UID) + + // Add the UID again. + captivePortal.setDelegateUidAndAwait(APP2_UID) + inOrder.verify(netd).networkSetProtectAllow(APP2_UID) + + // Add another UID again. The old UID should be removed and the new one added. + captivePortal.setDelegateUidAndAwait(APP1_UID) + inOrder.verify(netd).networkSetProtectDeny(APP2_UID) + inOrder.verify(netd).networkSetProtectAllow(APP1_UID) + + // Launch another captive portal app on a different network with the same APP1_UID. + val (ethernetAgent, captivePortal2) = connectToEthernetWithPortal() + + // Netd API should not be called because APP1_UID is already in the allow list. + captivePortal2.setDelegateUidAndAwait(APP1_UID) + inOrder.verify(netd, never()).networkSetProtectAllow(anyInt()) + + // Netd API should not be called because APP1_UID is still in the allow list. + captivePortal.setDelegateUidAndAwait(android.os.Process.INVALID_UID) + inOrder.verify(netd, never()).networkSetProtectDeny(anyInt()) + + wifiAgent.disconnect() + ethernetAgent.disconnect() + } + @Test fun testCaptivePortalApp_Reevaluate_Nopermission() { val captivePortalCallback = TestableNetworkCallback() diff --git a/tests/unit/java/com/android/server/connectivityservice/CSNetdBypassRulesTests.kt b/tests/unit/java/com/android/server/connectivityservice/CSNetdBypassRulesTests.kt index 2c692dcfe1..52d3950533 100644 --- a/tests/unit/java/com/android/server/connectivityservice/CSNetdBypassRulesTests.kt +++ b/tests/unit/java/com/android/server/connectivityservice/CSNetdBypassRulesTests.kt @@ -36,10 +36,12 @@ import androidx.test.filters.SmallTest import com.android.server.ConnectivityService.CaptivePortalImpl import com.android.server.connectivity.NetworkAgentInfo import com.android.server.connectivity.QosCallbackTracker +import com.android.testutils.DevSdkIgnoreRule import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo import com.android.testutils.DevSdkIgnoreRunner import com.android.testutils.postAndWait import kotlin.test.assertEquals +import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.mockito.ArgumentMatchers.eq @@ -50,8 +52,6 @@ import org.mockito.Mockito.doThrow import org.mockito.Mockito.inOrder import org.mockito.Mockito.mock import org.mockito.Mockito.never -import org.mockito.Mockito.verify -import org.mockito.Mockito.`when` private const val TEST_UID = 101 private const val TEST_TIMEOUT_MS = 500L @@ -59,10 +59,12 @@ private const val TEST_TIMEOUT_MS = 500L @DevSdkIgnoreRunner.MonitorThreadLeak @RunWith(DevSdkIgnoreRunner::class) @SmallTest -@IgnoreUpTo(Build.VERSION_CODES.R) +@IgnoreUpTo(Build.VERSION_CODES.UPSIDE_DOWN_CAKE) class CSNetdBypassRulesTests : CSTest() { private val TEST_LINGER_DELAY_MS = 400 + @get:Rule val ignoreRule = DevSdkIgnoreRule() + private val caller = mock(CaptivePortalImpl::class.java) private val na = mock(INetworkAgent::class.java) private val info = mock(NetworkInfo::class.java)