From 3dfb544626c7f199437d5e28461c4562086f9f7e Mon Sep 17 00:00:00 2001 From: "Philip P. Moltmann" Date: Wed, 28 Oct 2020 11:42:03 -0700 Subject: Ensure permissions are revoked on state changes If a permission owner changes, or a permission level is upgraded, revoke the permission from all packages Test: Manual Bug: 154505240 Merged-In: I0dec9eb7c2fecd3147e33e04d3f79f6dffcf7721 Change-Id: I2b3780ba3ae5147026d4c85b3526fe1807724be6 (manually backported from commit a28931a09814a89e1c55816c794c1e1f20dc0c91) --- .../java/com/android/server/pm/BasePermission.java | 10 ++ .../android/server/pm/PackageManagerService.java | 105 ++++++++++++++++++++- 2 files changed, 112 insertions(+), 3 deletions(-) diff --git a/services/core/java/com/android/server/pm/BasePermission.java b/services/core/java/com/android/server/pm/BasePermission.java index 21000388e746..f609db0648aa 100644 --- a/services/core/java/com/android/server/pm/BasePermission.java +++ b/services/core/java/com/android/server/pm/BasePermission.java @@ -35,6 +35,8 @@ final class BasePermission { final int type; + private boolean mPermissionDefinitionChanged; + int protectionLevel; PackageParser.Permission perm; @@ -67,11 +69,19 @@ final class BasePermission { + "}"; } + public boolean isPermissionDefinitionChanged() { + return mPermissionDefinitionChanged; + } + public void setGids(int[] gids, boolean perUser) { this.gids = gids; this.perUser = perUser; } + public void setPermissionDefinitionChanged(boolean shouldOverride) { + mPermissionDefinitionChanged = shouldOverride; + } + public int[] computeGids(int userId) { if (perUser) { final int[] userGids = new int[gids.length]; diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index b4f521893377..fd9c4bbadc85 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -10784,7 +10784,7 @@ public class PackageManagerService extends IPackageManager.Stub } else { final int userId = user == null ? 0 : user.getIdentifier(); // Modify state for the given package setting - commitPackageSettings(pkg, pkgSetting, user, scanFlags, + commitPackageSettings(pkg, oldPkg, pkgSetting, user, scanFlags, (policyFlags & PackageParser.PARSE_CHATTY) != 0 /*chatty*/); if (pkgSetting.getInstantApp(userId)) { mInstantAppRegistry.addInstantAppLPw(userId, pkgSetting.appId); @@ -11146,11 +11146,74 @@ public class PackageManagerService extends IPackageManager.Stub return true; } + /** + * If permissions are upgraded to runtime, or their owner changes to the system, then any + * granted permissions must be revoked. + * + * @param permissionsToRevoke A list of permission names to revoke + * @param allPackageNames All package names + */ + private void revokeRuntimePermissionsIfPermissionDefinitionChanged( + @NonNull List permissionsToRevoke, + @NonNull ArrayList allPackageNames) { + + final int[] userIds = UserManagerService.getInstance().getUserIds(); + final int numPermissions = permissionsToRevoke.size(); + final int numUserIds = userIds.length; + final int numPackages = allPackageNames.size(); + final int callingUid = Binder.getCallingUid(); + + for (int permNum = 0; permNum < numPermissions; permNum++) { + String permName = permissionsToRevoke.get(permNum); + + BasePermission bp = mSettings.mPermissions.get(permName); + if (bp == null || !bp.isRuntime()) { + continue; + } + for (int userIdNum = 0; userIdNum < numUserIds; userIdNum++) { + final int userId = userIds[userIdNum]; + for (int packageNum = 0; packageNum < numPackages; packageNum++) { + final String packageName = allPackageNames.get(packageNum); + final int uid = getPackageUid(packageName, 0, userId); + if (uid < Process.FIRST_APPLICATION_UID) { + // do not revoke from system apps + continue; + } + final int permissionState = checkPermission(permName, packageName, + userId); + final int flags = getPermissionFlags(permName, packageName, userId); + final int flagMask = FLAG_PERMISSION_SYSTEM_FIXED + | FLAG_PERMISSION_POLICY_FIXED + | FLAG_PERMISSION_GRANTED_BY_DEFAULT; + if (permissionState == PackageManager.PERMISSION_GRANTED + && (flags & flagMask) == 0) { + EventLog.writeEvent(0x534e4554, "154505240", uid, + "Revoking permission " + permName + " from package " + + packageName + " due to definition change"); + EventLog.writeEvent(0x534e4554, "168319670", uid, + "Revoking permission " + permName + " from package " + + packageName + " due to definition change"); + Slog.e(TAG, "Revoking permission " + permName + " from package " + + packageName + " due to definition change"); + try { + revokeRuntimePermission(packageName, permName, userId, false); + } catch (Exception e) { + Slog.e(TAG, "Could not revoke " + permName + " from " + + packageName, e); + } + } + } + } + bp.setPermissionDefinitionChanged(false); + } + } + /** * Adds a scanned package to the system. When this method is finished, the package will * be available for query, resolution, etc... */ - private void commitPackageSettings(PackageParser.Package pkg, PackageSetting pkgSetting, + private void commitPackageSettings(PackageParser.Package pkg, PackageParser.Package oldPkg, + PackageSetting pkgSetting, UserHandle user, int scanFlags, boolean chatty) throws PackageManagerException { final String pkgName = pkg.packageName; if (mCustomResolverComponentName != null && @@ -11482,6 +11545,10 @@ public class PackageManagerService extends IPackageManager.Stub if (DEBUG_PACKAGE_SCANNING) Log.d(TAG, " Permission Groups: " + r); } + // If a permission has had its defining app changed, or it has had its protection + // upgraded, we need to revoke apps that hold it + final List permissionsWithChangedDefinition = new ArrayList(); + N = pkg.permissions.size(); r = null; for (i=0; i allPackageNames = new ArrayList<>(mPackages.keySet()); + + AsyncTask.execute(() -> { + if (hasPermissionDefinitionChanges) { + revokeRuntimePermissionsIfPermissionDefinitionChanged( + permissionsWithChangedDefinition, allPackageNames); + } + }); + } } Trace.traceEnd(TRACE_TAG_PACKAGE_MANAGER); -- cgit v1.2.3-59-g8ed1b