diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index fccb2a68b9..078fff0bfb 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -610,13 +610,6 @@ public class ConnectivityService extends IConnectivityManager.Stub // Handle private DNS validation status updates. private static final int EVENT_PRIVATE_DNS_VALIDATION_UPDATE = 38; - /** - * used to remove a network request, either a listener or a real request and call unavailable - * arg1 = UID of caller - * obj = NetworkRequest - */ - private static final int EVENT_RELEASE_NETWORK_REQUEST_AND_CALL_UNAVAILABLE = 39; - /** * Event for NetworkMonitor/NetworkAgentInfo to inform ConnectivityService that the network has * been tested. @@ -2628,7 +2621,7 @@ public class ConnectivityService extends IConnectivityManager.Stub verifyCallingUidAndPackage(callingPackageName, mDeps.getCallingUid()); enforceChangePermission(callingPackageName, callingAttributionTag); if (mProtectedNetworks.contains(networkType)) { - enforceConnectivityRestrictedNetworksPermission(); + enforceConnectivityRestrictedNetworksPermission(true /* checkUidsAllowedList */); } InetAddress addr; @@ -2982,18 +2975,35 @@ public class ConnectivityService extends IConnectivityManager.Stub android.Manifest.permission.NETWORK_SETTINGS); } - private void enforceConnectivityRestrictedNetworksPermission() { - try { - mContext.enforceCallingOrSelfPermission( - android.Manifest.permission.CONNECTIVITY_USE_RESTRICTED_NETWORKS, - "ConnectivityService"); - return; - } catch (SecurityException e) { /* fallback to ConnectivityInternalPermission */ } - // TODO: Remove this fallback check after all apps have declared - // CONNECTIVITY_USE_RESTRICTED_NETWORKS. - mContext.enforceCallingOrSelfPermission( - android.Manifest.permission.CONNECTIVITY_INTERNAL, - "ConnectivityService"); + private boolean checkConnectivityRestrictedNetworksPermission(int callingUid, + boolean checkUidsAllowedList) { + if (PermissionUtils.checkAnyPermissionOf(mContext, + android.Manifest.permission.CONNECTIVITY_USE_RESTRICTED_NETWORKS)) { + return true; + } + + // fallback to ConnectivityInternalPermission + // TODO: Remove this fallback check after all apps have declared + // CONNECTIVITY_USE_RESTRICTED_NETWORKS. + if (PermissionUtils.checkAnyPermissionOf(mContext, + android.Manifest.permission.CONNECTIVITY_INTERNAL)) { + return true; + } + + // Check whether uid is in allowed on restricted networks list. + if (checkUidsAllowedList + && mPermissionMonitor.isUidAllowedOnRestrictedNetworks(callingUid)) { + return true; + } + return false; + } + + private void enforceConnectivityRestrictedNetworksPermission(boolean checkUidsAllowedList) { + final int callingUid = mDeps.getCallingUid(); + if (!checkConnectivityRestrictedNetworksPermission(callingUid, checkUidsAllowedList)) { + throw new SecurityException("ConnectivityService: user " + callingUid + + " has no permission to access restricted network."); + } } private void enforceKeepalivePermission() { @@ -4495,7 +4505,7 @@ public class ConnectivityService extends IConnectivityManager.Stub private boolean hasCarrierPrivilegeForNetworkCaps(final int callingUid, @NonNull final NetworkCapabilities caps) { - if (SdkLevel.isAtLeastT() && mCarrierPrivilegeAuthenticator != null) { + if (mCarrierPrivilegeAuthenticator != null) { return mCarrierPrivilegeAuthenticator.hasCarrierPrivilegeForNetworkCapabilities( callingUid, caps); } @@ -4525,7 +4535,6 @@ public class ConnectivityService extends IConnectivityManager.Stub private void handleRegisterNetworkRequests(@NonNull final Set nris) { ensureRunningOnConnectivityServiceThread(); - NetworkRequest requestToBeReleased = null; for (final NetworkRequestInfo nri : nris) { mNetworkRequestInfoLogs.log("REGISTER " + nri); checkNrisConsistency(nri); @@ -4540,13 +4549,6 @@ public class ConnectivityService extends IConnectivityManager.Stub } } } - if (req.hasCapability(NetworkCapabilities.NET_CAPABILITY_CBS)) { - if (!hasCarrierPrivilegeForNetworkCaps(nri.mUid, req.networkCapabilities) - && !checkConnectivityRestrictedNetworksPermission( - nri.mPid, nri.mUid)) { - requestToBeReleased = req; - } - } } // If this NRI has a satisfier already, it is replacing an older request that @@ -4558,11 +4560,6 @@ public class ConnectivityService extends IConnectivityManager.Stub } } - if (requestToBeReleased != null) { - releaseNetworkRequestAndCallOnUnavailable(requestToBeReleased); - return; - } - if (mFlags.noRematchAllRequestsOnRegister()) { rematchNetworksAndRequests(nris); } else { @@ -5402,11 +5399,6 @@ public class ConnectivityService extends IConnectivityManager.Stub /* callOnUnavailable */ false); break; } - case EVENT_RELEASE_NETWORK_REQUEST_AND_CALL_UNAVAILABLE: { - handleReleaseNetworkRequest((NetworkRequest) msg.obj, msg.arg1, - /* callOnUnavailable */ true); - break; - } case EVENT_SET_ACCEPT_UNVALIDATED: { Network network = (Network) msg.obj; handleSetAcceptUnvalidated(network, toBool(msg.arg1), toBool(msg.arg2)); @@ -6625,7 +6617,7 @@ public class ConnectivityService extends IConnectivityManager.Stub case REQUEST: networkCapabilities = new NetworkCapabilities(networkCapabilities); enforceNetworkRequestPermissions(networkCapabilities, callingPackageName, - callingAttributionTag); + callingAttributionTag, callingUid); // TODO: this is incorrect. We mark the request as metered or not depending on // the state of the app when the request is filed, but we never change the // request if the app changes network state. http://b/29964605 @@ -6715,26 +6707,19 @@ public class ConnectivityService extends IConnectivityManager.Stub } private void enforceNetworkRequestPermissions(NetworkCapabilities networkCapabilities, - String callingPackageName, String callingAttributionTag) { + String callingPackageName, String callingAttributionTag, final int callingUid) { if (networkCapabilities.hasCapability(NET_CAPABILITY_NOT_RESTRICTED) == false) { - if (!networkCapabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_CBS)) { - enforceConnectivityRestrictedNetworksPermission(); + // For T+ devices, callers with carrier privilege could request with CBS capabilities. + if (networkCapabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_CBS) + && hasCarrierPrivilegeForNetworkCaps(callingUid, networkCapabilities)) { + return; } + enforceConnectivityRestrictedNetworksPermission(true /* checkUidsAllowedList */); } else { enforceChangePermission(callingPackageName, callingAttributionTag); } } - private boolean checkConnectivityRestrictedNetworksPermission(int callerPid, int callerUid) { - if (checkAnyPermissionOf(callerPid, callerUid, - android.Manifest.permission.CONNECTIVITY_USE_RESTRICTED_NETWORKS) - || checkAnyPermissionOf(callerPid, callerUid, - android.Manifest.permission.CONNECTIVITY_INTERNAL)) { - return true; - } - return false; - } - @Override public boolean requestBandwidthUpdate(Network network) { enforceAccessPermission(); @@ -6793,7 +6778,7 @@ public class ConnectivityService extends IConnectivityManager.Stub final int callingUid = mDeps.getCallingUid(); networkCapabilities = new NetworkCapabilities(networkCapabilities); enforceNetworkRequestPermissions(networkCapabilities, callingPackageName, - callingAttributionTag); + callingAttributionTag, callingUid); enforceMeteredApnPolicy(networkCapabilities); ensureRequestableCapabilities(networkCapabilities); ensureSufficientPermissionsForRequest(networkCapabilities, @@ -6916,13 +6901,6 @@ public class ConnectivityService extends IConnectivityManager.Stub EVENT_RELEASE_NETWORK_REQUEST, mDeps.getCallingUid(), 0, networkRequest)); } - private void releaseNetworkRequestAndCallOnUnavailable(NetworkRequest networkRequest) { - ensureNetworkRequestHasType(networkRequest); - mHandler.sendMessage(mHandler.obtainMessage( - EVENT_RELEASE_NETWORK_REQUEST_AND_CALL_UNAVAILABLE, mDeps.getCallingUid(), 0, - networkRequest)); - } - private void handleRegisterNetworkProvider(NetworkProviderInfo npi) { if (mNetworkProviderInfos.containsKey(npi.messenger)) { // Avoid creating duplicates. even if an app makes a direct AIDL call. @@ -10625,7 +10603,11 @@ public class ConnectivityService extends IConnectivityManager.Stub if (callback == null) throw new IllegalArgumentException("callback must be non-null"); if (!nai.networkCapabilities.hasCapability(NET_CAPABILITY_NOT_RESTRICTED)) { - enforceConnectivityRestrictedNetworksPermission(); + // TODO: Check allowed list here and ensure that either a) any QoS callback registered + // on this network is unregistered when the app loses permission or b) no QoS + // callbacks are sent for restricted networks unless the app currently has permission + // to access restricted networks. + enforceConnectivityRestrictedNetworksPermission(false /* checkUidsAllowedList */); } mQosCallbackTracker.registerCallback(callback, filter, nai); } diff --git a/service/src/com/android/server/connectivity/PermissionMonitor.java b/service/src/com/android/server/connectivity/PermissionMonitor.java index c02d9cfc7a..8d99cb477a 100755 --- a/service/src/com/android/server/connectivity/PermissionMonitor.java +++ b/service/src/com/android/server/connectivity/PermissionMonitor.java @@ -421,7 +421,14 @@ public class PermissionMonitor { if (appInfo == null) return false; // Check whether package's uid is in allowed on restricted networks uid list. If so, this // uid can have netd system permission. - return mUidsAllowedOnRestrictedNetworks.contains(appInfo.uid); + return isUidAllowedOnRestrictedNetworks(appInfo.uid); + } + + /** + * Returns whether the given uid is in allowed on restricted networks list. + */ + public synchronized boolean isUidAllowedOnRestrictedNetworks(final int uid) { + return mUidsAllowedOnRestrictedNetworks.contains(uid); } @VisibleForTesting diff --git a/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java b/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java index a2c7d4a4e8..8f6786f151 100644 --- a/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java +++ b/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java @@ -52,6 +52,7 @@ import static android.net.ConnectivityManager.TYPE_MOBILE_SUPL; import static android.net.ConnectivityManager.TYPE_PROXY; import static android.net.ConnectivityManager.TYPE_VPN; import static android.net.ConnectivityManager.TYPE_WIFI_P2P; +import static android.net.ConnectivitySettingsManager.setUidsAllowedOnRestrictedNetworks; import static android.net.NetworkCapabilities.NET_CAPABILITY_FOREGROUND; import static android.net.NetworkCapabilities.NET_CAPABILITY_IMS; import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET; @@ -3168,7 +3169,7 @@ public class ConnectivityManagerTest { @AppModeFull(reason = "WRITE_SECURE_SETTINGS permission can't be granted to instant apps") @Test public void testUidsAllowedOnRestrictedNetworks() throws Exception { - assumeTrue(TestUtils.shouldTestSApis()); + assumeTestSApis(); // TODO (b/175199465): figure out a reasonable permission check for // setUidsAllowedOnRestrictedNetworks that allows tests but not system-external callers. @@ -3181,10 +3182,10 @@ public class ConnectivityManagerTest { // because it has been just installed to device. In case the uid is existed in setting // mistakenly, try to remove the uid and set correct uids to setting. originalUidsAllowedOnRestrictedNetworks.remove(uid); - runWithShellPermissionIdentity(() -> - ConnectivitySettingsManager.setUidsAllowedOnRestrictedNetworks( - mContext, originalUidsAllowedOnRestrictedNetworks), NETWORK_SETTINGS); + runWithShellPermissionIdentity(() -> setUidsAllowedOnRestrictedNetworks( + mContext, originalUidsAllowedOnRestrictedNetworks), NETWORK_SETTINGS); + // File a restricted network request with permission first to hold the connection. final TestableNetworkCallback testNetworkCb = new TestableNetworkCallback(); final NetworkRequest testRequest = new NetworkRequest.Builder() .addTransportType(NetworkCapabilities.TRANSPORT_TEST) @@ -3196,6 +3197,19 @@ public class ConnectivityManagerTest { runWithShellPermissionIdentity(() -> requestNetwork(testRequest, testNetworkCb), CONNECTIVITY_USE_RESTRICTED_NETWORKS); + // File another restricted network request without permission. + final TestableNetworkCallback restrictedNetworkCb = new TestableNetworkCallback(); + final NetworkRequest restrictedRequest = new NetworkRequest.Builder() + .addTransportType(NetworkCapabilities.TRANSPORT_TEST) + .removeCapability(NetworkCapabilities.NET_CAPABILITY_TRUSTED) + .removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED) + .setNetworkSpecifier(CompatUtil.makeTestNetworkSpecifier( + TEST_RESTRICTED_NW_IFACE_NAME)) + .build(); + // Uid is not in allowed list and no permissions. Expect that SecurityException will throw. + assertThrows(SecurityException.class, + () -> mCm.requestNetwork(restrictedRequest, restrictedNetworkCb)); + final NetworkAgent agent = createRestrictedNetworkAgent(mContext); final Network network = agent.getNetwork(); @@ -3215,19 +3229,26 @@ public class ConnectivityManagerTest { final Set newUidsAllowedOnRestrictedNetworks = new ArraySet<>(originalUidsAllowedOnRestrictedNetworks); newUidsAllowedOnRestrictedNetworks.add(uid); - runWithShellPermissionIdentity(() -> - ConnectivitySettingsManager.setUidsAllowedOnRestrictedNetworks( - mContext, newUidsAllowedOnRestrictedNetworks), NETWORK_SETTINGS); + runWithShellPermissionIdentity(() -> setUidsAllowedOnRestrictedNetworks( + mContext, newUidsAllowedOnRestrictedNetworks), NETWORK_SETTINGS); // Wait a while for sending allowed uids on the restricted network to netd. - // TODD: Have a significant signal to know the uids has been send to netd. + // TODD: Have a significant signal to know the uids has been sent to netd. assertBindSocketToNetworkSuccess(network); + + // Uid is in allowed list. Try file network request again. + requestNetwork(restrictedRequest, restrictedNetworkCb); + // Verify that the network is restricted. + restrictedNetworkCb.eventuallyExpect(CallbackEntry.NETWORK_CAPS_UPDATED, + NETWORK_CALLBACK_TIMEOUT_MS, + entry -> network.equals(entry.getNetwork()) + && (!((CallbackEntry.CapabilitiesChanged) entry).getCaps() + .hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED))); } finally { agent.unregister(); // Restore setting. - runWithShellPermissionIdentity(() -> - ConnectivitySettingsManager.setUidsAllowedOnRestrictedNetworks( - mContext, originalUidsAllowedOnRestrictedNetworks), NETWORK_SETTINGS); + runWithShellPermissionIdentity(() -> setUidsAllowedOnRestrictedNetworks( + mContext, originalUidsAllowedOnRestrictedNetworks), NETWORK_SETTINGS); } } @@ -3251,6 +3272,12 @@ public class ConnectivityManagerTest { assertTrue(dumpOutput, dumpOutput.contains("BPF map content")); } + private void assumeTestSApis() { + // Cannot use @IgnoreUpTo(Build.VERSION_CODES.R) because this test also requires API 31 + // shims, and @IgnoreUpTo does not check that. + assumeTrue(TestUtils.shouldTestSApis()); + } + private void unregisterRegisteredCallbacks() { for (NetworkCallback callback: mRegisteredCallbacks) { mCm.unregisterNetworkCallback(callback); diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index 8c6e3a184d..39a82bd39a 100644 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -5875,7 +5875,7 @@ public class ConnectivityServiceTest { } /** - * Validate the callback flow CBS request without carrier privilege. + * Validate the service throws if request with CBS but without carrier privilege. */ @Test public void testCBSRequestWithoutCarrierPrivilege() throws Exception { @@ -5884,10 +5884,8 @@ public class ConnectivityServiceTest { final TestNetworkCallback networkCallback = new TestNetworkCallback(); mServiceContext.setPermission(CONNECTIVITY_USE_RESTRICTED_NETWORKS, PERMISSION_DENIED); - // Now file the test request and expect it. - mCm.requestNetwork(nr, networkCallback); - networkCallback.expectCallback(CallbackEntry.UNAVAILABLE, (Network) null); - mCm.unregisterNetworkCallback(networkCallback); + // Now file the test request and expect the service throws. + assertThrows(SecurityException.class, () -> mCm.requestNetwork(nr, networkCallback)); } private static class TestKeepaliveCallback extends PacketKeepaliveCallback {