Replace permission-specific methods with package change callback.
This change replaces the following PermissionManagerInternal methods:
- revokeRuntimePermissionsIfGroupChanged()
- revokeRuntimePermissionsIfPermissionDefinitionChanged()
- addAllPermissionGroups()
- addAllPermissions()
- transferPermissions()
with a single onPackageAdded() callback, to ensure we have an API
surface that is generic enough for future new permission systems, and
exposes details that will remain absolutely necessary due to
compatibility.
We cannot use a generic package change callback because most of the
time, those callbacks should not be blocking the package changes -
however permission is different and needs synchronous callbacks.
Permission was tightly coupled with package manager, so that we need
to keep compatibility with the behavior that the permission state and
any permission changes will be ready right after package changes.
Meanwhile, permission is the only system component that needs this
level of integration with package manager so it doesn't make sense to
build a generic package change callback that may be misused by other
system components.
Bug: 158736025
Test: presubmit
Change-Id: Ia4b7acc0579408eca7bcd08d4fdce6108c31c209
diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java
index ad686f2..b11abde 100644
--- a/services/core/java/com/android/server/pm/PackageManagerService.java
+++ b/services/core/java/com/android/server/pm/PackageManagerService.java
@@ -246,7 +246,6 @@
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;
@@ -339,7 +338,6 @@
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;
@@ -11592,22 +11590,6 @@
}
pkgSetting.signatures.mSigningDetails = reconciledPkg.signingDetails;
- if (!pkg.getAdoptPermissions().isEmpty()) {
- // This package wants to adopt ownership of permissions from
- // another package.
- for (int i = pkg.getAdoptPermissions().size() - 1; i >= 0; i--) {
- final String origName = pkg.getAdoptPermissions().get(i);
- final PackageSetting orig = mSettings.getPackageLPr(origName);
- if (orig != null) {
- if (verifyPackageUpdateLPr(orig, pkg)) {
- Slog.i(TAG, "Adopting permissions from " + origName + " to "
- + pkg.getPackageName());
- mPermissionManager.transferPermissions(origName, pkg.getPackageName());
- }
- }
- }
- }
-
if (changedAbiCodePath != null && changedAbiCodePath.size() > 0) {
for (int i = changedAbiCodePath.size() - 1; i >= 0; --i) {
final String codePathString = changedAbiCodePath.get(i);
@@ -12854,27 +12836,6 @@
reconciledPkg.prepareResult != null && reconciledPkg.prepareResult.replace;
mAppsFilter.addPackage(pkgSetting, isReplace);
- // Don't allow ephemeral applications to define new permissions groups.
- if ((scanFlags & SCAN_AS_INSTANT_APP) != 0) {
- Slog.w(TAG, "Permission groups from package " + pkg.getPackageName()
- + " ignored: instant apps cannot define new permission groups.");
- } else {
- 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 {
- permissionsWithChangedDefinition =
- mPermissionManager.addAllPermissions(pkg, chatty);
- }
-
int collectionSize = ArrayUtils.size(pkg.getInstrumentations());
StringBuilder r = null;
int i;
@@ -12901,31 +12862,7 @@
}
}
- 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.
- //
- // 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<String> allPackageNames = new ArrayList<>(mPackages.keySet());
-
- AsyncTask.execute(() -> {
- if (hasOldPkg) {
- mPermissionManager.revokeRuntimePermissionsIfGroupChanged(pkg, oldPkg,
- allPackageNames);
- }
- if (hasPermissionDefinitionChanges) {
- mPermissionManager.revokeRuntimePermissionsIfPermissionDefinitionChanged(
- permissionsWithChangedDefinition, allPackageNames);
- }
- });
- }
+ mPermissionManager.onPackageAdded(pkg, (scanFlags & SCAN_AS_INSTANT_APP) != 0, oldPkg);
}
Trace.traceEnd(TRACE_TAG_PACKAGE_MANAGER);
diff --git a/services/core/java/com/android/server/pm/permission/Permission.java b/services/core/java/com/android/server/pm/permission/Permission.java
index 0245b28..687e96c 100644
--- a/services/core/java/com/android/server/pm/permission/Permission.java
+++ b/services/core/java/com/android/server/pm/permission/Permission.java
@@ -399,8 +399,7 @@
@NonNull
public static Permission createOrUpdate(@Nullable Permission permission,
@NonNull PermissionInfo permissionInfo, @NonNull AndroidPackage pkg,
- @NonNull Collection<Permission> permissionTrees, boolean isOverridingSystemPermission,
- boolean chatty) {
+ @NonNull Collection<Permission> permissionTrees, boolean isOverridingSystemPermission) {
// Allow system apps to redefine non-system permissions
boolean ownerChanged = false;
if (permission != null && !Objects.equals(permission.mPermissionInfo.packageName,
@@ -437,7 +436,7 @@
permission.mPermissionInfo = permissionInfo;
permission.mReconciled = true;
permission.mUid = pkg.getUid();
- if (chatty) {
+ if (PackageManagerService.DEBUG_PACKAGE_SCANNING) {
if (r == null) {
r = new StringBuilder(256);
} else {
@@ -456,7 +455,7 @@
+ permissionInfo.packageName + " ignored: original from "
+ permission.mPermissionInfo.packageName);
}
- } else if (chatty) {
+ } else if (PackageManagerService.DEBUG_PACKAGE_SCANNING) {
if (r == null) {
r = new StringBuilder(256);
} else {
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 6c03a28..092167d0 100644
--- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java
+++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java
@@ -93,6 +93,7 @@
import android.content.pm.parsing.component.ParsedPermissionGroup;
import android.content.pm.permission.SplitPermissionInfoParcelable;
import android.metrics.LogMaker;
+import android.os.AsyncTask;
import android.os.Binder;
import android.os.Build;
import android.os.Debug;
@@ -133,6 +134,7 @@
import com.android.internal.logging.nano.MetricsProto.MetricsEvent;
import com.android.internal.os.RoSystemProperties;
import com.android.internal.util.ArrayUtils;
+import com.android.internal.util.CollectionUtils;
import com.android.internal.util.DumpUtils;
import com.android.internal.util.IntPair;
import com.android.internal.util.Preconditions;
@@ -2363,14 +2365,9 @@
*
* @param newPackage The new package that was installed
* @param oldPackage The old package that was updated
- * @param allPackageNames All package names
- * @param permissionCallback Callback for permission changed
*/
- private void revokeRuntimePermissionsIfGroupChanged(
- @NonNull AndroidPackage newPackage,
- @NonNull AndroidPackage oldPackage,
- @NonNull ArrayList<String> allPackageNames,
- @NonNull PermissionCallback permissionCallback) {
+ private void revokeRuntimePermissionsIfGroupChangedInternal(@NonNull AndroidPackage newPackage,
+ @NonNull AndroidPackage oldPackage) {
final int numOldPackagePermissions = ArrayUtils.size(oldPackage.getPermissions());
final ArrayMap<String, String> oldPermissionNameToGroupName
= new ArrayMap<>(numOldPackagePermissions);
@@ -2403,13 +2400,9 @@
if (newPermissionGroupName != null
&& !newPermissionGroupName.equals(oldPermissionGroupName)) {
final int[] userIds = mUserManagerInt.getUserIds();
- final int numUserIds = userIds.length;
- for (int userIdNum = 0; userIdNum < numUserIds; userIdNum++) {
- final int userId = userIds[userIdNum];
-
- final int numPackages = allPackageNames.size();
- for (int packageNum = 0; packageNum < numPackages; packageNum++) {
- final String packageName = allPackageNames.get(packageNum);
+ mPackageManagerInt.forEachPackage(pkg -> {
+ final String packageName = pkg.getPackageName();
+ for (final int userId : userIds) {
final int permissionState = checkPermission(permissionName, packageName,
userId);
if (permissionState == PackageManager.PERMISSION_GRANTED) {
@@ -2422,14 +2415,15 @@
try {
revokeRuntimePermissionInternal(permissionName, packageName,
- false, callingUid, userId, null, permissionCallback);
+ false, callingUid, userId, null,
+ mDefaultPermissionCallback);
} catch (IllegalArgumentException e) {
Slog.e(TAG, "Could not revoke " + permissionName + " from "
+ packageName, e);
}
}
}
- }
+ });
}
}
}
@@ -2440,18 +2434,11 @@
* 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) {
-
+ private void revokeRuntimePermissionsIfPermissionDefinitionChangedInternal(
+ @NonNull List<String> permissionsToRevoke) {
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++) {
@@ -2462,15 +2449,14 @@
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;
- }
+ mPackageManagerInt.forEachPackage(pkg -> {
+ final String packageName = pkg.getPackageName();
+ final int appId = pkg.getUid();
+ if (appId < Process.FIRST_APPLICATION_UID) {
+ // do not revoke from system apps
+ return;
+ }
+ for (final int userId : userIds) {
final int permissionState = checkPermissionImpl(permName, packageName,
userId);
final int flags = getPermissionFlags(permName, packageName, userId);
@@ -2480,6 +2466,7 @@
| FLAG_PERMISSION_GRANTED_BY_ROLE;
if (permissionState == PackageManager.PERMISSION_GRANTED
&& (flags & flagMask) == 0) {
+ final int uid = UserHandle.getUid(userId, appId);
EventLog.writeEvent(0x534e4554, "154505240", uid,
"Revoking permission " + permName + " from package "
+ packageName + " due to definition change");
@@ -2490,18 +2477,18 @@
+ packageName + " due to definition change");
try {
revokeRuntimePermissionInternal(permName, packageName,
- false, callingUid, userId, null, permissionCallback);
+ false, callingUid, userId, null, mDefaultPermissionCallback);
} catch (Exception e) {
Slog.e(TAG, "Could not revoke " + permName + " from "
+ packageName, e);
}
}
}
- }
+ });
}
}
- private List<String> addAllPermissions(AndroidPackage pkg, boolean chatty) {
+ private List<String> addAllPermissionsInternal(@NonNull AndroidPackage pkg) {
final int N = ArrayUtils.size(pkg.getPermissions());
ArrayList<String> definitionChangedPermissions = new ArrayList<>();
for (int i=0; i<N; i++) {
@@ -2539,7 +2526,7 @@
synchronized (mLock) {
final Permission permission = Permission.createOrUpdate(oldPermission,
permissionInfo, pkg, mRegistry.getPermissionTrees(),
- isOverridingSystemPermission, chatty);
+ isOverridingSystemPermission);
if (p.isTree()) {
mRegistry.addPermissionTree(permission);
} else {
@@ -2557,7 +2544,7 @@
return definitionChangedPermissions;
}
- private void addAllPermissionGroups(AndroidPackage pkg, boolean chatty) {
+ private void addAllPermissionGroupsInternal(@NonNull AndroidPackage pkg) {
synchronized (mLock) {
final int N = ArrayUtils.size(pkg.getPermissionGroups());
StringBuilder r = null;
@@ -2568,7 +2555,7 @@
final boolean isPackageUpdate = pg.getPackageName().equals(curPackageName);
if (cur == null || isPackageUpdate) {
mRegistry.addPermissionGroup(pg);
- if (chatty && DEBUG_PACKAGE_SCANNING) {
+ if (DEBUG_PACKAGE_SCANNING) {
if (r == null) {
r = new StringBuilder(256);
} else {
@@ -2583,7 +2570,7 @@
Slog.w(TAG, "Permission group " + pg.getName() + " from package "
+ pg.getPackageName() + " ignored: original from "
+ cur.getPackageName());
- if (chatty && DEBUG_PACKAGE_SCANNING) {
+ if (DEBUG_PACKAGE_SCANNING) {
if (r == null) {
r = new StringBuilder(256);
} else {
@@ -4848,11 +4835,81 @@
}
}
- private void transferPermissions(@NonNull String oldPackageName,
- @NonNull String newPackageName) {
- synchronized (mLock) {
- mRegistry.transferPermissions(oldPackageName, newPackageName);
+ private void onPackageAddedInternal(@NonNull AndroidPackage pkg, boolean isInstantApp,
+ @Nullable AndroidPackage oldPkg) {
+ if (!pkg.getAdoptPermissions().isEmpty()) {
+ // This package wants to adopt ownership of permissions from
+ // another package.
+ for (int i = pkg.getAdoptPermissions().size() - 1; i >= 0; i--) {
+ final String origName = pkg.getAdoptPermissions().get(i);
+ if (canAdoptPermissionsInternal(origName, pkg)) {
+ Slog.i(TAG, "Adopting permissions from " + origName + " to "
+ + pkg.getPackageName());
+ synchronized (mLock) {
+ mRegistry.transferPermissions(origName, pkg.getPackageName());
+ }
+ }
+ }
}
+
+ // Don't allow ephemeral applications to define new permissions groups.
+ if (isInstantApp) {
+ Slog.w(TAG, "Permission groups from package " + pkg.getPackageName()
+ + " ignored: instant apps cannot define new permission groups.");
+ } else {
+ addAllPermissionGroupsInternal(pkg);
+ }
+
+ // 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 (isInstantApp) {
+ permissionsWithChangedDefinition = null;
+ Slog.w(TAG, "Permissions from package " + pkg.getPackageName()
+ + " ignored: instant apps cannot define new permissions.");
+ } else {
+ permissionsWithChangedDefinition = addAllPermissionsInternal(pkg);
+ }
+
+ 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.
+ AsyncTask.execute(() -> {
+ if (hasOldPkg) {
+ revokeRuntimePermissionsIfGroupChangedInternal(pkg, oldPkg);
+ }
+ if (hasPermissionDefinitionChanges) {
+ revokeRuntimePermissionsIfPermissionDefinitionChangedInternal(
+ permissionsWithChangedDefinition);
+ }
+ });
+ }
+ }
+
+ private boolean canAdoptPermissionsInternal(@NonNull String oldPackageName,
+ @NonNull AndroidPackage newPkg) {
+ final PackageSetting oldPs = mPackageManagerInt.getPackageSetting(oldPackageName);
+ if (oldPs == null) {
+ return false;
+ }
+ if (!oldPs.isSystem()) {
+ Slog.w(TAG, "Unable to update from " + oldPs.name
+ + " to " + newPkg.getPackageName()
+ + ": old package not in system partition");
+ return false;
+ }
+ if (mPackageManagerInt.getPackage(oldPs.name) != null) {
+ Slog.w(TAG, "Unable to update from " + oldPs.name
+ + " to " + newPkg.getPackageName()
+ + ": old package still exists");
+ return false;
+ }
+ return true;
}
private boolean canPropagatePermissionToInstantApp(@NonNull String permissionName) {
@@ -4955,31 +5012,6 @@
}
@Override
- public void revokeRuntimePermissionsIfGroupChanged(
- @NonNull AndroidPackage newPackage,
- @NonNull AndroidPackage oldPackage,
- @NonNull ArrayList<String> allPackageNames) {
- PermissionManagerService.this.revokeRuntimePermissionsIfGroupChanged(newPackage,
- oldPackage, allPackageNames, mDefaultPermissionCallback);
- }
-
- @Override
- 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) {
- PermissionManagerService.this.addAllPermissionGroups(pkg, chatty);
- }
- @Override
public void removeAllPermissions(AndroidPackage pkg, boolean chatty) {
PermissionManagerService.this.removeAllPermissions(pkg, chatty);
}
@@ -5313,9 +5345,10 @@
}
@Override
- public void transferPermissions(@NonNull String oldPackageName,
- @NonNull String newPackageName) {
- PermissionManagerService.this.transferPermissions(oldPackageName, newPackageName);
+ public void onPackageAdded(@NonNull AndroidPackage pkg, boolean isInstantApp,
+ @Nullable AndroidPackage oldPkg) {
+ Objects.requireNonNull(pkg);
+ onPackageAddedInternal(pkg, isInstantApp, oldPkg);
}
@Override
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 df9d0d3..11b6f31 100644
--- a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java
+++ b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java
@@ -276,42 +276,6 @@
//@SystemApi(client = SystemApi.Client.SYSTEM_SERVER)
public abstract void resetAllRuntimePermissions(@UserIdInt int userId);
- /**
- * 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 packages
- */
- public abstract void revokeRuntimePermissionsIfGroupChanged(
- @NonNull AndroidPackage newPackage,
- @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 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);
/**
@@ -578,10 +542,15 @@
@NonNull LegacyPermissionSettings legacyPermissionSettings);
/**
- * Transfers ownership of permissions from one package to another.
+ * Callback when a package has been added.
+ *
+ * @param pkg the added package
+ * @param isInstantApp whether the added package is an instant app
+ * @param oldPkg the old package, or {@code null} if none
*/
- public abstract void transferPermissions(@NonNull String oldPackageName,
- @NonNull String newPackageName);
+ //@SystemApi(client = SystemApi.Client.SYSTEM_SERVER)
+ public abstract void onPackageAdded(@NonNull AndroidPackage pkg, boolean isInstantApp,
+ @Nullable AndroidPackage oldPkg);
/**
* Check whether a permission can be propagated to instant app.