From 04c59bf4ed3efaff35d36fe675cec38cb4c9e781 Mon Sep 17 00:00:00 2001 From: Rhed Jao Date: Fri, 13 May 2022 17:51:54 +0800 Subject: [PATCH 1/3] Add a user id parameter to getProperty API Starting from U, the PackageManager#getProperty API returns failed if the package does not installed on the calling user. This cl adds a user id parameter to the API for system modules to specify a correct user id when getting the package's property. Bug: 229684723 Test: atest PackageManagerGetPropertyTest Test: atest CtsCompanionDeviceManagerCoreTestCases:SelfPresenceReportingTest Test: atest BackupEligibilityRulesTest Test: atest ExternalStorageHostTest Change-Id: Id447c5ae62b0ead8fb706f541d8c71282a605a95 --- .../app/ApplicationPackageManager.java | 23 ++++++++++--------- .../android/content/pm/IPackageManager.aidl | 3 ++- .../android/content/pm/PackageManager.java | 15 ++++++++++++ .../backup/utils/BackupEligibilityRules.java | 5 ++-- .../server/companion/PackageUtils.java | 7 +++--- .../android/server/StorageManagerService.java | 3 ++- .../server/pm/IPackageManagerBase.java | 13 +++++++---- .../utils/BackupEligibilityRulesTest.java | 13 ++++++----- 8 files changed, 54 insertions(+), 28 deletions(-) diff --git a/core/java/android/app/ApplicationPackageManager.java b/core/java/android/app/ApplicationPackageManager.java index d16f3318a3c31..e9ebd729ef9fe 100644 --- a/core/java/android/app/ApplicationPackageManager.java +++ b/core/java/android/app/ApplicationPackageManager.java @@ -3716,15 +3716,7 @@ public class ApplicationPackageManager extends PackageManager { throws NameNotFoundException { Objects.requireNonNull(packageName); Objects.requireNonNull(propertyName); - try { - final Property property = mPM.getProperty(propertyName, packageName, null); - if (property == null) { - throw new NameNotFoundException(); - } - return property; - } catch (RemoteException e) { - throw e.rethrowAsRuntimeException(); - } + return getPropertyAsUser(propertyName, packageName, null /* className */, getUserId()); } @Override @@ -3732,9 +3724,18 @@ public class ApplicationPackageManager extends PackageManager { throws NameNotFoundException { Objects.requireNonNull(component); Objects.requireNonNull(propertyName); + return getPropertyAsUser(propertyName, + component.getPackageName(), component.getClassName(), getUserId()); + } + + @Override + public Property getPropertyAsUser(@NonNull String propertyName, @NonNull String packageName, + @Nullable String className, int userId) throws NameNotFoundException { + Objects.requireNonNull(packageName); + Objects.requireNonNull(propertyName); try { - final Property property = mPM.getProperty( - propertyName, component.getPackageName(), component.getClassName()); + final Property property = mPM.getPropertyAsUser(propertyName, + packageName, className, userId); if (property == null) { throw new NameNotFoundException(); } diff --git a/core/java/android/content/pm/IPackageManager.aidl b/core/java/android/content/pm/IPackageManager.aidl index f49ab5711bbf1..cb188fbc703e9 100644 --- a/core/java/android/content/pm/IPackageManager.aidl +++ b/core/java/android/content/pm/IPackageManager.aidl @@ -795,7 +795,8 @@ interface IPackageManager { void holdLock(in IBinder token, in int durationMs); - PackageManager.Property getProperty(String propertyName, String packageName, String className); + PackageManager.Property getPropertyAsUser(String propertyName, String packageName, + String className, int userId); ParceledListSlice queryProperty(String propertyName, int componentType); void setKeepUninstalledPackages(in List packageList); diff --git a/core/java/android/content/pm/PackageManager.java b/core/java/android/content/pm/PackageManager.java index df3578f36fe18..53c23f3fffa76 100644 --- a/core/java/android/content/pm/PackageManager.java +++ b/core/java/android/content/pm/PackageManager.java @@ -10224,6 +10224,21 @@ public abstract class PackageManager { "getProperty not implemented in subclass"); } + /** + * If the provided className is {@code null}, returns the property defined on the application. + * Otherwise, returns the property defined on the component. + * + * @throws NameNotFoundException if the given package is not installed on the calling user or + * component does not exist or if the given property is not defined within the manifest. + * @hide + */ + @NonNull + public Property getPropertyAsUser(@NonNull String propertyName, @NonNull String packageName, + @Nullable String className, int userId) throws NameNotFoundException { + throw new UnsupportedOperationException( + "getPropertyAsUser not implemented in subclass"); + } + /** * Returns the property definition for all <application> tags. *

If the property is not defined with any <application> tag, diff --git a/services/backup/java/com/android/server/backup/utils/BackupEligibilityRules.java b/services/backup/java/com/android/server/backup/utils/BackupEligibilityRules.java index bd1ac2dcffda4..5dacdb40713bc 100644 --- a/services/backup/java/com/android/server/backup/utils/BackupEligibilityRules.java +++ b/services/backup/java/com/android/server/backup/utils/BackupEligibilityRules.java @@ -189,8 +189,9 @@ public class BackupEligibilityRules { boolean isDebuggable = (app.flags & ApplicationInfo.FLAG_DEBUGGABLE) != 0; if (UserHandle.isCore(app.uid) || isPrivileged) { try { - return mPackageManager.getProperty(PackageManager.PROPERTY_ALLOW_ADB_BACKUP, - packageName).getBoolean(); + return mPackageManager.getPropertyAsUser( + PackageManager.PROPERTY_ALLOW_ADB_BACKUP, packageName, + null /* className */, mUserId).getBoolean(); } catch (PackageManager.NameNotFoundException e) { Slog.w(TAG, "Failed to read allowAdbBackup property for + " + packageName); diff --git a/services/companion/java/com/android/server/companion/PackageUtils.java b/services/companion/java/com/android/server/companion/PackageUtils.java index 42c7687f0f28b..622396ccfa368 100644 --- a/services/companion/java/com/android/server/companion/PackageUtils.java +++ b/services/companion/java/com/android/server/companion/PackageUtils.java @@ -110,7 +110,7 @@ final class PackageUtils { final ComponentName componentName = service.getComponentName(); - if (isPrimaryCompanionDeviceService(pm, componentName)) { + if (isPrimaryCompanionDeviceService(pm, componentName, userId)) { // "Primary" service should be at the head of the list. services.add(0, componentName); } else { @@ -122,9 +122,10 @@ final class PackageUtils { } private static boolean isPrimaryCompanionDeviceService(@NonNull PackageManager pm, - @NonNull ComponentName componentName) { + @NonNull ComponentName componentName, @UserIdInt int userId) { try { - return pm.getProperty(PROPERTY_PRIMARY_TAG, componentName).getBoolean(); + return pm.getPropertyAsUser(PROPERTY_PRIMARY_TAG, componentName.getPackageName(), + componentName.getClassName(), userId).getBoolean(); } catch (PackageManager.NameNotFoundException e) { return false; } diff --git a/services/core/java/com/android/server/StorageManagerService.java b/services/core/java/com/android/server/StorageManagerService.java index 8edd75e61478b..77b582e968999 100644 --- a/services/core/java/com/android/server/StorageManagerService.java +++ b/services/core/java/com/android/server/StorageManagerService.java @@ -3728,7 +3728,8 @@ class StorageManagerService extends IStorageManager.Stub try { final PackageManager.Property noAppStorageProp = mContext.getPackageManager() - .getProperty(PackageManager.PROPERTY_NO_APP_DATA_STORAGE, callingPkg); + .getPropertyAsUser(PackageManager.PROPERTY_NO_APP_DATA_STORAGE, callingPkg, + null /* className */, userId); if (noAppStorageProp != null && noAppStorageProp.getBoolean()) { throw new SecurityException(callingPkg + " should not have " + appPath); } diff --git a/services/core/java/com/android/server/pm/IPackageManagerBase.java b/services/core/java/com/android/server/pm/IPackageManagerBase.java index 759b6a3d4acab..c69bc50948aac 100644 --- a/services/core/java/com/android/server/pm/IPackageManagerBase.java +++ b/services/core/java/com/android/server/pm/IPackageManagerBase.java @@ -711,12 +711,17 @@ public abstract class IPackageManagerBase extends IPackageManager.Stub { @Override @Deprecated - public final PackageManager.Property getProperty(String propertyName, String packageName, - String className) { + public final PackageManager.Property getPropertyAsUser(String propertyName, String packageName, + String className, int userId) { Objects.requireNonNull(propertyName); Objects.requireNonNull(packageName); - PackageStateInternal packageState = snapshot().getPackageStateForInstalledAndFiltered( - packageName, Binder.getCallingUid(), UserHandle.getCallingUserId()); + final int callingUid = Binder.getCallingUid(); + final Computer snapshot = snapshot(); + snapshot.enforceCrossUserOrProfilePermission(callingUid, userId, + /* requireFullPermission */ false, + /* checkShell */ false, "getPropertyAsUser"); + PackageStateInternal packageState = snapshot.getPackageStateForInstalledAndFiltered( + packageName, callingUid, userId); if (packageState == null) { return null; } diff --git a/services/tests/servicestests/src/com/android/server/backup/utils/BackupEligibilityRulesTest.java b/services/tests/servicestests/src/com/android/server/backup/utils/BackupEligibilityRulesTest.java index 7aea65e78616e..310c8f4b5d698 100644 --- a/services/tests/servicestests/src/com/android/server/backup/utils/BackupEligibilityRulesTest.java +++ b/services/tests/servicestests/src/com/android/server/backup/utils/BackupEligibilityRulesTest.java @@ -19,6 +19,7 @@ package com.android.server.backup.utils; import static com.google.common.truth.Truth.assertThat; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.when; @@ -250,9 +251,9 @@ public class BackupEligibilityRulesTest { /* flags */ ApplicationInfo.PRIVATE_FLAG_PRIVILEGED, CUSTOM_BACKUP_AGENT_NAME); BackupEligibilityRules eligibilityRules = getBackupEligibilityRules( OperationType.ADB_BACKUP); - when(mPackageManager.getProperty(eq(PackageManager.PROPERTY_ALLOW_ADB_BACKUP), - eq(TEST_PACKAGE_NAME))).thenReturn(getAdbBackupProperty( - /* allowAdbBackup */ false)); + when(mPackageManager.getPropertyAsUser(eq(PackageManager.PROPERTY_ALLOW_ADB_BACKUP), + eq(TEST_PACKAGE_NAME), isNull(), eq(mUserId))) + .thenReturn(getAdbBackupProperty(/* allowAdbBackup */ false)); boolean isEligible = eligibilityRules.appIsEligibleForBackup(applicationInfo); @@ -267,9 +268,9 @@ public class BackupEligibilityRulesTest { /* flags */ ApplicationInfo.PRIVATE_FLAG_PRIVILEGED, CUSTOM_BACKUP_AGENT_NAME); BackupEligibilityRules eligibilityRules = getBackupEligibilityRules( OperationType.ADB_BACKUP); - when(mPackageManager.getProperty(eq(PackageManager.PROPERTY_ALLOW_ADB_BACKUP), - eq(TEST_PACKAGE_NAME))).thenReturn(getAdbBackupProperty( - /* allowAdbBackup */ true)); + when(mPackageManager.getPropertyAsUser(eq(PackageManager.PROPERTY_ALLOW_ADB_BACKUP), + eq(TEST_PACKAGE_NAME), isNull(), eq(mUserId))) + .thenReturn(getAdbBackupProperty(/* allowAdbBackup */ true)); boolean isEligible = eligibilityRules.appIsEligibleForBackup(applicationInfo); From 08544604c217aac2c0461fa5dfffc249e57e0074 Mon Sep 17 00:00:00 2001 From: Rhed Jao Date: Wed, 18 May 2022 17:22:40 +0800 Subject: [PATCH 2/3] Fix cross user package visibility leakage for PackageManager (8/n) Fix APIs: - PackageManager#getApplicationHiddenSettingAsUser - PackageManager#getInstallReason - PackageManager#setDistractingPackageRestrictions - PackageManager#movePackage - PackageManager#setPackageStoppedState - PackageManager#queryProperty - PackageManager#setInstallerPackageName - PackageManager#setPackagesSuspended - IPackageManager#getBlockUninstallForUser Bug: 229684723 Test: atest CrossUserPackageVisibilityTests Change-Id: I8d9f93041ec2a283ecdef38ea23d104201db9f77 --- .../com/android/server/pm/ComputerEngine.java | 20 ++++---- .../server/pm/DistractingPackageHelper.java | 6 +-- .../android/server/pm/MovePackageHelper.java | 7 ++- .../server/pm/PackageManagerService.java | 30 ++++++------ .../server/pm/SuspendPackageHelper.java | 6 +-- .../CrossUserPackageVisibilityTests.java | 46 +++++++++++++++++++ 6 files changed, 77 insertions(+), 38 deletions(-) diff --git a/services/core/java/com/android/server/pm/ComputerEngine.java b/services/core/java/com/android/server/pm/ComputerEngine.java index 9ee7db3bb11d3..e52832f8ad94c 100644 --- a/services/core/java/com/android/server/pm/ComputerEngine.java +++ b/services/core/java/com/android/server/pm/ComputerEngine.java @@ -5011,7 +5011,7 @@ public class ComputerEngine implements Computer { if (ps == null) { return true; } - if (shouldFilterApplication(ps, callingUid, userId)) { + if (shouldFilterApplicationIncludingUninstalled(ps, callingUid, userId)) { return true; } return ps.getUserStateOrDefault(userId).isHidden(); @@ -5075,11 +5075,12 @@ public class ComputerEngine implements Computer { @Override public boolean getBlockUninstallForUser(@NonNull String packageName, @UserIdInt int userId) { - final PackageStateInternal ps = mSettings.getPackage(packageName); - if (ps == null || shouldFilterApplication(ps, Binder.getCallingUid(), userId)) { - return false; - } - return mSettings.getBlockUninstall(userId, packageName); + final PackageStateInternal ps = mSettings.getPackage(packageName); + final int callingUid = Binder.getCallingUid(); + if (ps == null || shouldFilterApplicationIncludingUninstalled(ps, callingUid, userId)) { + return false; + } + return mSettings.getBlockUninstall(userId, packageName); } @Nullable @@ -5471,13 +5472,10 @@ public class ComputerEngine implements Computer { enforceCrossUserPermission(callingUid, userId, true /* requireFullPermission */, false /* checkShell */, "get install reason"); final PackageStateInternal ps = mSettings.getPackage(packageName); - if (shouldFilterApplication(ps, callingUid, userId)) { + if (ps == null || shouldFilterApplicationIncludingUninstalled(ps, callingUid, userId)) { return PackageManager.INSTALL_REASON_UNKNOWN; } - if (ps != null) { - return ps.getUserStateOrDefault(userId).getInstallReason(); - } - return PackageManager.INSTALL_REASON_UNKNOWN; + return ps.getUserStateOrDefault(userId).getInstallReason(); } @Override diff --git a/services/core/java/com/android/server/pm/DistractingPackageHelper.java b/services/core/java/com/android/server/pm/DistractingPackageHelper.java index b9fb3bd8b8734..53e97540de23d 100644 --- a/services/core/java/com/android/server/pm/DistractingPackageHelper.java +++ b/services/core/java/com/android/server/pm/DistractingPackageHelper.java @@ -93,9 +93,9 @@ public final class DistractingPackageHelper { for (int i = 0; i < packageNames.length; i++) { final String packageName = packageNames[i]; final PackageStateInternal packageState = - snapshot.getPackageStateInternal(packageName); - if (packageState == null - || snapshot.shouldFilterApplication(packageState, callingUid, userId)) { + snapshot.getPackageStateForInstalledAndFiltered( + packageName, callingUid, userId); + if (packageState == null) { Slog.w(PackageManagerService.TAG, "Could not find package setting for package: " + packageName + ". Skipping..."); diff --git a/services/core/java/com/android/server/pm/MovePackageHelper.java b/services/core/java/com/android/server/pm/MovePackageHelper.java index c5ca06cc7b84d..ff73987999e74 100644 --- a/services/core/java/com/android/server/pm/MovePackageHelper.java +++ b/services/core/java/com/android/server/pm/MovePackageHelper.java @@ -80,10 +80,9 @@ public final class MovePackageHelper { final PackageManager pm = mPm.mContext.getPackageManager(); Computer snapshot = mPm.snapshotComputer(); - final PackageStateInternal packageState = snapshot.getPackageStateInternal(packageName); - if (packageState == null - || packageState.getPkg() == null - || snapshot.shouldFilterApplication(packageState, callingUid, user.getIdentifier())) { + final PackageStateInternal packageState = snapshot.getPackageStateForInstalledAndFiltered( + packageName, callingUid, user.getIdentifier()); + if (packageState == null || packageState.getPkg() == null) { throw new PackageManagerException(MOVE_FAILED_DOESNT_EXIST, "Missing package"); } final AndroidPackage pkg = packageState.getPkg(); diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index b5013ae0ccb0a..c858ed51da0d5 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -4378,12 +4378,11 @@ public class PackageManagerService implements PackageSender, TestUtilityService true /* requireFullPermission */, true /* checkShell */, "stop package"); final PackageStateInternal packageState = - snapshot.getPackageStateInternal(packageName); + snapshot.getPackageStateForInstalledAndFiltered( + packageName, callingUid, userId); final PackageUserState packageUserState = packageState == null ? null : packageState.getUserStateOrDefault(userId); - if (packageState != null - && !snapshot.shouldFilterApplication(packageState, callingUid, userId) - && packageUserState.isStopped() != stopped) { + if (packageState != null && packageUserState.isStopped() != stopped) { boolean wasNotLaunched = packageUserState.isNotLaunched(); commitPackageStateMutation(null, packageName, state -> { PackageUserStateWrite userState = state.userState(userId); @@ -5237,11 +5236,10 @@ public class PackageManagerService implements PackageSender, TestUtilityService final int callingUserId = UserHandle.getCallingUserId(); final Computer snapshot = snapshotComputer(); final List result = - mPackageProperty.queryProperty(propertyName, componentType, packageName -> { - final PackageStateInternal ps = - snapshot.getPackageStateInternal(packageName); - return snapshot.shouldFilterApplication(ps, callingUid, callingUserId); - }); + mPackageProperty.queryProperty(propertyName, componentType, + packageName -> snapshot.getPackageStateForInstalledAndFiltered( + packageName, callingUid, callingUserId) == null + ); if (result == null) { return ParceledListSlice.emptyList(); } @@ -5600,19 +5598,17 @@ public class PackageManagerService implements PackageSender, TestUtilityService } PackageStateInternal targetPackageState = - snapshot.getPackageStateInternal(targetPackage); - if (targetPackageState == null - || snapshot.shouldFilterApplication(targetPackageState, callingUid, - callingUserId)) { + snapshot.getPackageStateForInstalledAndFiltered( + targetPackage, callingUid, callingUserId); + if (targetPackageState == null) { throw new IllegalArgumentException("Unknown target package: " + targetPackage); } PackageStateInternal installerPackageState = null; if (installerPackageName != null) { - installerPackageState = snapshot.getPackageStateInternal(installerPackageName); - if (installerPackageState == null - || snapshot.shouldFilterApplication( - installerPackageState, callingUid, callingUserId)) { + installerPackageState = snapshot.getPackageStateForInstalledAndFiltered( + installerPackageName, callingUid, callingUserId); + if (installerPackageState == null) { throw new IllegalArgumentException("Unknown installer package: " + installerPackageName); } diff --git a/services/core/java/com/android/server/pm/SuspendPackageHelper.java b/services/core/java/com/android/server/pm/SuspendPackageHelper.java index f476091766b52..f00b34a4ef170 100644 --- a/services/core/java/com/android/server/pm/SuspendPackageHelper.java +++ b/services/core/java/com/android/server/pm/SuspendPackageHelper.java @@ -127,9 +127,9 @@ public final class SuspendPackageHelper { continue; } final PackageStateInternal packageState = - snapshot.getPackageStateInternal(packageName); - if (packageState == null - || snapshot.shouldFilterApplication(packageState, callingUid, userId)) { + snapshot.getPackageStateForInstalledAndFiltered( + packageName, callingUid, userId); + if (packageState == null) { Slog.w(TAG, "Could not find package setting for package: " + packageName + ". Skipping suspending/un-suspending."); unmodifiablePackages.add(packageName); diff --git a/services/tests/PackageManagerServiceTests/appenumeration/src/com/android/server/pm/test/appenumeration/CrossUserPackageVisibilityTests.java b/services/tests/PackageManagerServiceTests/appenumeration/src/com/android/server/pm/test/appenumeration/CrossUserPackageVisibilityTests.java index cacf2cc152c0d..2fd6090faceee 100644 --- a/services/tests/PackageManagerServiceTests/appenumeration/src/com/android/server/pm/test/appenumeration/CrossUserPackageVisibilityTests.java +++ b/services/tests/PackageManagerServiceTests/appenumeration/src/com/android/server/pm/test/appenumeration/CrossUserPackageVisibilityTests.java @@ -17,6 +17,10 @@ package com.android.server.pm.test.appenumeration; import static android.Manifest.permission.CLEAR_APP_USER_DATA; +import static android.Manifest.permission.DELETE_PACKAGES; +import static android.Manifest.permission.MOUNT_UNMOUNT_FILESYSTEMS; +import static android.Manifest.permission.MOVE_PACKAGE; +import static android.content.pm.PackageManager.MOVE_FAILED_DOESNT_EXIST; import static com.android.compatibility.common.util.ShellUtils.runShellCommand; @@ -41,6 +45,7 @@ import com.android.bedstead.harrier.BedsteadJUnit4; import com.android.bedstead.harrier.DeviceState; import com.android.bedstead.harrier.annotations.EnsureHasSecondaryUser; import com.android.bedstead.nene.users.UserReference; +import com.android.compatibility.common.util.PollingCheck; import com.android.compatibility.common.util.TestUtils; import org.junit.After; @@ -203,6 +208,38 @@ public class CrossUserPackageVisibilityTests { assertThat(clearApplicationUserData(CROSS_USER_TEST_PACKAGE_NAME)).isFalse(); } + @Test + public void testGetBlockUninstallForUser_cannotDetectStubPkg() throws Exception { + mInstrumentation.getUiAutomation().adoptShellPermissionIdentity(DELETE_PACKAGES); + assertThat(mIPackageManager.setBlockUninstallForUser( + CROSS_USER_TEST_PACKAGE_NAME, true, mCurrentUser.id())).isTrue(); + try { + assertThat(mIPackageManager.getBlockUninstallForUser( + CROSS_USER_TEST_PACKAGE_NAME, mCurrentUser.id())).isFalse(); + + installPackageForUser(CROSS_USER_TEST_APK_FILE, mOtherUser); + + assertThat(mIPackageManager.getBlockUninstallForUser( + CROSS_USER_TEST_PACKAGE_NAME, mCurrentUser.id())).isFalse(); + } finally { + assertThat(mIPackageManager.setBlockUninstallForUser( + CROSS_USER_TEST_PACKAGE_NAME, false, mCurrentUser.id())).isTrue(); + } + } + + @Test + public void testMovePackage_cannotDetectStubPkg() throws Exception { + mInstrumentation.getUiAutomation().adoptShellPermissionIdentity( + MOVE_PACKAGE, MOUNT_UNMOUNT_FILESYSTEMS); + assertThat(movePackage(CROSS_USER_TEST_PACKAGE_NAME, null /* volumeUuid */)) + .isEqualTo(MOVE_FAILED_DOESNT_EXIST); + + installPackageForUser(CROSS_USER_TEST_APK_FILE, mOtherUser); + + assertThat(movePackage(CROSS_USER_TEST_PACKAGE_NAME, null /* volumeUuid */)) + .isEqualTo(MOVE_FAILED_DOESNT_EXIST); + } + private boolean clearApplicationUserData(String packageName) throws Exception { final AtomicInteger result = new AtomicInteger(-1); final IPackageDataObserver localObserver = new IPackageDataObserver.Stub() { @@ -221,6 +258,15 @@ public class CrossUserPackageVisibilityTests { return result.get() == 1; } + private int movePackage(String packageName, String volumeUuid) throws Exception { + final int moveId = mIPackageManager.movePackage(packageName, volumeUuid); + PollingCheck.check( + "Waiting for the package " + packageName + " moving timeout", + DEFAULT_TIMEOUT_MS, + () -> PackageManager.isMoveStatusFinished(mIPackageManager.getMoveStatus(moveId))); + return mIPackageManager.getMoveStatus(moveId); + } + private static void installPackage(File apk) { installPackageForUser(apk, null, false /* forceQueryable */); } From 0df1d5a262debc7000d3d7ce9d06e3a60ca46905 Mon Sep 17 00:00:00 2001 From: Rhed Jao Date: Thu, 19 May 2022 18:03:05 +0800 Subject: [PATCH 3/3] Fix cross user package visibility leakage for activitySupportsIntent Returns false if the target package does not install in the calling user to fix the cross user package visibility leakage for the activitySupportsIntent API. This cl also adds a user id parameter to the API for the activity starter module to specify a correct user id to query the package's activity intent. Bug: 229684723 Test: atest CrossUserPackageVisibilityTests Test: atest ActivityStarterTests Change-Id: I0e84c762a6861db263adbc5a2fd713f35468213f --- .../android/content/pm/IPackageManager.aidl | 4 ++-- .../java/com/android/server/pm/Computer.java | 5 ++-- .../com/android/server/pm/ComputerEngine.java | 12 ++++++---- .../server/pm/IPackageManagerBase.java | 8 +++---- .../android/server/wm/ActivityStarter.java | 8 +++---- .../CrossUserPackageVisibilityTests.java | 24 +++++++++++++++++++ ...oidManifest-crossUserPackageVisibility.xml | 6 +++++ .../server/wm/ActivityStarterTests.java | 8 +++---- 8 files changed, 54 insertions(+), 21 deletions(-) diff --git a/core/java/android/content/pm/IPackageManager.aidl b/core/java/android/content/pm/IPackageManager.aidl index cb188fbc703e9..6bb7e3e7d140e 100644 --- a/core/java/android/content/pm/IPackageManager.aidl +++ b/core/java/android/content/pm/IPackageManager.aidl @@ -89,8 +89,8 @@ interface IPackageManager { ActivityInfo getActivityInfo(in ComponentName className, long flags, int userId); - boolean activitySupportsIntent(in ComponentName className, in Intent intent, - String resolvedType); + boolean activitySupportsIntentAsUser(in ComponentName className, in Intent intent, + String resolvedType, int userId); ActivityInfo getReceiverInfo(in ComponentName className, long flags, int userId); diff --git a/services/core/java/com/android/server/pm/Computer.java b/services/core/java/com/android/server/pm/Computer.java index b875252a0f1c1..dfe06cf921df4 100644 --- a/services/core/java/com/android/server/pm/Computer.java +++ b/services/core/java/com/android/server/pm/Computer.java @@ -349,8 +349,9 @@ public interface Computer extends PackageDataSnapshot { int getTargetSdkVersion(@NonNull String packageName); - boolean activitySupportsIntent(@NonNull ComponentName resolveComponentName, - @NonNull ComponentName component, @NonNull Intent intent, String resolvedType); + boolean activitySupportsIntentAsUser(@NonNull ComponentName resolveComponentName, + @NonNull ComponentName component, @NonNull Intent intent, String resolvedType, + int userId); @Nullable ActivityInfo getReceiverInfo(@NonNull ComponentName component, diff --git a/services/core/java/com/android/server/pm/ComputerEngine.java b/services/core/java/com/android/server/pm/ComputerEngine.java index e52832f8ad94c..45a139dfb28fe 100644 --- a/services/core/java/com/android/server/pm/ComputerEngine.java +++ b/services/core/java/com/android/server/pm/ComputerEngine.java @@ -3821,14 +3821,16 @@ public class ComputerEngine implements Computer { } @Override - public boolean activitySupportsIntent(@NonNull ComponentName resolveComponentName, - @NonNull ComponentName component, @NonNull Intent intent, String resolvedType) { + public boolean activitySupportsIntentAsUser(@NonNull ComponentName resolveComponentName, + @NonNull ComponentName component, @NonNull Intent intent, String resolvedType, + int userId) { + final int callingUid = Binder.getCallingUid(); + enforceCrossUserPermission(callingUid, userId, false /* requireFullPermission */, + false /* checkShell */, "activitySupportsIntentAsUser"); if (component.equals(resolveComponentName)) { // The resolver supports EVERYTHING! return true; } - final int callingUid = Binder.getCallingUid(); - final int callingUserId = UserHandle.getUserId(callingUid); ParsedActivity a = mComponentResolver.getActivity(component); if (a == null) { return false; @@ -3838,7 +3840,7 @@ public class ComputerEngine implements Computer { return false; } if (shouldFilterApplication( - ps, callingUid, component, TYPE_ACTIVITY, callingUserId)) { + ps, callingUid, component, TYPE_ACTIVITY, userId, true /* filterUninstall */)) { return false; } for (int i=0; i< a.getIntents().size(); i++) { diff --git a/services/core/java/com/android/server/pm/IPackageManagerBase.java b/services/core/java/com/android/server/pm/IPackageManagerBase.java index c69bc50948aac..37541312f9b77 100644 --- a/services/core/java/com/android/server/pm/IPackageManagerBase.java +++ b/services/core/java/com/android/server/pm/IPackageManagerBase.java @@ -148,10 +148,10 @@ public abstract class IPackageManagerBase extends IPackageManager.Stub { @Override @Deprecated - public final boolean activitySupportsIntent(ComponentName component, Intent intent, - String resolvedType) { - return snapshot().activitySupportsIntent(mResolveComponentName, component, intent, - resolvedType); + public final boolean activitySupportsIntentAsUser(ComponentName component, Intent intent, + String resolvedType, int userId) { + return snapshot().activitySupportsIntentAsUser(mResolveComponentName, component, intent, + resolvedType, userId); } @Override diff --git a/services/core/java/com/android/server/wm/ActivityStarter.java b/services/core/java/com/android/server/wm/ActivityStarter.java index c1e1a39dd09bf..684f5e4a7e5f5 100644 --- a/services/core/java/com/android/server/wm/ActivityStarter.java +++ b/services/core/java/com/android/server/wm/ActivityStarter.java @@ -957,8 +957,8 @@ class ActivityStarter { && sourceRecord.info.applicationInfo.uid != aInfo.applicationInfo.uid) { try { intent.addCategory(Intent.CATEGORY_VOICE); - if (!mService.getPackageManager().activitySupportsIntent( - intent.getComponent(), intent, resolvedType)) { + if (!mService.getPackageManager().activitySupportsIntentAsUser( + intent.getComponent(), intent, resolvedType, userId)) { Slog.w(TAG, "Activity being started in current voice task does not support " + "voice: " + intent); err = ActivityManager.START_NOT_VOICE_COMPATIBLE; @@ -974,8 +974,8 @@ class ActivityStarter { // If the caller is starting a new voice session, just make sure the target // is actually allowing it to run this way. try { - if (!mService.getPackageManager().activitySupportsIntent(intent.getComponent(), - intent, resolvedType)) { + if (!mService.getPackageManager().activitySupportsIntentAsUser( + intent.getComponent(), intent, resolvedType, userId)) { Slog.w(TAG, "Activity being started in new voice task does not support: " + intent); err = ActivityManager.START_NOT_VOICE_COMPATIBLE; diff --git a/services/tests/PackageManagerServiceTests/appenumeration/src/com/android/server/pm/test/appenumeration/CrossUserPackageVisibilityTests.java b/services/tests/PackageManagerServiceTests/appenumeration/src/com/android/server/pm/test/appenumeration/CrossUserPackageVisibilityTests.java index 2fd6090faceee..0f742a39c6796 100644 --- a/services/tests/PackageManagerServiceTests/appenumeration/src/com/android/server/pm/test/appenumeration/CrossUserPackageVisibilityTests.java +++ b/services/tests/PackageManagerServiceTests/appenumeration/src/com/android/server/pm/test/appenumeration/CrossUserPackageVisibilityTests.java @@ -30,7 +30,9 @@ import static org.junit.Assert.assertThrows; import android.app.AppGlobals; import android.app.Instrumentation; +import android.content.ComponentName; import android.content.Context; +import android.content.Intent; import android.content.pm.IPackageDataObserver; import android.content.pm.IPackageManager; import android.content.pm.KeySet; @@ -76,6 +78,11 @@ public class CrossUserPackageVisibilityTests { private static final File SHARED_USER_TEST_APK_FILE = new File(TEST_DATA_DIR, "AppEnumerationSharedUserTestApp.apk"); + private static final String ACTION_CROSS_USER_TEST = + "com.android.appenumeration.action.CROSS_USER_TEST"; + private static final ComponentName TEST_ACTIVITY_COMPONENT_NAME = new ComponentName( + CROSS_USER_TEST_PACKAGE_NAME, "com.android.appenumeration.testapp.DummyActivity"); + private static final long DEFAULT_TIMEOUT_MS = 5000; @ClassRule @@ -240,6 +247,23 @@ public class CrossUserPackageVisibilityTests { .isEqualTo(MOVE_FAILED_DOESNT_EXIST); } + @Test + public void testActivitySupportsIntentAsUser_cannotDetectStubPkg() throws Exception { + assertThat(mIPackageManager.activitySupportsIntentAsUser( + TEST_ACTIVITY_COMPONENT_NAME, + new Intent(ACTION_CROSS_USER_TEST), + null, + mCurrentUser.id())).isFalse(); + + installPackageForUser(CROSS_USER_TEST_APK_FILE, mOtherUser); + + assertThat(mIPackageManager.activitySupportsIntentAsUser( + TEST_ACTIVITY_COMPONENT_NAME, + new Intent(ACTION_CROSS_USER_TEST), + null, + mCurrentUser.id())).isFalse(); + } + private boolean clearApplicationUserData(String packageName) throws Exception { final AtomicInteger result = new AtomicInteger(-1); final IPackageDataObserver localObserver = new IPackageDataObserver.Stub() { diff --git a/services/tests/PackageManagerServiceTests/appenumeration/test-apps/target/AndroidManifest-crossUserPackageVisibility.xml b/services/tests/PackageManagerServiceTests/appenumeration/test-apps/target/AndroidManifest-crossUserPackageVisibility.xml index 9d38ddfab5f10..c36d77455e80e 100644 --- a/services/tests/PackageManagerServiceTests/appenumeration/test-apps/target/AndroidManifest-crossUserPackageVisibility.xml +++ b/services/tests/PackageManagerServiceTests/appenumeration/test-apps/target/AndroidManifest-crossUserPackageVisibility.xml @@ -18,5 +18,11 @@ + + + + + diff --git a/services/tests/wmtests/src/com/android/server/wm/ActivityStarterTests.java b/services/tests/wmtests/src/com/android/server/wm/ActivityStarterTests.java index b9432753c17fb..d4d9fca45079e 100644 --- a/services/tests/wmtests/src/com/android/server/wm/ActivityStarterTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/ActivityStarterTests.java @@ -261,12 +261,12 @@ public class ActivityStarterTests extends WindowTestsBase { PRECONDITION_ACTIVITY_SUPPORTS_INTENT_EXCEPTION)) { doAnswer((inv) -> { throw new RemoteException(); - }).when(packageManager).activitySupportsIntent( - eq(source.mActivityComponent), eq(intent), any()); + }).when(packageManager).activitySupportsIntentAsUser( + eq(source.mActivityComponent), eq(intent), any(), anyInt()); } else { doReturn(!containsConditions(preconditions, PRECONDITION_NO_VOICE_SESSION_SUPPORT)) - .when(packageManager).activitySupportsIntent(eq(source.mActivityComponent), - eq(intent), any()); + .when(packageManager).activitySupportsIntentAsUser( + eq(source.mActivityComponent), eq(intent), any(), anyInt()); } } catch (RemoteException e) { }