From b5743e7137e8d7cb5f242622f937538cacc87ca3 Mon Sep 17 00:00:00 2001 From: Valentin Iftime Date: Mon, 29 Jul 2024 15:00:02 +0200 Subject: 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. Flag: EXEMPT security fix Test: atest ManagedServicesTest Bug: 347674739 Change-Id: I6ba585d123fbe88cbff2011b62f02774dd1efda5 (cherry picked from commit 4bab9e28e4463c38c6aff80e9eced2b113786385) Merged-In: I6ba585d123fbe88cbff2011b62f02774dd1efda5 --- .../server/notification/ManagedServices.java | 12 ++- .../server/notification/ManagedServicesTest.java | 109 +++++++++++++++++++-- 2 files changed, 109 insertions(+), 12 deletions(-) diff --git a/services/core/java/com/android/server/notification/ManagedServices.java b/services/core/java/com/android/server/notification/ManagedServices.java index 98b288cd7676..abf7364d700a 100644 --- a/services/core/java/com/android/server/notification/ManagedServices.java +++ b/services/core/java/com/android/server/notification/ManagedServices.java @@ -945,7 +945,7 @@ abstract public class ManagedServices { || isPackageOrComponentAllowed(component.getPackageName(), userId))) { return false; } - return componentHasBindPermission(component, userId); + return isValidService(component, userId); } private boolean componentHasBindPermission(ComponentName component, int userId) { @@ -1282,11 +1282,12 @@ abstract public class ManagedServices { 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); } } @@ -1305,6 +1306,11 @@ abstract public class ManagedServices { } } + 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); } 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 4ef3a6eb12ab..0c35d6054d26 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/ManagedServicesTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/ManagedServicesTest.java @@ -884,7 +884,7 @@ public class ManagedServicesTest extends UiServiceTestCase { 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); @@ -915,7 +915,7 @@ public class ManagedServicesTest extends UiServiceTestCase { 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); @@ -946,7 +946,7 @@ public class ManagedServicesTest extends UiServiceTestCase { 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); @@ -977,7 +977,7 @@ public class ManagedServicesTest extends UiServiceTestCase { 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); @@ -1186,6 +1186,64 @@ public class ManagedServicesTest extends UiServiceTestCase { assertTrue(service.isComponentEnabledForCurrentProfiles(approvedComponent)); } + @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 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>() { + @Override + public List 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 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}) { @@ -1882,7 +1940,7 @@ public class ManagedServicesTest extends UiServiceTestCase { 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); @@ -1927,7 +1985,7 @@ public class ManagedServicesTest extends UiServiceTestCase { 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); @@ -1966,7 +2024,7 @@ public class ManagedServicesTest extends UiServiceTestCase { 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); @@ -1991,8 +2049,8 @@ public class ManagedServicesTest extends UiServiceTestCase { } private void mockServiceInfoWithMetaData(List componentNames, - ManagedServices service, ArrayMap metaDatas) - throws RemoteException { + ManagedServices service, PackageManager packageManager, + ArrayMap metaDatas) throws RemoteException { when(mIpm.getServiceInfo(any(), anyLong(), anyInt())).thenAnswer( (Answer) invocation -> { ComponentName invocationCn = invocation.getArgument(0); @@ -2007,6 +2065,39 @@ public class ManagedServicesTest extends UiServiceTestCase { return null; } ); + + // add components to queryIntentServicesAsUser response + final List 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>() { + @Override + public List 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 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() { -- cgit v1.2.3-59-g8ed1b