diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index cd6a71a47f..f1e451e243 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -325,7 +325,8 @@ public class ConnectivityService extends IConnectivityManager.Stub private static final int DEFAULT_NASCENT_DELAY_MS = 5_000; // The maximum number of network request allowed per uid before an exception is thrown. - private static final int MAX_NETWORK_REQUESTS_PER_UID = 100; + @VisibleForTesting + static final int MAX_NETWORK_REQUESTS_PER_UID = 100; // The maximum number of network request allowed for system UIDs before an exception is thrown. @VisibleForTesting @@ -345,7 +346,8 @@ public class ConnectivityService extends IConnectivityManager.Stub @VisibleForTesting protected final PermissionMonitor mPermissionMonitor; - private final PerUidCounter mNetworkRequestCounter; + @VisibleForTesting + final PerUidCounter mNetworkRequestCounter; @VisibleForTesting final PerUidCounter mSystemNetworkRequestCounter; @@ -1155,9 +1157,20 @@ public class ConnectivityService extends IConnectivityManager.Stub private void incrementCountOrThrow(final int uid, final int numToIncrement) { final int newRequestCount = mUidToNetworkRequestCount.get(uid, 0) + numToIncrement; - if (newRequestCount >= mMaxCountPerUid) { + if (newRequestCount >= mMaxCountPerUid + // HACK : the system server is allowed to go over the request count limit + // when it is creating requests on behalf of another app (but not itself, + // so it can still detect its own request leaks). This only happens in the + // per-app API flows in which case the old requests for that particular + // UID will be removed soon. + // TODO : instead of this hack, addPerAppDefaultNetworkRequests and other + // users of transact() should unregister the requests to decrease the count + // before they increase it again by creating a new NRI. Then remove the + // transact() method. + && (Process.myUid() == uid || Process.myUid() != Binder.getCallingUid())) { throw new ServiceSpecificException( - ConnectivityManager.Errors.TOO_MANY_REQUESTS); + ConnectivityManager.Errors.TOO_MANY_REQUESTS, + "Uid " + uid + " exceeded its allotted requests limit"); } mUidToNetworkRequestCount.put(uid, newRequestCount); } @@ -5853,7 +5866,7 @@ public class ConnectivityService extends IConnectivityManager.Stub mUid = nri.mUid; mAsUid = nri.mAsUid; mPendingIntent = nri.mPendingIntent; - mPerUidCounter = getRequestCounter(this); + mPerUidCounter = nri.mPerUidCounter; mPerUidCounter.incrementCountOrThrow(mUid); mCallbackFlags = nri.mCallbackFlags; mCallingAttributionTag = nri.mCallingAttributionTag; @@ -10247,7 +10260,7 @@ public class ConnectivityService extends IConnectivityManager.Stub final NetworkRequestInfo trackingNri = getDefaultRequestTrackingUid(callbackRequest.mAsUid); - // If this nri is not being tracked, the change it back to an untracked nri. + // If this nri is not being tracked, then change it back to an untracked nri. if (trackingNri == mDefaultRequest) { callbackRequestsToRegister.add(new NetworkRequestInfo( callbackRequest, diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index d6cf026427..9e731e361e 100644 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -125,6 +125,7 @@ import static android.net.resolv.aidl.IDnsResolverUnsolicitedEventListener.VALID import static android.os.Process.INVALID_UID; import static android.system.OsConstants.IPPROTO_TCP; +import static com.android.server.ConnectivityService.MAX_NETWORK_REQUESTS_PER_SYSTEM_UID; import static com.android.server.ConnectivityService.PREFERENCE_ORDER_MOBILE_DATA_PREFERERRED; import static com.android.server.ConnectivityService.PREFERENCE_ORDER_OEM; import static com.android.server.ConnectivityService.PREFERENCE_ORDER_PROFILE; @@ -540,6 +541,9 @@ public class ConnectivityServiceTest { private final LinkedBlockingQueue mStartedActivities = new LinkedBlockingQueue<>(); // Map of permission name -> PermissionManager.Permission_{GRANTED|DENIED} constant + // For permissions granted across the board, the key is only the permission name. + // For permissions only granted to a combination of uid/pid, the key + // is ",,". PID+UID permissons have priority over generic ones. private final HashMap mMockedPermissions = new HashMap<>(); MockContext(Context base, ContentProvider settingsProvider) { @@ -641,30 +645,40 @@ public class ConnectivityServiceTest { return mPackageManager; } - private int checkMockedPermission(String permission, Supplier ifAbsent) { - final Integer granted = mMockedPermissions.get(permission); - return granted != null ? granted : ifAbsent.get(); + private int checkMockedPermission(String permission, int pid, int uid, + Supplier ifAbsent) { + final Integer granted = mMockedPermissions.get(permission + "," + pid + "," + uid); + if (null != granted) { + return granted; + } + final Integer allGranted = mMockedPermissions.get(permission); + if (null != allGranted) { + return allGranted; + } + return ifAbsent.get(); } @Override public int checkPermission(String permission, int pid, int uid) { - return checkMockedPermission( - permission, () -> super.checkPermission(permission, pid, uid)); + return checkMockedPermission(permission, pid, uid, + () -> super.checkPermission(permission, pid, uid)); } @Override public int checkCallingOrSelfPermission(String permission) { - return checkMockedPermission( - permission, () -> super.checkCallingOrSelfPermission(permission)); + return checkMockedPermission(permission, Process.myPid(), Process.myUid(), + () -> super.checkCallingOrSelfPermission(permission)); } @Override public void enforceCallingOrSelfPermission(String permission, String message) { - final Integer granted = mMockedPermissions.get(permission); - if (granted == null) { - super.enforceCallingOrSelfPermission(permission, message); - return; - } + final Integer granted = checkMockedPermission(permission, + Process.myPid(), Process.myUid(), + () -> { + super.enforceCallingOrSelfPermission(permission, message); + // enforce will crash if the permission is not granted + return PERMISSION_GRANTED; + }); if (!granted.equals(PERMISSION_GRANTED)) { throw new SecurityException("[Test] permission denied: " + permission); @@ -674,6 +688,8 @@ public class ConnectivityServiceTest { /** * Mock checks for the specified permission, and have them behave as per {@code granted}. * + * This will apply across the board no matter what the checked UID and PID are. + * *

Passing null reverts to default behavior, which does a real permission check on the * test package. * @param granted One of {@link PackageManager#PERMISSION_GRANTED} or @@ -683,6 +699,21 @@ public class ConnectivityServiceTest { mMockedPermissions.put(permission, granted); } + /** + * Mock checks for the specified permission, and have them behave as per {@code granted}. + * + * This will only apply to the passed UID and PID. + * + *

Passing null reverts to default behavior, which does a real permission check on the + * test package. + * @param granted One of {@link PackageManager#PERMISSION_GRANTED} or + * {@link PackageManager#PERMISSION_DENIED}. + */ + public void setPermission(String permission, int pid, int uid, Integer granted) { + final String key = permission + "," + pid + "," + uid; + mMockedPermissions.put(key, granted); + } + @Override public Intent registerReceiverForAllUsers(@Nullable BroadcastReceiver receiver, @NonNull IntentFilter filter, @Nullable String broadcastPermission, @@ -1564,15 +1595,21 @@ public class ConnectivityServiceTest { } private void withPermission(String permission, ExceptionalRunnable r) throws Exception { - if (mServiceContext.checkCallingOrSelfPermission(permission) == PERMISSION_GRANTED) { - r.run(); - return; - } try { mServiceContext.setPermission(permission, PERMISSION_GRANTED); r.run(); } finally { - mServiceContext.setPermission(permission, PERMISSION_DENIED); + mServiceContext.setPermission(permission, null); + } + } + + private void withPermission(String permission, int pid, int uid, ExceptionalRunnable r) + throws Exception { + try { + mServiceContext.setPermission(permission, pid, uid, PERMISSION_GRANTED); + r.run(); + } finally { + mServiceContext.setPermission(permission, pid, uid, null); } } @@ -13401,17 +13438,45 @@ public class ConnectivityServiceTest { @Test public void testProfileNetworkPrefCountsRequestsCorrectlyOnSet() throws Exception { final UserHandle testHandle = setupEnterpriseNetwork(); - testRequestCountLimits(() -> { - // Set initially to test the limit prior to having existing requests. - final TestOnCompleteListener listener = new TestOnCompleteListener(); - mCm.setProfileNetworkPreference(testHandle, PROFILE_NETWORK_PREFERENCE_ENTERPRISE, - Runnable::run, listener); + final TestOnCompleteListener listener = new TestOnCompleteListener(); + // Leave one request available so the profile preference can be set. + testRequestCountLimits(1 /* countToLeaveAvailable */, () -> { + withPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, + Process.myPid(), Process.myUid(), () -> { + // Set initially to test the limit prior to having existing requests. + mCm.setProfileNetworkPreference(testHandle, + PROFILE_NETWORK_PREFERENCE_ENTERPRISE, + Runnable::run, listener); + }); listener.expectOnComplete(); - // re-set so as to test the limit as part of replacing existing requests. - mCm.setProfileNetworkPreference(testHandle, PROFILE_NETWORK_PREFERENCE_ENTERPRISE, - Runnable::run, listener); + // Simulate filing requests as some app on the work profile + final int otherAppUid = UserHandle.getUid(TEST_WORK_PROFILE_USER_ID, + UserHandle.getAppId(Process.myUid() + 1)); + final int remainingCount = ConnectivityService.MAX_NETWORK_REQUESTS_PER_UID + - mService.mNetworkRequestCounter.mUidToNetworkRequestCount.get(otherAppUid) + - 1; + final NetworkCallback[] callbacks = new NetworkCallback[remainingCount]; + doAsUid(otherAppUid, () -> { + for (int i = 0; i < remainingCount; ++i) { + callbacks[i] = new TestableNetworkCallback(); + mCm.registerDefaultNetworkCallback(callbacks[i]); + } + }); + + withPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, + Process.myPid(), Process.myUid(), () -> { + // re-set so as to test the limit as part of replacing existing requests. + mCm.setProfileNetworkPreference(testHandle, + PROFILE_NETWORK_PREFERENCE_ENTERPRISE, Runnable::run, listener); + }); listener.expectOnComplete(); + + doAsUid(otherAppUid, () -> { + for (final NetworkCallback callback : callbacks) { + mCm.unregisterNetworkCallback(callback); + } + }); }); } @@ -13423,39 +13488,45 @@ public class ConnectivityServiceTest { mockHasSystemFeature(PackageManager.FEATURE_AUTOMOTIVE, true); @OemNetworkPreferences.OemNetworkPreference final int networkPref = OEM_NETWORK_PREFERENCE_OEM_PRIVATE_ONLY; - testRequestCountLimits(() -> { - // Set initially to test the limit prior to having existing requests. - final TestOemListenerCallback listener = new TestOemListenerCallback(); - mService.setOemNetworkPreference( - createDefaultOemNetworkPreferences(networkPref), listener); - listener.expectOnComplete(); + // Leave one request available so the OEM preference can be set. + testRequestCountLimits(1 /* countToLeaveAvailable */, () -> + withPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, () -> { + // Set initially to test the limit prior to having existing requests. + final TestOemListenerCallback listener = new TestOemListenerCallback(); + mService.setOemNetworkPreference( + createDefaultOemNetworkPreferences(networkPref), listener); + listener.expectOnComplete(); - // re-set so as to test the limit as part of replacing existing requests. - mService.setOemNetworkPreference( - createDefaultOemNetworkPreferences(networkPref), listener); - listener.expectOnComplete(); - }); + // re-set so as to test the limit as part of replacing existing requests. + mService.setOemNetworkPreference( + createDefaultOemNetworkPreferences(networkPref), listener); + listener.expectOnComplete(); + })); } - private void testRequestCountLimits(@NonNull final Runnable r) throws Exception { + private void testRequestCountLimits(final int countToLeaveAvailable, + @NonNull final ExceptionalRunnable r) throws Exception { final ArraySet callbacks = new ArraySet<>(); try { final int requestCount = mService.mSystemNetworkRequestCounter .mUidToNetworkRequestCount.get(Process.myUid()); - // The limit is hit when total requests <= limit. - final int maxCount = - ConnectivityService.MAX_NETWORK_REQUESTS_PER_SYSTEM_UID - requestCount; + // The limit is hit when total requests = limit - 1, and exceeded with a crash when + // total requests >= limit. + final int countToFile = + MAX_NETWORK_REQUESTS_PER_SYSTEM_UID - requestCount - countToLeaveAvailable; // Need permission so registerDefaultNetworkCallback uses mSystemNetworkRequestCounter withPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, () -> { - for (int i = 1; i < maxCount - 1; i++) { + for (int i = 1; i < countToFile; i++) { final TestNetworkCallback cb = new TestNetworkCallback(); mCm.registerDefaultNetworkCallback(cb); callbacks.add(cb); } - - // Code to run to check if it triggers a max request count limit error. - r.run(); + assertEquals(MAX_NETWORK_REQUESTS_PER_SYSTEM_UID - 1 - countToLeaveAvailable, + mService.mSystemNetworkRequestCounter + .mUidToNetworkRequestCount.get(Process.myUid())); }); + // Code to run to check if it triggers a max request count limit error. + r.run(); } finally { for (final TestNetworkCallback cb : callbacks) { mCm.unregisterNetworkCallback(cb); @@ -13700,15 +13771,18 @@ public class ConnectivityServiceTest { public void testMobileDataPreferredUidsChangedCountsRequestsCorrectlyOnSet() throws Exception { ConnectivitySettingsManager.setMobileDataPreferredUids(mServiceContext, Set.of(PRIMARY_USER_HANDLE.getUid(TEST_PACKAGE_UID))); - testRequestCountLimits(() -> { - // Set initially to test the limit prior to having existing requests. - mService.updateMobileDataPreferredUids(); - waitForIdle(); + // Leave one request available so MDO preference set up above can be set. + testRequestCountLimits(1 /* countToLeaveAvailable */, () -> + withPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, + Process.myPid(), Process.myUid(), () -> { + // Set initially to test the limit prior to having existing requests. + mService.updateMobileDataPreferredUids(); + waitForIdle(); - // re-set so as to test the limit as part of replacing existing requests. - mService.updateMobileDataPreferredUids(); - waitForIdle(); - }); + // re-set so as to test the limit as part of replacing existing requests + mService.updateMobileDataPreferredUids(); + waitForIdle(); + })); } @Test