Check for NLS service intent filter when rebinding services
Also:
- after updating packages with NLS components, check
the approved services and remove from approved list if missing the NLS service intent filter.
- check for services permission and intent when user enables NLS.
Flag: EXEMPT security fix
Test: atest ManagedServicesTest
Bug: 347674739
Change-Id: I1e216a4bbdddc6e254cd43553ff6a676cf63a3a7
diff --git a/services/core/java/com/android/server/notification/ManagedServices.java b/services/core/java/com/android/server/notification/ManagedServices.java
index 03fc60c..cd0a2a7 100644
--- a/services/core/java/com/android/server/notification/ManagedServices.java
+++ b/services/core/java/com/android/server/notification/ManagedServices.java
@@ -106,7 +106,8 @@
protected final String TAG = getClass().getSimpleName().replace('$', '.');
protected final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG);
- private static final int ON_BINDING_DIED_REBIND_DELAY_MS = 10000;
+ protected static final int ON_BINDING_DIED_REBIND_DELAY_MS = 10000;
+ protected static final int ON_BINDING_DIED_REBIND_MSG = 1234;
protected static final String ENABLED_SERVICES_SEPARATOR = ":";
private static final String DB_VERSION_1 = "1";
private static final String DB_VERSION_2 = "2";
@@ -856,7 +857,13 @@
String approvedItem = getApprovedValue(pkgOrComponent);
if (approvedItem != null) {
+ final ComponentName component = ComponentName.unflattenFromString(approvedItem);
if (enabled) {
+ if (component != null && !isValidService(component, userId)) {
+ Log.e(TAG, "Skip allowing " + mConfig.caption + " " + pkgOrComponent
+ + " (userSet: " + userSet + ") for invalid service");
+ return;
+ }
approved.add(approvedItem);
} else {
approved.remove(approvedItem);
@@ -954,7 +961,7 @@
|| isPackageOrComponentAllowed(component.getPackageName(), userId))) {
return false;
}
- return componentHasBindPermission(component, userId);
+ return isValidService(component, userId);
}
private boolean componentHasBindPermission(ComponentName component, int userId) {
@@ -1306,11 +1313,12 @@
if (TextUtils.equals(getPackageName(approvedPackageOrComponent), packageName)) {
final ComponentName component = ComponentName.unflattenFromString(
approvedPackageOrComponent);
- if (component != null && !componentHasBindPermission(component, userId)) {
+ if (component != null && !isValidService(component, userId)) {
approved.removeAt(j);
if (DEBUG) {
Slog.v(TAG, "Removing " + approvedPackageOrComponent
- + " from approved list; no bind permission found "
+ + " from approved list; no bind permission or "
+ + "service interface filter found "
+ mConfig.bindPermission);
}
}
@@ -1329,6 +1337,11 @@
}
}
+ protected boolean isValidService(ComponentName component, int userId) {
+ return componentHasBindPermission(component, userId) && queryPackageForServices(
+ component.getPackageName(), userId).contains(component);
+ }
+
protected boolean isValidEntry(String packageOrComponent, int userId) {
return hasMatchingServices(packageOrComponent, userId);
}
@@ -1486,23 +1499,25 @@
* Called when user switched to unbind all services from other users.
*/
@VisibleForTesting
- void unbindOtherUserServices(int currentUser) {
+ void unbindOtherUserServices(int switchedToUser) {
TimingsTraceAndSlog t = new TimingsTraceAndSlog();
- t.traceBegin("ManagedServices.unbindOtherUserServices_current" + currentUser);
- unbindServicesImpl(currentUser, true /* allExceptUser */);
+ t.traceBegin("ManagedServices.unbindOtherUserServices_current" + switchedToUser);
+ unbindServicesImpl(switchedToUser, true /* allExceptUser */);
t.traceEnd();
}
- void unbindUserServices(int user) {
+ void unbindUserServices(int removedUser) {
TimingsTraceAndSlog t = new TimingsTraceAndSlog();
- t.traceBegin("ManagedServices.unbindUserServices" + user);
- unbindServicesImpl(user, false /* allExceptUser */);
+ t.traceBegin("ManagedServices.unbindUserServices" + removedUser);
+ unbindServicesImpl(removedUser, false /* allExceptUser */);
t.traceEnd();
}
void unbindServicesImpl(int user, boolean allExceptUser) {
final SparseArray<Set<ComponentName>> componentsToUnbind = new SparseArray<>();
synchronized (mMutex) {
+ // Remove enqueued rebinds to avoid rebinding services for a switched user
+ mHandler.removeMessages(ON_BINDING_DIED_REBIND_MSG);
final Set<ManagedServiceInfo> removableBoundServices = getRemovableConnectedServices();
for (ManagedServiceInfo info : removableBoundServices) {
if ((allExceptUser && (info.userid != user))
@@ -1697,6 +1712,7 @@
mServicesRebinding.add(servicesBindingTag);
mHandler.postDelayed(() ->
reregisterService(name, userid),
+ ON_BINDING_DIED_REBIND_MSG,
ON_BINDING_DIED_REBIND_DELAY_MS);
} else {
Slog.v(TAG, getCaption() + " not rebinding in user " + userid
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 48bc9d7..3bbc6b2 100644
--- a/services/tests/uiservicestests/src/com/android/server/notification/ManagedServicesTest.java
+++ b/services/tests/uiservicestests/src/com/android/server/notification/ManagedServicesTest.java
@@ -63,11 +63,13 @@
import android.os.Bundle;
import android.os.IBinder;
import android.os.IInterface;
+import android.os.Looper;
import android.os.RemoteException;
import android.os.UserHandle;
import android.os.UserManager;
import android.platform.test.annotations.EnableFlags;
import android.provider.Settings;
+import android.testing.TestableLooper;
import android.text.TextUtils;
import android.util.ArrayMap;
import android.util.ArraySet;
@@ -82,6 +84,7 @@
import com.google.android.collect.Lists;
+import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.mockito.ArgumentCaptor;
@@ -103,6 +106,7 @@
import java.util.Set;
import java.util.concurrent.CountDownLatch;
+
public class ManagedServicesTest extends UiServiceTestCase {
@Mock
@@ -115,6 +119,7 @@
private ManagedServices.UserProfiles mUserProfiles;
@Mock private DevicePolicyManager mDpm;
Object mLock = new Object();
+ private TestableLooper mTestableLooper;
UserInfo mZero = new UserInfo(0, "zero", 0);
UserInfo mTen = new UserInfo(10, "ten", 0);
@@ -142,6 +147,7 @@
@Before
public void setUp() throws Exception {
MockitoAnnotations.initMocks(this);
+ mTestableLooper = new TestableLooper(Looper.getMainLooper());
mContext.setMockPackageManager(mPm);
mContext.addMockSystemService(Context.USER_SERVICE, mUm);
@@ -199,6 +205,11 @@
mIpm, APPROVAL_BY_COMPONENT);
}
+ @After
+ public void tearDown() throws Exception {
+ mTestableLooper.destroy();
+ }
+
@Test
public void testBackupAndRestore_migration() throws Exception {
for (int approvalLevel : new int[] {APPROVAL_BY_COMPONENT, APPROVAL_BY_PACKAGE}) {
@@ -888,7 +899,7 @@
return true;
});
- mockServiceInfoWithMetaData(List.of(cn), service, new ArrayMap<>());
+ mockServiceInfoWithMetaData(List.of(cn), service, pm, new ArrayMap<>());
service.addApprovedList("a", 0, true);
service.reregisterService(cn, 0);
@@ -919,7 +930,7 @@
return true;
});
- mockServiceInfoWithMetaData(List.of(cn), service, new ArrayMap<>());
+ mockServiceInfoWithMetaData(List.of(cn), service, pm, new ArrayMap<>());
service.addApprovedList("a", 0, false);
service.reregisterService(cn, 0);
@@ -950,7 +961,7 @@
return true;
});
- mockServiceInfoWithMetaData(List.of(cn), service, new ArrayMap<>());
+ mockServiceInfoWithMetaData(List.of(cn), service, pm, new ArrayMap<>());
service.addApprovedList("a/a", 0, true);
service.reregisterService(cn, 0);
@@ -981,7 +992,7 @@
return true;
});
- mockServiceInfoWithMetaData(List.of(cn), service, new ArrayMap<>());
+ mockServiceInfoWithMetaData(List.of(cn), service, pm, new ArrayMap<>());
service.addApprovedList("a/a", 0, false);
service.reregisterService(cn, 0);
@@ -1053,6 +1064,77 @@
}
@Test
+ public void registerService_bindingDied_rebindIsClearedOnUserSwitch() throws Exception {
+ Context context = mock(Context.class);
+ PackageManager pm = mock(PackageManager.class);
+ ApplicationInfo ai = new ApplicationInfo();
+ ai.targetSdkVersion = Build.VERSION_CODES.CUR_DEVELOPMENT;
+
+ when(context.getPackageName()).thenReturn(mPkg);
+ when(context.getUserId()).thenReturn(mUser.getIdentifier());
+ when(context.getPackageManager()).thenReturn(pm);
+ when(pm.getApplicationInfo(anyString(), anyInt())).thenReturn(ai);
+
+ ManagedServices service = new TestManagedServices(context, mLock, mUserProfiles, mIpm,
+ APPROVAL_BY_PACKAGE);
+ service = spy(service);
+ ComponentName cn = ComponentName.unflattenFromString("a/a");
+
+ // Trigger onBindingDied for component when registering
+ // => will schedule a rebind in 10 seconds
+ when(context.bindServiceAsUser(any(), any(), anyInt(), any())).thenAnswer(invocation -> {
+ Object[] args = invocation.getArguments();
+ ServiceConnection sc = (ServiceConnection) args[1];
+ sc.onBindingDied(cn);
+ return true;
+ });
+ service.registerService(cn, 0);
+ assertThat(service.isBound(cn, 0)).isFalse();
+
+ // Switch to user 10
+ service.onUserSwitched(10);
+
+ // Check that the scheduled rebind for user 0 was cleared
+ mTestableLooper.moveTimeForward(ManagedServices.ON_BINDING_DIED_REBIND_DELAY_MS);
+ mTestableLooper.processAllMessages();
+ verify(service, never()).reregisterService(any(), anyInt());
+ }
+
+ @Test
+ public void registerService_bindingDied_rebindIsExecutedAfterTimeout() throws Exception {
+ Context context = mock(Context.class);
+ PackageManager pm = mock(PackageManager.class);
+ ApplicationInfo ai = new ApplicationInfo();
+ ai.targetSdkVersion = Build.VERSION_CODES.CUR_DEVELOPMENT;
+
+ when(context.getPackageName()).thenReturn(mPkg);
+ when(context.getUserId()).thenReturn(mUser.getIdentifier());
+ when(context.getPackageManager()).thenReturn(pm);
+ when(pm.getApplicationInfo(anyString(), anyInt())).thenReturn(ai);
+
+ ManagedServices service = new TestManagedServices(context, mLock, mUserProfiles, mIpm,
+ APPROVAL_BY_PACKAGE);
+ service = spy(service);
+ ComponentName cn = ComponentName.unflattenFromString("a/a");
+
+ // Trigger onBindingDied for component when registering
+ // => will schedule a rebind in 10 seconds
+ when(context.bindServiceAsUser(any(), any(), anyInt(), any())).thenAnswer(invocation -> {
+ Object[] args = invocation.getArguments();
+ ServiceConnection sc = (ServiceConnection) args[1];
+ sc.onBindingDied(cn);
+ return true;
+ });
+ service.registerService(cn, 0);
+ assertThat(service.isBound(cn, 0)).isFalse();
+
+ // Check that the scheduled rebind is run
+ mTestableLooper.moveTimeForward(ManagedServices.ON_BINDING_DIED_REBIND_DELAY_MS);
+ mTestableLooper.processAllMessages();
+ verify(service, times(1)).reregisterService(eq(cn), eq(0));
+ }
+
+ @Test
public void testPackageUninstall_packageNoLongerInApprovedList() throws Exception {
for (int approvalLevel : new int[] {APPROVAL_BY_COMPONENT, APPROVAL_BY_PACKAGE}) {
ManagedServices service = new TestManagedServices(getContext(), mLock, mUserProfiles,
@@ -1211,6 +1293,64 @@
}
@Test
+ public void testUpgradeAppNoIntentFilterNoRebind() throws Exception {
+ Context context = spy(getContext());
+ doReturn(true).when(context).bindServiceAsUser(any(), any(), anyInt(), any());
+
+ ManagedServices service = new TestManagedServices(context, mLock, mUserProfiles,
+ mIpm, APPROVAL_BY_COMPONENT);
+
+ List<String> packages = new ArrayList<>();
+ packages.add("package");
+ addExpectedServices(service, packages, 0);
+
+ final ComponentName unapprovedComponent = ComponentName.unflattenFromString("package/C1");
+ final ComponentName approvedComponent = ComponentName.unflattenFromString("package/C2");
+
+ // Both components are approved initially
+ mExpectedPrimaryComponentNames.clear();
+ mExpectedPrimaryPackages.clear();
+ mExpectedPrimaryComponentNames.put(0, "package/C1:package/C2");
+ mExpectedSecondaryComponentNames.clear();
+ mExpectedSecondaryPackages.clear();
+
+ loadXml(service);
+
+ //Component package/C1 loses serviceInterface intent filter
+ ManagedServices.Config config = service.getConfig();
+ when(mPm.queryIntentServicesAsUser(any(), anyInt(), anyInt()))
+ .thenAnswer(new Answer<List<ResolveInfo>>() {
+ @Override
+ public List<ResolveInfo> answer(InvocationOnMock invocationOnMock)
+ throws Throwable {
+ Object[] args = invocationOnMock.getArguments();
+ Intent invocationIntent = (Intent) args[0];
+ if (invocationIntent != null) {
+ if (invocationIntent.getAction().equals(config.serviceInterface)
+ && packages.contains(invocationIntent.getPackage())) {
+ List<ResolveInfo> dummyServices = new ArrayList<>();
+ ResolveInfo resolveInfo = new ResolveInfo();
+ ServiceInfo serviceInfo = new ServiceInfo();
+ serviceInfo.packageName = invocationIntent.getPackage();
+ serviceInfo.name = approvedComponent.getClassName();
+ serviceInfo.permission = service.getConfig().bindPermission;
+ resolveInfo.serviceInfo = serviceInfo;
+ dummyServices.add(resolveInfo);
+ return dummyServices;
+ }
+ }
+ return new ArrayList<>();
+ }
+ });
+
+ // Trigger package update
+ service.onPackagesChanged(false, new String[]{"package"}, new int[]{0});
+
+ assertFalse(service.isComponentEnabledForCurrentProfiles(unapprovedComponent));
+ assertTrue(service.isComponentEnabledForCurrentProfiles(approvedComponent));
+ }
+
+ @Test
public void testSetPackageOrComponentEnabled() throws Exception {
for (int approvalLevel : new int[] {APPROVAL_BY_COMPONENT, APPROVAL_BY_PACKAGE}) {
ManagedServices service = new TestManagedServices(getContext(), mLock, mUserProfiles,
@@ -1223,6 +1363,21 @@
"user10package1/K", "user10.3/Component", "user10package2/L",
"user10.4/Component"}));
+ // mock permissions for services
+ PackageManager pm = mock(PackageManager.class);
+ when(getContext().getPackageManager()).thenReturn(pm);
+ List<ComponentName> enabledComponents = List.of(
+ ComponentName.unflattenFromString("package/Comp"),
+ ComponentName.unflattenFromString("package/C2"),
+ ComponentName.unflattenFromString("again/M4"),
+ ComponentName.unflattenFromString("user10package/B"),
+ ComponentName.unflattenFromString("user10/Component"),
+ ComponentName.unflattenFromString("user10package1/K"),
+ ComponentName.unflattenFromString("user10.3/Component"),
+ ComponentName.unflattenFromString("user10package2/L"),
+ ComponentName.unflattenFromString("user10.4/Component"));
+ mockServiceInfoWithMetaData(enabledComponents, service, pm, new ArrayMap<>());
+
for (int userId : expectedEnabled.keySet()) {
ArrayList<String> expectedForUser = expectedEnabled.get(userId);
for (int i = 0; i < expectedForUser.size(); i++) {
@@ -1944,7 +2099,7 @@
metaDataAutobindAllow.putBoolean(META_DATA_DEFAULT_AUTOBIND, true);
metaDatas.put(cn_allowed, metaDataAutobindAllow);
- mockServiceInfoWithMetaData(componentNames, service, metaDatas);
+ mockServiceInfoWithMetaData(componentNames, service, pm, metaDatas);
service.addApprovedList(cn_allowed.flattenToString(), 0, true);
service.addApprovedList(cn_disallowed.flattenToString(), 0, true);
@@ -1989,7 +2144,7 @@
metaDataAutobindDisallow.putBoolean(META_DATA_DEFAULT_AUTOBIND, false);
metaDatas.put(cn_disallowed, metaDataAutobindDisallow);
- mockServiceInfoWithMetaData(componentNames, service, metaDatas);
+ mockServiceInfoWithMetaData(componentNames, service, pm, metaDatas);
service.addApprovedList(cn_disallowed.flattenToString(), 0, true);
@@ -2028,7 +2183,7 @@
metaDataAutobindDisallow.putBoolean(META_DATA_DEFAULT_AUTOBIND, false);
metaDatas.put(cn_disallowed, metaDataAutobindDisallow);
- mockServiceInfoWithMetaData(componentNames, service, metaDatas);
+ mockServiceInfoWithMetaData(componentNames, service, pm, metaDatas);
service.addApprovedList(cn_disallowed.flattenToString(), 0, true);
@@ -2099,8 +2254,8 @@
}
private void mockServiceInfoWithMetaData(List<ComponentName> componentNames,
- ManagedServices service, ArrayMap<ComponentName, Bundle> metaDatas)
- throws RemoteException {
+ ManagedServices service, PackageManager packageManager,
+ ArrayMap<ComponentName, Bundle> metaDatas) throws RemoteException {
when(mIpm.getServiceInfo(any(), anyLong(), anyInt())).thenAnswer(
(Answer<ServiceInfo>) invocation -> {
ComponentName invocationCn = invocation.getArgument(0);
@@ -2115,6 +2270,39 @@
return null;
}
);
+
+ // add components to queryIntentServicesAsUser response
+ final List<String> packages = new ArrayList<>();
+ for (ComponentName cn: componentNames) {
+ packages.add(cn.getPackageName());
+ }
+ ManagedServices.Config config = service.getConfig();
+ when(packageManager.queryIntentServicesAsUser(any(), anyInt(), anyInt())).
+ thenAnswer(new Answer<List<ResolveInfo>>() {
+ @Override
+ public List<ResolveInfo> answer(InvocationOnMock invocationOnMock)
+ throws Throwable {
+ Object[] args = invocationOnMock.getArguments();
+ Intent invocationIntent = (Intent) args[0];
+ if (invocationIntent != null) {
+ if (invocationIntent.getAction().equals(config.serviceInterface)
+ && packages.contains(invocationIntent.getPackage())) {
+ List<ResolveInfo> dummyServices = new ArrayList<>();
+ for (ComponentName cn: componentNames) {
+ ResolveInfo resolveInfo = new ResolveInfo();
+ ServiceInfo serviceInfo = new ServiceInfo();
+ serviceInfo.packageName = invocationIntent.getPackage();
+ serviceInfo.name = cn.getClassName();
+ serviceInfo.permission = service.getConfig().bindPermission;
+ resolveInfo.serviceInfo = serviceInfo;
+ dummyServices.add(resolveInfo);
+ }
+ return dummyServices;
+ }
+ }
+ return new ArrayList<>();
+ }
+ });
}
private void resetComponentsAndPackages() {
diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationAssistantsTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationAssistantsTest.java
index 797b95b5..7e4ae67 100644
--- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationAssistantsTest.java
+++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationAssistantsTest.java
@@ -25,6 +25,7 @@
import static junit.framework.Assert.assertTrue;
import static org.junit.Assert.assertNull;
+
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Matchers.any;
@@ -193,6 +194,8 @@
public void testWriteXml_userTurnedOffNAS() throws Exception {
int userId = ActivityManager.getCurrentUser();
+ doReturn(true).when(mAssistants).isValidService(eq(mCn), eq(userId));
+
mAssistants.loadDefaultsFromConfig(true);
mAssistants.setPackageOrComponentEnabled(mCn.flattenToString(), userId, true,
@@ -398,6 +401,10 @@
public void testSetPackageOrComponentEnabled_onlyOnePackage() throws Exception {
ComponentName component1 = ComponentName.unflattenFromString("package/Component1");
ComponentName component2 = ComponentName.unflattenFromString("package/Component2");
+
+ doReturn(true).when(mAssistants).isValidService(eq(component1), eq(mZero.id));
+ doReturn(true).when(mAssistants).isValidService(eq(component2), eq(mZero.id));
+
mAssistants.setPackageOrComponentEnabled(component1.flattenToString(), mZero.id, true,
true, true);
verify(mNm, never()).setNotificationAssistantAccessGrantedForUserInternal(
@@ -543,6 +550,7 @@
public void testSetAdjustmentTypeSupportedState() throws Exception {
int userId = ActivityManager.getCurrentUser();
+ doReturn(true).when(mAssistants).isValidService(eq(mCn), eq(userId));
mAssistants.loadDefaultsFromConfig(true);
mAssistants.setPackageOrComponentEnabled(mCn.flattenToString(), userId, true,
true, true);
@@ -566,6 +574,7 @@
public void testSetAdjustmentTypeSupportedState_readWriteXml_entries() throws Exception {
int userId = ActivityManager.getCurrentUser();
+ doReturn(true).when(mAssistants).isValidService(eq(mCn), eq(userId));
mAssistants.loadDefaultsFromConfig(true);
mAssistants.setPackageOrComponentEnabled(mCn.flattenToString(), userId, true,
true, true);
@@ -589,6 +598,7 @@
public void testSetAdjustmentTypeSupportedState_readWriteXml_empty() throws Exception {
int userId = ActivityManager.getCurrentUser();
+ doReturn(true).when(mAssistants).isValidService(eq(mCn), eq(userId));
mAssistants.loadDefaultsFromConfig(true);
mAssistants.setPackageOrComponentEnabled(mCn.flattenToString(), userId, true,
true, true);