Ensure permissions are revoked on state changes am: bc71724034 (f31a906e) · Commits · e / os / android_frameworks_base

services/core/java/com/android/server/pm/PackageManagerService.java

+21 −4

Original line number	Diff line number	Diff line
		@@ -290,6 +290,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;
		@@ -12292,11 +12293,16 @@ 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 {
		permissionsWithChangedDefinition =
		mPermissionManager.addAllPermissions(pkg, chatty);
		}

		@@ -12342,7 +12348,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.
		@@ -12353,9 +12362,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);
		}
		});
		}
		}

services/core/java/com/android/server/pm/permission/BasePermission.java

+19 −0

Original line number	Diff line number	Diff line
		@@ -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 wasNormal = bp.isNormal();
		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 \|\| 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.mPermissionDefinitionChanged = true;
		}
		if (PackageManagerService.DEBUG_PACKAGE_SCANNING && r != null) {
		Log.d(TAG, " Permissions: " + r);
		}

services/core/java/com/android/server/pm/permission/PermissionManagerService.java

+87 −6

Original line number	Diff line number	Diff line
		@@ -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) {
		@@ -3043,9 +3114,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) {

services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java

+16 −1

Original line number	Diff line number	Diff line
		@@ -113,13 +113,28 @@ public abstract class PermissionManagerServiceInternal extends PermissionManager
		@NonNull ArrayList<String> allPackageNames,
		@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,