From 36e91a3a0b7ecb464e4aebcf821b43a7edddb2dc Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Mon, 30 Nov 2020 16:02:54 +0900 Subject: [PATCH 1/3] Add tests for always-on VPN lockdown mode. This requires mocking lots of new things that weren't mocked before but is otherwise fairly straightforward. A few changes to MockVpn are needed as well: 1. Set the VPN's NetworkInfo to CONNECTED, so methods such as isBlockingUid will work. While I'm at it, set the interface on the LinkProperties as well to make things a bit more realistic. 2. Constructs the VpnConfig when registering the agent, not when the MockVpn is created. This is needed because starting and stopping lockdown VPN calls prepare, which nulls out mConfig. But constructing the VpnConfig when registering the agent is more realistic anyway. The production code does that in establish, but we can't do that in ConnectivityServiceTest because some of the test cases don't call establish and call registerAgent directly. Bug: 173331190 Test: atest FrameworksNetTests Change-Id: I827543751dbf5e626a24ec02cd6f50b423f5f761 --- .../server/ConnectivityServiceTest.java | 258 ++++++++++++++++-- .../android/server/connectivity/VpnTest.java | 6 +- 2 files changed, 246 insertions(+), 18 deletions(-) diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 8c403f1b68..43d3e4e692 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -39,6 +39,7 @@ import static android.net.ConnectivityManager.TYPE_MOBILE; import static android.net.ConnectivityManager.TYPE_MOBILE_FOTA; import static android.net.ConnectivityManager.TYPE_MOBILE_MMS; import static android.net.ConnectivityManager.TYPE_MOBILE_SUPL; +import static android.net.ConnectivityManager.TYPE_VPN; import static android.net.ConnectivityManager.TYPE_WIFI; import static android.net.INetworkMonitor.NETWORK_VALIDATION_PROBE_DNS; import static android.net.INetworkMonitor.NETWORK_VALIDATION_PROBE_FALLBACK; @@ -101,6 +102,8 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.mockito.AdditionalMatchers.aryEq; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.argThat; @@ -132,6 +135,7 @@ import android.app.AppOpsManager; import android.app.NotificationManager; import android.app.PendingIntent; import android.content.BroadcastReceiver; +import android.content.ComponentName; import android.content.ContentProvider; import android.content.ContentResolver; import android.content.Context; @@ -140,6 +144,8 @@ import android.content.IntentFilter; import android.content.pm.ApplicationInfo; import android.content.pm.PackageInfo; import android.content.pm.PackageManager; +import android.content.pm.ResolveInfo; +import android.content.pm.ServiceInfo; import android.content.pm.UserInfo; import android.content.res.Resources; import android.location.LocationManager; @@ -176,6 +182,7 @@ import android.net.NetworkAgentConfig; import android.net.NetworkCapabilities; import android.net.NetworkFactory; import android.net.NetworkInfo; +import android.net.NetworkInfo.DetailedState; import android.net.NetworkRequest; import android.net.NetworkSpecifier; import android.net.NetworkStack; @@ -332,7 +339,7 @@ public class ConnectivityServiceTest { private static final String WIFI_WOL_IFNAME = "test_wlan_wol"; private static final String VPN_IFNAME = "tun10042"; private static final String TEST_PACKAGE_NAME = "com.android.test.package"; - private static final String[] EMPTY_STRING_ARRAY = new String[0]; + private static final String ALWAYS_ON_PACKAGE = "com.android.test.alwaysonvpn"; private static final String INTERFACE_NAME = "interface"; @@ -353,6 +360,7 @@ public class ConnectivityServiceTest { @Mock IIpConnectivityMetrics mIpConnectivityMetrics; @Mock IpConnectivityMetrics.Logger mMetricsService; @Mock DefaultNetworkMetrics mDefaultNetworkMetrics; + @Mock DeviceIdleInternal mDeviceIdleInternal; @Mock INetworkManagementService mNetworkManagementService; @Mock INetworkStatsService mStatsService; @Mock IBatteryStats mBatteryStatsService; @@ -449,6 +457,15 @@ public class ConnectivityServiceTest { } catch (InterruptedException e) {} } + @Override + public ComponentName startService(Intent service) { + final String action = service.getAction(); + if (!VpnConfig.SERVICE_INTERFACE.equals(action)) { + fail("Attempt to start unknown service, action=" + action); + } + return new ComponentName(service.getPackage(), "com.android.test.Service"); + } + @Override public Object getSystemService(String name) { if (Context.CONNECTIVITY_SERVICE.equals(name)) return mCm; @@ -1055,9 +1072,19 @@ public class ConnectivityServiceTest { private VpnInfo mVpnInfo; public MockVpn(int userId) { - super(startHandlerThreadAndReturnLooper(), mServiceContext, mNetworkManagementService, - mMockNetd, userId, mock(KeyStore.class)); - mConfig = new VpnConfig(); + super(startHandlerThreadAndReturnLooper(), mServiceContext, + new Dependencies() { + @Override + public boolean isCallerSystem() { + return true; + } + + @Override + public DeviceIdleInternal getDeviceIdleInternal() { + return mDeviceIdleInternal; + } + }, + mNetworkManagementService, mMockNetd, userId, mock(KeyStore.class)); } public void setUids(Set uids) { @@ -1086,9 +1113,16 @@ public class ConnectivityServiceTest { return mVpnType; } + private LinkProperties makeLinkProperties() { + final LinkProperties lp = new LinkProperties(); + lp.setInterfaceName(VPN_IFNAME); + return lp; + } + private void registerAgent(boolean isAlwaysMetered, Set uids, LinkProperties lp) throws Exception { if (mAgentRegistered) throw new IllegalStateException("already registered"); + mConfig = new VpnConfig(); setUids(uids); if (!isAlwaysMetered) mNetworkCapabilities.addCapability(NET_CAPABILITY_NOT_METERED); mInterface = VPN_IFNAME; @@ -1101,12 +1135,13 @@ public class ConnectivityServiceTest { verify(mMockNetd, never()) .networkRemoveUidRanges(eq(mMockVpn.getNetId()), any()); mAgentRegistered = true; + updateState(NetworkInfo.DetailedState.CONNECTED, "registerAgent"); mNetworkCapabilities.set(mMockNetworkAgent.getNetworkCapabilities()); mNetworkAgent = mMockNetworkAgent.getNetworkAgent(); } private void registerAgent(Set uids) throws Exception { - registerAgent(false /* isAlwaysMetered */, uids, new LinkProperties()); + registerAgent(false /* isAlwaysMetered */, uids, makeLinkProperties()); } private void connect(boolean validated, boolean hasInternet, boolean isStrictMode) { @@ -1142,12 +1177,12 @@ public class ConnectivityServiceTest { public void establishForMyUid(boolean validated, boolean hasInternet, boolean isStrictMode) throws Exception { final int uid = Process.myUid(); - establish(new LinkProperties(), uid, uidRangesForUid(uid), validated, hasInternet, + establish(makeLinkProperties(), uid, uidRangesForUid(uid), validated, hasInternet, isStrictMode); } public void establishForMyUid() throws Exception { - establishForMyUid(new LinkProperties()); + establishForMyUid(makeLinkProperties()); } public void sendLinkProperties(LinkProperties lp) { @@ -1155,7 +1190,10 @@ public class ConnectivityServiceTest { } public void disconnect() { - if (mMockNetworkAgent != null) mMockNetworkAgent.disconnect(); + if (mMockNetworkAgent != null) { + mMockNetworkAgent.disconnect(); + updateState(NetworkInfo.DetailedState.DISCONNECTED, "disconnect"); + } mAgentRegistered = false; } @@ -1376,13 +1414,13 @@ public class ConnectivityServiceTest { } private void mockDefaultPackages() throws Exception { - final String testPackageName = mContext.getPackageName(); - final PackageInfo testPackageInfo = mContext.getPackageManager().getPackageInfo( - testPackageName, PackageManager.GET_PERMISSIONS); + final String myPackageName = mContext.getPackageName(); + final PackageInfo myPackageInfo = mContext.getPackageManager().getPackageInfo( + myPackageName, PackageManager.GET_PERMISSIONS); when(mPackageManager.getPackagesForUid(Binder.getCallingUid())).thenReturn( - new String[] {testPackageName}); - when(mPackageManager.getPackageInfoAsUser(eq(testPackageName), anyInt(), - eq(UserHandle.getCallingUserId()))).thenReturn(testPackageInfo); + new String[] {myPackageName}); + when(mPackageManager.getPackageInfoAsUser(eq(myPackageName), anyInt(), + eq(UserHandle.getCallingUserId()))).thenReturn(myPackageInfo); when(mPackageManager.getInstalledPackages(eq(GET_PERMISSIONS | MATCH_ANY_USER))).thenReturn( Arrays.asList(new PackageInfo[] { @@ -1390,6 +1428,25 @@ public class ConnectivityServiceTest { buildPackageInfo(/* SYSTEM */ false, APP2_UID), buildPackageInfo(/* SYSTEM */ false, VPN_UID) })); + + // Create a fake always-on VPN package. + final int userId = UserHandle.getCallingUserId(); + final ApplicationInfo applicationInfo = new ApplicationInfo(); + applicationInfo.targetSdkVersion = Build.VERSION_CODES.R; // Always-on supported in N+. + when(mPackageManager.getApplicationInfoAsUser(eq(ALWAYS_ON_PACKAGE), anyInt(), + eq(userId))).thenReturn(applicationInfo); + + // Minimal mocking to keep Vpn#isAlwaysOnPackageSupported happy. + ResolveInfo rInfo = new ResolveInfo(); + rInfo.serviceInfo = new ServiceInfo(); + rInfo.serviceInfo.metaData = new Bundle(); + final List services = Arrays.asList(new ResolveInfo[]{rInfo}); + when(mPackageManager.queryIntentServicesAsUser(any(), eq(PackageManager.GET_META_DATA), + eq(userId))).thenReturn(services); + when(mPackageManager.getPackageUidAsUser(TEST_PACKAGE_NAME, userId)) + .thenReturn(Process.myUid()); + when(mPackageManager.getPackageUidAsUser(ALWAYS_ON_PACKAGE, userId)) + .thenReturn(VPN_UID); } private void verifyActiveNetwork(int transport) { @@ -2252,10 +2309,10 @@ public class ConnectivityServiceTest { } private void grantUsingBackgroundNetworksPermissionForUid(final int uid) throws Exception { - final String testPackageName = mContext.getPackageName(); - when(mPackageManager.getPackageInfo(eq(testPackageName), eq(GET_PERMISSIONS))) + final String myPackageName = mContext.getPackageName(); + when(mPackageManager.getPackageInfo(eq(myPackageName), eq(GET_PERMISSIONS))) .thenReturn(buildPackageInfo(true, uid)); - mService.mPermissionMonitor.onPackageAdded(testPackageName, uid); + mService.mPermissionMonitor.onPackageAdded(myPackageName, uid); } @Test @@ -6389,6 +6446,173 @@ public class ConnectivityServiceTest { mCm.unregisterNetworkCallback(defaultCallback); } + private void expectNetworkRejectNonSecureVpn(InOrder inOrder, boolean add, + UidRangeParcel... expected) throws Exception { + inOrder.verify(mMockNetd).networkRejectNonSecureVpn(eq(add), aryEq(expected)); + } + + private void checkNetworkInfo(NetworkInfo ni, int type, DetailedState state) { + assertNotNull(ni); + assertEquals(type, ni.getType()); + assertEquals(ConnectivityManager.getNetworkTypeName(type), state, ni.getDetailedState()); + } + + private void assertActiveNetworkInfo(int type, DetailedState state) { + checkNetworkInfo(mCm.getActiveNetworkInfo(), type, state); + } + private void assertNetworkInfo(int type, DetailedState state) { + checkNetworkInfo(mCm.getNetworkInfo(type), type, state); + } + + @Test + public void testNetworkBlockedStatusAlwaysOnVpn() throws Exception { + mServiceContext.setPermission( + Manifest.permission.CONTROL_ALWAYS_ON_VPN, PERMISSION_GRANTED); + mServiceContext.setPermission( + Manifest.permission.CONTROL_VPN, PERMISSION_GRANTED); + mServiceContext.setPermission( + Manifest.permission.NETWORK_SETTINGS, PERMISSION_GRANTED); + + final TestNetworkCallback callback = new TestNetworkCallback(); + final NetworkRequest request = new NetworkRequest.Builder() + .removeCapability(NET_CAPABILITY_NOT_VPN) + .build(); + mCm.registerNetworkCallback(request, callback); + + final TestNetworkCallback defaultCallback = new TestNetworkCallback(); + mCm.registerDefaultNetworkCallback(defaultCallback); + + final int uid = Process.myUid(); + final int userId = UserHandle.getUserId(uid); + final ArrayList allowList = new ArrayList<>(); + mService.setAlwaysOnVpnPackage(userId, ALWAYS_ON_PACKAGE, true /* lockdown */, allowList); + + UidRangeParcel firstHalf = new UidRangeParcel(1, VPN_UID - 1); + UidRangeParcel secondHalf = new UidRangeParcel(VPN_UID + 1, 99999); + InOrder inOrder = inOrder(mMockNetd); + expectNetworkRejectNonSecureVpn(inOrder, true, firstHalf, secondHalf); + + // Connect a network when lockdown is active, expect to see it blocked. + mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI); + mWiFiNetworkAgent.connect(false /* validated */); + callback.expectAvailableCallbacksUnvalidatedAndBlocked(mWiFiNetworkAgent); + defaultCallback.expectAvailableCallbacksUnvalidatedAndBlocked(mWiFiNetworkAgent); + assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetworkForUid(VPN_UID)); + assertNull(mCm.getActiveNetwork()); + assertActiveNetworkInfo(TYPE_WIFI, DetailedState.BLOCKED); + // Mobile is BLOCKED even though it's not actually connected. + assertNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED); + assertNetworkInfo(TYPE_WIFI, DetailedState.BLOCKED); + + // Disable lockdown, expect to see the network unblocked. + // There are no callbacks because they are not implemented yet. + mService.setAlwaysOnVpnPackage(userId, null, false /* lockdown */, allowList); + expectNetworkRejectNonSecureVpn(inOrder, false, firstHalf, secondHalf); + assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetworkForUid(VPN_UID)); + assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork()); + assertActiveNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED); + assertNetworkInfo(TYPE_MOBILE, DetailedState.DISCONNECTED); + assertNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED); + + // Add our UID to the allowlist and re-enable lockdown, expect network is not blocked. + allowList.add(TEST_PACKAGE_NAME); + mService.setAlwaysOnVpnPackage(userId, ALWAYS_ON_PACKAGE, true /* lockdown */, allowList); + defaultCallback.assertNoCallback(); + + // The following requires that the UID of this test package is greater than VPN_UID. This + // is always true in practice because a plain AOSP build with no apps installed has almost + // 200 packages installed. + final UidRangeParcel piece1 = new UidRangeParcel(1, VPN_UID - 1); + final UidRangeParcel piece2 = new UidRangeParcel(VPN_UID + 1, uid - 1); + final UidRangeParcel piece3 = new UidRangeParcel(uid + 1, 99999); + expectNetworkRejectNonSecureVpn(inOrder, true, piece1, piece2, piece3); + assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetworkForUid(VPN_UID)); + assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork()); + assertActiveNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED); + assertNetworkInfo(TYPE_MOBILE, DetailedState.DISCONNECTED); + assertNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED); + + // Connect a new network, expect it to be unblocked. + mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR); + mCellNetworkAgent.connect(false /* validated */); + callback.expectAvailableCallbacksUnvalidated(mCellNetworkAgent); + defaultCallback.assertNoCallback(); + assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetworkForUid(VPN_UID)); + assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork()); + assertActiveNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED); + // Cellular is DISCONNECTED because it's not the default and there are no requests for it. + assertNetworkInfo(TYPE_MOBILE, DetailedState.DISCONNECTED); + assertNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED); + + // Disable lockdown, remove our UID from the allowlist, and re-enable lockdown. + // Everything should now be blocked. + mService.setAlwaysOnVpnPackage(userId, null, false /* lockdown */, allowList); + expectNetworkRejectNonSecureVpn(inOrder, false, piece1, piece2, piece3); + allowList.clear(); + mService.setAlwaysOnVpnPackage(userId, ALWAYS_ON_PACKAGE, true /* lockdown */, allowList); + expectNetworkRejectNonSecureVpn(inOrder, true, firstHalf, secondHalf); + assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetworkForUid(VPN_UID)); + assertNull(mCm.getActiveNetwork()); + assertActiveNetworkInfo(TYPE_WIFI, DetailedState.BLOCKED); + assertNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED); + assertNetworkInfo(TYPE_WIFI, DetailedState.BLOCKED); + + // Disable lockdown. Everything is unblocked. + mService.setAlwaysOnVpnPackage(userId, null, false /* lockdown */, allowList); + assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetworkForUid(VPN_UID)); + assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork()); + assertActiveNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED); + assertNetworkInfo(TYPE_MOBILE, DetailedState.DISCONNECTED); + assertNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED); + + // Enable and disable an always-on VPN package without lockdown. Expect no changes. + reset(mMockNetd); + mService.setAlwaysOnVpnPackage(userId, ALWAYS_ON_PACKAGE, false /* lockdown */, allowList); + inOrder.verify(mMockNetd, never()).networkRejectNonSecureVpn(anyBoolean(), any()); + callback.assertNoCallback(); + defaultCallback.assertNoCallback(); + assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetworkForUid(VPN_UID)); + assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork()); + assertActiveNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED); + assertNetworkInfo(TYPE_MOBILE, DetailedState.DISCONNECTED); + assertNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED); + + mService.setAlwaysOnVpnPackage(userId, null, false /* lockdown */, allowList); + inOrder.verify(mMockNetd, never()).networkRejectNonSecureVpn(anyBoolean(), any()); + callback.assertNoCallback(); + defaultCallback.assertNoCallback(); + assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetworkForUid(VPN_UID)); + assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork()); + assertActiveNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED); + assertNetworkInfo(TYPE_MOBILE, DetailedState.DISCONNECTED); + assertNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED); + + // Enable lockdown and connect a VPN. The VPN is not blocked. + mService.setAlwaysOnVpnPackage(userId, ALWAYS_ON_PACKAGE, true /* lockdown */, allowList); + assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetworkForUid(VPN_UID)); + assertNull(mCm.getActiveNetwork()); + assertActiveNetworkInfo(TYPE_WIFI, DetailedState.BLOCKED); + assertNetworkInfo(TYPE_MOBILE, DetailedState.BLOCKED); + assertNetworkInfo(TYPE_WIFI, DetailedState.BLOCKED); + + mMockVpn.establishForMyUid(); + defaultCallback.expectAvailableThenValidatedCallbacks(mMockVpn); + assertEquals(mMockVpn.getNetwork(), mCm.getActiveNetwork()); + assertEquals(null, mCm.getActiveNetworkForUid(VPN_UID)); // BUG? + assertActiveNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED); + assertNetworkInfo(TYPE_MOBILE, DetailedState.DISCONNECTED); + assertNetworkInfo(TYPE_VPN, DetailedState.CONNECTED); + assertNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED); + + mMockVpn.disconnect(); + defaultCallback.expectCallback(CallbackEntry.LOST, mMockVpn); + defaultCallback.expectAvailableCallbacksUnvalidatedAndBlocked(mWiFiNetworkAgent); + assertNull(mCm.getActiveNetwork()); + + mCm.unregisterNetworkCallback(callback); + mCm.unregisterNetworkCallback(defaultCallback); + } + @Test public final void testLoseTrusted() throws Exception { final NetworkRequest trustedRequest = new NetworkRequest.Builder() diff --git a/tests/net/java/com/android/server/connectivity/VpnTest.java b/tests/net/java/com/android/server/connectivity/VpnTest.java index 6e380be6c5..cc47317554 100644 --- a/tests/net/java/com/android/server/connectivity/VpnTest.java +++ b/tests/net/java/com/android/server/connectivity/VpnTest.java @@ -228,7 +228,6 @@ public class VpnTest { R.string.config_customVpnAlwaysOnDisconnectedDialogComponent)); when(mPackageManager.hasSystemFeature(PackageManager.FEATURE_IPSEC_TUNNELS)) .thenReturn(true); - when(mSystemServices.isCallerSystem()).thenReturn(true); // Used by {@link Notification.Builder} ApplicationInfo applicationInfo = new ApplicationInfo(); @@ -1101,6 +1100,11 @@ public class VpnTest { } } + @Override + public boolean isCallerSystem() { + return true; + } + @Override public void startService(final String serviceName) { mRunningServices.put(serviceName, true); From 1b17648534db5f44fd5d74cdf8a1da572d816d78 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Tue, 15 Dec 2020 00:45:30 +0900 Subject: [PATCH 2/3] Add a test for getDefaultNetworkCapabilitiesForUser. Bug: 173331190 Test: test-only change Test: new test passes 100 times in a row Change-Id: I210284578e38cd25b8b95235d3390d5bd66a5a70 --- .../server/ConnectivityServiceTest.java | 33 +++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 43d3e4e692..af1f75e0f1 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -5888,10 +5888,21 @@ public class ConnectivityServiceTest { assertTrue(nc.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)); } + private void assertDefaultNetworkCapabilities(int userId, NetworkAgentWrapper... networks) { + final NetworkCapabilities[] defaultCaps = mService.getDefaultNetworkCapabilitiesForUser( + userId, "com.android.calling.package"); + final String defaultCapsString = Arrays.toString(defaultCaps); + assertEquals(defaultCapsString, defaultCaps.length, networks.length); + final Set defaultCapsSet = new ArraySet<>(defaultCaps); + for (NetworkAgentWrapper network : networks) { + final NetworkCapabilities nc = mCm.getNetworkCapabilities(network.getNetwork()); + final String msg = "Did not find " + nc + " in " + Arrays.toString(defaultCaps); + assertTrue(msg, defaultCapsSet.contains(nc)); + } + } + @Test public void testVpnSetUnderlyingNetworks() throws Exception { - final int uid = Process.myUid(); - final TestNetworkCallback vpnNetworkCallback = new TestNetworkCallback(); final NetworkRequest vpnNetworkRequest = new NetworkRequest.Builder() .removeCapability(NET_CAPABILITY_NOT_VPN) @@ -5914,6 +5925,9 @@ public class ConnectivityServiceTest { // A VPN without underlying networks is not suspended. assertTrue(nc.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)); + final int userId = UserHandle.getUserId(Process.myUid()); + assertDefaultNetworkCapabilities(userId /* no networks */); + // Connect cell and use it as an underlying network. mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR); mCellNetworkAgent.addCapability(NET_CAPABILITY_NOT_SUSPENDED); @@ -5927,6 +5941,7 @@ public class ConnectivityServiceTest { && caps.hasTransport(TRANSPORT_CELLULAR) && !caps.hasTransport(TRANSPORT_WIFI) && !caps.hasCapability(NET_CAPABILITY_NOT_METERED) && caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)); + assertDefaultNetworkCapabilities(userId, mCellNetworkAgent); mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI); mWiFiNetworkAgent.addCapability(NET_CAPABILITY_NOT_METERED); @@ -5941,6 +5956,7 @@ public class ConnectivityServiceTest { && caps.hasTransport(TRANSPORT_CELLULAR) && caps.hasTransport(TRANSPORT_WIFI) && !caps.hasCapability(NET_CAPABILITY_NOT_METERED) && caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)); + assertDefaultNetworkCapabilities(userId, mCellNetworkAgent, mWiFiNetworkAgent); // Don't disconnect, but note the VPN is not using wifi any more. mService.setUnderlyingNetworksForVpn( @@ -5951,6 +5967,9 @@ public class ConnectivityServiceTest { && caps.hasTransport(TRANSPORT_CELLULAR) && !caps.hasTransport(TRANSPORT_WIFI) && !caps.hasCapability(NET_CAPABILITY_NOT_METERED) && caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)); + // The return value of getDefaultNetworkCapabilitiesForUser always includes the default + // network (wifi) as well as the underlying networks (cell). + assertDefaultNetworkCapabilities(userId, mCellNetworkAgent, mWiFiNetworkAgent); // Remove NOT_SUSPENDED from the only network and observe VPN is now suspended. mCellNetworkAgent.removeCapability(NET_CAPABILITY_NOT_SUSPENDED); @@ -5979,6 +5998,7 @@ public class ConnectivityServiceTest { && !caps.hasTransport(TRANSPORT_CELLULAR) && caps.hasTransport(TRANSPORT_WIFI) && caps.hasCapability(NET_CAPABILITY_NOT_METERED) && caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)); + assertDefaultNetworkCapabilities(userId, mWiFiNetworkAgent); // Use both again. mService.setUnderlyingNetworksForVpn( @@ -5989,6 +6009,7 @@ public class ConnectivityServiceTest { && caps.hasTransport(TRANSPORT_CELLULAR) && caps.hasTransport(TRANSPORT_WIFI) && !caps.hasCapability(NET_CAPABILITY_NOT_METERED) && caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)); + assertDefaultNetworkCapabilities(userId, mCellNetworkAgent, mWiFiNetworkAgent); // Cell is suspended again. As WiFi is not, this should not cause a callback. mCellNetworkAgent.removeCapability(NET_CAPABILITY_NOT_SUSPENDED); @@ -6006,6 +6027,7 @@ public class ConnectivityServiceTest { // a bug in ConnectivityService, but as the SUSPENDED and RESUMED callbacks have never // been public and are deprecated and slated for removal, there is no sense in spending // resources fixing this bug now. + assertDefaultNetworkCapabilities(userId, mCellNetworkAgent, mWiFiNetworkAgent); // Use both again. mService.setUnderlyingNetworksForVpn( @@ -6018,6 +6040,7 @@ public class ConnectivityServiceTest { && caps.hasCapability(NET_CAPABILITY_NOT_SUSPENDED)); // As above, the RESUMED callback not being sent here is a bug, but not a bug that's // worth anybody's time to fix. + assertDefaultNetworkCapabilities(userId, mCellNetworkAgent, mWiFiNetworkAgent); // Disconnect cell. Receive update without even removing the dead network from the // underlying networks – it's dead anyway. Not metered any more. @@ -6026,6 +6049,7 @@ public class ConnectivityServiceTest { (caps) -> caps.hasTransport(TRANSPORT_VPN) && !caps.hasTransport(TRANSPORT_CELLULAR) && caps.hasTransport(TRANSPORT_WIFI) && caps.hasCapability(NET_CAPABILITY_NOT_METERED)); + assertDefaultNetworkCapabilities(userId, mWiFiNetworkAgent); // Disconnect wifi too. No underlying networks means this is now metered. mWiFiNetworkAgent.disconnect(); @@ -6033,6 +6057,11 @@ public class ConnectivityServiceTest { (caps) -> caps.hasTransport(TRANSPORT_VPN) && !caps.hasTransport(TRANSPORT_CELLULAR) && !caps.hasTransport(TRANSPORT_WIFI) && !caps.hasCapability(NET_CAPABILITY_NOT_METERED)); + // When a network disconnects, the callbacks are fired before all state is updated, so for a + // short time, synchronous calls will behave as if the network is still connected. Wait for + // things to settle. + waitForIdle(); + assertDefaultNetworkCapabilities(userId /* no networks */); mMockVpn.disconnect(); } From f61ca94e2cf23f6f5566eb6fd1e36fe0e10e04ae Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Tue, 15 Dec 2020 11:02:22 +0900 Subject: [PATCH 3/3] Allow ConnectivityServiceTest to change the calling UID. Allow ConnectivityServiceTest to change the UID by replacing static calls to Binder.getCallingUid() with a method that can be mocked. Add registerNetworkCallbackAsUid as an initial way to exercise this, and add some test coverage to the always-on lockdown test to confirm that things are working as expected. Bug: 173331190 Test: new unit tests Change-Id: Ie0b32460e20e5906a0f479191e11a062f21cc608 --- .../android/server/ConnectivityService.java | 100 +++++++++--------- .../server/ConnectivityServiceTest.java | 40 ++++++- 2 files changed, 89 insertions(+), 51 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 2c765bd896..184f1bfaa0 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -876,6 +876,10 @@ public class ConnectivityService extends IConnectivityManager.Stub */ @VisibleForTesting public static class Dependencies { + public int getCallingUid() { + return Binder.getCallingUid(); + } + /** * Get system properties to use in ConnectivityService. */ @@ -1408,7 +1412,7 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public NetworkInfo getActiveNetworkInfo() { enforceAccessPermission(); - final int uid = Binder.getCallingUid(); + final int uid = mDeps.getCallingUid(); final NetworkState state = getUnfilteredActiveNetworkState(uid); filterNetworkStateForUid(state, uid, false); maybeLogBlockedNetworkInfo(state.networkInfo, uid); @@ -1418,7 +1422,7 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public Network getActiveNetwork() { enforceAccessPermission(); - return getActiveNetworkForUidInternal(Binder.getCallingUid(), false); + return getActiveNetworkForUidInternal(mDeps.getCallingUid(), false); } @Override @@ -1458,7 +1462,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // Public because it's used by mLockdownTracker. public NetworkInfo getActiveNetworkInfoUnfiltered() { enforceAccessPermission(); - final int uid = Binder.getCallingUid(); + final int uid = mDeps.getCallingUid(); NetworkState state = getUnfilteredActiveNetworkState(uid); return state.networkInfo; } @@ -1474,7 +1478,7 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public NetworkInfo getNetworkInfo(int networkType) { enforceAccessPermission(); - final int uid = Binder.getCallingUid(); + final int uid = mDeps.getCallingUid(); if (getVpnUnderlyingNetworks(uid) != null) { // A VPN is active, so we may need to return one of its underlying networks. This // information is not available in LegacyTypeTracker, so we have to get it from @@ -1519,7 +1523,7 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public Network getNetworkForType(int networkType) { enforceAccessPermission(); - final int uid = Binder.getCallingUid(); + final int uid = mDeps.getCallingUid(); NetworkState state = getFilteredNetworkState(networkType, uid); if (!isNetworkWithLinkPropertiesBlocked(state.linkProperties, uid, false)) { return state.network; @@ -1566,7 +1570,7 @@ public class ConnectivityService extends IConnectivityManager.Stub result.put( nai.network, maybeSanitizeLocationInfoForCaller( - nc, Binder.getCallingUid(), callingPackageName)); + nc, mDeps.getCallingUid(), callingPackageName)); } synchronized (mVpns) { @@ -1581,7 +1585,7 @@ public class ConnectivityService extends IConnectivityManager.Stub result.put( network, maybeSanitizeLocationInfoForCaller( - nc, Binder.getCallingUid(), callingPackageName)); + nc, mDeps.getCallingUid(), callingPackageName)); } } } @@ -1611,7 +1615,7 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public LinkProperties getActiveLinkProperties() { enforceAccessPermission(); - final int uid = Binder.getCallingUid(); + final int uid = mDeps.getCallingUid(); NetworkState state = getUnfilteredActiveNetworkState(uid); if (state.linkProperties == null) return null; return linkPropertiesRestrictedForCallerPermissions(state.linkProperties, @@ -1625,7 +1629,7 @@ public class ConnectivityService extends IConnectivityManager.Stub final LinkProperties lp = getLinkProperties(nai); if (lp == null) return null; return linkPropertiesRestrictedForCallerPermissions( - lp, Binder.getCallingPid(), Binder.getCallingUid()); + lp, Binder.getCallingPid(), mDeps.getCallingUid()); } // TODO - this should be ALL networks @@ -1635,7 +1639,7 @@ public class ConnectivityService extends IConnectivityManager.Stub final LinkProperties lp = getLinkProperties(getNetworkAgentInfoForNetwork(network)); if (lp == null) return null; return linkPropertiesRestrictedForCallerPermissions( - lp, Binder.getCallingPid(), Binder.getCallingUid()); + lp, Binder.getCallingPid(), mDeps.getCallingUid()); } @Nullable @@ -1657,17 +1661,17 @@ public class ConnectivityService extends IConnectivityManager.Stub synchronized (nai) { if (nai.networkCapabilities == null) return null; return networkCapabilitiesRestrictedForCallerPermissions( - nai.networkCapabilities, Binder.getCallingPid(), Binder.getCallingUid()); + nai.networkCapabilities, Binder.getCallingPid(), mDeps.getCallingUid()); } } @Override public NetworkCapabilities getNetworkCapabilities(Network network, String callingPackageName) { - mAppOpsManager.checkPackage(Binder.getCallingUid(), callingPackageName); + mAppOpsManager.checkPackage(mDeps.getCallingUid(), callingPackageName); enforceAccessPermission(); return maybeSanitizeLocationInfoForCaller( getNetworkCapabilitiesInternal(network), - Binder.getCallingUid(), callingPackageName); + mDeps.getCallingUid(), callingPackageName); } @VisibleForTesting @@ -1755,7 +1759,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } private void restrictBackgroundRequestForCaller(NetworkCapabilities nc) { - if (!mPermissionMonitor.hasUseBackgroundNetworksPermission(Binder.getCallingUid())) { + if (!mPermissionMonitor.hasUseBackgroundNetworksPermission(mDeps.getCallingUid())) { nc.addCapability(NET_CAPABILITY_FOREGROUND); } } @@ -1808,7 +1812,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // requestRouteToHost. In Q, GnssLocationProvider is changed to not call requestRouteToHost // for devices launched with Q and above. However, existing devices upgrading to Q and // above must continued to be supported for few more releases. - if (isSystem(Binder.getCallingUid()) && SystemProperties.getInt( + if (isSystem(mDeps.getCallingUid()) && SystemProperties.getInt( "ro.product.first_api_level", 0) > Build.VERSION_CODES.P) { log("This method exists only for app backwards compatibility" + " and must not be called by system services."); @@ -1874,7 +1878,7 @@ public class ConnectivityService extends IConnectivityManager.Stub return false; } - final int uid = Binder.getCallingUid(); + final int uid = mDeps.getCallingUid(); final long token = Binder.clearCallingIdentity(); try { LinkProperties lp; @@ -2294,7 +2298,7 @@ public class ConnectivityService extends IConnectivityManager.Stub */ @Override public void systemReady() { - if (Binder.getCallingUid() != Process.SYSTEM_UID) { + if (mDeps.getCallingUid() != Process.SYSTEM_UID) { throw new SecurityException("Calling Uid is not system uid."); } systemReadyInternal(); @@ -2520,7 +2524,7 @@ public class ConnectivityService extends IConnectivityManager.Stub if (context.checkCallingOrSelfPermission(android.Manifest.permission.DUMP) != PackageManager.PERMISSION_GRANTED) { pw.println("Permission Denial: can't dump " + tag + " from from pid=" - + Binder.getCallingPid() + ", uid=" + Binder.getCallingUid() + + Binder.getCallingPid() + ", uid=" + mDeps.getCallingUid() + " due to missing android.permission.DUMP permission"); return false; } else { @@ -3900,7 +3904,7 @@ public class ConnectivityService extends IConnectivityManager.Stub if (request == CaptivePortal.APP_REQUEST_REEVALUATION_REQUIRED) { checkNetworkStackPermission(); - nm.forceReevaluation(Binder.getCallingUid()); + nm.forceReevaluation(mDeps.getCallingUid()); } } @@ -4367,7 +4371,7 @@ public class ConnectivityService extends IConnectivityManager.Stub public void reportNetworkConnectivity(Network network, boolean hasConnectivity) { enforceAccessPermission(); enforceInternetPermission(); - final int uid = Binder.getCallingUid(); + final int uid = mDeps.getCallingUid(); final int connectivityInfo = encodeBool(hasConnectivity); // Handle ConnectivityDiagnostics event before attempting to revalidate the network. This @@ -4437,13 +4441,13 @@ public class ConnectivityService extends IConnectivityManager.Stub if (globalProxy != null) return globalProxy; if (network == null) { // Get the network associated with the calling UID. - final Network activeNetwork = getActiveNetworkForUidInternal(Binder.getCallingUid(), + final Network activeNetwork = getActiveNetworkForUidInternal(mDeps.getCallingUid(), true); if (activeNetwork == null) { return null; } return getLinkPropertiesProxyInfo(activeNetwork); - } else if (mDeps.queryUserAccess(Binder.getCallingUid(), network.getNetId())) { + } else if (mDeps.queryUserAccess(mDeps.getCallingUid(), network.getNetId())) { // Don't call getLinkProperties() as it requires ACCESS_NETWORK_STATE permission, which // caller may not have. return getLinkPropertiesProxyInfo(network); @@ -4612,7 +4616,7 @@ public class ConnectivityService extends IConnectivityManager.Stub */ @Override public ParcelFileDescriptor establishVpn(VpnConfig config) { - int user = UserHandle.getUserId(Binder.getCallingUid()); + int user = UserHandle.getUserId(mDeps.getCallingUid()); synchronized (mVpns) { throwIfLockdownEnabled(); return mVpns.get(user).establish(config); @@ -4633,7 +4637,7 @@ public class ConnectivityService extends IConnectivityManager.Stub */ @Override public boolean provisionVpnProfile(@NonNull VpnProfile profile, @NonNull String packageName) { - final int user = UserHandle.getUserId(Binder.getCallingUid()); + final int user = UserHandle.getUserId(mDeps.getCallingUid()); synchronized (mVpns) { return mVpns.get(user).provisionVpnProfile(packageName, profile, mKeyStore); } @@ -4651,7 +4655,7 @@ public class ConnectivityService extends IConnectivityManager.Stub */ @Override public void deleteVpnProfile(@NonNull String packageName) { - final int user = UserHandle.getUserId(Binder.getCallingUid()); + final int user = UserHandle.getUserId(mDeps.getCallingUid()); synchronized (mVpns) { mVpns.get(user).deleteVpnProfile(packageName, mKeyStore); } @@ -4668,7 +4672,7 @@ public class ConnectivityService extends IConnectivityManager.Stub */ @Override public void startVpnProfile(@NonNull String packageName) { - final int user = UserHandle.getUserId(Binder.getCallingUid()); + final int user = UserHandle.getUserId(mDeps.getCallingUid()); synchronized (mVpns) { throwIfLockdownEnabled(); mVpns.get(user).startVpnProfile(packageName, mKeyStore); @@ -4685,7 +4689,7 @@ public class ConnectivityService extends IConnectivityManager.Stub */ @Override public void stopVpnProfile(@NonNull String packageName) { - final int user = UserHandle.getUserId(Binder.getCallingUid()); + final int user = UserHandle.getUserId(mDeps.getCallingUid()); synchronized (mVpns) { mVpns.get(user).stopVpnProfile(packageName); } @@ -4697,7 +4701,7 @@ public class ConnectivityService extends IConnectivityManager.Stub */ @Override public void startLegacyVpn(VpnProfile profile) { - int user = UserHandle.getUserId(Binder.getCallingUid()); + int user = UserHandle.getUserId(mDeps.getCallingUid()); final LinkProperties egress = getActiveLinkProperties(); if (egress == null) { throw new IllegalStateException("Missing active network connection"); @@ -4846,7 +4850,7 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public boolean updateLockdownVpn() { - if (Binder.getCallingUid() != Process.SYSTEM_UID) { + if (mDeps.getCallingUid() != Process.SYSTEM_UID) { logw("Lockdown VPN only available to AID_SYSTEM"); return false; } @@ -4868,7 +4872,7 @@ public class ConnectivityService extends IConnectivityManager.Stub setLockdownTracker(null); return true; } - int user = UserHandle.getUserId(Binder.getCallingUid()); + int user = UserHandle.getUserId(mDeps.getCallingUid()); Vpn vpn = mVpns.get(user); if (vpn == null) { logw("VPN for user " + user + " not ready yet. Skipping lockdown"); @@ -5433,7 +5437,7 @@ public class ConnectivityService extends IConnectivityManager.Stub messenger = null; mBinder = null; mPid = getCallingPid(); - mUid = getCallingUid(); + mUid = mDeps.getCallingUid(); enforceRequestCountLimit(); } @@ -5445,7 +5449,7 @@ public class ConnectivityService extends IConnectivityManager.Stub ensureAllNetworkRequestsHaveType(mRequests); mBinder = binder; mPid = getCallingPid(); - mUid = getCallingUid(); + mUid = mDeps.getCallingUid(); mPendingIntent = null; enforceRequestCountLimit(); @@ -5588,7 +5592,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } private boolean checkUnsupportedStartingFrom(int version, String callingPackageName) { - final UserHandle user = UserHandle.getUserHandleForUid(Binder.getCallingUid()); + final UserHandle user = UserHandle.getUserHandleForUid(mDeps.getCallingUid()); final PackageManager pm = mContext.createContextAsUser(user, 0 /* flags */).getPackageManager(); try { @@ -5608,7 +5612,7 @@ public class ConnectivityService extends IConnectivityManager.Stub throw new SecurityException("Insufficient permissions to specify legacy type"); } } - final int callingUid = Binder.getCallingUid(); + final int callingUid = mDeps.getCallingUid(); final NetworkRequest.Type type = (networkCapabilities == null) ? NetworkRequest.Type.TRACK_DEFAULT : NetworkRequest.Type.REQUEST; @@ -5678,7 +5682,7 @@ public class ConnectivityService extends IConnectivityManager.Stub if (nai != null) { nai.asyncChannel.sendMessage(android.net.NetworkAgent.CMD_REQUEST_BANDWIDTH_UPDATE); synchronized (mBandwidthRequests) { - final int uid = Binder.getCallingUid(); + final int uid = mDeps.getCallingUid(); Integer uidReqs = mBandwidthRequests.get(uid); if (uidReqs == null) { uidReqs = 0; @@ -5695,7 +5699,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } private void enforceMeteredApnPolicy(NetworkCapabilities networkCapabilities) { - final int uid = Binder.getCallingUid(); + final int uid = mDeps.getCallingUid(); if (isSystem(uid)) { // Exemption for system uid. return; @@ -5715,7 +5719,7 @@ public class ConnectivityService extends IConnectivityManager.Stub PendingIntent operation, @NonNull String callingPackageName, @Nullable String callingAttributionTag) { Objects.requireNonNull(operation, "PendingIntent cannot be null."); - final int callingUid = Binder.getCallingUid(); + final int callingUid = mDeps.getCallingUid(); networkCapabilities = new NetworkCapabilities(networkCapabilities); enforceNetworkRequestPermissions(networkCapabilities, callingPackageName, callingAttributionTag); @@ -5774,7 +5778,7 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public NetworkRequest listenForNetwork(NetworkCapabilities networkCapabilities, Messenger messenger, IBinder binder, @NonNull String callingPackageName) { - final int callingUid = Binder.getCallingUid(); + final int callingUid = mDeps.getCallingUid(); if (!hasWifiNetworkListenPermission(networkCapabilities)) { enforceAccessPermission(); } @@ -5804,7 +5808,7 @@ public class ConnectivityService extends IConnectivityManager.Stub public void pendingListenForNetwork(NetworkCapabilities networkCapabilities, PendingIntent operation, @NonNull String callingPackageName) { Objects.requireNonNull(operation, "PendingIntent cannot be null."); - final int callingUid = Binder.getCallingUid(); + final int callingUid = mDeps.getCallingUid(); if (!hasWifiNetworkListenPermission(networkCapabilities)) { enforceAccessPermission(); } @@ -5905,7 +5909,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } else { enforceNetworkFactoryPermission(); } - mHandler.post(() -> handleReleaseNetworkRequest(request, Binder.getCallingUid(), true)); + mHandler.post(() -> handleReleaseNetworkRequest(request, mDeps.getCallingUid(), true)); } // NOTE: Accessed on multiple threads, must be synchronized on itself. @@ -5999,7 +6003,7 @@ public class ConnectivityService extends IConnectivityManager.Stub enforceNetworkFactoryPermission(); } - final int uid = Binder.getCallingUid(); + final int uid = mDeps.getCallingUid(); final long token = Binder.clearCallingIdentity(); try { return registerNetworkAgentInternal(messenger, networkInfo, linkProperties, @@ -7653,7 +7657,7 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public boolean addVpnAddress(String address, int prefixLength) { - int user = UserHandle.getUserId(Binder.getCallingUid()); + int user = UserHandle.getUserId(mDeps.getCallingUid()); synchronized (mVpns) { throwIfLockdownEnabled(); return mVpns.get(user).addAddress(address, prefixLength); @@ -7662,7 +7666,7 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public boolean removeVpnAddress(String address, int prefixLength) { - int user = UserHandle.getUserId(Binder.getCallingUid()); + int user = UserHandle.getUserId(mDeps.getCallingUid()); synchronized (mVpns) { throwIfLockdownEnabled(); return mVpns.get(user).removeAddress(address, prefixLength); @@ -7671,7 +7675,7 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public boolean setUnderlyingNetworksForVpn(Network[] networks) { - int user = UserHandle.getUserId(Binder.getCallingUid()); + int user = UserHandle.getUserId(mDeps.getCallingUid()); final boolean success; synchronized (mVpns) { throwIfLockdownEnabled(); @@ -7898,7 +7902,7 @@ public class ConnectivityService extends IConnectivityManager.Stub @GuardedBy("mVpns") private Vpn getVpnIfOwner() { - return getVpnIfOwner(Binder.getCallingUid()); + return getVpnIfOwner(mDeps.getCallingUid()); } @GuardedBy("mVpns") @@ -8376,7 +8380,7 @@ public class ConnectivityService extends IConnectivityManager.Stub throw new IllegalArgumentException("ConnectivityManager.TYPE_* are deprecated." + " Please use NetworkCapabilities instead."); } - final int callingUid = Binder.getCallingUid(); + final int callingUid = mDeps.getCallingUid(); mAppOpsManager.checkPackage(callingUid, callingPackageName); // This NetworkCapabilities is only used for matching to Networks. Clear out its owner uid @@ -8411,7 +8415,7 @@ public class ConnectivityService extends IConnectivityManager.Stub mConnectivityDiagnosticsHandler.obtainMessage( ConnectivityDiagnosticsHandler .EVENT_UNREGISTER_CONNECTIVITY_DIAGNOSTICS_CALLBACK, - Binder.getCallingUid(), + mDeps.getCallingUid(), 0, callback)); } @@ -8427,7 +8431,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } final NetworkAgentInfo nai = getNetworkAgentInfoForNetwork(network); - if (nai == null || nai.creatorUid != Binder.getCallingUid()) { + if (nai == null || nai.creatorUid != mDeps.getCallingUid()) { throw new SecurityException("Data Stall simulation is only possible for network " + "creators"); } diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index af1f75e0f1..21dbbc6fed 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -345,6 +345,7 @@ public class ConnectivityServiceTest { private MockContext mServiceContext; private HandlerThread mCsHandlerThread; + private ConnectivityService.Dependencies mDeps; private ConnectivityService mService; private WrappedConnectivityManager mCm; private TestNetworkAgentWrapper mWiFiNetworkAgent; @@ -1267,6 +1268,17 @@ public class ConnectivityServiceTest { fail("ConditionVariable was blocked for more than " + TIMEOUT_MS + "ms"); } + private void registerNetworkCallbackAsUid(NetworkRequest request, NetworkCallback callback, + int uid) { + when(mDeps.getCallingUid()).thenReturn(uid); + try { + mCm.registerNetworkCallback(request, callback); + waitForIdle(); + } finally { + returnRealCallingUid(); + } + } + private static final int VPN_USER = 0; private static final int APP1_UID = UserHandle.getUid(VPN_USER, 10100); private static final int APP2_UID = UserHandle.getUid(VPN_USER, 10101); @@ -1309,7 +1321,8 @@ public class ConnectivityServiceTest { initAlarmManager(mAlarmManager, mAlarmManagerThread.getThreadHandler()); mCsHandlerThread = new HandlerThread("TestConnectivityService"); - final ConnectivityService.Dependencies deps = makeDependencies(); + mDeps = makeDependencies(); + returnRealCallingUid(); mService = new ConnectivityService(mServiceContext, mNetworkManagementService, mStatsService, @@ -1317,9 +1330,9 @@ public class ConnectivityServiceTest { mMockDnsResolver, mock(IpConnectivityLog.class), mMockNetd, - deps); + mDeps); mService.mLingerDelayMs = TEST_LINGER_DELAY_MS; - verify(deps).makeMultinetworkPolicyTracker(any(), any(), any()); + verify(mDeps).makeMultinetworkPolicyTracker(any(), any(), any()); final ArgumentCaptor policyListenerCaptor = ArgumentCaptor.forClass(INetworkPolicyListener.class); @@ -1339,6 +1352,10 @@ public class ConnectivityServiceTest { setPrivateDnsSettings(PRIVATE_DNS_MODE_OFF, "ignored.example.com"); } + private void returnRealCallingUid() { + doAnswer((invocationOnMock) -> Binder.getCallingUid()).when(mDeps).getCallingUid(); + } + private ConnectivityService.Dependencies makeDependencies() { doReturn(TEST_TCP_INIT_RWND).when(mSystemProperties) .getInt("net.tcp.default_init_rwnd", 0); @@ -6362,6 +6379,7 @@ public class ConnectivityServiceTest { // Despite VPN using WiFi (which is unmetered), VPN itself is marked as always metered. assertTrue(mCm.isActiveNetworkMetered()); + // VPN explicitly declares WiFi as its underlying network. mService.setUnderlyingNetworksForVpn( new Network[] { mWiFiNetworkAgent.getNetwork() }); @@ -6511,6 +6529,10 @@ public class ConnectivityServiceTest { final TestNetworkCallback defaultCallback = new TestNetworkCallback(); mCm.registerDefaultNetworkCallback(defaultCallback); + final TestNetworkCallback vpnUidCallback = new TestNetworkCallback(); + final NetworkRequest vpnUidRequest = new NetworkRequest.Builder().build(); + registerNetworkCallbackAsUid(vpnUidRequest, vpnUidCallback, VPN_UID); + final int uid = Process.myUid(); final int userId = UserHandle.getUserId(uid); final ArrayList allowList = new ArrayList<>(); @@ -6526,6 +6548,7 @@ public class ConnectivityServiceTest { mWiFiNetworkAgent.connect(false /* validated */); callback.expectAvailableCallbacksUnvalidatedAndBlocked(mWiFiNetworkAgent); defaultCallback.expectAvailableCallbacksUnvalidatedAndBlocked(mWiFiNetworkAgent); + vpnUidCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetworkForUid(VPN_UID)); assertNull(mCm.getActiveNetwork()); assertActiveNetworkInfo(TYPE_WIFI, DetailedState.BLOCKED); @@ -6537,6 +6560,7 @@ public class ConnectivityServiceTest { // There are no callbacks because they are not implemented yet. mService.setAlwaysOnVpnPackage(userId, null, false /* lockdown */, allowList); expectNetworkRejectNonSecureVpn(inOrder, false, firstHalf, secondHalf); + vpnUidCallback.assertNoCallback(); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetworkForUid(VPN_UID)); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork()); assertActiveNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED); @@ -6546,7 +6570,9 @@ public class ConnectivityServiceTest { // Add our UID to the allowlist and re-enable lockdown, expect network is not blocked. allowList.add(TEST_PACKAGE_NAME); mService.setAlwaysOnVpnPackage(userId, ALWAYS_ON_PACKAGE, true /* lockdown */, allowList); + callback.assertNoCallback(); defaultCallback.assertNoCallback(); + vpnUidCallback.assertNoCallback(); // The following requires that the UID of this test package is greater than VPN_UID. This // is always true in practice because a plain AOSP build with no apps installed has almost @@ -6566,6 +6592,7 @@ public class ConnectivityServiceTest { mCellNetworkAgent.connect(false /* validated */); callback.expectAvailableCallbacksUnvalidated(mCellNetworkAgent); defaultCallback.assertNoCallback(); + vpnUidCallback.expectAvailableCallbacksUnvalidated(mCellNetworkAgent); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetworkForUid(VPN_UID)); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork()); assertActiveNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED); @@ -6580,6 +6607,7 @@ public class ConnectivityServiceTest { allowList.clear(); mService.setAlwaysOnVpnPackage(userId, ALWAYS_ON_PACKAGE, true /* lockdown */, allowList); expectNetworkRejectNonSecureVpn(inOrder, true, firstHalf, secondHalf); + vpnUidCallback.assertNoCallback(); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetworkForUid(VPN_UID)); assertNull(mCm.getActiveNetwork()); assertActiveNetworkInfo(TYPE_WIFI, DetailedState.BLOCKED); @@ -6588,6 +6616,7 @@ public class ConnectivityServiceTest { // Disable lockdown. Everything is unblocked. mService.setAlwaysOnVpnPackage(userId, null, false /* lockdown */, allowList); + vpnUidCallback.assertNoCallback(); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetworkForUid(VPN_UID)); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork()); assertActiveNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED); @@ -6600,6 +6629,7 @@ public class ConnectivityServiceTest { inOrder.verify(mMockNetd, never()).networkRejectNonSecureVpn(anyBoolean(), any()); callback.assertNoCallback(); defaultCallback.assertNoCallback(); + vpnUidCallback.assertNoCallback(); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetworkForUid(VPN_UID)); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork()); assertActiveNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED); @@ -6610,6 +6640,7 @@ public class ConnectivityServiceTest { inOrder.verify(mMockNetd, never()).networkRejectNonSecureVpn(anyBoolean(), any()); callback.assertNoCallback(); defaultCallback.assertNoCallback(); + vpnUidCallback.assertNoCallback(); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetworkForUid(VPN_UID)); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork()); assertActiveNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED); @@ -6618,6 +6649,7 @@ public class ConnectivityServiceTest { // Enable lockdown and connect a VPN. The VPN is not blocked. mService.setAlwaysOnVpnPackage(userId, ALWAYS_ON_PACKAGE, true /* lockdown */, allowList); + vpnUidCallback.assertNoCallback(); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetworkForUid(VPN_UID)); assertNull(mCm.getActiveNetwork()); assertActiveNetworkInfo(TYPE_WIFI, DetailedState.BLOCKED); @@ -6626,6 +6658,7 @@ public class ConnectivityServiceTest { mMockVpn.establishForMyUid(); defaultCallback.expectAvailableThenValidatedCallbacks(mMockVpn); + vpnUidCallback.assertNoCallback(); // vpnUidCallback has NOT_VPN capability. assertEquals(mMockVpn.getNetwork(), mCm.getActiveNetwork()); assertEquals(null, mCm.getActiveNetworkForUid(VPN_UID)); // BUG? assertActiveNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED); @@ -6640,6 +6673,7 @@ public class ConnectivityServiceTest { mCm.unregisterNetworkCallback(callback); mCm.unregisterNetworkCallback(defaultCallback); + mCm.unregisterNetworkCallback(vpnUidCallback); } @Test