From ef9acb6ed97ececa4c8554adb693ec948a963db1 Mon Sep 17 00:00:00 2001 From: Todd Kennedy Date: Tue, 29 May 2018 15:18:06 -0700 Subject: Ensure permission held for MATCH_KNOWN_PACKAGES There's an escape clause that passes the cross user permissions if the caller UID is identical to the target user ID [eg. we're not operating across users]. However, the method getInstalledPackagesList() uses android.permission.INTERACT_ACROSS_USERS to filter the results and a calling UID check is not sufficient. Ensuure the permission is actually held, regardless of the calling UID or target user. Change-Id: Iebf88668766d387a15246d6eea6420610665105a Fixes: 80435086 Test: atest CtsAppSecurityHostTestCases:ApplicationVisibilityTest --- api/test-current.txt | 2 ++ core/java/android/content/pm/PackageManager.java | 2 ++ .../android/server/pm/PackageManagerService.java | 6 ++-- .../pm/permission/PermissionManagerInternal.java | 8 +++++ .../pm/permission/PermissionManagerService.java | 34 +++++++++++++++++----- 5 files changed, 42 insertions(+), 10 deletions(-) diff --git a/api/test-current.txt b/api/test-current.txt index ba2e4ecd53be..adb06c79eaf5 100644 --- a/api/test-current.txt +++ b/api/test-current.txt @@ -248,6 +248,7 @@ package android.content.pm { public abstract class PackageManager { method public abstract java.lang.String getDefaultBrowserPackageNameAsUser(int); method public abstract int getInstallReason(java.lang.String, android.os.UserHandle); + method public abstract java.util.List getInstalledPackagesAsUser(int, int); method public abstract java.lang.String[] getNamesForUids(int[]); method public abstract java.lang.String getPermissionControllerPackageName(); method public abstract java.lang.String getServicesSystemSharedLibraryPackageName(); @@ -256,6 +257,7 @@ package android.content.pm { field public static final java.lang.String FEATURE_ADOPTABLE_STORAGE = "android.software.adoptable_storage"; field public static final java.lang.String FEATURE_FILE_BASED_ENCRYPTION = "android.software.file_based_encryption"; field public static final int MATCH_FACTORY_ONLY = 2097152; // 0x200000 + field public static final int MATCH_KNOWN_PACKAGES = 4202496; // 0x402000 } public class PermissionInfo extends android.content.pm.PackageItemInfo implements android.os.Parcelable { diff --git a/core/java/android/content/pm/PackageManager.java b/core/java/android/content/pm/PackageManager.java index 63de8bf49f8b..3f9798b7d6c9 100644 --- a/core/java/android/content/pm/PackageManager.java +++ b/core/java/android/content/pm/PackageManager.java @@ -461,6 +461,7 @@ public abstract class PackageManager { * package. * @hide */ + @TestApi public static final int MATCH_KNOWN_PACKAGES = MATCH_UNINSTALLED_PACKAGES | MATCH_ANY_USER; /** @@ -3420,6 +3421,7 @@ public abstract class PackageManager { * deleted with {@code DONT_DELETE_DATA} flag set). * @hide */ + @TestApi @SystemApi @RequiresPermission(android.Manifest.permission.INTERACT_ACROSS_USERS_FULL) public abstract List getInstalledPackagesAsUser(@PackageInfoFlags int flags, diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index d496ab67fec4..7b7bc41e4bbb 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -4786,8 +4786,10 @@ public class PackageManagerService extends IPackageManager.Stub triaged = false; } if ((flags & PackageManager.MATCH_ANY_USER) != 0) { + // require the permission to be held; the calling uid and given user id referring + // to the same user is not sufficient mPermissionManager.enforceCrossUserPermission( - Binder.getCallingUid(), userId, false, false, + Binder.getCallingUid(), userId, false, false, true, "MATCH_ANY_USER flag requires INTERACT_ACROSS_USERS permission at " + Debug.getCallers(5)); } else if ((flags & PackageManager.MATCH_UNINSTALLED_PACKAGES) != 0 && isCallerSystemUser @@ -7871,7 +7873,7 @@ public class PackageManagerService extends IPackageManager.Stub flags = updateFlagsForPackage(flags, userId, null); final boolean listUninstalled = (flags & MATCH_KNOWN_PACKAGES) != 0; mPermissionManager.enforceCrossUserPermission(callingUid, userId, - true /* requireFullPermission */, false /* checkShell */, + false /* requireFullPermission */, false /* checkShell */, "get installed packages"); // writer diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerInternal.java b/services/core/java/com/android/server/pm/permission/PermissionManagerInternal.java index 2bd5b6793b10..a042fedf8b47 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerInternal.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerInternal.java @@ -174,6 +174,14 @@ public abstract class PermissionManagerInternal { */ public abstract void enforceCrossUserPermission(int callingUid, int userId, boolean requireFullPermission, boolean checkShell, @NonNull String message); + /** + * @see #enforceCrossUserPermission(int, int, boolean, boolean, String) + * @param requirePermissionWhenSameUser When {@code true}, still require the cross user + * permission to be held even if the callingUid and userId reference the same user. + */ + public abstract void enforceCrossUserPermission(int callingUid, int userId, + boolean requireFullPermission, boolean checkShell, + boolean requirePermissionWhenSameUser, @NonNull String message); public abstract void enforceGrantRevokeRuntimePermissionPermissions(@NonNull String message); public abstract @NonNull PermissionSettings getPermissionSettings(); diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java index 36fc1202006f..c51a72406b53 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -1381,7 +1381,9 @@ public class PermissionManagerService { "grantRuntimePermission"); enforceCrossUserPermission(callingUid, userId, - true /* requireFullPermission */, true /* checkShell */, + true, // requireFullPermission + true, // checkShell + false, // requirePermissionWhenSameUser "grantRuntimePermission"); final PackageParser.Package pkg = mPackageManagerInt.getPackage(packageName); @@ -1501,7 +1503,9 @@ public class PermissionManagerService { "revokeRuntimePermission"); enforceCrossUserPermission(Binder.getCallingUid(), userId, - true /* requireFullPermission */, true /* checkShell */, + true, // requireFullPermission + true, // checkShell + false, // requirePermissionWhenSameUser "revokeRuntimePermission"); final int appId; @@ -1658,7 +1662,9 @@ public class PermissionManagerService { enforceGrantRevokeRuntimePermissionPermissions("getPermissionFlags"); enforceCrossUserPermission(callingUid, userId, - true /* requireFullPermission */, false /* checkShell */, + true, // requireFullPermission + false, // checkShell + false, // requirePermissionWhenSameUser "getPermissionFlags"); final PackageParser.Package pkg = mPackageManagerInt.getPackage(packageName); @@ -1849,7 +1855,9 @@ public class PermissionManagerService { enforceGrantRevokeRuntimePermissionPermissions("updatePermissionFlags"); enforceCrossUserPermission(callingUid, userId, - true /* requireFullPermission */, true /* checkShell */, + true, // requireFullPermission + true, // checkShell + false, // requirePermissionWhenSameUser "updatePermissionFlags"); // Only the system can change these flags and nothing else. @@ -1904,7 +1912,9 @@ public class PermissionManagerService { enforceGrantRevokeRuntimePermissionPermissions( "updatePermissionFlagsForAllApps"); enforceCrossUserPermission(callingUid, userId, - true /* requireFullPermission */, true /* checkShell */, + true, // requireFullPermission + true, // checkShell + false, // requirePermissionWhenSameUser "updatePermissionFlagsForAllApps"); // Only the system can change system fixed flags. @@ -1944,7 +1954,8 @@ public class PermissionManagerService { * @param message the message to log on security exception */ private void enforceCrossUserPermission(int callingUid, int userId, - boolean requireFullPermission, boolean checkShell, String message) { + boolean requireFullPermission, boolean checkShell, + boolean requirePermissionWhenSameUser, String message) { if (userId < 0) { throw new IllegalArgumentException("Invalid userId " + userId); } @@ -1952,7 +1963,7 @@ public class PermissionManagerService { PackageManagerServiceUtils.enforceShellRestriction( UserManager.DISALLOW_DEBUGGING_FEATURES, callingUid, userId); } - if (userId == UserHandle.getUserId(callingUid)) return; + if (!requirePermissionWhenSameUser && userId == UserHandle.getUserId(callingUid)) return; if (callingUid != Process.SYSTEM_UID && callingUid != Process.ROOT_UID) { if (requireFullPermission) { mContext.enforceCallingOrSelfPermission( @@ -2139,7 +2150,14 @@ public class PermissionManagerService { public void enforceCrossUserPermission(int callingUid, int userId, boolean requireFullPermission, boolean checkShell, String message) { PermissionManagerService.this.enforceCrossUserPermission(callingUid, userId, - requireFullPermission, checkShell, message); + requireFullPermission, checkShell, false, message); + } + @Override + public void enforceCrossUserPermission(int callingUid, int userId, + boolean requireFullPermission, boolean checkShell, + boolean requirePermissionWhenSameUser, String message) { + PermissionManagerService.this.enforceCrossUserPermission(callingUid, userId, + requireFullPermission, checkShell, requirePermissionWhenSameUser, message); } @Override public void enforceGrantRevokeRuntimePermissionPermissions(String message) { -- cgit v1.2.3-59-g8ed1b