diff options
4 files changed, 143 insertions, 11 deletions
diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index cfc49392967e..df05b56bd0e0 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -288,6 +288,7 @@ import com.android.internal.os.SomeArgs; import com.android.internal.os.Zygote; import com.android.internal.telephony.CarrierAppUtils; import com.android.internal.util.ArrayUtils; +import com.android.internal.util.CollectionUtils; import com.android.internal.util.ConcurrentUtils; import com.android.internal.util.DumpUtils; import com.android.internal.util.FastXmlSerializer; @@ -12247,12 +12248,17 @@ public class PackageManagerService extends IPackageManager.Stub mPermissionManager.addAllPermissionGroups(pkg, chatty); } + // 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; // Don't allow ephemeral applications to define new permissions. if ((scanFlags & SCAN_AS_INSTANT_APP) != 0) { + permissionsWithChangedDefinition = null; Slog.w(TAG, "Permissions from package " + pkg.packageName + " ignored: instant apps cannot define new permissions."); } else { - mPermissionManager.addAllPermissions(pkg, chatty); + permissionsWithChangedDefinition = + mPermissionManager.addAllPermissions(pkg, chatty); } int collectionSize = pkg.instrumentation.size(); @@ -12297,7 +12303,10 @@ public class PackageManagerService extends IPackageManager.Stub } } - if (oldPkg != null) { + boolean hasOldPkg = oldPkg != null; + boolean hasPermissionDefinitionChanges = + !CollectionUtils.isEmpty(permissionsWithChangedDefinition); + if (hasOldPkg || hasPermissionDefinitionChanges) { // We need to call revokeRuntimePermissionsIfGroupChanged 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. @@ -12308,9 +12317,17 @@ public class PackageManagerService extends IPackageManager.Stub // won't be granted yet, hence new packages are no problem. final ArrayList<String> allPackageNames = new ArrayList<>(mPackages.keySet()); - AsyncTask.execute(() -> + AsyncTask.execute(() -> { + if (hasOldPkg) { mPermissionManager.revokeRuntimePermissionsIfGroupChanged(pkg, oldPkg, - allPackageNames, mPermissionCallback)); + allPackageNames, mPermissionCallback); + } + if (hasPermissionDefinitionChanges) { + mPermissionManager.revokeRuntimePermissionsIfPermissionDefinitionChanged( + permissionsWithChangedDefinition, allPackageNames, + mPermissionCallback); + } + }); } } diff --git a/services/core/java/com/android/server/pm/permission/BasePermission.java b/services/core/java/com/android/server/pm/permission/BasePermission.java index 6d22faa7032e..1f029000f8a5 100644 --- a/services/core/java/com/android/server/pm/permission/BasePermission.java +++ b/services/core/java/com/android/server/pm/permission/BasePermission.java @@ -82,6 +82,8 @@ public final class BasePermission { final @PermissionType int type; + private boolean mPermissionDefinitionChanged; + String sourcePackageName; // TODO: Can we get rid of this? Seems we only use some signature info from the setting @@ -134,6 +136,11 @@ public final class BasePermission { public Signature[] getSourceSignatures() { return sourcePackageSetting.getSignatures(); } + + public boolean isPermissionDefinitionChanged() { + return mPermissionDefinitionChanged; + } + public int getType() { return type; } @@ -151,6 +158,10 @@ public final class BasePermission { this.sourcePackageSetting = sourcePackageSetting; } + public void setPermissionDefinitionChanged(boolean shouldOverride) { + mPermissionDefinitionChanged = shouldOverride; + } + public int[] computeGids(int userId) { if (perUser) { final int[] userGids = new int[gids.length]; @@ -330,6 +341,7 @@ public final class BasePermission { boolean chatty) { final PackageSettingBase pkgSetting = (PackageSettingBase) pkg.mExtras; // Allow system apps to redefine non-system permissions + boolean ownerChanged = false; if (bp != null && !Objects.equals(bp.sourcePackageName, p.info.packageName)) { final boolean currentOwnerIsSystem = (bp.perm != null && bp.perm.owner.isSystem()); @@ -345,6 +357,7 @@ public final class BasePermission { String msg = "New decl " + p.owner + " of permission " + p.info.name + " is system; overriding " + bp.sourcePackageName; PackageManagerService.reportSettingsProblem(Log.WARN, msg); + ownerChanged = true; bp = null; } } @@ -352,6 +365,7 @@ public final class BasePermission { if (bp == null) { bp = new BasePermission(p.info.name, p.info.packageName, TYPE_NORMAL); } + boolean wasNonRuntime = !bp.isRuntime(); StringBuilder r = null; if (bp.perm == null) { if (bp.sourcePackageName == null @@ -395,6 +409,11 @@ public final class BasePermission { if (bp.perm == p) { bp.protectionLevel = p.info.protectionLevel; } + if (bp.isRuntime() && (ownerChanged || wasNonRuntime)) { + // 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.mPermissionDefinitionChanged = true; + } if (PackageManagerService.DEBUG_PACKAGE_SCANNING && r != null) { Log.d(TAG, " Permissions: " + r); } 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 e35d25676de5..18f3a0a81c7d 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -666,8 +666,74 @@ public class PermissionManagerService { } } - private void addAllPermissions(PackageParser.Package pkg, boolean chatty) { - final int N = pkg.permissions.size(); + /** + * 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 + * @param permissionCallback Callback for permission changed + */ + private void revokeRuntimePermissionsIfPermissionDefinitionChanged( + @NonNull List<String> permissionsToRevoke, + @NonNull ArrayList<String> allPackageNames, + @NonNull PermissionCallback permissionCallback) { + + final int[] userIds = mUserManagerInt.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.getPermission(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 = mPackageManagerInt.getPackageUid(packageName, 0, userId); + if (uid < Process.FIRST_APPLICATION_UID) { + // do not revoke from system apps + continue; + } + final int permissionState = checkPermission(permName, packageName, + Binder.getCallingUid(), userId); + final int flags = getPermissionFlags(permName, packageName, + Binder.getCallingUid(), 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(permName, packageName, + false, userId, permissionCallback); + } catch (Exception e) { + Slog.e(TAG, "Could not revoke " + permName + " from " + + packageName, e); + } + } + } + } + bp.setPermissionDefinitionChanged(false); + } + } + + private List<String> addAllPermissions(PackageParser.Package pkg, boolean chatty) { + final int N = ArrayUtils.size(pkg.permissions); + ArrayList<String> definitionChangedPermissions = new ArrayList<>(); for (int i=0; i<N; i++) { PackageParser.Permission p = pkg.permissions.get(i); @@ -689,19 +755,24 @@ public class PermissionManagerService { } } + final BasePermission bp; if (p.tree) { - final BasePermission bp = BasePermission.createOrUpdate( + bp = BasePermission.createOrUpdate( mSettings.getPermissionTreeLocked(p.info.name), p, pkg, mSettings.getAllPermissionTreesLocked(), chatty); mSettings.putPermissionTreeLocked(p.info.name, bp); } else { - final BasePermission bp = BasePermission.createOrUpdate( + bp = BasePermission.createOrUpdate( mSettings.getPermissionLocked(p.info.name), p, pkg, mSettings.getAllPermissionTreesLocked(), chatty); mSettings.putPermissionLocked(p.info.name, bp); } + if (bp.isPermissionDefinitionChanged()) { + definitionChangedPermissions.add(p.info.name); + } } } + return definitionChangedPermissions; } private void addAllPermissionGroups(PackageParser.Package pkg, boolean chatty) { @@ -3029,9 +3100,19 @@ public class PermissionManagerService { PermissionManagerService.this.revokeRuntimePermissionsIfGroupChanged(newPackage, oldPackage, allPackageNames, permissionCallback); } + + @Override + public void revokeRuntimePermissionsIfPermissionDefinitionChanged( + @NonNull List<String> permissionsToRevoke, + @NonNull ArrayList<String> allPackageNames, + @NonNull PermissionCallback permissionCallback) { + PermissionManagerService.this.revokeRuntimePermissionsIfPermissionDefinitionChanged( + permissionsToRevoke, allPackageNames, permissionCallback); + } + @Override - public void addAllPermissions(Package pkg, boolean chatty) { - PermissionManagerService.this.addAllPermissions(pkg, chatty); + public List<String> addAllPermissions(Package pkg, boolean chatty) { + return PermissionManagerService.this.addAllPermissions(pkg, chatty); } @Override public void addAllPermissionGroups(Package pkg, boolean chatty) { diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java index e5e9598bec57..40ff4a231751 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java @@ -114,12 +114,27 @@ public abstract class PermissionManagerServiceInternal extends PermissionManager @NonNull PermissionCallback permissionCallback); /** + * Some permissions might have been owned by a non-system package, and the system then defined + * said permission. Some other permissions may one have been install permissions, but are now + * runtime or higher. These permissions should be revoked. + * + * @param permissionsToRevoke A list of permission names to revoke + * @param allPackageNames All packages + */ + public abstract void revokeRuntimePermissionsIfPermissionDefinitionChanged( + @NonNull List<String> permissionsToRevoke, + @NonNull ArrayList<String> allPackageNames, + @NonNull PermissionCallback permissionCallback); + + /** * Add all permissions in the given package. * <p> * NOTE: argument {@code groupTEMP} is temporary until mPermissionGroups is moved to * the permission settings. + * + * @return A list of BasePermissions that were updated, and need to be revoked from packages */ - public abstract void addAllPermissions(@NonNull PackageParser.Package pkg, boolean chatty); + public abstract List<String> addAllPermissions(@NonNull PackageParser.Package pkg, boolean chatty); public abstract void addAllPermissionGroups(@NonNull PackageParser.Package pkg, boolean chatty); public abstract void removeAllPermissions(@NonNull PackageParser.Package pkg, boolean chatty); public abstract boolean addDynamicPermission(@NonNull PermissionInfo info, boolean async, |