From 0ed1b472af194896fcf421dc862c443343c7c127 Mon Sep 17 00:00:00 2001 From: "Philip P. Moltmann" Date: Thu, 12 Apr 2018 14:45:14 -0700 Subject: DO NOT MERGE (O) 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 Change-Id: Ib2165d1ae53b80455ebe02e07775853e37a2e339 Fixes: 72710897 --- .../android/server/pm/PackageManagerService.java | 105 +++++++++++++++++++++ 1 file changed, 105 insertions(+) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 06600bf75ffa..d6b572835950 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -184,6 +184,7 @@ import android.database.ContentObserver; import android.graphics.Bitmap; import android.hardware.display.DisplayManager; import android.net.Uri; +import android.os.AsyncTask; import android.os.Binder; import android.os.Build; import android.os.Bundle; @@ -5849,6 +5850,83 @@ 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, + false); + } catch (IllegalArgumentException e) { + Slog.e(TAG, "Could not revoke " + permissionName + " from " + + packageName, e); + } + } + } + } + } + } + } + } + + /** * Get the first event id for the permission. * @@ -10743,6 +10821,8 @@ public class PackageManagerService extends IPackageManager.Stub String primaryCpuAbiFromSettings = null; String secondaryCpuAbiFromSettings = null; + final PackageParser.Package oldPkg; + // writer synchronized (mPackages) { if (pkg.mSharedUserId != null) { @@ -10843,6 +10923,12 @@ public class PackageManagerService extends IPackageManager.Stub final PackageSetting disabledPkgSetting = mSettings.getDisabledSystemPkgLPr(pkg.packageName); + if (oldPkgSetting == null) { + oldPkg = null; + } else { + oldPkg = oldPkgSetting.pkg; + } + String[] usesStaticLibraries = null; if (pkg.usesStaticLibraries != null) { usesStaticLibraries = new String[pkg.usesStaticLibraries.size()]; @@ -11175,6 +11261,25 @@ public class PackageManagerService extends IPackageManager.Stub mInstantAppRegistry.addInstantAppLPw(userId, pkgSetting.appId); } } + + 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; } -- cgit v1.2.3-59-g8ed1b