diff --git a/core/java/android/net/ConnectivityDiagnosticsManager.java b/core/java/android/net/ConnectivityDiagnosticsManager.java index 275e38c744..704f31d7f7 100644 --- a/core/java/android/net/ConnectivityDiagnosticsManager.java +++ b/core/java/android/net/ConnectivityDiagnosticsManager.java @@ -711,6 +711,13 @@ public class ConnectivityDiagnosticsManager { * not currently registered. If a ConnectivityDiagnosticsCallback instance is registered with * multiple NetworkRequests, an IllegalArgumentException will be thrown. * + *

To avoid performance issues due to apps leaking callbacks, the system will limit the + * number of outstanding requests to 100 per app (identified by their UID), shared with + * callbacks in {@link ConnectivityManager}. Registering a callback with this method will count + * toward this limit. If this limit is exceeded, an exception will be thrown. To avoid hitting + * this issue and to conserve resources, make sure to unregister the callbacks with + * {@link #unregisterConnectivityDiagnosticsCallback}. + * * @param request The NetworkRequest that will be used to match with Networks for which * callbacks will be fired * @param e The Executor to be used for running the callback method invocations @@ -718,6 +725,7 @@ public class ConnectivityDiagnosticsManager { * System * @throws IllegalArgumentException if the same callback instance is registered with multiple * NetworkRequests + * @throws RuntimeException if the app already has too many callbacks registered. */ public void registerConnectivityDiagnosticsCallback( @NonNull NetworkRequest request, diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index 36ffe50ef8..a29f878260 100644 --- a/core/java/android/net/ConnectivityManager.java +++ b/core/java/android/net/ConnectivityManager.java @@ -2246,26 +2246,6 @@ public class ConnectivityManager { .getPackageNameForUid(context, uid), true /* throwException */); } - /** {@hide} */ - public static final void enforceTetherChangePermission(Context context, String callingPkg) { - Preconditions.checkNotNull(context, "Context cannot be null"); - Preconditions.checkNotNull(callingPkg, "callingPkg cannot be null"); - - if (context.getResources().getStringArray( - com.android.internal.R.array.config_mobile_hotspot_provision_app).length == 2) { - // Have a provisioning app - must only let system apps (which check this app) - // turn on tethering - context.enforceCallingOrSelfPermission( - android.Manifest.permission.TETHER_PRIVILEGED, "ConnectivityService"); - } else { - int uid = Binder.getCallingUid(); - // If callingPkg's uid is not same as Binder.getCallingUid(), - // AppOpsService throws SecurityException. - Settings.checkAndNoteWriteSettingsOperation(context, uid, callingPkg, - true /* throwException */); - } - } - /** * @deprecated - use getSystemService. This is a kludge to support static access in certain * situations where a Context pointer is unavailable. @@ -3814,13 +3794,22 @@ public class ConnectivityManager { * or the ability to modify system settings as determined by * {@link android.provider.Settings.System#canWrite}.

* + *

To avoid performance issues due to apps leaking callbacks, the system will limit the + * number of outstanding requests to 100 per app (identified by their UID), shared with + * all variants of this method, of {@link #registerNetworkCallback} as well as + * {@link ConnectivityDiagnosticsManager#registerConnectivityDiagnosticsCallback}. + * Requesting a network with this method will count toward this limit. If this limit is + * exceeded, an exception will be thrown. To avoid hitting this issue and to conserve resources, + * make sure to unregister the callbacks with + * {@link #unregisterNetworkCallback(NetworkCallback)}. + * * @param request {@link NetworkRequest} describing this request. * @param networkCallback The {@link NetworkCallback} to be utilized for this request. Note * the callback must not be shared - it uniquely specifies this request. * The callback is invoked on the default internal Handler. * @throws IllegalArgumentException if {@code request} contains invalid network capabilities. * @throws SecurityException if missing the appropriate permissions. - * @throws RuntimeException if request limit per UID is exceeded. + * @throws RuntimeException if the app already has too many callbacks registered. */ public void requestNetwork(@NonNull NetworkRequest request, @NonNull NetworkCallback networkCallback) { @@ -3834,8 +3823,8 @@ public class ConnectivityManager { * but runs all the callbacks on the passed Handler. * *

This method has the same permission requirements as - * {@link #requestNetwork(NetworkRequest, NetworkCallback)} and throws the same exceptions in - * the same conditions. + * {@link #requestNetwork(NetworkRequest, NetworkCallback)}, is subject to the same limitations, + * and throws the same exceptions in the same conditions. * * @param request {@link NetworkRequest} describing this request. * @param networkCallback The {@link NetworkCallback} to be utilized for this request. Note @@ -3866,8 +3855,8 @@ public class ConnectivityManager { * for that purpose. Calling this method will attempt to bring up the requested network. * *

This method has the same permission requirements as - * {@link #requestNetwork(NetworkRequest, NetworkCallback)} and throws the same exceptions in - * the same conditions. + * {@link #requestNetwork(NetworkRequest, NetworkCallback)}, is subject to the same limitations, + * and throws the same exceptions in the same conditions. * * @param request {@link NetworkRequest} describing this request. * @param networkCallback The {@link NetworkCallback} to be utilized for this request. Note @@ -3893,8 +3882,8 @@ public class ConnectivityManager { * on the passed Handler. * *

This method has the same permission requirements as - * {@link #requestNetwork(NetworkRequest, NetworkCallback, int)} and throws the same exceptions - * in the same conditions. + * {@link #requestNetwork(NetworkRequest, NetworkCallback)}, is subject to the same limitations, + * and throws the same exceptions in the same conditions. * * @param request {@link NetworkRequest} describing this request. * @param networkCallback The {@link NetworkCallback} to be utilized for this request. Note @@ -3963,6 +3952,15 @@ public class ConnectivityManager { * is unknown prior to bringing up the network so the framework does not * know how to go about satisfying a request with these capabilities. * + *

To avoid performance issues due to apps leaking callbacks, the system will limit the + * number of outstanding requests to 100 per app (identified by their UID), shared with + * all variants of this method, of {@link #registerNetworkCallback} as well as + * {@link ConnectivityDiagnosticsManager#registerConnectivityDiagnosticsCallback}. + * Requesting a network with this method will count toward this limit. If this limit is + * exceeded, an exception will be thrown. To avoid hitting this issue and to conserve resources, + * make sure to unregister the callbacks with {@link #unregisterNetworkCallback(PendingIntent)} + * or {@link #releaseNetworkRequest(PendingIntent)}. + * *

This method requires the caller to hold either the * {@link android.Manifest.permission#CHANGE_NETWORK_STATE} permission * or the ability to modify system settings as determined by @@ -3974,7 +3972,7 @@ public class ConnectivityManager { * comes from {@link PendingIntent#getBroadcast}. Cannot be null. * @throws IllegalArgumentException if {@code request} contains invalid network capabilities. * @throws SecurityException if missing the appropriate permissions. - * @throws RuntimeException if request limit per UID is exceeded. + * @throws RuntimeException if the app already has too many callbacks registered. */ public void requestNetwork(@NonNull NetworkRequest request, @NonNull PendingIntent operation) { @@ -4030,10 +4028,20 @@ public class ConnectivityManager { * either the application exits or {@link #unregisterNetworkCallback(NetworkCallback)} is * called. * + *

To avoid performance issues due to apps leaking callbacks, the system will limit the + * number of outstanding requests to 100 per app (identified by their UID), shared with + * all variants of this method, of {@link #requestNetwork} as well as + * {@link ConnectivityDiagnosticsManager#registerConnectivityDiagnosticsCallback}. + * Requesting a network with this method will count toward this limit. If this limit is + * exceeded, an exception will be thrown. To avoid hitting this issue and to conserve resources, + * make sure to unregister the callbacks with + * {@link #unregisterNetworkCallback(NetworkCallback)}. + * * @param request {@link NetworkRequest} describing this request. * @param networkCallback The {@link NetworkCallback} that the system will call as suitable * networks change state. * The callback is invoked on the default internal Handler. + * @throws RuntimeException if the app already has too many callbacks registered. */ @RequiresPermission(android.Manifest.permission.ACCESS_NETWORK_STATE) public void registerNetworkCallback(@NonNull NetworkRequest request, @@ -4047,10 +4055,21 @@ public class ConnectivityManager { * either the application exits or {@link #unregisterNetworkCallback(NetworkCallback)} is * called. * + *

To avoid performance issues due to apps leaking callbacks, the system will limit the + * number of outstanding requests to 100 per app (identified by their UID), shared with + * all variants of this method, of {@link #requestNetwork} as well as + * {@link ConnectivityDiagnosticsManager#registerConnectivityDiagnosticsCallback}. + * Requesting a network with this method will count toward this limit. If this limit is + * exceeded, an exception will be thrown. To avoid hitting this issue and to conserve resources, + * make sure to unregister the callbacks with + * {@link #unregisterNetworkCallback(NetworkCallback)}. + * + * * @param request {@link NetworkRequest} describing this request. * @param networkCallback The {@link NetworkCallback} that the system will call as suitable * networks change state. * @param handler {@link Handler} to specify the thread upon which the callback will be invoked. + * @throws RuntimeException if the app already has too many callbacks registered. */ @RequiresPermission(android.Manifest.permission.ACCESS_NETWORK_STATE) public void registerNetworkCallback(@NonNull NetworkRequest request, @@ -4084,10 +4103,21 @@ public class ConnectivityManager { *

* The request may be released normally by calling * {@link #unregisterNetworkCallback(android.app.PendingIntent)}. + * + *

To avoid performance issues due to apps leaking callbacks, the system will limit the + * number of outstanding requests to 100 per app (identified by their UID), shared with + * all variants of this method, of {@link #requestNetwork} as well as + * {@link ConnectivityDiagnosticsManager#registerConnectivityDiagnosticsCallback}. + * Requesting a network with this method will count toward this limit. If this limit is + * exceeded, an exception will be thrown. To avoid hitting this issue and to conserve resources, + * make sure to unregister the callbacks with {@link #unregisterNetworkCallback(PendingIntent)} + * or {@link #releaseNetworkRequest(PendingIntent)}. + * * @param request {@link NetworkRequest} describing this request. * @param operation Action to perform when the network is available (corresponds * to the {@link NetworkCallback#onAvailable} call. Typically * comes from {@link PendingIntent#getBroadcast}. Cannot be null. + * @throws RuntimeException if the app already has too many callbacks registered. */ @RequiresPermission(android.Manifest.permission.ACCESS_NETWORK_STATE) public void registerNetworkCallback(@NonNull NetworkRequest request, @@ -4109,9 +4139,19 @@ public class ConnectivityManager { * will continue to be called until either the application exits or * {@link #unregisterNetworkCallback(NetworkCallback)} is called. * + *

To avoid performance issues due to apps leaking callbacks, the system will limit the + * number of outstanding requests to 100 per app (identified by their UID), shared with + * all variants of this method, of {@link #requestNetwork} as well as + * {@link ConnectivityDiagnosticsManager#registerConnectivityDiagnosticsCallback}. + * Requesting a network with this method will count toward this limit. If this limit is + * exceeded, an exception will be thrown. To avoid hitting this issue and to conserve resources, + * make sure to unregister the callbacks with + * {@link #unregisterNetworkCallback(NetworkCallback)}. + * * @param networkCallback The {@link NetworkCallback} that the system will call as the * system default network changes. * The callback is invoked on the default internal Handler. + * @throws RuntimeException if the app already has too many callbacks registered. */ @RequiresPermission(android.Manifest.permission.ACCESS_NETWORK_STATE) public void registerDefaultNetworkCallback(@NonNull NetworkCallback networkCallback) { @@ -4123,9 +4163,19 @@ public class ConnectivityManager { * will continue to be called until either the application exits or * {@link #unregisterNetworkCallback(NetworkCallback)} is called. * + *

To avoid performance issues due to apps leaking callbacks, the system will limit the + * number of outstanding requests to 100 per app (identified by their UID), shared with + * all variants of this method, of {@link #requestNetwork} as well as + * {@link ConnectivityDiagnosticsManager#registerConnectivityDiagnosticsCallback}. + * Requesting a network with this method will count toward this limit. If this limit is + * exceeded, an exception will be thrown. To avoid hitting this issue and to conserve resources, + * make sure to unregister the callbacks with + * {@link #unregisterNetworkCallback(NetworkCallback)}. + * * @param networkCallback The {@link NetworkCallback} that the system will call as the * system default network changes. * @param handler {@link Handler} to specify the thread upon which the callback will be invoked. + * @throws RuntimeException if the app already has too many callbacks registered. */ @RequiresPermission(android.Manifest.permission.ACCESS_NETWORK_STATE) public void registerDefaultNetworkCallback(@NonNull NetworkCallback networkCallback, @@ -4217,7 +4267,6 @@ public class ConnectivityManager { * Cannot be null. */ public void unregisterNetworkCallback(@NonNull PendingIntent operation) { - checkPendingIntentNotNull(operation); releaseNetworkRequest(operation); } diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 0ab571854c..2958fd2ae6 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -6426,7 +6426,7 @@ public class ConnectivityService extends IConnectivityManager.Stub final boolean shouldFilter = requiresVpnIsolation(nai, newNc, nai.linkProperties); final String iface = nai.linkProperties.getInterfaceName(); // For VPN uid interface filtering, old ranges need to be removed before new ranges can - // be added, due to the range being expanded and stored as invidiual UIDs. For example + // be added, due to the range being expanded and stored as individual UIDs. For example // the UIDs might be updated from [0, 99999] to ([0, 10012], [10014, 99999]) which means // prevRanges = [0, 99999] while newRanges = [0, 10012], [10014, 99999]. If prevRanges // were added first and then newRanges got removed later, there would be only one uid diff --git a/tests/net/java/com/android/server/net/NetworkStatsSubscriptionsMonitorTest.java b/tests/net/java/com/android/server/net/NetworkStatsSubscriptionsMonitorTest.java new file mode 100644 index 0000000000..058856dcd6 --- /dev/null +++ b/tests/net/java/com/android/server/net/NetworkStatsSubscriptionsMonitorTest.java @@ -0,0 +1,218 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.net; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.anyInt; +import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import android.annotation.NonNull; +import android.content.Context; +import android.os.Looper; +import android.telephony.PhoneStateListener; +import android.telephony.ServiceState; +import android.telephony.SubscriptionManager; +import android.telephony.TelephonyManager; + +import com.android.internal.util.CollectionUtils; +import com.android.server.net.NetworkStatsSubscriptionsMonitor.Delegate; +import com.android.server.net.NetworkStatsSubscriptionsMonitor.RatTypeListener; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.Executor; +import java.util.concurrent.Executors; + +@RunWith(JUnit4.class) +public final class NetworkStatsSubscriptionsMonitorTest { + private static final int TEST_SUBID1 = 3; + private static final int TEST_SUBID2 = 5; + private static final String TEST_IMSI1 = "466921234567890"; + private static final String TEST_IMSI2 = "466920987654321"; + private static final String TEST_IMSI3 = "466929999999999"; + + @Mock private Context mContext; + @Mock private PhoneStateListener mPhoneStateListener; + @Mock private SubscriptionManager mSubscriptionManager; + @Mock private TelephonyManager mTelephonyManager; + @Mock private Delegate mDelegate; + private final List mTestSubList = new ArrayList<>(); + + private final Executor mExecutor = Executors.newSingleThreadExecutor(); + private NetworkStatsSubscriptionsMonitor mMonitor; + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + + if (Looper.myLooper() == null) { + Looper.prepare(); + } + + when(mTelephonyManager.createForSubscriptionId(anyInt())).thenReturn(mTelephonyManager); + + when(mContext.getSystemService(eq(Context.TELEPHONY_SUBSCRIPTION_SERVICE))) + .thenReturn(mSubscriptionManager); + when(mContext.getSystemService(eq(Context.TELEPHONY_SERVICE))) + .thenReturn(mTelephonyManager); + + mMonitor = new NetworkStatsSubscriptionsMonitor(mContext, mExecutor, mDelegate); + } + + @Test + public void testStartStop() { + // Verify that addOnSubscriptionsChangedListener() is never called before start(). + verify(mSubscriptionManager, never()) + .addOnSubscriptionsChangedListener(mExecutor, mMonitor); + mMonitor.start(); + verify(mSubscriptionManager).addOnSubscriptionsChangedListener(mExecutor, mMonitor); + + // Verify that removeOnSubscriptionsChangedListener() is never called before stop() + verify(mSubscriptionManager, never()).removeOnSubscriptionsChangedListener(mMonitor); + mMonitor.stop(); + verify(mSubscriptionManager).removeOnSubscriptionsChangedListener(mMonitor); + } + + @NonNull + private static int[] convertArrayListToIntArray(@NonNull List arrayList) { + final int[] list = new int[arrayList.size()]; + for (int i = 0; i < arrayList.size(); i++) { + list[i] = arrayList.get(i); + } + return list; + } + + private void setRatTypeForSub(List listeners, + int subId, int type) { + final ServiceState serviceState = mock(ServiceState.class); + when(serviceState.getDataNetworkType()).thenReturn(type); + final RatTypeListener match = CollectionUtils + .find(listeners, it -> it.getSubId() == subId); + if (match != null) { + match.onServiceStateChanged(serviceState); + } + } + + private void addTestSub(int subId, String subscriberId) { + // add SubId to TestSubList. + if (!mTestSubList.contains(subId)) { + mTestSubList.add(subId); + } + final int[] subList = convertArrayListToIntArray(mTestSubList); + when(mSubscriptionManager.getCompleteActiveSubscriptionIdList()).thenReturn(subList); + when(mTelephonyManager.getSubscriberId(subId)).thenReturn(subscriberId); + mMonitor.onSubscriptionsChanged(); + } + + private void removeTestSub(int subId) { + // Remove subId from TestSubList. + mTestSubList.removeIf(it -> it == subId); + final int[] subList = convertArrayListToIntArray(mTestSubList); + when(mSubscriptionManager.getCompleteActiveSubscriptionIdList()).thenReturn(subList); + mMonitor.onSubscriptionsChanged(); + } + + private void assertRatTypeChangedForSub(String subscriberId, int ratType) { + assertEquals(mMonitor.getRatTypeForSubscriberId(subscriberId), ratType); + final ArgumentCaptor typeCaptor = ArgumentCaptor.forClass(Integer.class); + // Verify callback with the subscriberId and the RAT type should be as expected. + // It will fail if get a callback with an unexpected RAT type. + verify(mDelegate).onCollapsedRatTypeChanged(eq(subscriberId), typeCaptor.capture()); + final int type = typeCaptor.getValue(); + assertEquals(ratType, type); + } + + private void assertRatTypeNotChangedForSub(String subscriberId, int ratType) { + assertEquals(mMonitor.getRatTypeForSubscriberId(subscriberId), ratType); + // Should never get callback with any RAT type. + verify(mDelegate, never()).onCollapsedRatTypeChanged(eq(subscriberId), anyInt()); + } + + @Test + public void testSubChangedAndRatTypeChanged() { + final ArgumentCaptor ratTypeListenerCaptor = + ArgumentCaptor.forClass(RatTypeListener.class); + + mMonitor.start(); + // Insert sim1, verify RAT type is NETWORK_TYPE_UNKNOWN, and never get any callback + // before changing RAT type. + addTestSub(TEST_SUBID1, TEST_IMSI1); + assertRatTypeNotChangedForSub(TEST_IMSI1, TelephonyManager.NETWORK_TYPE_UNKNOWN); + + // Insert sim2. + addTestSub(TEST_SUBID2, TEST_IMSI2); + assertRatTypeNotChangedForSub(TEST_IMSI1, TelephonyManager.NETWORK_TYPE_UNKNOWN); + verify(mTelephonyManager, times(2)).listen(ratTypeListenerCaptor.capture(), + eq(PhoneStateListener.LISTEN_SERVICE_STATE)); + reset(mDelegate); + + // Set RAT type of sim1 to UMTS. + // Verify RAT type of sim1 after subscription gets onCollapsedRatTypeChanged() callback + // and others remain untouched. + setRatTypeForSub(ratTypeListenerCaptor.getAllValues(), TEST_SUBID1, + TelephonyManager.NETWORK_TYPE_UMTS); + assertRatTypeChangedForSub(TEST_IMSI1, TelephonyManager.NETWORK_TYPE_UMTS); + assertRatTypeNotChangedForSub(TEST_IMSI2, TelephonyManager.NETWORK_TYPE_UNKNOWN); + assertRatTypeNotChangedForSub(TEST_IMSI3, TelephonyManager.NETWORK_TYPE_UNKNOWN); + reset(mDelegate); + + // Set RAT type of sim2 to LTE. + // Verify RAT type of sim2 after subscription gets onCollapsedRatTypeChanged() callback + // and others remain untouched. + setRatTypeForSub(ratTypeListenerCaptor.getAllValues(), TEST_SUBID2, + TelephonyManager.NETWORK_TYPE_LTE); + assertRatTypeNotChangedForSub(TEST_IMSI1, TelephonyManager.NETWORK_TYPE_UMTS); + assertRatTypeChangedForSub(TEST_IMSI2, TelephonyManager.NETWORK_TYPE_LTE); + assertRatTypeNotChangedForSub(TEST_IMSI3, TelephonyManager.NETWORK_TYPE_UNKNOWN); + reset(mDelegate); + + // Remove sim2 and verify that callbacks are fired and RAT type is correct for sim2. + // while the other two remain untouched. + removeTestSub(TEST_SUBID2); + verify(mTelephonyManager).listen(any(), eq(PhoneStateListener.LISTEN_NONE)); + assertRatTypeNotChangedForSub(TEST_IMSI1, TelephonyManager.NETWORK_TYPE_UMTS); + assertRatTypeChangedForSub(TEST_IMSI2, TelephonyManager.NETWORK_TYPE_UNKNOWN); + assertRatTypeNotChangedForSub(TEST_IMSI3, TelephonyManager.NETWORK_TYPE_UNKNOWN); + reset(mDelegate); + + // Set RAT type of sim1 to UNKNOWN. Then stop monitoring subscription changes + // and verify that the listener for sim1 is removed. + setRatTypeForSub(ratTypeListenerCaptor.getAllValues(), TEST_SUBID1, + TelephonyManager.NETWORK_TYPE_UNKNOWN); + assertRatTypeChangedForSub(TEST_IMSI1, TelephonyManager.NETWORK_TYPE_UNKNOWN); + reset(mDelegate); + + mMonitor.stop(); + verify(mTelephonyManager, times(2)).listen(any(), eq(PhoneStateListener.LISTEN_NONE)); + assertRatTypeChangedForSub(TEST_IMSI1, TelephonyManager.NETWORK_TYPE_UNKNOWN); + } +}