RESTRICT AUTOMERGE Fix to restrict admin from granting permission to …

…a sensor permission group Bug: 308138085 Test: atest PermissionControllerMockingTests:AdminRestrictedPermissionsUtilsTest LOW_COVERAGE_REASON=b/330904893 Relnote: Security bug fix to restrict admin from granting permission to a sensor permission group (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:cee0dab747af4563998f6225f120db4f318843f1) Merged-In: Id9f7b9e1d73deec867ee87d0bde9a0868dd440f2 Change-Id: Id9f7b9e1d73deec867ee87d0bde9a0868dd440f2
AICP · Sep 17, 2024 · 4ebc385 · 4ebc385
1 parent cec0ea6
commit 4ebc385
Show file tree

Hide file tree

Showing 4 changed files with 97 additions and 3 deletions.
diff --git a/.../com/android/permissioncontroller/permission/service/PermissionControllerServiceImpl.java b/.../com/android/permissioncontroller/permission/service/PermissionControllerServiceImpl.java
@@ -557,7 +557,7 @@ private boolean onSetRuntimePermissionGrantStateByDeviceAdmin(@NonNull String ca
             switch (grantState) {
                 case PERMISSION_GRANT_STATE_GRANTED:
                     if (AdminRestrictedPermissionsUtils.mayAdminGrantPermission(perm.getName(),
-                            canAdminGrantSensorsPermissions, isManagedProfile)) {
+                            group.getName(), canAdminGrantSensorsPermissions, isManagedProfile)) {
                         perm.setPolicyFixed(true);
                         group.grantRuntimePermissions(false, false, new String[]{permName});
                         autoGrantPermissionsNotifier.onPermissionAutoGranted(permName);

diff --git a/...rc/com/android/permissioncontroller/permission/utils/AdminRestrictedPermissionsUtils.java b/...rc/com/android/permissioncontroller/permission/utils/AdminRestrictedPermissionsUtils.java
@@ -24,6 +24,7 @@
 import android.util.ArraySet;
 
 import com.android.modules.utils.build.SdkLevel;
+import com.android.permissioncontroller.permission.utils.Utils;
 
 /**
  * A class for dealing with permissions that the admin may not grant in certain configurations.
@@ -88,15 +89,20 @@ public static boolean mayAdminGrantPermission(Context context, String permission
     /**
      * Returns true if the admin may grant this permission, false otherwise.
      */
-    public static boolean mayAdminGrantPermission(String permission,
+    public static boolean mayAdminGrantPermission(String permission, String permissionGroup,
             boolean canAdminGrantSensorsPermissions, boolean isManagedProfile) {
         if (!SdkLevel.isAtLeastS()) {
             return true;
         }
         if (isManagedProfile && MANAGED_PROFILE_OWNER_RESTRICTED_PERMISSIONS.contains(permission)) {
             return false;
         }
-        if (!ADMIN_RESTRICTED_SENSORS_PERMISSIONS.contains(permission)) {
+
+        boolean isAdminRestrictedSensorPermissionGroup = permissionGroup != null
+                && Utils.getPlatformPermissionNamesOfGroup(permissionGroup).stream()
+                .anyMatch(ADMIN_RESTRICTED_SENSORS_PERMISSIONS::contains);
+        if (!ADMIN_RESTRICTED_SENSORS_PERMISSIONS.contains(permission)
+                && !isAdminRestrictedSensorPermissionGroup) {
             return true;
         }
 

diff --git a/PermissionController/tests/mocking/Android.bp b/PermissionController/tests/mocking/Android.bp
@@ -110,6 +110,7 @@ android_test {
         "androidx.test.ext.junit",
         "kotlinx_coroutines_test",
         "mockito-target-extended-minus-junit4",
+        "platform-test-annotations",
     ],
 
     jni_libs: [

diff --git a/...ermissioncontroller/tests/mocking/permission/utils/AdminRestrictedPermissionsUtilsTest.kt b/...ermissioncontroller/tests/mocking/permission/utils/AdminRestrictedPermissionsUtilsTest.kt
@@ -0,0 +1,87 @@
+/*
+ * Copyright (C) 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.permissioncontroller.tests.mocking.permission.utils
+
+import android.platform.test.annotations.AsbSecurityTest
+import com.android.modules.utils.build.SdkLevel
+import com.android.permissioncontroller.permission.utils.AdminRestrictedPermissionsUtils
+import org.junit.Assert.assertEquals
+import org.junit.Assume
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.Parameterized
+import org.mockito.Mockito.mock
+
+@RunWith(Parameterized::class)
+class AdminRestrictedPermissionsUtilsTest(
+    private val permission: String,
+    private val group: String?,
+    private val canAdminGrantSensorsPermissions: Boolean,
+    private val expected: Boolean
+) {
+    @Before
+    fun setup() {
+        Assume.assumeTrue(SdkLevel.isAtLeastS())
+    }
+
+    @AsbSecurityTest(cveBugId = [308138085])
+    @Test
+    fun mayAdminGrantPermissionTest() {
+        val canGrant =
+            AdminRestrictedPermissionsUtils.mayAdminGrantPermission(
+                permission,
+                group,
+                canAdminGrantSensorsPermissions,
+                false
+            )
+        assertEquals(expected, canGrant)
+    }
+
+    companion object {
+        /**
+         * Returns a list of arrays containing the following values:
+         *
+         * 0. Permission name (String)
+         * 1. Permission group name (String)
+         * 2. Can admin grant sensors permissions (Boolean)
+         * 3. Expected return from mayAdminGrantPermission method (Boolean)
+         */
+        @JvmStatic
+        @Parameterized.Parameters(name = "{index}: validate({0}, {1}, {3}) = {4}")
+        fun getParameters(): List<Array<out Any?>> {
+            return listOf(
+                arrayOf("abc", "xyz", false, true),
+                arrayOf("abc", null, false, true),
+                arrayOf("android.permission.RECORD_AUDIO", "xyz", false, false),
+                arrayOf("abc", "android.permission-group.MICROPHONE", false, false),
+                arrayOf(
+                    "android.permission.RECORD_AUDIO",
+                    "android.permission-group.MICROPHONE",
+                    false,
+                    false
+                ),
+                arrayOf(
+                    "android.permission.RECORD_AUDIO",
+                    "android.permission-group.MICROPHONE",
+                    true,
+                    true
+                ),
+            )
+        }
+    }
+}