From c90f85e70a86c4a3b4287097e08e8f3f727feb4b Mon Sep 17 00:00:00 2001 From: Hai Zhang Date: Fri, 6 Jan 2023 18:48:12 -0800 Subject: Compute PermissionInfo.FLAG_INSTALLED instead of mutating ParsedPermission. PermissionInfo.FLAG_INSTALLED is about the current system state instead of the permission definition itself as in its APK, so we shouldn't be mutating the ParsedPermission object to return it. Instead, we now follow what we did for granted permissions and gather and pass installed permissions similarly, so that we return the right flag in PackageInfo.permissions. For other methods returning PermissionInfo, they are all returning permission definitions that are registered in the system, so we can just unconditionally add FLAG_INSTALLED to them. This resolves the issue when the new subsystem is enabled that PermissionController refuses to manage a permission if its definition doesn't have FLAG_INSTALLED. Bug: 263504888 Test: manual Change-Id: I891fa8b3adf7095a3c09ac448e1e377d432dafdc --- .../java/com/android/server/pm/ComputerEngine.java | 10 +++++++--- .../server/pm/parsing/PackageInfoUtils.java | 20 +++++++++++++------- .../android/server/pm/permission/Permission.java | 9 ++------- .../pm/permission/PermissionManagerService.java | 5 +++++ .../permission/PermissionManagerServiceImpl.java | 22 +++++++++++++++------- .../PermissionManagerServiceInterface.java | 12 ++++++++++++ .../PermissionManagerServiceInternal.java | 13 +++++++++++++ .../PermissionManagerServiceLoggingDecorator.java | 7 +++++++ .../access/permission/PermissionService.kt | 18 ++++++++++++++++++ .../com/android/server/pm/PackageParserTest.java | 6 ++++-- 10 files changed, 96 insertions(+), 26 deletions(-) diff --git a/services/core/java/com/android/server/pm/ComputerEngine.java b/services/core/java/com/android/server/pm/ComputerEngine.java index f433d6adc713..e40910b78cd2 100644 --- a/services/core/java/com/android/server/pm/ComputerEngine.java +++ b/services/core/java/com/android/server/pm/ComputerEngine.java @@ -1475,14 +1475,18 @@ public class ComputerEngine implements Computer { // Compute GIDs only if requested final int[] gids = (flags & PackageManager.GET_GIDS) == 0 ? EMPTY_INT_ARRAY : mPermissionManager.getGidsForUid(UserHandle.getUid(userId, ps.getAppId())); + // Compute installed permissions only if requested + final Set installedPermissions = ((flags & PackageManager.GET_PERMISSIONS) == 0 + || ArrayUtils.isEmpty(p.getPermissions())) ? Collections.emptySet() + : mPermissionManager.getInstalledPermissions(ps.getPackageName()); // Compute granted permissions only if package has requested permissions - final Set permissions = ((flags & PackageManager.GET_PERMISSIONS) == 0 + final Set grantedPermissions = ((flags & PackageManager.GET_PERMISSIONS) == 0 || ArrayUtils.isEmpty(p.getRequestedPermissions())) ? Collections.emptySet() : mPermissionManager.getGrantedPermissions(ps.getPackageName(), userId); PackageInfo packageInfo = PackageInfoUtils.generate(p, gids, flags, - state.getFirstInstallTimeMillis(), ps.getLastUpdateTime(), permissions, state, - userId, ps); + state.getFirstInstallTimeMillis(), ps.getLastUpdateTime(), installedPermissions, + grantedPermissions, state, userId, ps); if (packageInfo == null) { return null; diff --git a/services/core/java/com/android/server/pm/parsing/PackageInfoUtils.java b/services/core/java/com/android/server/pm/parsing/PackageInfoUtils.java index 41ed4dbdb720..59256d34a018 100644 --- a/services/core/java/com/android/server/pm/parsing/PackageInfoUtils.java +++ b/services/core/java/com/android/server/pm/parsing/PackageInfoUtils.java @@ -104,10 +104,11 @@ public class PackageInfoUtils { @Nullable public static PackageInfo generate(AndroidPackage pkg, int[] gids, @PackageManager.PackageInfoFlagsBits long flags, long firstInstallTime, - long lastUpdateTime, Set grantedPermissions, PackageUserStateInternal state, - @UserIdInt int userId, @NonNull PackageStateInternal pkgSetting) { + long lastUpdateTime, Set installedPermissions, Set grantedPermissions, + PackageUserStateInternal state, @UserIdInt int userId, + @NonNull PackageStateInternal pkgSetting) { return generateWithComponents(pkg, gids, flags, firstInstallTime, lastUpdateTime, - grantedPermissions, state, userId, pkgSetting); + installedPermissions, grantedPermissions, state, userId, pkgSetting); } /** @@ -115,8 +116,9 @@ public class PackageInfoUtils { */ private static PackageInfo generateWithComponents(AndroidPackage pkg, int[] gids, @PackageManager.PackageInfoFlagsBits long flags, long firstInstallTime, - long lastUpdateTime, Set grantedPermissions, PackageUserStateInternal state, - @UserIdInt int userId, @NonNull PackageStateInternal pkgSetting) { + long lastUpdateTime, Set installedPermissions, Set grantedPermissions, + PackageUserStateInternal state, @UserIdInt int userId, + @NonNull PackageStateInternal pkgSetting) { ApplicationInfo applicationInfo = generateApplicationInfo(pkg, flags, state, userId, pkgSetting); if (applicationInfo == null) { @@ -174,8 +176,12 @@ public class PackageInfoUtils { if (size > 0) { info.permissions = new PermissionInfo[size]; for (int i = 0; i < size; i++) { - info.permissions[i] = generatePermissionInfo(pkg.getPermissions().get(i), - flags); + final var permission = pkg.getPermissions().get(i); + final var permissionInfo = generatePermissionInfo(permission, flags); + if (installedPermissions.contains(permission.getName())) { + permissionInfo.flags |= PermissionInfo.FLAG_INSTALLED; + } + info.permissions[i] = permissionInfo; } } final List usesPermissions = pkg.getUsesPermissions(); 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 74b178e8708e..f3b9246de371 100644 --- a/services/core/java/com/android/server/pm/permission/Permission.java +++ b/services/core/java/com/android/server/pm/permission/Permission.java @@ -29,7 +29,6 @@ import android.util.Log; import android.util.Slog; import com.android.server.pm.PackageManagerService; -import com.android.server.pm.pkg.AndroidPackage; import com.android.server.pm.pkg.PackageState; import com.android.server.pm.pkg.component.ParsedPermission; @@ -215,10 +214,6 @@ public final class Permission { == PermissionInfo.PROTECTION_DANGEROUS; } - public boolean isInstalled() { - return (mPermissionInfo.flags & PermissionInfo.FLAG_INSTALLED) != 0; - } - public boolean isRemoved() { return (mPermissionInfo.flags & PermissionInfo.FLAG_REMOVED) != 0; } @@ -423,7 +418,6 @@ public final class Permission { if (packageState.isSystem()) { if (permission.mType == Permission.TYPE_CONFIG && !permission.mReconciled) { // It's a built-in permission and no owner, take ownership now - permissionInfo.flags |= PermissionInfo.FLAG_INSTALLED; permission.mPermissionInfo = permissionInfo; permission.mReconciled = true; permission.mUid = packageState.getAppId(); @@ -451,7 +445,6 @@ public final class Permission { final Permission tree = findPermissionTree(permissionTrees, permissionInfo.name); if (tree == null || tree.mPermissionInfo.packageName.equals(permissionInfo.packageName)) { - permissionInfo.flags |= PermissionInfo.FLAG_INSTALLED; permission.mPermissionInfo = permissionInfo; permission.mReconciled = true; permission.mUid = packageState.getAppId(); @@ -562,6 +555,8 @@ public final class Permission { permissionInfo.packageName = mPermissionInfo.packageName; permissionInfo.nonLocalizedLabel = mPermissionInfo.name; } + // A Permission in PermissionRegistry is always installed. + permissionInfo.flags |= PermissionInfo.FLAG_INSTALLED; if (targetSdkVersion >= Build.VERSION_CODES.O) { permissionInfo.protectionLevel = mPermissionInfo.protectionLevel; } 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 268a36f39a60..8973adc9c07e 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -760,6 +760,11 @@ public class PermissionManagerService extends IPermissionManager.Stub { } @NonNull @Override + public Set getInstalledPermissions(@NonNull String packageName) { + return mPermissionManagerServiceImpl.getInstalledPermissions(packageName); + } + @NonNull + @Override public Set getGrantedPermissions(@NonNull String packageName, @UserIdInt int userId) { return mPermissionManagerServiceImpl.getGrantedPermissions(packageName, userId); diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceImpl.java b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceImpl.java index f6362f267616..936ef676ab16 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceImpl.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceImpl.java @@ -2345,9 +2345,6 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt for (int i=0; i getInstalledPermissions(@NonNull String packageName) { + Objects.requireNonNull(packageName, "packageName"); + final Set installedPermissions = new ArraySet<>(); + synchronized (mLock) { + for (final Permission permission : mRegistry.getPermissions()) { + if (Objects.equals(permission.getPackageName(), packageName)) { + installedPermissions.add(permission.getName()); + } + } + } + return installedPermissions; + } + @NonNull @Override public Set getGrantedPermissions(@NonNull String packageName, diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInterface.java b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInterface.java index 08938a566754..b62c64b25c36 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInterface.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInterface.java @@ -452,6 +452,18 @@ public interface PermissionManagerServiceInterface extends PermissionManagerInte */ void writeLegacyPermissionStateTEMP(); + /** + * Get all the permissions definitions from a package that's installed in the system. + *

+ * A permission definition in a normal app may not be installed if it's overridden by the + * platform or system app that contains a conflicting definition after system upgrade. + * + * @param packageName the name of the package + * @return the names of the installed permissions + */ + @NonNull + Set getInstalledPermissions(@NonNull String packageName); + /** * Get all the permissions granted to a package. * 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 fdcf765151e2..ea85c9d87d24 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java @@ -127,6 +127,19 @@ public interface PermissionManagerServiceInternal extends PermissionManagerInter */ void writeLegacyPermissionStateTEMP(); + /** + * Get all the permissions definitions from a package that's installed in the system. + *

+ * A permission definition in a normal app may not be installed if it's overridden by the + * platform or system app that contains a conflicting definition after system upgrade. + * + * @param packageName the name of the package + * @return the names of the installed permissions + */ + //@SystemApi(client = SystemApi.Client.SYSTEM_SERVER) + @NonNull + Set getInstalledPermissions(@NonNull String packageName); + /** * Get all the permissions granted to a package. * diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceLoggingDecorator.java b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceLoggingDecorator.java index bfe0008bc9e3..a6fa304cfa84 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceLoggingDecorator.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceLoggingDecorator.java @@ -291,6 +291,13 @@ public class PermissionManagerServiceLoggingDecorator implements PermissionManag mService.writeLegacyPermissionStateTEMP(); } + @NonNull + @Override + public Set getInstalledPermissions(@NonNull String packageName) { + Log.i(LOG_TAG, "getInstalledPermissions(packageName = " + packageName + ")"); + return mService.getInstalledPermissions(packageName); + } + @NonNull @Override public Set getGrantedPermissions(@NonNull String packageName, int userId) { diff --git a/services/permission/java/com/android/server/permission/access/permission/PermissionService.kt b/services/permission/java/com/android/server/permission/access/permission/PermissionService.kt index dd36c38c1bd4..acd0a3cbbb98 100644 --- a/services/permission/java/com/android/server/permission/access/permission/PermissionService.kt +++ b/services/permission/java/com/android/server/permission/access/permission/PermissionService.kt @@ -241,6 +241,9 @@ class PermissionService( ): PermissionInfo = @Suppress("DEPRECATION") PermissionInfo(permissionInfo).apply { + // All Permission objects are registered so the PermissionInfo generated for it should + // also have FLAG_INSTALLED. + this.flags = this.flags or PermissionInfo.FLAG_INSTALLED if (!flags.hasBits(PackageManager.GET_META_DATA)) { metaData = null } @@ -322,6 +325,21 @@ class PermissionService( return permission.getGidsForUser(userId) } + override fun getInstalledPermissions(packageName: String): Set { + requireNotNull(packageName) { "packageName cannot be null" } + + val permissions = service.getState { + with(policy) { getPermissions() } + } + return permissions.mapNotNullIndexedToSet { _, _, permission -> + if (permission.packageName == packageName) { + permission.name + } else { + null + } + } + } + override fun addPermission(permissionInfo: PermissionInfo, async: Boolean): Boolean { val permissionName = permissionInfo.name requireNotNull(permissionName) { "permissionName cannot be null" } diff --git a/services/tests/PackageManagerServiceTests/server/src/com/android/server/pm/PackageParserTest.java b/services/tests/PackageManagerServiceTests/server/src/com/android/server/pm/PackageParserTest.java index c6b7736eafb5..9895e7ccdac1 100644 --- a/services/tests/PackageManagerServiceTests/server/src/com/android/server/pm/PackageParserTest.java +++ b/services/tests/PackageManagerServiceTests/server/src/com/android/server/pm/PackageParserTest.java @@ -780,9 +780,11 @@ public class PackageParserTest { assertArrayEquals(a.getSplitFlags(), b.getSplitFlags()); PackageInfo aInfo = PackageInfoUtils.generate(a, new int[]{}, 0, 0, 0, - Collections.emptySet(), PackageUserStateInternal.DEFAULT, 0, mockPkgSetting(a)); + Collections.emptySet(), Collections.emptySet(), PackageUserStateInternal.DEFAULT, 0, + mockPkgSetting(a)); PackageInfo bInfo = PackageInfoUtils.generate(b, new int[]{}, 0, 0, 0, - Collections.emptySet(), PackageUserStateInternal.DEFAULT, 0, mockPkgSetting(b)); + Collections.emptySet(), Collections.emptySet(), PackageUserStateInternal.DEFAULT, 0, + mockPkgSetting(b)); assertApplicationInfoEqual(aInfo.applicationInfo, bInfo.applicationInfo); assertEquals(ArrayUtils.size(a.getPermissions()), ArrayUtils.size(b.getPermissions())); -- cgit v1.2.3-59-g8ed1b