Merge "Allow role protected permissions to be granted even if not requested by factory APK" into main

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) {