Ensure permissions are revoked on state changes

If a permission owner changes, or a permission level is upgraded, revoke
the permission from all packages

Test: Manual
Bug: 154505240
Change-Id: I0dec9eb7c2fecd3147e33e04d3f79f6dffcf7721
This commit is contained in:
Nate Myren
2020-10-14 15:14:04 -07:00
parent dd9cc9f4cc
commit a28931a098
4 changed files with 135 additions and 8 deletions

View File

@@ -335,6 +335,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;
@@ -12686,12 +12687,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.getPackageName()
+ " ignored: instant apps cannot define new permissions.");
} else {
mPermissionManager.addAllPermissions(pkg, chatty);
permissionsWithChangedDefinition =
mPermissionManager.addAllPermissions(pkg, chatty);
}
int collectionSize = ArrayUtils.size(pkg.getInstrumentations());
@@ -12720,7 +12726,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.
@@ -12731,9 +12740,16 @@ 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));
allPackageNames);
}
if (hasPermissionDefinitionChanges) {
mPermissionManager.revokeRuntimePermissionsIfPermissionDefinitionChanged(
permissionsWithChangedDefinition, allPackageNames);
}
});
}
}

View File

@@ -74,6 +74,8 @@ public final class BasePermission {
private static final String ATTR_PACKAGE = "package";
private static final String TAG_ITEM = "item";
private boolean mPermissionDefinitionChanged;
@NonNull
private PermissionInfo mPermissionInfo;
@@ -122,6 +124,10 @@ public final class BasePermission {
return mPermissionInfo.packageName;
}
public boolean isPermissionDefinitionChanged() {
return mPermissionDefinitionChanged;
}
public int getType() {
return mType;
}
@@ -148,6 +154,10 @@ public final class BasePermission {
mReconciled = permissionInfo != null;
}
public void setPermissionDefinitionChanged(boolean shouldOverride) {
mPermissionDefinitionChanged = shouldOverride;
}
public boolean hasGids() {
return mGids.length != 0;
}
@@ -364,6 +374,7 @@ public final class BasePermission {
@NonNull AndroidPackage pkg, Collection<BasePermission> permissionTrees,
boolean chatty) {
// Allow system apps to redefine non-system permissions
boolean ownerChanged = false;
if (bp != null && !Objects.equals(bp.mPermissionInfo.packageName, p.packageName)) {
final boolean currentOwnerIsSystem;
if (!bp.mReconciled) {
@@ -389,6 +400,7 @@ public final class BasePermission {
String msg = "New decl " + pkg + " of permission "
+ p.name + " is system; overriding " + bp.mPermissionInfo.packageName;
PackageManagerService.reportSettingsProblem(Log.WARN, msg);
ownerChanged = true;
bp = null;
}
}
@@ -396,6 +408,7 @@ public final class BasePermission {
if (bp == null) {
bp = new BasePermission(p.name, p.packageName, TYPE_MANIFEST);
}
boolean wasNormal = bp.isNormal();
StringBuilder r = null;
if (!bp.mReconciled) {
if (bp.mPermissionInfo.packageName == null
@@ -435,6 +448,11 @@ public final class BasePermission {
r.append("DUP:");
r.append(p.name);
}
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);
}

View File

@@ -2327,8 +2327,74 @@ public class PermissionManagerService extends IPermissionManager.Stub {
}
}
private void addAllPermissions(AndroidPackage pkg, boolean chatty) {
/**
* 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 = checkPermissionImpl(permName, packageName,
userId);
final int flags = getPermissionFlags(permName, packageName, userId);
final int flagMask = FLAG_PERMISSION_SYSTEM_FIXED
| FLAG_PERMISSION_POLICY_FIXED
| FLAG_PERMISSION_GRANTED_BY_DEFAULT
| FLAG_PERMISSION_GRANTED_BY_ROLE;
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 {
revokeRuntimePermissionInternal(permName, packageName,
false, callingUid, userId, null, permissionCallback);
} catch (Exception e) {
Slog.e(TAG, "Could not revoke " + permName + " from "
+ packageName, e);
}
}
}
}
bp.setPermissionDefinitionChanged(false);
}
}
private List<String> addAllPermissions(AndroidPackage pkg, boolean chatty) {
final int N = ArrayUtils.size(pkg.getPermissions());
ArrayList<String> definitionChangedPermissions = new ArrayList<>();
for (int i=0; i<N; i++) {
ParsedPermission p = pkg.getPermissions().get(i);
@@ -2369,8 +2435,12 @@ public class PermissionManagerService extends IPermissionManager.Stub {
if (bp.isInstalled()) {
p.setFlags(p.getFlags() | PermissionInfo.FLAG_INSTALLED);
}
if (bp.isPermissionDefinitionChanged()) {
definitionChangedPermissions.add(p.getName());
}
}
}
return definitionChangedPermissions;
}
private void addAllPermissionGroups(AndroidPackage pkg, boolean chatty) {
@@ -4750,9 +4820,18 @@ public class PermissionManagerService extends IPermissionManager.Stub {
PermissionManagerService.this.revokeRuntimePermissionsIfGroupChanged(newPackage,
oldPackage, allPackageNames, mDefaultPermissionCallback);
}
@Override
public void addAllPermissions(AndroidPackage pkg, boolean chatty) {
PermissionManagerService.this.addAllPermissions(pkg, chatty);
public void revokeRuntimePermissionsIfPermissionDefinitionChanged(
@NonNull List<String> permissionsToRevoke,
@NonNull ArrayList<String> allPackageNames) {
PermissionManagerService.this.revokeRuntimePermissionsIfPermissionDefinitionChanged(
permissionsToRevoke, allPackageNames, mDefaultPermissionCallback);
}
@Override
public List<String> addAllPermissions(AndroidPackage pkg, boolean chatty) {
return PermissionManagerService.this.addAllPermissions(pkg, chatty);
}
@Override
public void addAllPermissionGroups(AndroidPackage pkg, boolean chatty) {

View File

@@ -255,13 +255,27 @@ public abstract class PermissionManagerServiceInternal extends PermissionManager
@NonNull AndroidPackage oldPackage,
@NonNull ArrayList<String> allPackageNames);
/**
* 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);
/**
* 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 AndroidPackage pkg, boolean chatty);
public abstract List<String> addAllPermissions(@NonNull AndroidPackage pkg, boolean chatty);
public abstract void addAllPermissionGroups(@NonNull AndroidPackage pkg, boolean chatty);
public abstract void removeAllPermissions(@NonNull AndroidPackage pkg, boolean chatty);