diff options
5 files changed, 70 insertions, 98 deletions
diff --git a/PermissionController/res/xml/roles.xml b/PermissionController/res/xml/roles.xml index 9fcca22c0..b067c41b5 100644 --- a/PermissionController/res/xml/roles.xml +++ b/PermissionController/res/xml/roles.xml @@ -1278,7 +1278,6 @@ name="android.app.role.SYSTEM_SUPERVISION" defaultHolders="config_systemSupervision" exclusive="true" - ignoreDisabledSystemPackageWhenGranting="true" minSdkVersion="33" static="true" systemOnly="true" diff --git a/PermissionController/role-controller/java/com/android/role/controller/model/Permissions.java b/PermissionController/role-controller/java/com/android/role/controller/model/Permissions.java index 6de52ca42..ed21db7bb 100644 --- a/PermissionController/role-controller/java/com/android/role/controller/model/Permissions.java +++ b/PermissionController/role-controller/java/com/android/role/controller/model/Permissions.java @@ -51,11 +51,13 @@ public class Permissions { private static final boolean DEBUG = false; + private static final Object sPermissionInfoLock = new Object(); + private static final ArrayMap<String, Boolean> sIsRuntimePermission = new ArrayMap<>(); + private static final ArrayMap<String, Boolean> sIsRestrictedPermission = new ArrayMap<>(); + + private static final Object sForegroundBackgroundPermissionMappingsLock = new Object(); private static ArrayMap<String, String> sForegroundToBackgroundPermission; private static ArrayMap<String, List<String>> sBackgroundToForegroundPermissions; - private static final Object sForegroundBackgroundPermissionMappingsLock = new Object(); - - private static final ArrayMap<String, Boolean> sRestrictedPermissions = new ArrayMap<>(); /** * Filter a list of permissions based on their SDK versions. @@ -88,7 +90,7 @@ public class Permissions { * @param permissions the list of permissions to be granted * @param ignoreDisabledSystemPackage whether to ignore the requested permissions of a disabled * system package (if this package is an updated system - * package) + * package) when granting runtime permissions * @param overrideUserSetAndFixed whether to override user set and fixed flags on the permission * @param setGrantedByRole whether the permissions will be granted as granted-by-role * @param setGrantedByDefault whether the permissions will be granted as granted-by-default @@ -150,11 +152,13 @@ public class Permissions { PackageInfo disabledSystemPackageInfo = getFactoryPackageInfoAsUser(packageName, user, context); if (disabledSystemPackageInfo != null) { - if (ArrayUtils.isEmpty(disabledSystemPackageInfo.requestedPermissions)) { - return false; + for (int i = permissionsToGrant.size() - 1; i >= 0; i--) { + String permission = permissionsToGrant.valueAt(i); + if (isRuntimePermission(permission, context) && !ArrayUtils.contains( + disabledSystemPackageInfo.requestedPermissions, permission)) { + permissionsToGrant.removeAt(i); + } } - CollectionUtils.retainAll(permissionsToGrant, - disabledSystemPackageInfo.requestedPermissions); if (permissionsToGrant.isEmpty()) { return false; } @@ -702,6 +706,50 @@ public class Permissions { return true; } + private static boolean isRuntimePermission(@NonNull String permission, + @NonNull Context context) { + synchronized (sPermissionInfoLock) { + Boolean isRuntimePermission = sIsRuntimePermission.get(permission); + if (isRuntimePermission != null) { + return isRuntimePermission; + } + fetchPermissionInfoLocked(permission, context); + return sIsRuntimePermission.get(permission); + } + } + + private static boolean isRestrictedPermission(@NonNull String permission, + @NonNull Context context) { + synchronized (sPermissionInfoLock) { + Boolean isRestrictedPermission = sIsRestrictedPermission.get(permission); + if (isRestrictedPermission != null) { + return isRestrictedPermission; + } + fetchPermissionInfoLocked(permission, context); + return sIsRestrictedPermission.get(permission); + } + } + + private static void fetchPermissionInfoLocked(@NonNull String permission, + @NonNull Context context) { + PackageManager packageManager = context.getPackageManager(); + PermissionInfo permissionInfo = null; + try { + permissionInfo = packageManager.getPermissionInfo(permission, 0); + } catch (PackageManager.NameNotFoundException e) { + Log.e(LOG_TAG, "Cannot get PermissionInfo for permission: " + permission); + } + + // Don't expect that to be a transient error, so we can still cache the failed information. + boolean isRuntimePermission = permissionInfo != null + && permissionInfo.getProtection() == PermissionInfo.PROTECTION_DANGEROUS; + boolean isRestrictedPermission = permissionInfo != null + && (permissionInfo.flags & (PermissionInfo.FLAG_SOFT_RESTRICTED + | PermissionInfo.FLAG_HARD_RESTRICTED)) != 0; + sIsRuntimePermission.put(permission, isRuntimePermission); + sIsRestrictedPermission.put(permission, isRestrictedPermission); + } + private static boolean isForegroundPermission(@NonNull String permission, @NonNull Context context) { ensureForegroundBackgroundPermissionMappings(context); @@ -732,40 +780,13 @@ public class Permissions { synchronized (sForegroundBackgroundPermissionMappingsLock) { if (sForegroundToBackgroundPermission == null && sBackgroundToForegroundPermissions == null) { - createForegroundBackgroundPermissionMappings(context); + createForegroundBackgroundPermissionMappingsLocked(context); } } } - private static boolean isRestrictedPermission(@NonNull String permission, + private static void createForegroundBackgroundPermissionMappingsLocked( @NonNull Context context) { - synchronized (sRestrictedPermissions) { - if (sRestrictedPermissions.containsKey(permission)) { - return sRestrictedPermissions.get(permission); - } - } - - PackageManager packageManager = context.getPackageManager(); - PermissionInfo permissionInfo = null; - try { - permissionInfo = packageManager.getPermissionInfo(permission, 0); - } catch (PackageManager.NameNotFoundException e) { - Log.e(LOG_TAG, "Cannot get PermissionInfo for permission: " + permission); - } - - // Don't expect that to be a transient error, so we can still cache the failed information. - boolean isRestrictedPermission = permissionInfo != null - && (permissionInfo.flags & (PermissionInfo.FLAG_SOFT_RESTRICTED - | PermissionInfo.FLAG_HARD_RESTRICTED)) != 0; - - synchronized (sRestrictedPermissions) { - sRestrictedPermissions.put(permission, isRestrictedPermission); - } - - return isRestrictedPermission; - } - - private static void createForegroundBackgroundPermissionMappings(@NonNull Context context) { List<String> permissions = new ArrayList<>(); sBackgroundToForegroundPermissions = new ArrayMap<>(); diff --git a/PermissionController/role-controller/java/com/android/role/controller/model/Role.java b/PermissionController/role-controller/java/com/android/role/controller/model/Role.java index 942535b0f..2f2431ece 100644 --- a/PermissionController/role-controller/java/com/android/role/controller/model/Role.java +++ b/PermissionController/role-controller/java/com/android/role/controller/model/Role.java @@ -126,12 +126,6 @@ public class Role { private final Supplier<Boolean> mFeatureFlag; /** - * Whether this role should ignore the requested permissions of a disabled system package when - * granting. - */ - private final boolean mIgnoreDisabledSystemPackageWhenGranting; - - /** * The string resource for the label of this role. */ @StringRes @@ -248,8 +242,7 @@ public class Role { public Role(@NonNull String name, boolean allowBypassingQualification, @Nullable RoleBehavior behavior, @Nullable String defaultHoldersResourceName, @StringRes int descriptionResource, boolean exclusive, boolean fallBackToDefaultHolder, - @Nullable Supplier<Boolean> featureFlag, - boolean ignoreDisabledSystemPackageWhenGranting, @StringRes int labelResource, + @Nullable Supplier<Boolean> featureFlag, @StringRes int labelResource, int maxSdkVersion, int minSdkVersion, boolean onlyGrantWhenAdded, boolean overrideUserWhenGranting, @StringRes int requestDescriptionResource, @StringRes int requestTitleResource, boolean requestable, @@ -267,7 +260,6 @@ public class Role { mExclusive = exclusive; mFallBackToDefaultHolder = fallBackToDefaultHolder; mFeatureFlag = featureFlag; - mIgnoreDisabledSystemPackageWhenGranting = ignoreDisabledSystemPackageWhenGranting; mLabelResource = labelResource; mMaxSdkVersion = maxSdkVersion; mMinSdkVersion = minSdkVersion; @@ -314,13 +306,6 @@ public class Role { return mFeatureFlag; } - /** - * @see #mIgnoreDisabledSystemPackageWhenGranting - */ - public boolean shouldIgnoreDisabledSystemPackageWhenGranting() { - return mIgnoreDisabledSystemPackageWhenGranting; - } - @StringRes public int getLabelResource() { return mLabelResource; @@ -371,10 +356,6 @@ public class Role { return mShowNone; } - public boolean isSystemOnly() { - return mSystemOnly; - } - public boolean isVisible() { return mVisible; } @@ -840,7 +821,7 @@ public class Role { boolean overrideUser, @NonNull UserHandle user, @NonNull Context context) { boolean permissionOrAppOpChanged = Permissions.grantAsUser(packageName, Permissions.filterBySdkVersionAsUser(mPermissions, user, context), - mIgnoreDisabledSystemPackageWhenGranting, overrideUser, true, false, false, + SdkLevel.isAtLeastS() ? !mSystemOnly : true, overrideUser, true, false, false, user, context); List<String> appOpPermissionsToGrant = @@ -1126,8 +1107,6 @@ public class Role { + ", mExclusive=" + mExclusive + ", mFallBackToDefaultHolder=" + mFallBackToDefaultHolder + ", mFeatureFlag=" + mFeatureFlag - + ", mIgnoreDisabledSystemPackageWhenGranting=" - + mIgnoreDisabledSystemPackageWhenGranting + ", mLabelResource=" + mLabelResource + ", mMaxSdkVersion=" + mMaxSdkVersion + ", mMinSdkVersion=" + mMinSdkVersion diff --git a/PermissionController/role-controller/java/com/android/role/controller/model/RoleParser.java b/PermissionController/role-controller/java/com/android/role/controller/model/RoleParser.java index d4e5fa2d6..6d5cf69bd 100644 --- a/PermissionController/role-controller/java/com/android/role/controller/model/RoleParser.java +++ b/PermissionController/role-controller/java/com/android/role/controller/model/RoleParser.java @@ -19,12 +19,9 @@ package com.android.role.controller.model; import android.app.AppOpsManager; import android.content.Context; import android.content.Intent; -import android.content.pm.PackageManager; -import android.content.pm.PermissionInfo; import android.content.res.Resources; import android.content.res.XmlResourceParser; import android.os.Build; -import android.os.Process; import android.permission.flags.Flags; import android.util.ArrayMap; import android.util.Log; @@ -94,8 +91,6 @@ public class RoleParser { private static final String ATTRIBUTE_EXCLUSIVE = "exclusive"; private static final String ATTRIBUTE_FALL_BACK_TO_DEFAULT_HOLDER = "fallBackToDefaultHolder"; private static final String ATTRIBUTE_FEATURE_FLAG = "featureFlag"; - private static final String ATTRIBUTE_IGNORE_DISABLED_SYSTEM_PACKAGE_WHEN_GRANTING = - "ignoreDisabledSystemPackageWhenGranting"; private static final String ATTRIBUTE_LABEL = "label"; private static final String ATTRIBUTE_MAX_SDK_VERSION = "maxSdkVersion"; private static final String ATTRIBUTE_MIN_SDK_VERSION = "minSdkVersion"; @@ -431,11 +426,6 @@ public class RoleParser { Supplier<Boolean> featureFlag = getAttributeMethodValue(parser, ATTRIBUTE_FEATURE_FLAG, boolean.class, sFeatureFlagFallback, TAG_ROLE); - boolean systemOnly = getAttributeBooleanValue(parser, ATTRIBUTE_SYSTEM_ONLY, false); - boolean ignoreDisabledSystemPackageWhenGranting = getAttributeBooleanValue(parser, - ATTRIBUTE_IGNORE_DISABLED_SYSTEM_PACKAGE_WHEN_GRANTING, - SdkLevel.isAtLeastS() ? !systemOnly : true); - int maxSdkVersion = getAttributeIntValue(parser, ATTRIBUTE_MAX_SDK_VERSION, Build.VERSION_CODES.CUR_DEVELOPMENT); int minSdkVersion = getAttributeIntValue(parser, ATTRIBUTE_MIN_SDK_VERSION, @@ -494,6 +484,8 @@ public class RoleParser { return null; } + boolean systemOnly = getAttributeBooleanValue(parser, ATTRIBUTE_SYSTEM_ONLY, false); + String uiBehaviorName = getAttributeValue(parser, ATTRIBUTE_UI_BEHAVIOR); List<RequiredComponent> requiredComponents = null; @@ -575,9 +567,8 @@ public class RoleParser { preferredActivities = Collections.emptyList(); } return new Role(name, allowBypassingQualification, behavior, defaultHoldersResourceName, - descriptionResource, exclusive, fallBackToDefaultHolder, featureFlag, - ignoreDisabledSystemPackageWhenGranting, labelResource, maxSdkVersion, - minSdkVersion, onlyGrantWhenAdded, overrideUserWhenGranting, + descriptionResource, exclusive, fallBackToDefaultHolder, featureFlag, labelResource, + maxSdkVersion, minSdkVersion, onlyGrantWhenAdded, overrideUserWhenGranting, requestDescriptionResource, requestTitleResource, requestable, searchKeywordsResource, shortLabelResource, showNone, statik, systemOnly, visible, requiredComponents, permissions, appOpPermissions, appOps, preferredActivities, diff --git a/PermissionController/tests/mocking/src/com/android/permissioncontroller/tests/mocking/role/model/RoleParserTest.kt b/PermissionController/tests/mocking/src/com/android/permissioncontroller/tests/mocking/role/model/RoleParserTest.kt index f1accb6e5..da1430c0a 100644 --- a/PermissionController/tests/mocking/src/com/android/permissioncontroller/tests/mocking/role/model/RoleParserTest.kt +++ b/PermissionController/tests/mocking/src/com/android/permissioncontroller/tests/mocking/role/model/RoleParserTest.kt @@ -22,7 +22,6 @@ import android.content.pm.PermissionInfo import android.os.Process import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.platform.app.InstrumentationRegistry -import com.android.modules.utils.build.SdkLevel import com.android.permissioncontroller.role.model.RoleParserInitializer import com.android.role.controller.model.AppOp import com.android.role.controller.model.Permission @@ -75,7 +74,7 @@ class RoleParserTest { fun validateRoles(permissionSets: Map<String, PermissionSet>, roles: Map<String, Role>) { for (permissionSet in permissionSets.values) { for (permission in permissionSet.permissions) { - validatePermission(permission, false) + validatePermission(permission) } } @@ -92,14 +91,7 @@ class RoleParserTest { } for (permission in role.permissions) { - // Prevent system-only roles that ignore disabled system packages from - // granting runtime permissions for now, since that may allow apps to update and - // silently obtain a new runtime permission. - val enforceNotRuntime = - SdkLevel.isAtLeastS() && - role.isSystemOnly && - role.shouldIgnoreDisabledSystemPackageWhenGranting() - validatePermission(permission, enforceNotRuntime) + validatePermission(permission) } for (appOp in role.appOps) { @@ -119,22 +111,18 @@ class RoleParserTest { } } - private fun validatePermission(permission: Permission, enforceNotRuntime: Boolean) { + private fun validatePermission(permission: Permission) { if (!permission.isAvailableAsUser(Process.myUserHandle(), targetContext)) { return } - validatePermission(permission.name, true, enforceNotRuntime) + validatePermission(permission.name, true) } private fun validatePermission(permissionName: String) { - validatePermission(permissionName, false, false) + validatePermission(permissionName, false) } - private fun validatePermission( - permissionName: String, - enforceIsRuntimeOrRole: Boolean, - enforceNotRuntime: Boolean, - ) { + private fun validatePermission(permissionName: String, enforceIsRuntimeOrRole: Boolean) { val isAutomotive = packageManager.hasSystemFeature(PackageManager.FEATURE_AUTOMOTIVE) // Skip validation for car permissions which may not be available on all build targets. if (!isAutomotive && permissionName.startsWith("android.car")) { @@ -157,12 +145,6 @@ class RoleParserTest { "Permission is not a runtime or role permission: $permissionName" } } - - if (enforceNotRuntime) { - require(permissionInfo.protection != PermissionInfo.PROTECTION_DANGEROUS) { - "Permission is a runtime permission: $permissionName" - } - } } private fun validateAppOpPermission(appOpPermission: Permission) { |