diff options
author | Philip P. Moltmann <moltmann@google.com> | 2020-10-28 11:42:03 -0700 |
---|---|---|
committer | Anis Assi <anisassi@google.com> | 2020-11-12 11:44:41 -0800 |
commit | 3d9c0920aa2d287eda8e285e2000f35746d39267 (patch) | |
tree | 9b5a38499ba336f38b7ca707bbd8d7dd24a79916 | |
parent | 87d060e9d53a79fb2cde0331bec070f65dfa5479 (diff) | |
download | base-3d9c0920aa2d287eda8e285e2000f35746d39267.tar.gz |
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)
(cherry picked from commit 3dfb544626c7f199437d5e28461c4562086f9f7e)
-rw-r--r-- | services/core/java/com/android/server/pm/BasePermission.java | 10 | ||||
-rw-r--r-- | services/core/java/com/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 30fda1e5665e..15828537e0b1 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 5ebcadcc8d2c..4625b5acc2c7 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -11302,7 +11302,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); @@ -11672,10 +11672,73 @@ public class PackageManagerService extends IPackageManager.Stub } /** + * 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<String> permissionsToRevoke, + @NonNull ArrayList<String> 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 && @@ -12007,6 +12070,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<String> permissionsWithChangedDefinition = new ArrayList<String>(); + N = pkg.permissions.size(); r = null; for (i=0; i<N; i++) { @@ -12042,6 +12109,7 @@ public class PackageManagerService extends IPackageManager.Stub BasePermission bp = permissionMap.get(p.info.name); // Allow system apps to redefine non-system permissions + boolean ownerChanged = false; if (bp != null && !Objects.equals(bp.sourcePackage, p.info.packageName)) { final boolean currentOwnerIsSystem = (bp.perm != null && isSystemApp(bp.perm.owner)); @@ -12057,6 +12125,7 @@ public class PackageManagerService extends IPackageManager.Stub String msg = "New decl " + p.owner + " of permission " + p.info.name + " is system; overriding " + bp.sourcePackage; reportSettingsProblem(Log.WARN, msg); + ownerChanged = true; bp = null; } } @@ -12068,6 +12137,7 @@ public class PackageManagerService extends IPackageManager.Stub permissionMap.put(p.info.name, bp); } + boolean wasNormal = bp.type == BasePermission.TYPE_NORMAL; if (bp.perm == null) { if (bp.sourcePackage == null || bp.sourcePackage.equals(p.info.packageName)) { @@ -12110,8 +12180,15 @@ public class PackageManagerService extends IPackageManager.Stub if (bp.perm == p) { bp.protectionLevel = p.info.protectionLevel; } - } + if (bp.isRuntime() && (ownerChanged || wasNormal)) { + // If this is a runtime permission and the owner has changed, or this was a normal + // permission, then permission state should be cleaned up + bp.setPermissionDefinitionChanged(true); + + permissionsWithChangedDefinition.add(p.info.name); + } + } if (r != null) { if (DEBUG_PACKAGE_SCANNING) Log.d(TAG, " Permissions: " + r); } @@ -12154,6 +12231,28 @@ public class PackageManagerService extends IPackageManager.Stub } } } + + boolean hasOldPkg = oldPkg != null; + boolean hasPermissionDefinitionChanges = !permissionsWithChangedDefinition.isEmpty(); + if (hasOldPkg || hasPermissionDefinitionChanges) { + // We need to call revokeRuntimePermissionsIfPermissionDefinitionChanged async + // as permission + // revoke callbacks from this method might need to kill apps which need the + // mPackages lock on a different thread. This would dead lock. + // + // Hence create a copy of all package names and pass it into + // revokeRuntimePermissionsIfGroupChanged. Only for those permissions might get + // revoked. If a new package is added before the async code runs the permission + // won't be granted yet, hence new packages are no problem. + final ArrayList<String> allPackageNames = new ArrayList<>(mPackages.keySet()); + + AsyncTask.execute(() -> { + if (hasPermissionDefinitionChanges) { + revokeRuntimePermissionsIfPermissionDefinitionChanged( + permissionsWithChangedDefinition, allPackageNames); + } + }); + } } Trace.traceEnd(TRACE_TAG_PACKAGE_MANAGER); |