Limit the number of services (NLSes, etc) that can be approved per user Trying to activate additional packages/components will be silently rejected. Bug: 428701593 Test: atest ManagedServicesTest NotificationManagerServiceTest Flag: com.android.server.notification.limit_managed_services_count (cherry picked from commit a132684a093d9e1750100b39d4e4168f2d27d349) Cherrypick-From: https://googleplex-android-review.googlesource.com/q/commit:3b41dfeec7ebfcd313ff26b7f27b7e7971af4497 Merged-In: Iddd8044997c41f97369b768f4da5e49efc43ad06 Change-Id: Iddd8044997c41f97369b768f4da5e49efc43ad06
diff --git a/services/core/java/com/android/server/notification/ManagedServices.java b/services/core/java/com/android/server/notification/ManagedServices.java index 62e26e1..159d339 100644 --- a/services/core/java/com/android/server/notification/ManagedServices.java +++ b/services/core/java/com/android/server/notification/ManagedServices.java
@@ -133,6 +133,13 @@ static final int APPROVAL_BY_PACKAGE = 0; static final int APPROVAL_BY_COMPONENT = 1; + /** + * Maximum number of entries allowed in the lists of packages/components contained in + * {@link #mApproved} or {@link #mUserSetServices}. For the first, this effectively limits + * the number of services (e.g. NLSes) that will be bound per user. + */ + private static final int MAX_SERVICE_ENTRIES = 100; + protected final Context mContext; protected final Object mMutex; private final UserProfiles mUserProfiles; @@ -915,16 +922,22 @@ } } - protected void setPackageOrComponentEnabled(String pkgOrComponent, int userId, + protected boolean setPackageOrComponentEnabled(String pkgOrComponent, int userId, boolean isPrimary, boolean enabled) { - setPackageOrComponentEnabled(pkgOrComponent, userId, isPrimary, enabled, true); + return setPackageOrComponentEnabled(pkgOrComponent, userId, isPrimary, enabled, true); } - protected void setPackageOrComponentEnabled(String pkgOrComponent, int userId, + /** + * Changes the enabled state of a managed service. + * + * @return true if the change (enabling or disabling) was applied; false otherwise + */ + protected boolean setPackageOrComponentEnabled(String pkgOrComponent, int userId, boolean isPrimary, boolean enabled, boolean userSet) { Slog.i(TAG, (enabled ? " Allowing " : "Disallowing ") + mConfig.caption + " " + pkgOrComponent + " (userSet: " + userSet + ")"); + boolean changed = false; synchronized (mApproved) { ArrayMap<Boolean, ArraySet<String>> allowedByType = mApproved.get(userId); if (allowedByType == null) { @@ -940,24 +953,42 @@ if (approvedItem != null) { if (enabled) { - approved.add(approvedItem); + if (approved.size() < MAX_SERVICE_ENTRIES) { + approved.add(approvedItem); + changed = true; + } else { + Slog.w(TAG, TextUtils.formatSimple( + "Failed to allow %s %s because there are too many already", + mConfig.caption, pkgOrComponent)); + } } else { approved.remove(approvedItem); + changed = true; } } - ArraySet<String> userSetServices = mUserSetServices.get(userId); - if (userSetServices == null) { - userSetServices = new ArraySet<>(); - mUserSetServices.put(userId, userSetServices); - } - if (userSet) { - userSetServices.add(pkgOrComponent); - } else { - userSetServices.remove(pkgOrComponent); + + if (changed) { + ArraySet<String> userSetServices = mUserSetServices.get(userId); + if (userSetServices == null) { + userSetServices = new ArraySet<>(); + mUserSetServices.put(userId, userSetServices); + } + if (userSet) { + if (userSetServices.size() < MAX_SERVICE_ENTRIES) { + userSetServices.add(pkgOrComponent); + } + } else { + userSetServices.remove(pkgOrComponent); + } + } } - rebindServices(false, userId); + if (changed) { + rebindServices(false, userId); + } + + return changed; } private String getApprovedValue(String pkgOrComponent) {
diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index d7913ba..da76c82 100644 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java
@@ -6692,8 +6692,11 @@ try { if (mAllowedManagedServicePackages.test( pkg, userId, mConditionProviders.getRequiredPermission())) { - mConditionProviders.setPackageOrComponentEnabled( - pkg, userId, true, granted); + boolean changed = mConditionProviders.setPackageOrComponentEnabled(pkg, userId, + /* isPrimary= */ true, granted); + if (!changed) { + return; + } getContext().sendBroadcastAsUser(new Intent( ACTION_NOTIFICATION_POLICY_ACCESS_GRANTED_CHANGED) @@ -6963,10 +6966,15 @@ try { if (mAllowedManagedServicePackages.test( listener.getPackageName(), userId, mListeners.getRequiredPermission())) { + boolean changed = mListeners.setPackageOrComponentEnabled( + listener.flattenToString(), userId, /* isPrimary= */ true, granted, + userSet); + if (!changed) { + return; + } + mConditionProviders.setPackageOrComponentEnabled(listener.flattenToString(), userId, false, granted, userSet); - mListeners.setPackageOrComponentEnabled(listener.flattenToString(), - userId, true, granted, userSet); getContext().sendBroadcastAsUser(new Intent( ACTION_NOTIFICATION_POLICY_ACCESS_GRANTED_CHANGED) @@ -12698,19 +12706,20 @@ } @Override - protected void setPackageOrComponentEnabled(String pkgOrComponent, int userId, + protected boolean setPackageOrComponentEnabled(String pkgOrComponent, int userId, boolean isPrimary, boolean enabled, boolean userSet) { // Ensures that only one component is enabled at a time if (enabled) { List<ComponentName> allowedComponents = getAllowedComponents(userId); if (!allowedComponents.isEmpty()) { ComponentName currentComponent = CollectionUtils.firstOrNull(allowedComponents); - if (currentComponent.flattenToString().equals(pkgOrComponent)) return; + if (currentComponent.flattenToString().equals(pkgOrComponent)) return false; setNotificationAssistantAccessGrantedForUserInternal( currentComponent, userId, false, userSet); } } - super.setPackageOrComponentEnabled(pkgOrComponent, userId, isPrimary, enabled, userSet); + return super.setPackageOrComponentEnabled(pkgOrComponent, userId, isPrimary, enabled, + userSet); } private boolean isVerboseLogEnabled() { @@ -13057,9 +13066,14 @@ } @Override - protected void setPackageOrComponentEnabled(String pkgOrComponent, int userId, + protected boolean setPackageOrComponentEnabled(String pkgOrComponent, int userId, boolean isPrimary, boolean enabled, boolean userSet) { - super.setPackageOrComponentEnabled(pkgOrComponent, userId, isPrimary, enabled, userSet); + boolean changed = super.setPackageOrComponentEnabled(pkgOrComponent, userId, isPrimary, + enabled, userSet); + if (!changed) { + return false; + } + String pkgName = getPackageName(pkgOrComponent); if (redactSensitiveNotificationsFromUntrustedListeners()) { int uid = mPackageManagerInternal.getPackageUid(pkgName, 0, userId); @@ -13079,6 +13093,8 @@ new Intent(ACTION_NOTIFICATION_LISTENER_ENABLED_CHANGED) .addFlags(Intent.FLAG_RECEIVER_REGISTERED_ONLY), UserHandle.of(userId), null); + + return true; } @Override
diff --git a/services/tests/uiservicestests/src/com/android/server/notification/ManagedServicesTest.java b/services/tests/uiservicestests/src/com/android/server/notification/ManagedServicesTest.java index 9c85b04..cff652c 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/ManagedServicesTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/ManagedServicesTest.java
@@ -33,6 +33,7 @@ import static com.android.server.notification.NotificationManagerService.privateSpaceFlagsEnabled; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth.assertWithMessage; import static junit.framework.Assert.assertEquals; import static junit.framework.Assert.assertFalse; @@ -2534,6 +2535,68 @@ assertThat(listener.enabledAndUserMatches(visibleBackgroundUserId)).isFalse(); } + @Test + public void setPackageOrComponentEnabled_tooManyPackages_stopsAdding() { + ManagedServices service = new TestManagedServices(getContext(), mLock, mUserProfiles, + mIpm, APPROVAL_BY_PACKAGE); + int userId = 0; + + for (int i = 1; i <= 100; i++) { + assertWithMessage("Trying pkg" + i) + .that(service.setPackageOrComponentEnabled("pkg" + i, userId, true, true)) + .isTrue(); + assertThat(service.isPackageAllowed("pkg" + i, userId)).isTrue(); + } + + // And finally, monsieur, a wafer-thin mint. + assertThat(service.setPackageOrComponentEnabled("toomany", userId, true, true)).isFalse(); + assertThat(service.isPackageAllowed("toomany", userId)).isFalse(); + + // We can still DISABLE packages though. + assertThat(service.isPackageAllowed("pkg33", userId)).isTrue(); + assertThat(service.setPackageOrComponentEnabled("pkg33", userId, true, false)).isTrue(); + assertThat(service.isPackageAllowed("pkg33", userId)).isFalse(); + + // And that allows adding new ones. + assertThat(service.setPackageOrComponentEnabled("onemore", userId, true, true)).isTrue(); + assertThat(service.isPackageAllowed("onemore", userId)).isTrue(); + } + + @Test + public void setPackageOrComponentEnabled_tooManyChanges_stopsAddingToUserSet() { + ManagedServices service = new TestManagedServices(getContext(), mLock, mUserProfiles, + mIpm, APPROVAL_BY_PACKAGE); + int userId = 0; + + for (int i = 1; i <= 100; i++) { + assertWithMessage("Enabling pkg" + i) + .that(service.setPackageOrComponentEnabled("pkg" + i, userId, true, true)) + .isTrue(); + assertWithMessage("Disabling pkg" + i) + .that(service.setPackageOrComponentEnabled("pkg" + i, userId, true, false)) + .isTrue(); + assertThat(service.isPackageAllowed("pkg" + i, userId)).isFalse(); + assertThat(service.isPackageOrComponentUserSet("pkg" + i, userId)).isTrue(); + } + + // Too many disabled services. + assertThat(service.setPackageOrComponentEnabled("toomany", userId, true, true)).isTrue(); + assertThat(service.isPackageAllowed("toomany", userId)).isTrue(); + assertThat(service.isPackageOrComponentUserSet("toomany", userId)).isFalse(); + assertThat(service.setPackageOrComponentEnabled("toomany", userId, true, false)).isTrue(); + assertThat(service.isPackageAllowed("toomany", userId)).isFalse(); + assertThat(service.isPackageOrComponentUserSet("toomany", userId)).isFalse(); + + // We make space only when packages are uninstalled. + service.onPackagesChanged(/* removingPackage= */ true, new String[] { "pkg22" }, + new int[] { 22 }); + + // And that allows tracking new ones. + assertThat(service.setPackageOrComponentEnabled("onemore", userId, true, true)).isTrue(); + assertThat(service.setPackageOrComponentEnabled("onemore", userId, true, false)).isTrue(); + assertThat(service.isPackageOrComponentUserSet("onemore", userId)).isTrue(); + } + private void mockServiceInfoWithMetaData(List<ComponentName> componentNames, ManagedServices service, ArrayMap<ComponentName, Bundle> metaDatas) throws RemoteException {
diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java index 883c6cc..3eb35de 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java
@@ -713,6 +713,18 @@ mPolicyFile.finishWrite(fos); // Setup managed services + when(mListeners.setPackageOrComponentEnabled(any(), anyInt(), anyBoolean(), anyBoolean())) + .thenReturn(true); + when(mListeners.setPackageOrComponentEnabled(any(), anyInt(), anyBoolean(), anyBoolean(), + anyBoolean())).thenReturn(true); + when(mAssistants.setPackageOrComponentEnabled(any(), anyInt(), anyBoolean(), anyBoolean())) + .thenReturn(true); + when(mAssistants.setPackageOrComponentEnabled(any(), anyInt(), anyBoolean(), anyBoolean(), + anyBoolean())).thenReturn(true); + when(mConditionProviders.setPackageOrComponentEnabled(any(), anyInt(), anyBoolean(), + anyBoolean())).thenReturn(true); + when(mConditionProviders.setPackageOrComponentEnabled(any(), anyInt(), anyBoolean(), + anyBoolean(), anyBoolean())).thenReturn(true); when(mNlf.isTypeAllowed(anyInt())).thenReturn(true); when(mNlf.isPackageAllowed(any())).thenReturn(true); when(mNlf.isPackageAllowed(null)).thenReturn(true); @@ -6111,6 +6123,21 @@ } @Test + public void testSetListenerAccessForUser_tooManyListeners_skipsFollowups() throws Exception { + UserHandle user = UserHandle.of(mContext.getUserId() + 10); + ComponentName c = ComponentName.unflattenFromString("package/Component"); + when(mListeners.setPackageOrComponentEnabled(any(), anyInt(), anyBoolean(), anyBoolean(), + anyBoolean())).thenReturn(false); + + mBinderService.setNotificationListenerAccessGrantedForUser( + c, user.getIdentifier(), /* enabled= */ true, true); + + verify(mConditionProviders, never()).setPackageOrComponentEnabled(any(), anyInt(), + anyBoolean(), anyBoolean(), anyBoolean()); + verify(mContext, never()).sendBroadcastAsUser(any(), any(), any()); + } + + @Test public void testSetAssistantAccessForUser() throws Exception { UserInfo ui = new UserInfo(); ui.id = mContext.getUserId() + 10;