Merge changes I24580ea4,Id48b8eab

* changes:
  Allow the system to register 250 NetworkCallbacks.
  Add more test coverage for limiting the number of callbacks.

services/core/java/com/android/server/ConnectivityService.java

@@ -318,6 +318,9 @@ public class ConnectivityService extends IConnectivityManager.Stub
     // The maximum number of network request allowed per uid before an exception is thrown.
     private static final int MAX_NETWORK_REQUESTS_PER_UID = 100;
 
+    // The maximum number of network request allowed for system UIDs before an exception is thrown.
+    private static final int MAX_NETWORK_REQUESTS_PER_SYSTEM_UID = 250;
+
     @VisibleForTesting
     protected int mLingerDelayMs;  // Can't be final, or test subclass constructors can't change it.
     @VisibleForTesting
@@ -333,6 +336,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
     protected final PermissionMonitor mPermissionMonitor;
 
     private final PerUidCounter mNetworkRequestCounter;
+    private final PerUidCounter mSystemNetworkRequestCounter;
 
     private volatile boolean mLockdownEnabled;
 
@@ -1201,6 +1205,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
         mContext = Objects.requireNonNull(context, "missing Context");
         mResources = deps.getResources(mContext);
         mNetworkRequestCounter = new PerUidCounter(MAX_NETWORK_REQUESTS_PER_UID);
+        mSystemNetworkRequestCounter = new PerUidCounter(MAX_NETWORK_REQUESTS_PER_SYSTEM_UID);
 
         mMetricsLog = logger;
         mNetworkRanker = new NetworkRanker();
@@ -4029,7 +4034,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
                 }
             }
         }
-        mNetworkRequestCounter.decrementCount(nri.mUid);
+        decrementRequestCount(nri);
         mNetworkRequestInfoLogs.log("RELEASE " + nri);
 
         if (null != nri.getActiveRequest()) {
@@ -4140,6 +4145,20 @@ public class ConnectivityService extends IConnectivityManager.Stub
         }
     }
 
+    private PerUidCounter getRequestCounter(NetworkRequestInfo nri) {
+        return checkAnyPermissionOf(
+                nri.mPid, nri.mUid, NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK)
+                ? mSystemNetworkRequestCounter : mNetworkRequestCounter;
+    }
+
+    private void incrementRequestCountOrThrow(NetworkRequestInfo nri) {
+        getRequestCounter(nri).incrementCountOrThrow(nri.mUid);
+    }
+
+    private void decrementRequestCount(NetworkRequestInfo nri) {
+        getRequestCounter(nri).decrementCount(nri.mUid);
+    }
+
     @Override
     public void setAcceptUnvalidated(Network network, boolean accept, boolean always) {
         enforceNetworkStackSettingsOrSetup();
@@ -5488,7 +5507,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
             mPid = getCallingPid();
             mUid = mDeps.getCallingUid();
             mAsUid = asUid;
-            mNetworkRequestCounter.incrementCountOrThrow(mUid);
+            incrementRequestCountOrThrow(this);
             /**
              * Location sensitive data not included in pending intent. Only included in
              * {@link NetworkCallback}.
@@ -5520,7 +5539,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
             mUid = mDeps.getCallingUid();
             mAsUid = asUid;
             mPendingIntent = null;
-            mNetworkRequestCounter.incrementCountOrThrow(mUid);
+            incrementRequestCountOrThrow(this);
             mCallbackFlags = callbackFlags;
             mCallingAttributionTag = callingAttributionTag;
 
@@ -5563,7 +5582,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
             mUid = nri.mUid;
             mAsUid = nri.mAsUid;
             mPendingIntent = nri.mPendingIntent;
-            mNetworkRequestCounter.incrementCountOrThrow(mUid);
+            incrementRequestCountOrThrow(this);
             mCallbackFlags = nri.mCallbackFlags;
             mCallingAttributionTag = nri.mCallingAttributionTag;
         }
@@ -8748,7 +8767,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
             // Decrement the reference count for this NetworkRequestInfo. The reference count is
             // incremented when the NetworkRequestInfo is created as part of
             // enforceRequestCountLimit().
-            mNetworkRequestCounter.decrementCount(nri.mUid);
+            decrementRequestCount(nri);
             return;
         }
 
@@ -8814,7 +8833,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
         // Decrement the reference count for this NetworkRequestInfo. The reference count is
         // incremented when the NetworkRequestInfo is created as part of
         // enforceRequestCountLimit().
-        mNetworkRequestCounter.decrementCount(nri.mUid);
+        decrementRequestCount(nri);
 
         iCb.unlinkToDeath(cbInfo, 0);
     }

tests/net/java/com/android/server/ConnectivityServiceTest.java

@@ -18,6 +18,7 @@ package com.android.server;
 
 import static android.Manifest.permission.CHANGE_NETWORK_STATE;
 import static android.Manifest.permission.CONNECTIVITY_USE_RESTRICTED_NETWORKS;
+import static android.Manifest.permission.NETWORK_SETTINGS;
 import static android.app.PendingIntent.FLAG_IMMUTABLE;
 import static android.content.Intent.ACTION_USER_ADDED;
 import static android.content.Intent.ACTION_USER_REMOVED;
@@ -2483,8 +2484,7 @@ public class ConnectivityServiceTest {
     public void networkCallbacksSanitizationTest_Sanitize() throws Exception {
         mServiceContext.setPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK,
                 PERMISSION_DENIED);
-        mServiceContext.setPermission(Manifest.permission.NETWORK_SETTINGS,
+        mServiceContext.setPermission(NETWORK_SETTINGS, PERMISSION_DENIED);
-                PERMISSION_DENIED);
         doNetworkCallbacksSanitizationTest(true /* sanitized */);
     }
 
@@ -2492,7 +2492,7 @@ public class ConnectivityServiceTest {
     public void networkCallbacksSanitizationTest_NoSanitize_NetworkStack() throws Exception {
         mServiceContext.setPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK,
                 PERMISSION_GRANTED);
-        mServiceContext.setPermission(Manifest.permission.NETWORK_SETTINGS, PERMISSION_DENIED);
+        mServiceContext.setPermission(NETWORK_SETTINGS, PERMISSION_DENIED);
         doNetworkCallbacksSanitizationTest(false /* sanitized */);
     }
 
@@ -2500,7 +2500,7 @@ public class ConnectivityServiceTest {
     public void networkCallbacksSanitizationTest_NoSanitize_Settings() throws Exception {
         mServiceContext.setPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK,
                 PERMISSION_DENIED);
-        mServiceContext.setPermission(Manifest.permission.NETWORK_SETTINGS, PERMISSION_GRANTED);
+        mServiceContext.setPermission(NETWORK_SETTINGS, PERMISSION_GRANTED);
         doNetworkCallbacksSanitizationTest(false /* sanitized */);
     }
 
@@ -3572,8 +3572,7 @@ public class ConnectivityServiceTest {
 
     @Test
     public void testCaptivePortalApi() throws Exception {
-        mServiceContext.setPermission(
+        mServiceContext.setPermission(NETWORK_SETTINGS, PERMISSION_GRANTED);
-                android.Manifest.permission.NETWORK_SETTINGS, PERMISSION_GRANTED);
 
         final TestNetworkCallback captivePortalCallback = new TestNetworkCallback();
         final NetworkRequest captivePortalRequest = new NetworkRequest.Builder()
@@ -3607,8 +3606,7 @@ public class ConnectivityServiceTest {
     private TestNetworkCallback setupNetworkCallbackAndConnectToWifi() throws Exception {
         // Grant NETWORK_SETTINGS permission to be able to receive LinkProperties change callbacks
         // with sensitive (captive portal) data
-        mServiceContext.setPermission(
+        mServiceContext.setPermission(NETWORK_SETTINGS, PERMISSION_GRANTED);
-                android.Manifest.permission.NETWORK_SETTINGS, PERMISSION_GRANTED);
 
         final TestNetworkCallback captivePortalCallback = new TestNetworkCallback();
         final NetworkRequest captivePortalRequest = new NetworkRequest.Builder()
@@ -4042,8 +4040,7 @@ public class ConnectivityServiceTest {
     @Test
     public void testRegisterDefaultNetworkCallback() throws Exception {
         // NETWORK_SETTINGS is necessary to call registerSystemDefaultNetworkCallback.
-        mServiceContext.setPermission(Manifest.permission.NETWORK_SETTINGS,
+        mServiceContext.setPermission(NETWORK_SETTINGS, PERMISSION_GRANTED);
-                PERMISSION_GRANTED);
 
         final TestNetworkCallback defaultNetworkCallback = new TestNetworkCallback();
         mCm.registerDefaultNetworkCallback(defaultNetworkCallback);
@@ -4202,8 +4199,7 @@ public class ConnectivityServiceTest {
                 () -> mCm.registerDefaultNetworkCallbackAsUid(APP1_UID, callback, handler));
         callback.assertNoCallback();
 
-        mServiceContext.setPermission(Manifest.permission.NETWORK_SETTINGS,
+        mServiceContext.setPermission(NETWORK_SETTINGS, PERMISSION_GRANTED);
-                PERMISSION_GRANTED);
         mCm.registerSystemDefaultNetworkCallback(callback, handler);
         callback.expectAvailableCallbacksUnvalidated(mCellNetworkAgent);
         mCm.unregisterNetworkCallback(callback);
@@ -5524,10 +5520,11 @@ public class ConnectivityServiceTest {
     }
 
     @Test
-    public void testNetworkCallbackMaximum() {
+    public void testNetworkCallbackMaximum() throws Exception {
         final int MAX_REQUESTS = 100;
         final int CALLBACKS = 89;
         final int INTENTS = 11;
+        final int SYSTEM_ONLY_MAX_REQUESTS = 250;
         assertEquals(MAX_REQUESTS, CALLBACKS + INTENTS);
 
         NetworkRequest networkRequest = new NetworkRequest.Builder().build();
@@ -5576,6 +5573,33 @@ public class ConnectivityServiceTest {
                                 new Intent("d"), FLAG_IMMUTABLE))
         );
 
+        // The system gets another SYSTEM_ONLY_MAX_REQUESTS slots.
+        final Handler handler = new Handler(ConnectivityThread.getInstanceLooper());
+        withPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, () -> {
+            ArrayList<NetworkCallback> systemRegistered = new ArrayList<>();
+            for (int i = 0; i < SYSTEM_ONLY_MAX_REQUESTS - 1; i++) {
+                NetworkCallback cb = new NetworkCallback();
+                if (i % 2 == 0) {
+                    mCm.registerDefaultNetworkCallbackAsUid(1000000 + i, cb, handler);
+                } else {
+                    mCm.registerNetworkCallback(networkRequest, cb);
+                }
+                systemRegistered.add(cb);
+            }
+            waitForIdle();
+
+            assertThrows(TooManyRequestsException.class, () ->
+                    mCm.registerDefaultNetworkCallbackAsUid(1001042, new NetworkCallback(),
+                            handler));
+            assertThrows(TooManyRequestsException.class, () ->
+                    mCm.registerNetworkCallback(networkRequest, new NetworkCallback()));
+
+            for (NetworkCallback callback : systemRegistered) {
+                mCm.unregisterNetworkCallback(callback);
+            }
+            waitForIdle();  // Wait for requests to be unregistered before giving up the permission.
+        });
+
         for (Object o : registered) {
             if (o instanceof NetworkCallback) {
                 mCm.unregisterNetworkCallback((NetworkCallback)o);
@@ -5601,6 +5625,30 @@ public class ConnectivityServiceTest {
         }
         waitForIdle();
 
+        for (int i = 0; i < MAX_REQUESTS; i++) {
+            NetworkCallback networkCallback = new NetworkCallback();
+            mCm.registerDefaultNetworkCallback(networkCallback);
+            mCm.unregisterNetworkCallback(networkCallback);
+        }
+        waitForIdle();
+
+        for (int i = 0; i < MAX_REQUESTS; i++) {
+            NetworkCallback networkCallback = new NetworkCallback();
+            mCm.registerDefaultNetworkCallback(networkCallback);
+            mCm.unregisterNetworkCallback(networkCallback);
+        }
+        waitForIdle();
+
+        withPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, () -> {
+            for (int i = 0; i < MAX_REQUESTS; i++) {
+                NetworkCallback networkCallback = new NetworkCallback();
+                mCm.registerDefaultNetworkCallbackAsUid(1000000 + i, networkCallback,
+                        new Handler(ConnectivityThread.getInstanceLooper()));
+                mCm.unregisterNetworkCallback(networkCallback);
+            }
+        });
+        waitForIdle();
+
         for (int i = 0; i < MAX_REQUESTS; i++) {
             final PendingIntent pendingIntent = PendingIntent.getBroadcast(
                     mContext, 0 /* requestCode */, new Intent("e" + i), FLAG_IMMUTABLE);
@@ -6594,8 +6642,7 @@ public class ConnectivityServiceTest {
     @Test
     public void testVpnNetworkActive() throws Exception {
         // NETWORK_SETTINGS is necessary to call registerSystemDefaultNetworkCallback.
-        mServiceContext.setPermission(Manifest.permission.NETWORK_SETTINGS,
+        mServiceContext.setPermission(NETWORK_SETTINGS, PERMISSION_GRANTED);
-                PERMISSION_GRANTED);
 
         final int uid = Process.myUid();
 
@@ -7087,8 +7134,7 @@ public class ConnectivityServiceTest {
     @Test
     public void testRestrictedProfileAffectsVpnUidRanges() throws Exception {
         // NETWORK_SETTINGS is necessary to see the UID ranges in NetworkCapabilities.
-        mServiceContext.setPermission(Manifest.permission.NETWORK_SETTINGS,
+        mServiceContext.setPermission(NETWORK_SETTINGS, PERMISSION_GRANTED);
-                PERMISSION_GRANTED);
 
         final NetworkRequest request = new NetworkRequest.Builder()
                 .removeCapability(NET_CAPABILITY_NOT_VPN)
@@ -7174,8 +7220,7 @@ public class ConnectivityServiceTest {
         mServiceContext.setPermission(
                 Manifest.permission.CONTROL_VPN, PERMISSION_GRANTED);
         // Necessary to see the UID ranges in NetworkCapabilities.
-        mServiceContext.setPermission(
+        mServiceContext.setPermission(NETWORK_SETTINGS, PERMISSION_GRANTED);
-                Manifest.permission.NETWORK_SETTINGS, PERMISSION_GRANTED);
 
         final NetworkRequest request = new NetworkRequest.Builder()
                 .removeCapability(NET_CAPABILITY_NOT_VPN)
@@ -7663,8 +7708,7 @@ public class ConnectivityServiceTest {
                 Manifest.permission.CONTROL_ALWAYS_ON_VPN, PERMISSION_GRANTED);
         mServiceContext.setPermission(
                 Manifest.permission.CONTROL_VPN, PERMISSION_GRANTED);
-        mServiceContext.setPermission(
+        mServiceContext.setPermission(NETWORK_SETTINGS, PERMISSION_GRANTED);
-                Manifest.permission.NETWORK_SETTINGS, PERMISSION_GRANTED);
 
         final TestNetworkCallback callback = new TestNetworkCallback();
         final NetworkRequest request = new NetworkRequest.Builder()
@@ -7900,8 +7944,7 @@ public class ConnectivityServiceTest {
         mServiceContext.setPermission(
                 Manifest.permission.CONTROL_VPN, PERMISSION_GRANTED);
         // For LockdownVpnTracker to call registerSystemDefaultNetworkCallback.
-        mServiceContext.setPermission(
+        mServiceContext.setPermission(NETWORK_SETTINGS, PERMISSION_GRANTED);
-                Manifest.permission.NETWORK_SETTINGS, PERMISSION_GRANTED);
 
         final NetworkRequest request = new NetworkRequest.Builder().clearCapabilities().build();
         final TestNetworkCallback callback = new TestNetworkCallback();
@@ -9031,8 +9074,7 @@ public class ConnectivityServiceTest {
     private void denyAllLocationPrivilegedPermissions() {
         mServiceContext.setPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK,
                 PERMISSION_DENIED);
-        mServiceContext.setPermission(Manifest.permission.NETWORK_SETTINGS,
+        mServiceContext.setPermission(NETWORK_SETTINGS, PERMISSION_DENIED);
-                PERMISSION_DENIED);
         mServiceContext.setPermission(Manifest.permission.NETWORK_STACK,
                 PERMISSION_DENIED);
         mServiceContext.setPermission(Manifest.permission.NETWORK_SETUP_WIZARD,
@@ -9288,7 +9330,7 @@ public class ConnectivityServiceTest {
     @Test
     public void testCreateForCallerWithLocalMacAddressSanitizedWithSettingsPermission()
             throws Exception {
-        mServiceContext.setPermission(Manifest.permission.NETWORK_SETTINGS, PERMISSION_GRANTED);
+        mServiceContext.setPermission(NETWORK_SETTINGS, PERMISSION_GRANTED);
 
         final TransportInfo transportInfo = mock(TransportInfo.class);
         when(transportInfo.getApplicableRedactions())
@@ -10607,8 +10649,7 @@ public class ConnectivityServiceTest {
 
     private void registerDefaultNetworkCallbacks() {
         // Using Manifest.permission.NETWORK_SETTINGS for registerSystemDefaultNetworkCallback()
-        mServiceContext.setPermission(
+        mServiceContext.setPermission(NETWORK_SETTINGS, PERMISSION_GRANTED);
-                Manifest.permission.NETWORK_SETTINGS, PERMISSION_GRANTED);
         mSystemDefaultNetworkCallback = new TestNetworkCallback();
         mDefaultNetworkCallback = new TestNetworkCallback();
         mProfileDefaultNetworkCallback = new TestNetworkCallback();
@@ -10618,8 +10659,7 @@ public class ConnectivityServiceTest {
         registerDefaultNetworkCallbackAsUid(mProfileDefaultNetworkCallback,
                 TEST_WORK_PROFILE_APP_UID);
         // TODO: test using ConnectivityManager#registerDefaultNetworkCallbackAsUid as well.
-        mServiceContext.setPermission(
+        mServiceContext.setPermission(NETWORK_SETTINGS, PERMISSION_DENIED);
-                Manifest.permission.NETWORK_SETTINGS, PERMISSION_DENIED);
     }
 
     private void unregisterDefaultNetworkCallbacks() {
@@ -10774,7 +10814,7 @@ public class ConnectivityServiceTest {
         defaultNetworkCallback.assertNoCallback();
 
         final TestNetworkCallback otherUidDefaultCallback = new TestNetworkCallback();
-        withPermission(Manifest.permission.NETWORK_SETTINGS, () ->
+        withPermission(NETWORK_SETTINGS, () ->
                 mCm.registerDefaultNetworkCallbackAsUid(TEST_PACKAGE_UID, otherUidDefaultCallback,
                         new Handler(ConnectivityThread.getInstanceLooper())));
 
@@ -10822,7 +10862,7 @@ public class ConnectivityServiceTest {
         defaultNetworkCallback.assertNoCallback();
 
         final TestNetworkCallback otherUidDefaultCallback = new TestNetworkCallback();
-        withPermission(Manifest.permission.NETWORK_SETTINGS, () ->
+        withPermission(NETWORK_SETTINGS, () ->
                 mCm.registerDefaultNetworkCallbackAsUid(TEST_PACKAGE_UID, otherUidDefaultCallback,
                         new Handler(ConnectivityThread.getInstanceLooper())));
 
@@ -10864,7 +10904,7 @@ public class ConnectivityServiceTest {
         defaultNetworkCallback.assertNoCallback();
 
         final TestNetworkCallback otherUidDefaultCallback = new TestNetworkCallback();
-        withPermission(Manifest.permission.NETWORK_SETTINGS, () ->
+        withPermission(NETWORK_SETTINGS, () ->
                 mCm.registerDefaultNetworkCallbackAsUid(TEST_PACKAGE_UID, otherUidDefaultCallback,
                         new Handler(ConnectivityThread.getInstanceLooper())));