summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Rhed Jao <rhedjao@google.com> 2022-05-26 15:00:46 +0800
committer Rhed Jao <rhedjao@google.com> 2022-06-01 12:33:48 +0800
commit8f7e68c0f4b00f88aa0b8305aaa3ddc9ddfc1c29 (patch)
treef842934288be4dced2eac217abcceb4c7c2877f0
parent6d8dfd00779cf98fa94b560663664e5ee8bad23b (diff)
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
-rw-r--r--boot/hiddenapi/hiddenapi-unsupported.txt1
-rw-r--r--core/java/android/content/pm/IPackageManager.aidl2
-rw-r--r--services/core/java/com/android/server/pm/Computer.java2
-rw-r--r--services/core/java/com/android/server/pm/ComputerEngine.java15
-rw-r--r--services/core/java/com/android/server/pm/CrossProfileAppsServiceImpl.java14
-rw-r--r--services/core/java/com/android/server/pm/IPackageManagerBase.java4
-rw-r--r--services/robotests/src/com/android/server/pm/CrossProfileAppsServiceImplRoboTest.java4
-rw-r--r--services/tests/PackageManagerServiceTests/appenumeration/src/com/android/server/pm/test/appenumeration/AppEnumerationInternalTests.java5
-rw-r--r--services/tests/PackageManagerServiceTests/appenumeration/src/com/android/server/pm/test/appenumeration/CrossUserPackageVisibilityTests.java21
9 files changed, 45 insertions, 23 deletions
diff --git a/boot/hiddenapi/hiddenapi-unsupported.txt b/boot/hiddenapi/hiddenapi-unsupported.txt
index f6199137012f..6b71e24a1d35 100644
--- a/boot/hiddenapi/hiddenapi-unsupported.txt
+++ b/boot/hiddenapi/hiddenapi-unsupported.txt
@@ -110,7 +110,6 @@ Landroid/content/pm/IPackageInstallObserver2$Stub;-><init>()V
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 1b1143a992c9..2ef1c78474f2 100644
--- a/core/java/android/content/pm/IPackageManager.aidl
+++ b/core/java/android/content/pm/IPackageManager.aidl
@@ -746,7 +746,7 @@ interface IPackageManager {
// 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 33e9b2f27eb2..5a2f6feb4d83 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 @@ public interface Computer extends PackageDataSnapshot {
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 7a01f1ed5ba8..f4c750fbcff9 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 @@ public class ComputerEngine implements Computer {
// 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 daac7c04098a..ff6420c45c39 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 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub {
}
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 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub {
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 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub {
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 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub {
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 c16071dbbb11..f4285eb0d9d9 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 @@ public abstract class IPackageManagerBase extends IPackageManager.Stub {
@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 5dc048b9d7c6..df1d244ab879 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 @@ public class CrossProfileAppsServiceImplRoboTest {
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 @@ public class CrossProfileAppsServiceImplRoboTest {
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 ab004bea553e..08cea06fd314 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.app.AppGlobals;
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 @@ public class AppEnumerationInternalTests {
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 @@ public class AppEnumerationInternalTests {
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 0f742a39c679..e33ca7775e22 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 @@ public class CrossUserPackageVisibilityTests {
"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 @@ public class CrossUserPackageVisibilityTests {
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() {