diff options
5 files changed, 94 insertions, 80 deletions
diff --git a/Tethering/apex/manifest.json b/Tethering/apex/manifest.json index 40108a4c20..6bb324f504 100644 --- a/Tethering/apex/manifest.json +++ b/Tethering/apex/manifest.json @@ -1,4 +1,4 @@ { "name": "com.android.tethering", - "version": 311811040 + "version": 311811050 } diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index d79bdb8320..ff4c370f7a 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -608,13 +608,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. @@ -2613,7 +2606,7 @@ public class ConnectivityService extends IConnectivityManager.Stub verifyCallingUidAndPackage(callingPackageName, mDeps.getCallingUid()); enforceChangePermission(callingPackageName, callingAttributionTag); if (mProtectedNetworks.contains(networkType)) { - enforceConnectivityRestrictedNetworksPermission(); + enforceConnectivityRestrictedNetworksPermission(true /* checkUidsAllowedList */); } InetAddress addr; @@ -2967,18 +2960,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() { @@ -4464,7 +4474,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); } @@ -4494,7 +4504,6 @@ public class ConnectivityService extends IConnectivityManager.Stub private void handleRegisterNetworkRequests(@NonNull final Set<NetworkRequestInfo> nris) { ensureRunningOnConnectivityServiceThread(); - NetworkRequest requestToBeReleased = null; for (final NetworkRequestInfo nri : nris) { mNetworkRequestInfoLogs.log("REGISTER " + nri); checkNrisConsistency(nri); @@ -4509,13 +4518,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 @@ -4527,11 +4529,6 @@ public class ConnectivityService extends IConnectivityManager.Stub } } - if (requestToBeReleased != null) { - releaseNetworkRequestAndCallOnUnavailable(requestToBeReleased); - return; - } - if (mFlags.noRematchAllRequestsOnRegister()) { rematchNetworksAndRequests(nris); } else { @@ -5371,11 +5368,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)); @@ -6594,7 +6586,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 @@ -6684,26 +6676,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(); @@ -6762,7 +6747,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, @@ -6885,13 +6870,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. @@ -10593,7 +10571,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 62b3add8f5..cc1ad7a6d7 100755 --- a/service/src/com/android/server/connectivity/PermissionMonitor.java +++ b/service/src/com/android/server/connectivity/PermissionMonitor.java @@ -419,7 +419,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 d40bc9fd51..b24ceec76e 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; @@ -3195,7 +3196,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. @@ -3208,10 +3209,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) @@ -3223,6 +3224,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(); @@ -3242,19 +3256,26 @@ public class ConnectivityManagerTest { final Set<Integer> 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); } } @@ -3278,6 +3299,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 4c768030f8..0edfbc790a 100644 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -5851,7 +5851,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 { @@ -5860,10 +5860,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 { |