From 611b80456aa1990d19c85940ca0dea40565d0cf1 Mon Sep 17 00:00:00 2001 From: Ying Xu Date: Tue, 27 Feb 2024 10:07:43 -0800 Subject: [PATCH] Make the subscriptionIds and allowedUids public This CL allows everyone to see the subscriptionIds and the network owner to see the allowedUids from the NetworkCapabilities. Bug: 324357121 Test: atest ConnectivityManagerTest atest ConnectivityCoverageTests:android.net.connectivity.com.android.server.ConnectivityServiceTest Change-Id: I91e360827d2a9a64625f1d6ae0f808aa327c7d22 --- .../android/server/ConnectivityService.java | 13 +--- .../server/ConnectivityServiceTest.java | 75 +++++++++++-------- 2 files changed, 45 insertions(+), 43 deletions(-) diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 6839c22d06..cc40f4d466 100755 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -2769,6 +2769,7 @@ public class ConnectivityService extends IConnectivityManager.Stub private boolean canSeeAllowedUids(final int pid, final int uid, final int netOwnerUid) { return Process.SYSTEM_UID == uid + || netOwnerUid == uid || hasAnyPermissionOf(mContext, pid, uid, android.Manifest.permission.NETWORK_FACTORY); } @@ -2796,7 +2797,6 @@ public class ConnectivityService extends IConnectivityManager.Stub } if (!canSeeAllowedUids(callerPid, callerUid, newNc.getOwnerUid())) { newNc.setAllowedUids(new ArraySet<>()); - newNc.setSubscriptionIds(Collections.emptySet()); } redactUnderlyingNetworksForCapabilities(newNc, callerPid, callerUid); @@ -7572,15 +7572,6 @@ public class ConnectivityService extends IConnectivityManager.Stub "Insufficient permissions to request a specific signal strength"); } mAppOpsManager.checkPackage(callerUid, callerPackageName); - - if (nc.getSubscriptionIds().isEmpty()) { - return; - } - if (mRequestRestrictedWifiEnabled - && canRequestRestrictedNetworkDueToCarrierPrivileges(nc, callerUid)) { - return; - } - enforceNetworkFactoryPermission(); } private int[] getSignalStrengthThresholds(@NonNull final NetworkAgentInfo nai) { @@ -9162,7 +9153,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // 3. The app doesn't have Carrier Privileges // 4. The app doesn't have permission.CONNECTIVITY_USE_RESTRICTED_NETWORKS for (final NetworkRequest nr : mNetworkRequests.keySet()) { - if ((nr.isRequest() || nr.isListen()) + if (nr.isRequest() && !nr.hasCapability(NET_CAPABILITY_NOT_RESTRICTED) && nr.getRequestorUid() == uid && getSubscriptionIdFromNetworkCaps(nr.networkCapabilities) == subId diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index c5340257e2..5c09c23dbc 100755 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -17341,21 +17341,7 @@ public class ConnectivityServiceTest { } @Test - public void testSubIdsClearedWithoutNetworkFactoryPermission() throws Exception { - mServiceContext.setPermission(NETWORK_FACTORY, PERMISSION_DENIED); - final NetworkCapabilities nc = new NetworkCapabilities(); - nc.setSubscriptionIds(Collections.singleton(Process.myUid())); - - final NetworkCapabilities result = - mService.networkCapabilitiesRestrictedForCallerPermissions( - nc, Process.myPid(), Process.myUid()); - assertTrue(result.getSubscriptionIds().isEmpty()); - } - - @Test - public void testSubIdsExistWithNetworkFactoryPermission() throws Exception { - mServiceContext.setPermission(NETWORK_FACTORY, PERMISSION_GRANTED); - + public void testSubIdsExist() throws Exception { final Set subIds = Collections.singleton(Process.myUid()); final NetworkCapabilities nc = new NetworkCapabilities(); nc.setSubscriptionIds(subIds); @@ -17381,8 +17367,7 @@ public class ConnectivityServiceTest { } @Test - public void testNetworkRequestWithSubIdsWithNetworkFactoryPermission() throws Exception { - mServiceContext.setPermission(NETWORK_FACTORY, PERMISSION_GRANTED); + public void testNetworkRequestWithSubIds() throws Exception { final PendingIntent pendingIntent = PendingIntent.getBroadcast( mContext, 0 /* requestCode */, new Intent("a"), FLAG_IMMUTABLE); final NetworkCallback networkCallback1 = new NetworkCallback(); @@ -17397,21 +17382,6 @@ public class ConnectivityServiceTest { mCm.unregisterNetworkCallback(networkCallback2); } - @Test - public void testNetworkRequestWithSubIdsWithoutNetworkFactoryPermission() throws Exception { - mServiceContext.setPermission(NETWORK_FACTORY, PERMISSION_DENIED); - final PendingIntent pendingIntent = PendingIntent.getBroadcast( - mContext, 0 /* requestCode */, new Intent("a"), FLAG_IMMUTABLE); - - final Class expected = SecurityException.class; - assertThrows( - expected, () -> mCm.requestNetwork(getRequestWithSubIds(), new NetworkCallback())); - assertThrows(expected, () -> mCm.requestNetwork(getRequestWithSubIds(), pendingIntent)); - assertThrows( - expected, - () -> mCm.registerNetworkCallback(getRequestWithSubIds(), new NetworkCallback())); - } - @Test @IgnoreUpTo(Build.VERSION_CODES.TIRAMISU) public void testCarrierConfigAppSendNetworkRequestForRestrictedWifi() throws Exception { @@ -17547,6 +17517,47 @@ public class ConnectivityServiceTest { false /* expectUnavailable */, true /* expectCapChanged */); } + + @Test + public void testAllowedUidsExistWithoutNetworkFactoryPermission() throws Exception { + // Make sure NETWORK_FACTORY permission is not granted. + mServiceContext.setPermission(NETWORK_FACTORY, PERMISSION_DENIED); + mServiceContext.setPermission(MANAGE_TEST_NETWORKS, PERMISSION_GRANTED); + final TestNetworkCallback cb = new TestNetworkCallback(); + mCm.requestNetwork(new NetworkRequest.Builder() + .clearCapabilities() + .addTransportType(TRANSPORT_TEST) + .addTransportType(TRANSPORT_CELLULAR) + .build(), + cb); + + final ArraySet uids = new ArraySet<>(); + uids.add(200); + final NetworkCapabilities nc = new NetworkCapabilities.Builder() + .addTransportType(TRANSPORT_TEST) + .removeCapability(NET_CAPABILITY_NOT_RESTRICTED) + .setAllowedUids(uids) + .setOwnerUid(Process.myUid()) + .setAdministratorUids(new int[] {Process.myUid()}) + .build(); + final TestNetworkAgentWrapper agent = new TestNetworkAgentWrapper(TRANSPORT_TEST, + new LinkProperties(), nc); + agent.connect(true); + cb.expectAvailableThenValidatedCallbacks(agent); + + uids.add(300); + uids.add(400); + nc.setAllowedUids(uids); + agent.setNetworkCapabilities(nc, true /* sendToConnectivityService */); + if (mDeps.isAtLeastT()) { + // AllowedUids is not cleared even without the NETWORK_FACTORY permission + // because the caller is the owner of the network. + cb.expectCaps(agent, c -> c.getAllowedUids().equals(uids)); + } else { + cb.assertNoCallback(); + } + } + @Test public void testAllowedUids() throws Exception { final int preferenceOrder =