Merge changes from topic "12933478" into pi-dev
* changes: Revoke permission on non-runtime -> runtime upgrade Ensure permissions are revoked on state changes
This commit is contained in:
committed by
Android (Google) Code Review
commit
a15664c01f
@@ -284,6 +284,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;
|
||||
@@ -11638,12 +11639,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);
|
||||
}
|
||||
|
||||
N = pkg.instrumentation.size();
|
||||
@@ -11687,7 +11693,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.
|
||||
@@ -11698,9 +11707,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);
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -81,6 +81,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
|
||||
@@ -133,6 +135,11 @@ public final class BasePermission {
|
||||
public Signature[] getSourceSignatures() {
|
||||
return sourcePackageSetting.getSignatures();
|
||||
}
|
||||
|
||||
public boolean isPermissionDefinitionChanged() {
|
||||
return mPermissionDefinitionChanged;
|
||||
}
|
||||
|
||||
public int getType() {
|
||||
return type;
|
||||
}
|
||||
@@ -150,6 +157,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];
|
||||
@@ -286,6 +297,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());
|
||||
@@ -301,6 +313,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;
|
||||
}
|
||||
}
|
||||
@@ -308,6 +321,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
|
||||
@@ -351,6 +365,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);
|
||||
}
|
||||
|
||||
@@ -107,13 +107,18 @@ public abstract class PermissionManagerInternal {
|
||||
@NonNull ArrayList<String> allPackageNames,
|
||||
@NonNull PermissionCallback permissionCallback);
|
||||
|
||||
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.
|
||||
*/
|
||||
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,
|
||||
@@ -189,4 +194,4 @@ public abstract class PermissionManagerInternal {
|
||||
|
||||
/** HACK HACK methods to allow for partial migration of data to the PermissionManager class */
|
||||
public abstract @Nullable BasePermission getPermissionTEMP(@NonNull String permName);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -469,8 +469,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,
|
||||
callingUid, userId);
|
||||
final int flags = getPermissionFlags(permName, packageName, callingUid,
|
||||
userId);
|
||||
final int flagMask = PackageManager.FLAG_PERMISSION_SYSTEM_FIXED
|
||||
| PackageManager.FLAG_PERMISSION_POLICY_FIXED
|
||||
| PackageManager.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, callingUid, 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);
|
||||
|
||||
@@ -492,19 +558,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) {
|
||||
@@ -2064,9 +2135,19 @@ public class PermissionManagerService {
|
||||
PermissionManagerService.this.revokeRuntimePermissionsIfGroupChanged(newPackage,
|
||||
oldPackage, allPackageNames, permissionCallback);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void addAllPermissions(Package pkg, boolean chatty) {
|
||||
PermissionManagerService.this.addAllPermissions(pkg, chatty);
|
||||
public void revokeRuntimePermissionsIfPermissionDefinitionChanged(
|
||||
@NonNull List<String> permissionsToRevoke,
|
||||
@NonNull ArrayList<String> allPackageNames,
|
||||
@NonNull PermissionCallback permissionCallback) {
|
||||
PermissionManagerService.this.revokeRuntimePermissionsIfPermissionDefinitionChanged(
|
||||
permissionsToRevoke, allPackageNames, permissionCallback);
|
||||
}
|
||||
|
||||
@Override
|
||||
public List<String> addAllPermissions(Package pkg, boolean chatty) {
|
||||
return PermissionManagerService.this.addAllPermissions(pkg, chatty);
|
||||
}
|
||||
@Override
|
||||
public void addAllPermissionGroups(Package pkg, boolean chatty) {
|
||||
|
||||
Reference in New Issue
Block a user