Fix cross user package visibility leakage for getAppOpPermissionPackages
To fix cross user package visibility leakage, this CL filters out
packages that aren't installed in the calling user before the API
returns results to the caller.
Also adding a user id parameter to the API for the system modules to
specify the correct user id when querying the appop permission packages.
NoNonSdkCheck: Keep @UnsupportedAppUsage for new signature api
Bug: 229684723
Test: atest CrossUserPackageVisibilityTests
Change-Id: I9d3de91b0195d3396d2737673cb23ef899e23467
diff --git a/boot/hiddenapi/hiddenapi-unsupported.txt b/boot/hiddenapi/hiddenapi-unsupported.txt
index f619913..6b71e24a 100644
--- a/boot/hiddenapi/hiddenapi-unsupported.txt
+++ b/boot/hiddenapi/hiddenapi-unsupported.txt
@@ -110,7 +110,6 @@
Landroid/content/pm/IPackageInstallObserver2$Stub;->asInterface(Landroid/os/IBinder;)Landroid/content/pm/IPackageInstallObserver2;
Landroid/content/pm/IPackageManager$Stub$Proxy;-><init>(Landroid/os/IBinder;)V
Landroid/content/pm/IPackageManager$Stub$Proxy;->checkUidPermission(Ljava/lang/String;I)I
-Landroid/content/pm/IPackageManager$Stub$Proxy;->getAppOpPermissionPackages(Ljava/lang/String;)[Ljava/lang/String;
Landroid/content/pm/IPackageManager$Stub$Proxy;->getInstallLocation()I
Landroid/content/pm/IPackageManager$Stub$Proxy;->getLastChosenActivity(Landroid/content/Intent;Ljava/lang/String;I)Landroid/content/pm/ResolveInfo;
Landroid/content/pm/IPackageManager$Stub$Proxy;->getPackagesForUid(I)[Ljava/lang/String;
diff --git a/core/java/android/content/pm/IPackageManager.aidl b/core/java/android/content/pm/IPackageManager.aidl
index 1b1143a..2ef1c78 100644
--- a/core/java/android/content/pm/IPackageManager.aidl
+++ b/core/java/android/content/pm/IPackageManager.aidl
@@ -746,7 +746,7 @@
// We need to keep these in IPackageManager for app compatibility
//------------------------------------------------------------------------
@UnsupportedAppUsage(maxTargetSdk = 30, trackingBug = 170729553)
- String[] getAppOpPermissionPackages(String permissionName);
+ String[] getAppOpPermissionPackages(String permissionName, int userId);
@UnsupportedAppUsage
PermissionGroupInfo getPermissionGroupInfo(String name, int flags);
diff --git a/services/core/java/com/android/server/pm/Computer.java b/services/core/java/com/android/server/pm/Computer.java
index 33e9b2f..5a2f6fe 100644
--- a/services/core/java/com/android/server/pm/Computer.java
+++ b/services/core/java/com/android/server/pm/Computer.java
@@ -425,7 +425,7 @@
boolean isUidPrivileged(int uid);
@NonNull
- String[] getAppOpPermissionPackages(@NonNull String permissionName);
+ String[] getAppOpPermissionPackages(@NonNull String permissionName, int userId);
@NonNull
ParceledListSlice<PackageInfo> getPackagesHoldingPermissions(@NonNull String[] permissions,
diff --git a/services/core/java/com/android/server/pm/ComputerEngine.java b/services/core/java/com/android/server/pm/ComputerEngine.java
index 7a01f1e..f4c750f 100644
--- a/services/core/java/com/android/server/pm/ComputerEngine.java
+++ b/services/core/java/com/android/server/pm/ComputerEngine.java
@@ -4699,22 +4699,21 @@
// NOTE: Can't remove due to unsupported app usage
@NonNull
@Override
- public String[] getAppOpPermissionPackages(@NonNull String permissionName) {
- if (permissionName == null) {
- return EmptyArray.STRING;
- }
+ public String[] getAppOpPermissionPackages(@NonNull String permissionName, int userId) {
final int callingUid = Binder.getCallingUid();
- if (getInstantAppPackageName(callingUid) != null) {
+ enforceCrossUserPermission(callingUid, userId, false /* requireFullPermission */,
+ false /* checkShell */, "getAppOpPermissionPackages");
+ if (permissionName == null || getInstantAppPackageName(callingUid) != null
+ || !mUserManager.exists(userId)) {
return EmptyArray.STRING;
}
- final int callingUserId = UserHandle.getUserId(callingUid);
final ArraySet<String> packageNames = new ArraySet(
mPermissionManager.getAppOpPermissionPackages(permissionName));
for (int i = packageNames.size() - 1; i >= 0; i--) {
final String packageName = packageNames.valueAt(i);
- if (!shouldFilterApplication(mSettings.getPackage(packageName), callingUid,
- callingUserId)) {
+ if (!shouldFilterApplicationIncludingUninstalled(
+ mSettings.getPackage(packageName), callingUid, userId)) {
continue;
}
packageNames.removeAt(i);
diff --git a/services/core/java/com/android/server/pm/CrossProfileAppsServiceImpl.java b/services/core/java/com/android/server/pm/CrossProfileAppsServiceImpl.java
index daac7c0..ff6420c 100644
--- a/services/core/java/com/android/server/pm/CrossProfileAppsServiceImpl.java
+++ b/services/core/java/com/android/server/pm/CrossProfileAppsServiceImpl.java
@@ -265,8 +265,9 @@
}
private boolean canRequestInteractAcrossProfilesUnchecked(String packageName) {
+ final int callingUserId = mInjector.getCallingUserId();
final int[] enabledProfileIds =
- mInjector.getUserManager().getEnabledProfileIds(mInjector.getCallingUserId());
+ mInjector.getUserManager().getEnabledProfileIds(callingUserId);
if (enabledProfileIds.length < 2) {
return false;
}
@@ -274,13 +275,14 @@
return false;
}
return hasRequestedAppOpPermission(
- AppOpsManager.opToPermission(OP_INTERACT_ACROSS_PROFILES), packageName);
+ AppOpsManager.opToPermission(OP_INTERACT_ACROSS_PROFILES), packageName,
+ callingUserId);
}
- private boolean hasRequestedAppOpPermission(String permission, String packageName) {
+ private boolean hasRequestedAppOpPermission(String permission, String packageName, int userId) {
try {
String[] packages =
- mInjector.getIPackageManager().getAppOpPermissionPackages(permission);
+ mInjector.getIPackageManager().getAppOpPermissionPackages(permission, userId);
return ArrayUtils.contains(packages, packageName);
} catch (RemoteException exc) {
Slog.e(TAG, "PackageManager dead. Cannot get permission info");
@@ -604,7 +606,7 @@
return false;
}
if (!hasRequestedAppOpPermission(
- AppOpsManager.opToPermission(OP_INTERACT_ACROSS_PROFILES), packageName)) {
+ AppOpsManager.opToPermission(OP_INTERACT_ACROSS_PROFILES), packageName, userId)) {
return false;
}
return isCrossProfilePackageAllowlisted(packageName);
@@ -627,7 +629,7 @@
return false;
}
if (!hasRequestedAppOpPermission(
- AppOpsManager.opToPermission(OP_INTERACT_ACROSS_PROFILES), packageName)) {
+ AppOpsManager.opToPermission(OP_INTERACT_ACROSS_PROFILES), packageName, userId)) {
return false;
}
return !isPlatformSignedAppWithNonUserConfigurablePermission(packageName, profileIds);
diff --git a/services/core/java/com/android/server/pm/IPackageManagerBase.java b/services/core/java/com/android/server/pm/IPackageManagerBase.java
index c16071d..f4285eb 100644
--- a/services/core/java/com/android/server/pm/IPackageManagerBase.java
+++ b/services/core/java/com/android/server/pm/IPackageManagerBase.java
@@ -324,8 +324,8 @@
@NonNull
@Override
@Deprecated
- public final String[] getAppOpPermissionPackages(@NonNull String permissionName) {
- return snapshot().getAppOpPermissionPackages(permissionName);
+ public final String[] getAppOpPermissionPackages(@NonNull String permissionName, int userId) {
+ return snapshot().getAppOpPermissionPackages(permissionName, userId);
}
@Override
diff --git a/services/robotests/src/com/android/server/pm/CrossProfileAppsServiceImplRoboTest.java b/services/robotests/src/com/android/server/pm/CrossProfileAppsServiceImplRoboTest.java
index 5dc048b..df1d244 100644
--- a/services/robotests/src/com/android/server/pm/CrossProfileAppsServiceImplRoboTest.java
+++ b/services/robotests/src/com/android/server/pm/CrossProfileAppsServiceImplRoboTest.java
@@ -171,7 +171,7 @@
private void mockCrossProfileAppRequestsInteractAcrossProfiles() throws Exception {
final String permissionName = Manifest.permission.INTERACT_ACROSS_PROFILES;
- when(mIPackageManager.getAppOpPermissionPackages(permissionName))
+ when(mIPackageManager.getAppOpPermissionPackages(eq(permissionName), anyInt()))
.thenReturn(new String[] {CROSS_PROFILE_APP_PACKAGE_NAME});
}
@@ -488,7 +488,7 @@
private void mockCrossProfileAppDoesNotRequestInteractAcrossProfiles() throws Exception {
final String permissionName = Manifest.permission.INTERACT_ACROSS_PROFILES;
- when(mIPackageManager.getAppOpPermissionPackages(permissionName))
+ when(mIPackageManager.getAppOpPermissionPackages(eq(permissionName), anyInt()))
.thenReturn(new String[] {});
}
diff --git a/services/tests/PackageManagerServiceTests/appenumeration/src/com/android/server/pm/test/appenumeration/AppEnumerationInternalTests.java b/services/tests/PackageManagerServiceTests/appenumeration/src/com/android/server/pm/test/appenumeration/AppEnumerationInternalTests.java
index ab004be..08cea06 100644
--- a/services/tests/PackageManagerServiceTests/appenumeration/src/com/android/server/pm/test/appenumeration/AppEnumerationInternalTests.java
+++ b/services/tests/PackageManagerServiceTests/appenumeration/src/com/android/server/pm/test/appenumeration/AppEnumerationInternalTests.java
@@ -24,6 +24,7 @@
import android.content.pm.IPackageManager;
import android.content.pm.ProviderInfo;
import android.os.Process;
+import android.os.UserHandle;
import androidx.test.runner.AndroidJUnit4;
@@ -102,7 +103,7 @@
installPackage(HAS_APPOP_PERMISSION_APK_PATH, true /* forceQueryable */);
final String[] packageNames = mIPackageManager.getAppOpPermissionPackages(
- PERMISSION_REQUEST_INSTALL_PACKAGES);
+ PERMISSION_REQUEST_INSTALL_PACKAGES, UserHandle.myUserId());
assertThat(packageNames).asList().contains(TARGET_HAS_APPOP_PERMISSION);
}
@@ -112,7 +113,7 @@
installPackage(HAS_APPOP_PERMISSION_APK_PATH, false /* forceQueryable */);
final String[] packageNames = mIPackageManager.getAppOpPermissionPackages(
- PERMISSION_REQUEST_INSTALL_PACKAGES);
+ PERMISSION_REQUEST_INSTALL_PACKAGES, UserHandle.myUserId());
assertThat(packageNames).asList().doesNotContain(TARGET_HAS_APPOP_PERMISSION);
}
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 0f742a3..e33ca77 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
@@ -73,13 +73,20 @@
"com.android.appenumeration.crossuserpackagevisibility";
private static final String SHARED_USER_TEST_PACKAGE_NAME =
"com.android.appenumeration.shareduid";
+ private static final String HAS_APPOP_PERMISSION_PACKAGE_NAME =
+ "com.android.appenumeration.hasappoppermission";
+
private static final File CROSS_USER_TEST_APK_FILE =
new File(TEST_DATA_DIR, "AppEnumerationCrossUserPackageVisibilityTestApp.apk");
private static final File SHARED_USER_TEST_APK_FILE =
new File(TEST_DATA_DIR, "AppEnumerationSharedUserTestApp.apk");
+ private static final File HAS_APPOP_PERMISSION_APK_FILE =
+ new File(TEST_DATA_DIR, "AppEnumerationHasAppOpPermissionTestApp.apk");
private static final String ACTION_CROSS_USER_TEST =
"com.android.appenumeration.action.CROSS_USER_TEST";
+ private static final String PERMISSION_REQUEST_INSTALL_PACKAGES =
+ "android.permission.REQUEST_INSTALL_PACKAGES";
private static final ComponentName TEST_ACTIVITY_COMPONENT_NAME = new ComponentName(
CROSS_USER_TEST_PACKAGE_NAME, "com.android.appenumeration.testapp.DummyActivity");
@@ -264,6 +271,20 @@
mCurrentUser.id())).isFalse();
}
+ @Test
+ public void testGetAppOpPermissionPackages_cannotDetectPkg() throws Exception {
+ final int userId = mCurrentUser.id();
+ assertThat(mIPackageManager
+ .getAppOpPermissionPackages(PERMISSION_REQUEST_INSTALL_PACKAGES, userId))
+ .asList().doesNotContain(HAS_APPOP_PERMISSION_PACKAGE_NAME);
+
+ installPackageForUser(HAS_APPOP_PERMISSION_APK_FILE, mOtherUser, true /* forceQueryable */);
+
+ assertThat(mIPackageManager
+ .getAppOpPermissionPackages(PERMISSION_REQUEST_INSTALL_PACKAGES, userId))
+ .asList().doesNotContain(HAS_APPOP_PERMISSION_PACKAGE_NAME);
+ }
+
private boolean clearApplicationUserData(String packageName) throws Exception {
final AtomicInteger result = new AtomicInteger(-1);
final IPackageDataObserver localObserver = new IPackageDataObserver.Stub() {