From 83a3a4a9db7835d26fddd61e97d6f87c8f8c56d2 Mon Sep 17 00:00:00 2001 From: Svet Ganov Date: Fri, 3 May 2019 18:50:43 -0700 Subject: [PATCH] Restricted permission whitelisted by default To ensure existing installers would work without a change the default state of installing a package is now that all restricted permissions are whitelisted. If the installer specifies another whitelist at install time, it determines the install state. In addition to this we now enable the restricted permission checks as a prebuilt installer is no longer required. Test: atest CtsPermission2TestCases Test: atest CtsPermissionTestCases Test: atest CtsAppSecurityTestCases:android.appsecurity.cts.PermissionsHostTest bug:132160728 Change-Id: I705e341faebe62fc2d88fd37ad8870b98e1b71b1 --- api/test-current.txt | 1 - .../android/content/pm/PackageInstaller.java | 13 +++++---- .../android/content/pm/PackageManager.java | 5 ---- .../server/pm/PackageInstallerService.java | 10 ------- .../server/pm/PackageManagerShellCommand.java | 9 +++--- .../DefaultPermissionGrantPolicy.java | 29 ++++++++++++------- .../permission/PermissionManagerService.java | 11 ++++--- .../policy/PermissionPolicyService.java | 4 +-- 8 files changed, 39 insertions(+), 43 deletions(-) diff --git a/api/test-current.txt b/api/test-current.txt index 5dc7929d06dfe..181932cf12601 100644 --- a/api/test-current.txt +++ b/api/test-current.txt @@ -725,7 +725,6 @@ package android.content.pm { field public static final int FLAG_PERMISSION_USER_SET = 1; // 0x1 field public static final int MATCH_FACTORY_ONLY = 2097152; // 0x200000 field public static final int MATCH_KNOWN_PACKAGES = 4202496; // 0x402000 - field public static boolean RESTRICTED_PERMISSIONS_ENABLED; field public static final String SYSTEM_SHARED_LIBRARY_SERVICES = "android.ext.services"; field public static final String SYSTEM_SHARED_LIBRARY_SHARED = "android.ext.shared"; } diff --git a/core/java/android/content/pm/PackageInstaller.java b/core/java/android/content/pm/PackageInstaller.java index a8815ec6cfaac..89eabc285e38d 100644 --- a/core/java/android/content/pm/PackageInstaller.java +++ b/core/java/android/content/pm/PackageInstaller.java @@ -1278,7 +1278,7 @@ public class PackageInstaller { public int mode = MODE_INVALID; /** {@hide} */ @UnsupportedAppUsage - public int installFlags; + public int installFlags = PackageManager.INSTALL_ALL_WHITELIST_RESTRICTED_PERMISSIONS; /** {@hide} */ public int installLocation = PackageInfo.INSTALL_LOCATION_INTERNAL_ONLY; /** {@hide} */ @@ -1513,18 +1513,21 @@ public class PackageInstaller { * state of the permission can be determined only at install time and cannot be * changed on updated or at a later point via the package manager APIs. * + *

Initially, all restricted permissions are whitelisted but you can change + * which ones are whitelisted by calling this method or the corresponding ones + * on the {@link PackageManager}. + * * @see PackageManager#addWhitelistedRestrictedPermission(String, String, int) * @see PackageManager#removeWhitelistedRestrictedPermission(String, String, int) */ public void setWhitelistedRestrictedPermissions(@Nullable Set permissions) { if (permissions == RESTRICTED_PERMISSIONS_ALL) { installFlags |= PackageManager.INSTALL_ALL_WHITELIST_RESTRICTED_PERMISSIONS; - } - if (permissions != null) { - this.whitelistedRestrictedPermissions = new ArrayList<>(permissions); + whitelistedRestrictedPermissions = null; } else { installFlags &= ~PackageManager.INSTALL_ALL_WHITELIST_RESTRICTED_PERMISSIONS; - this.whitelistedRestrictedPermissions = null; + whitelistedRestrictedPermissions = (permissions != null) + ? new ArrayList<>(permissions) : null; } } diff --git a/core/java/android/content/pm/PackageManager.java b/core/java/android/content/pm/PackageManager.java index dd5ca6706316d..40561f02d55fc 100644 --- a/core/java/android/content/pm/PackageManager.java +++ b/core/java/android/content/pm/PackageManager.java @@ -86,11 +86,6 @@ public abstract class PackageManager { /** {@hide} */ public static final boolean APPLY_DEFAULT_TO_DEVICE_PROTECTED_STORAGE = true; - /** {@hide} */ - @TestApi - // STOPSHIP: Remove this once we get a Play prebuilt. - public static boolean RESTRICTED_PERMISSIONS_ENABLED = false; - /** * This exception is thrown when a given package, application, or component * name cannot be found. diff --git a/services/core/java/com/android/server/pm/PackageInstallerService.java b/services/core/java/com/android/server/pm/PackageInstallerService.java index e6313d9dcbe45..35f21496f2ccd 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerService.java +++ b/services/core/java/com/android/server/pm/PackageInstallerService.java @@ -531,16 +531,6 @@ public class PackageInstallerService extends IPackageInstaller.Stub implements + "to use the PackageManager.INSTALL_GRANT_RUNTIME_PERMISSIONS flag"); } - // Only system components can circumvent restricted whitelisting when installing. - if ((params.installFlags - & PackageManager.INSTALL_ALL_WHITELIST_RESTRICTED_PERMISSIONS) != 0 - && mContext.checkCallingOrSelfPermission(Manifest.permission - .WHITELIST_RESTRICTED_PERMISSIONS) == PackageManager.PERMISSION_DENIED) { - throw new SecurityException("You need the " - + "android.permission.WHITELIST_RESTRICTED_PERMISSIONS permission to" - + " use the PackageManager.INSTALL_WHITELIST_RESTRICTED_PERMISSIONS flag"); - } - // Defensively resize giant app icons if (params.appIcon != null) { final ActivityManager am = (ActivityManager) mContext.getSystemService( diff --git a/services/core/java/com/android/server/pm/PackageManagerShellCommand.java b/services/core/java/com/android/server/pm/PackageManagerShellCommand.java index 33dd48a1ac6a0..fbf074e3ba151 100644 --- a/services/core/java/com/android/server/pm/PackageManagerShellCommand.java +++ b/services/core/java/com/android/server/pm/PackageManagerShellCommand.java @@ -2351,9 +2351,10 @@ class PackageManagerShellCommand extends ShellCommand { break; case "-g": sessionParams.installFlags |= PackageManager.INSTALL_GRANT_RUNTIME_PERMISSIONS; - case "-w": - sessionParams.installFlags |= - PackageManager.INSTALL_ALL_WHITELIST_RESTRICTED_PERMISSIONS; + break; + case "--restrict-permissions": + sessionParams.installFlags &= + ~PackageManager.INSTALL_ALL_WHITELIST_RESTRICTED_PERMISSIONS; break; case "--dont-kill": sessionParams.installFlags |= PackageManager.INSTALL_DONT_KILL_APP; @@ -3004,10 +3005,10 @@ class PackageManagerShellCommand extends ShellCommand { pw.println(" -d: allow version code downgrade (debuggable packages only)"); pw.println(" -p: partial application install (new split on top of existing pkg)"); pw.println(" -g: grant all runtime permissions"); - pw.println(" -w: whitelist all restricted permissions"); pw.println(" -S: size in bytes of package, required for stdin"); pw.println(" --user: install under the given user."); pw.println(" --dont-kill: installing a new feature split, don't kill running app"); + pw.println(" --restrict-permissions: don't whitelist restricted permissions at install"); pw.println(" --originating-uri: set URI where app was downloaded from"); pw.println(" --referrer: set URI that instigated the install of the app"); pw.println(" --pkg: specify expected package name of app being installed"); diff --git a/services/core/java/com/android/server/pm/permission/DefaultPermissionGrantPolicy.java b/services/core/java/com/android/server/pm/permission/DefaultPermissionGrantPolicy.java index e0256460042a9..4fdf1bc58e050 100644 --- a/services/core/java/com/android/server/pm/permission/DefaultPermissionGrantPolicy.java +++ b/services/core/java/com/android/server/pm/permission/DefaultPermissionGrantPolicy.java @@ -1134,7 +1134,7 @@ public final class DefaultPermissionGrantPolicy { private void grantRuntimePermissions(PackageInfo pkg, Set permissionsWithoutSplits, boolean systemFixed, boolean ignoreSystemPackage, boolean whitelistRestrictedPermissions, int userId) { - UserHandle user = UserHandle.of(userId); + UserHandle user = UserHandle.of(userId); if (pkg == null) { return; } @@ -1203,7 +1203,7 @@ public final class DefaultPermissionGrantPolicy { if (ArrayUtils.isEmpty(disabledPkg.requestedPermissions)) { return; } - if (!requestedPermissions.equals(disabledPkg.requestedPermissions)) { + if (!Arrays.equals(requestedPermissions, disabledPkg.requestedPermissions)) { grantablePermissions = new ArraySet<>(Arrays.asList(requestedPermissions)); requestedPermissions = disabledPkg.requestedPermissions; } @@ -1213,7 +1213,7 @@ public final class DefaultPermissionGrantPolicy { final int numRequestedPermissions = requestedPermissions.length; // Sort requested permissions so that all permissions that are a foreground permission (i.e. - // permisions that have background permission) are before their background permissions. + // permissions that have a background permission) are before their background permissions. final String[] sortedRequestedPermissions = new String[numRequestedPermissions]; int numForeground = 0; int numOther = 0; @@ -1258,9 +1258,16 @@ public final class DefaultPermissionGrantPolicy { continue; } - int uid = UserHandle.getUid(userId, - UserHandle.getAppId(pkg.applicationInfo.uid)); - String op = AppOpsManager.permissionToOp(permission); + // Preserve whitelisting flags. + newFlags |= (flags & PackageManager.FLAGS_PERMISSION_RESTRICTION_ANY_EXEMPT); + + // If we are whitelisting the permission, update the exempt flag before grant. + if (whitelistRestrictedPermissions && isPermissionRestricted(permission)) { + mContext.getPackageManager().updatePermissionFlags(permission, + pkg.packageName, + PackageManager.FLAG_PERMISSION_RESTRICTION_SYSTEM_EXEMPT, + PackageManager.FLAG_PERMISSION_RESTRICTION_SYSTEM_EXEMPT, user); + } if (pm.checkPermission(permission, pkg.packageName) != PackageManager.PERMISSION_GRANTED) { @@ -1268,13 +1275,12 @@ public final class DefaultPermissionGrantPolicy { .grantRuntimePermission(pkg.packageName, permission, user); } - if (whitelistRestrictedPermissions && isPermissionRestricted(permission)) { - newFlags |= PackageManager.FLAG_PERMISSION_RESTRICTION_SYSTEM_EXEMPT; - } - mContext.getPackageManager().updatePermissionFlags(permission, pkg.packageName, newFlags, newFlags, user); + int uid = UserHandle.getUid(userId, + UserHandle.getAppId(pkg.applicationInfo.uid)); + List fgPerms = mPermissionManager.getBackgroundPermissions() .get(permission); if (fgPerms != null) { @@ -1285,6 +1291,7 @@ public final class DefaultPermissionGrantPolicy { if (pm.checkPermission(fgPerm, pkg.packageName) == PackageManager.PERMISSION_GRANTED) { // Upgrade the app-op state of the fg permission to allow bg access + // TODO: Dont' call app ops from package manager code. mContext.getSystemService(AppOpsManager.class).setUidMode( AppOpsManager.permissionToOp(fgPerm), uid, AppOpsManager.MODE_ALLOWED); @@ -1295,8 +1302,10 @@ public final class DefaultPermissionGrantPolicy { } String bgPerm = getBackgroundPermission(permission); + String op = AppOpsManager.permissionToOp(permission); if (bgPerm == null) { if (op != null) { + // TODO: Dont' call app ops from package manager code. mContext.getSystemService(AppOpsManager.class).setUidMode(op, uid, AppOpsManager.MODE_ALLOWED); } 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 448e595014bcf..27d9655d72135 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -32,7 +32,6 @@ import static android.content.pm.PackageManager.FLAG_PERMISSION_WHITELIST_INSTAL import static android.content.pm.PackageManager.FLAG_PERMISSION_WHITELIST_SYSTEM; import static android.content.pm.PackageManager.FLAG_PERMISSION_WHITELIST_UPGRADE; import static android.content.pm.PackageManager.MASK_PERMISSION_FLAGS_ALL; -import static android.content.pm.PackageManager.RESTRICTED_PERMISSIONS_ENABLED; import static android.os.Trace.TRACE_TAG_PACKAGE_MANAGER; import static com.android.server.pm.PackageManagerService.DEBUG_INSTALL; @@ -1070,8 +1069,8 @@ public class PermissionManagerService { boolean wasChanged = false; - boolean restrictionExempt = !RESTRICTED_PERMISSIONS_ENABLED - || (origPermissions.getPermissionFlags(bp.name, userId) + boolean restrictionExempt = + (origPermissions.getPermissionFlags(bp.name, userId) & FLAGS_PERMISSION_RESTRICTION_ANY_EXEMPT) != 0; boolean restrictionApplied = (origPermissions.getPermissionFlags( bp.name, userId) & FLAG_PERMISSION_APPLY_RESTRICTION) != 0; @@ -1189,8 +1188,8 @@ public class PermissionManagerService { for (int userId : currentUserIds) { boolean wasChanged = false; - boolean restrictionExempt = !RESTRICTED_PERMISSIONS_ENABLED - || (origPermissions.getPermissionFlags(bp.name, userId) + boolean restrictionExempt = + (origPermissions.getPermissionFlags(bp.name, userId) & FLAGS_PERMISSION_RESTRICTION_ANY_EXEMPT) != 0; boolean restrictionApplied = (origPermissions.getPermissionFlags( bp.name, userId) & FLAG_PERMISSION_APPLY_RESTRICTION) != 0; @@ -2066,7 +2065,7 @@ public class PermissionManagerService { return; } - if (RESTRICTED_PERMISSIONS_ENABLED && bp.isHardOrSoftRestricted() + if (bp.isHardOrSoftRestricted() && (flags & PackageManager.FLAGS_PERMISSION_RESTRICTION_ANY_EXEMPT) == 0) { Log.e(TAG, "Cannot grant restricted non-exempt permission " + permName + " for package " + packageName); diff --git a/services/core/java/com/android/server/policy/PermissionPolicyService.java b/services/core/java/com/android/server/policy/PermissionPolicyService.java index a280d83fac275..250f3313e3c45 100644 --- a/services/core/java/com/android/server/policy/PermissionPolicyService.java +++ b/services/core/java/com/android/server/policy/PermissionPolicyService.java @@ -326,8 +326,8 @@ public final class PermissionPolicyService extends SystemService { return; } - final boolean applyRestriction = PackageManager.RESTRICTED_PERMISSIONS_ENABLED - && (mPackageManager.getPermissionFlags(permission, pkg.packageName, + final boolean applyRestriction = + (mPackageManager.getPermissionFlags(permission, pkg.packageName, mContext.getUser()) & FLAG_PERMISSION_APPLY_RESTRICTION) != 0; if (permissionInfo.isHardRestricted()) {