From d87a1a7d5355b8e28f9bad031a5aebc2ba432ac1 Mon Sep 17 00:00:00 2001 From: "Philip P. Moltmann" Date: Thu, 12 Apr 2018 16:44:43 -0700 Subject: [PATCH] DO NOT MERGE (M) Revoke permision when group changed If a run time permission of a group is already granted we grant the other permission of the group automatically when requested. Hence if an already granted permission changed its group during an update suddenly permission of a potentially not approved group will get auto-granted. This is undesirable, hence we revoke the permission during the update process. Test: atest android.permission.cts.PermissionGroupChange Bug: 72710897 Change-Id: Ib2165d1ae53b80455ebe02e07775853e37a2e339 --- .../server/pm/PackageManagerService.java | 106 +++++++++++++++++- 1 file changed, 105 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 1fe7602c89fdd..1535273fec51b 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -144,6 +144,7 @@ import android.content.pm.VerifierInfo; import android.content.res.Resources; import android.hardware.display.DisplayManager; import android.net.Uri; +import android.os.AsyncTask; import android.os.Debug; import android.os.Binder; import android.os.Build; @@ -3614,6 +3615,81 @@ public class PackageManagerService extends IPackageManager.Stub { killUid(appId, userId, KILL_APP_REASON_PERMISSIONS_REVOKED); } + /** + * We might auto-grant permissions if any permission of the group is already granted. Hence if + * the group of a granted permission changes we need to revoke it to avoid having permissions of + * the new group auto-granted. + * + * @param newPackage The new package that was installed + * @param oldPackage The old package that was updated + * @param allPackageNames All package names + */ + private void revokeRuntimePermissionsIfGroupChanged( + PackageParser.Package newPackage, + PackageParser.Package oldPackage, + ArrayList allPackageNames) { + final int numOldPackagePermissions = oldPackage.permissions.size(); + final ArrayMap oldPermissionNameToGroupName + = new ArrayMap<>(numOldPackagePermissions); + + for (int i = 0; i < numOldPackagePermissions; i++) { + final PackageParser.Permission permission = oldPackage.permissions.get(i); + + if (permission.group != null) { + oldPermissionNameToGroupName.put(permission.info.name, + permission.group.info.name); + } + } + + final int numNewPackagePermissions = newPackage.permissions.size(); + for (int newPermissionNum = 0; newPermissionNum < numNewPackagePermissions; + newPermissionNum++) { + final PackageParser.Permission newPermission = + newPackage.permissions.get(newPermissionNum); + final int newProtection = newPermission.info.protectionLevel; + + if ((newProtection & PermissionInfo.PROTECTION_DANGEROUS) != 0) { + final String permissionName = newPermission.info.name; + final String newPermissionGroupName = + newPermission.group == null ? null : newPermission.group.info.name; + final String oldPermissionGroupName = oldPermissionNameToGroupName.get( + permissionName); + + if (newPermissionGroupName != null + && !newPermissionGroupName.equals(oldPermissionGroupName)) { + final List users = mContext.getSystemService(UserManager.class) + .getUsers(); + + final int numUsers = users.size(); + for (int userNum = 0; userNum < numUsers; userNum++) { + final int userId = users.get(userNum).id; + final int numPackages = allPackageNames.size(); + for (int packageNum = 0; packageNum < numPackages; packageNum++) { + final String packageName = allPackageNames.get(packageNum); + + if (checkPermission(permissionName, packageName, userId) + == PackageManager.PERMISSION_GRANTED) { + EventLog.writeEvent(0x534e4554, "72710897", + newPackage.applicationInfo.uid, + "Revoking permission", permissionName, "from package", + packageName, "as the group changed from", + oldPermissionGroupName, "to", newPermissionGroupName); + + try { + revokeRuntimePermission(packageName, permissionName, userId); + } catch (IllegalArgumentException e) { + Slog.e(TAG, "Could not revoke " + permissionName + " from " + + packageName, e); + } + } + } + } + } + } + } + } + + @Override public void resetRuntimePermissions() { mContext.enforceCallingOrSelfPermission( @@ -6496,7 +6572,7 @@ public class PackageManagerService extends IPackageManager.Stub { } } - private PackageParser.Package scanPackageDirtyLI(PackageParser.Package pkg, int parseFlags, + private PackageParser.Package scanPackageDirtyLI(final PackageParser.Package pkg, int parseFlags, int scanFlags, long currentTime, UserHandle user) throws PackageManagerException { final File scanFile = new File(pkg.codePath); if (pkg.applicationInfo.getCodePath() == null || @@ -6616,6 +6692,8 @@ public class PackageManagerService extends IPackageManager.Stub { pkg.mAdoptPermissions = null; } + final PackageParser.Package oldPkg; + // writer synchronized (mPackages) { if (pkg.mSharedUserId != null) { @@ -6632,6 +6710,14 @@ public class PackageManagerService extends IPackageManager.Stub { } } + final PackageSetting oldPkgSetting = mSettings.peekPackageLPr(pkg.packageName); + if (oldPkgSetting == null) { + oldPkg = null; + } else { + oldPkg = oldPkgSetting.pkg; + } + + // Check if we are renaming from an original package name. PackageSetting origPackage = null; String realName = null; @@ -7554,6 +7640,24 @@ public class PackageManagerService extends IPackageManager.Stub { // This is a regular package, with one or more known overlay packages. createIdmapsForPackageLI(pkg); } + + if (oldPkg != null) { + // We need to call revokeRuntimePermissionsIfGroupChanged async as permission + // revokation 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 allPackageNames = new ArrayList<>(mPackages.keySet()); + + AsyncTask.execute(new Runnable() { + public void run() { + revokeRuntimePermissionsIfGroupChanged(pkg, oldPkg, allPackageNames); + } + }); + } } return pkg;