From 05b75fb4282bfac8da921c1cbbd0510bcc910fb6 Mon Sep 17 00:00:00 2001 From: Hai Zhang Date: Wed, 14 Feb 2024 04:12:02 +0000 Subject: Create a better implementation for permission GIDs. The old subsystem creates "fake" permissions owned by the "android" package that will contain the GIDs and be overridden by system apps, however that approach is fragile because it depends a lot on the package scan order since these "fake" permissions will be trimmed after we scan the "android" package. In contrast, it's a lot easier and straightforward to just look up the GIDs upon scanning permission definitions, which will also fix the package scanning order issue. It also helps with removing one type of permission definition and the special cases we had to add for it. This is a better version of the easier but behavior changing fix ag/26216413. This change is behind a bug-fix flag since it's still a new way of doing things, despite that it's straightforward and there's no expected behavior change except for fixing GID assignment for permissions declared in APKs-in-APEX etc on first boot. Fixes: 325137277 Bug: 322197421 Test: manually check the GIDs in dumpsys permissionmgr after clean flash Change-Id: Ied24c45734e7c57ce8ed0d015121675bfcbae54f --- core/java/android/permission/flags.aconfig | 11 +++ .../access/permission/AppIdPermissionPolicy.kt | 85 ++++++++++++++-------- 2 files changed, 64 insertions(+), 32 deletions(-) diff --git a/core/java/android/permission/flags.aconfig b/core/java/android/permission/flags.aconfig index 95d6437ce3f6..9218cb8f497d 100644 --- a/core/java/android/permission/flags.aconfig +++ b/core/java/android/permission/flags.aconfig @@ -119,3 +119,14 @@ flag { description: "Enables the getEmergencyRoleHolder API." bug: "323157319" } + +flag { + name: "new_permission_gid_enabled" + is_fixed_read_only: true + namespace: "permissions" + description: "Enable new permission GID implementation" + bug: "325137277" + metadata { + purpose: PURPOSE_BUGFIX + } +} diff --git a/services/permission/java/com/android/server/permission/access/permission/AppIdPermissionPolicy.kt b/services/permission/java/com/android/server/permission/access/permission/AppIdPermissionPolicy.kt index 558827631dfe..cb3ee7307e36 100644 --- a/services/permission/java/com/android/server/permission/access/permission/AppIdPermissionPolicy.kt +++ b/services/permission/java/com/android/server/permission/access/permission/AppIdPermissionPolicy.kt @@ -46,6 +46,7 @@ import com.android.server.pm.KnownPackages import com.android.server.pm.parsing.PackageInfoUtils import com.android.server.pm.pkg.AndroidPackage import com.android.server.pm.pkg.PackageState +import libcore.util.EmptyArray class AppIdPermissionPolicy : SchemePolicy() { private val persistence = AppIdPermissionPersistence() @@ -73,40 +74,42 @@ class AppIdPermissionPolicy : SchemePolicy() { } override fun MutateStateScope.onInitialized() { - newState.externalState.configPermissions.forEach { (permissionName, permissionEntry) -> - val oldPermission = newState.systemState.permissions[permissionName] - val newPermission = - if (oldPermission != null) { - if (permissionEntry.gids != null) { - oldPermission.copy( - gids = permissionEntry.gids, - areGidsPerUser = permissionEntry.perUser - ) - } else { - return@forEach - } - } else { - @Suppress("DEPRECATION") - val permissionInfo = - PermissionInfo().apply { - name = permissionName - packageName = PLATFORM_PACKAGE_NAME - protectionLevel = PermissionInfo.PROTECTION_SIGNATURE + if (!Flags.newPermissionGidEnabled()) { + newState.externalState.configPermissions.forEach { (permissionName, permissionEntry) -> + val oldPermission = newState.systemState.permissions[permissionName] + val newPermission = + if (oldPermission != null) { + if (permissionEntry.gids != null) { + oldPermission.copy( + gids = permissionEntry.gids, + areGidsPerUser = permissionEntry.perUser + ) + } else { + return@forEach } - if (permissionEntry.gids != null) { - Permission( - permissionInfo, - false, - Permission.TYPE_CONFIG, - 0, - permissionEntry.gids, - permissionEntry.perUser - ) } else { - Permission(permissionInfo, false, Permission.TYPE_CONFIG, 0) + @Suppress("DEPRECATION") + val permissionInfo = + PermissionInfo().apply { + name = permissionName + packageName = PLATFORM_PACKAGE_NAME + protectionLevel = PermissionInfo.PROTECTION_SIGNATURE + } + if (permissionEntry.gids != null) { + Permission( + permissionInfo, + false, + Permission.TYPE_CONFIG, + 0, + permissionEntry.gids, + permissionEntry.perUser + ) + } else { + Permission(permissionInfo, false, Permission.TYPE_CONFIG, 0) + } } - } - newState.mutateSystemState().mutatePermissions()[permissionName] = newPermission + newState.mutateSystemState().mutatePermissions()[permissionName] = newPermission + } } } @@ -459,7 +462,7 @@ class AppIdPermissionPolicy : SchemePolicy() { ) return@forEachIndexed } - val newPermission = + var newPermission = if (oldPermission != null && newPackageName != oldPermission.packageName) { val oldPackageName = oldPermission.packageName // Only allow system apps to redefine non-system permissions. @@ -582,6 +585,24 @@ class AppIdPermissionPolicy : SchemePolicy() { ) } } + if (Flags.newPermissionGidEnabled()) { + var gids = EmptyArray.INT + var areGidsPerUser = false + if (!parsedPermission.isTree && packageState.isSystem) { + newState.externalState.configPermissions[permissionName]?.let { + gids = it.gids + areGidsPerUser = it.perUser + } + } + newPermission = Permission( + newPermissionInfo, + true, + Permission.TYPE_MANIFEST, + packageState.appId, + gids, + areGidsPerUser + ) + } if (parsedPermission.isTree) { newState.mutateSystemState().mutatePermissionTrees()[permissionName] = newPermission -- cgit v1.2.3-59-g8ed1b