AppOp mode for device aware permissions should come from their

permission states

The app op mode of a runtime permission is determined by its permission
state on the default device. Similarly for app op mode of device aware permissions
we should use their permission states to infer the mode

Bug: 332716249
Test: TODO
Change-Id: Id8bdf68804966a825933003f95f0461f1631af7f

5 files changed, 106 insertions, 55 deletions

diff --git a/core/api/test-current.txt b/core/api/test-current.txt
index 1383096e0941..6dab6b7aa86a 100644
--- a/core/api/test-current.txt
+++ b/core/api/test-current.txt
@@ -263,6 +263,7 @@ package android.app {
     method @RequiresPermission("android.permission.MANAGE_APPOPS") public void setHistoryParameters(int, long, int);
     method @RequiresPermission(android.Manifest.permission.MANAGE_APP_OPS_MODES) public void setMode(int, int, String, int);
     method public static int strOpToOp(@NonNull String);
+    method public int unsafeCheckOpRawNoThrow(@NonNull String, @NonNull android.content.AttributionSource);
     field public static final int ATTRIBUTION_CHAIN_ID_NONE = -1; // 0xffffffff
     field public static final int ATTRIBUTION_FLAGS_NONE = 0; // 0x0
     field public static final int ATTRIBUTION_FLAG_ACCESSOR = 1; // 0x1
diff --git a/core/java/android/app/AppOpsManager.java b/core/java/android/app/AppOpsManager.java
index 7ae514ac2491..947ceaca2f49 100644
--- a/core/java/android/app/AppOpsManager.java
+++ b/core/java/android/app/AppOpsManager.java
@@ -8790,6 +8790,18 @@ public class AppOpsManager {
      * Does not throw a security exception, does not translate {@link #MODE_FOREGROUND}.
      * @hide
      */
+    @TestApi
+    @SuppressLint("UnflaggedApi") // @TestApi without associated feature.
+    public int unsafeCheckOpRawNoThrow(
+            @NonNull String op, @NonNull AttributionSource attributionSource) {
+        return unsafeCheckOpRawNoThrow(strOpToOp(op), attributionSource);
+    }
+
+    /**
+     * Returns the <em>raw</em> mode associated with the op.
+     * Does not throw a security exception, does not translate {@link #MODE_FOREGROUND}.
+     * @hide
+     */
     public int unsafeCheckOpRawNoThrow(int op, int uid, @NonNull String packageName) {
         return unsafeCheckOpRawNoThrow(op, uid, packageName, Context.DEVICE_ID_DEFAULT);
     }
@@ -8800,8 +8812,8 @@ public class AppOpsManager {
             if (virtualDeviceId == Context.DEVICE_ID_DEFAULT) {
                 return mService.checkOperationRaw(op, uid, packageName, null);
             } else {
-                return mService.checkOperationRawForDevice(op, uid, packageName, null,
-                        Context.DEVICE_ID_DEFAULT);
+                return mService.checkOperationRawForDevice(
+                        op, uid, packageName, null, virtualDeviceId);
             }
         } catch (RemoteException e) {
             throw e.rethrowFromSystemServer();
diff --git a/services/core/java/com/android/server/appop/AppOpsService.java b/services/core/java/com/android/server/appop/AppOpsService.java
index 1bd93e4dbc36..6f6e862cecd1 100644
--- a/services/core/java/com/android/server/appop/AppOpsService.java
+++ b/services/core/java/com/android/server/appop/AppOpsService.java
@@ -2773,15 +2773,15 @@ public class AppOpsService extends IAppOpsService.Stub {
             }
             code = AppOpsManager.opToSwitch(code);
             UidState uidState = getUidStateLocked(uid, false);
-            if (uidState != null
-                    && mAppOpsCheckingService.getUidMode(
-                                    uidState.uid, getPersistentId(virtualDeviceId), code)
-                            != AppOpsManager.opToDefaultMode(code)) {
-                final int rawMode =
-                        mAppOpsCheckingService.getUidMode(
-                                uidState.uid, getPersistentId(virtualDeviceId), code);
-                return raw ? rawMode : uidState.evalMode(code, rawMode);
+            if (uidState != null) {
+                int rawUidMode = mAppOpsCheckingService.getUidMode(
+                        uidState.uid, getPersistentId(virtualDeviceId), code);
+
+                if (rawUidMode != AppOpsManager.opToDefaultMode(code)) {
+                    return raw ? rawUidMode : uidState.evalMode(code, rawUidMode);
+                }
             }
+
             Op op = getOpLocked(code, uid, packageName, null, false, pvr.bypass, /* edit */ false);
             if (op == null) {
                 return AppOpsManager.opToDefaultMode(code);
@@ -3682,26 +3682,24 @@ public class AppOpsService extends IAppOpsService.Stub {
             isRestricted = isOpRestrictedLocked(uid, code, packageName, attributionTag,
                     virtualDeviceId, pvr.bypass, false);
             final int switchCode = AppOpsManager.opToSwitch(code);
+
+            int rawUidMode;
             if (isOpAllowedForUid(uid)) {
                 // Op is always allowed for the UID, do nothing.
 
                 // If there is a non-default per UID policy (we set UID op mode only if
                 // non-default) it takes over, otherwise use the per package policy.
-            } else if (mAppOpsCheckingService.getUidMode(
-                    uidState.uid, getPersistentId(virtualDeviceId), switchCode)
+            } else if ((rawUidMode =
+                            mAppOpsCheckingService.getUidMode(
+                                    uidState.uid, getPersistentId(virtualDeviceId), switchCode))
                     != AppOpsManager.opToDefaultMode(switchCode)) {
-                final int uidMode =
-                        uidState.evalMode(
-                                code,
-                                mAppOpsCheckingService.getUidMode(
-                                        uidState.uid,
-                                        getPersistentId(virtualDeviceId),
-                                        switchCode));
+                final int uidMode = uidState.evalMode(code, rawUidMode);
                 if (!shouldStartForMode(uidMode, startIfModeDefault)) {
                     if (DEBUG) {
                         Slog.d(TAG, "startOperation: uid reject #" + uidMode + " for code "
                                 + switchCode + " (" + code + ") uid " + uid + " package "
-                                + packageName + " flags: " + AppOpsManager.flagsToString(flags));
+                                + packageName + " flags: "
+                                + AppOpsManager.flagsToString(flags));
                     }
                     attributedOp.rejected(uidState.getState(), flags);
                     scheduleOpStartedIfNeededLocked(code, uid, packageName, attributionTag,
@@ -3710,8 +3708,8 @@ public class AppOpsService extends IAppOpsService.Stub {
                     return new SyncNotedAppOp(uidMode, code, attributionTag, packageName);
                 }
             } else {
-                final Op switchOp = switchCode != code ? getOpLocked(ops, switchCode, uid, true)
-                        : op;
+                final Op switchOp =
+                        switchCode != code ? getOpLocked(ops, switchCode, uid, true) : op;
                 final int mode =
                         switchOp.uidState.evalMode(
                                 switchOp.op,
@@ -3721,9 +3719,12 @@ public class AppOpsService extends IAppOpsService.Stub {
                                         UserHandle.getUserId(switchOp.uid)));
                 if (mode != AppOpsManager.MODE_ALLOWED
                         && (!startIfModeDefault || mode != MODE_DEFAULT)) {
-                    if (DEBUG) Slog.d(TAG, "startOperation: reject #" + mode + " for code "
-                            + switchCode + " (" + code + ") uid " + uid + " package "
-                            + packageName + " flags: " + AppOpsManager.flagsToString(flags));
+                    if (DEBUG) {
+                        Slog.d(TAG, "startOperation: reject #" + mode + " for code "
+                                + switchCode + " (" + code + ") uid " + uid + " package "
+                                + packageName + " flags: "
+                                + AppOpsManager.flagsToString(flags));
+                    }
                     attributedOp.rejected(uidState.getState(), flags);
                     scheduleOpStartedIfNeededLocked(code, uid, packageName, attributionTag,
                             virtualDeviceId, flags, mode, startType, attributionFlags,
@@ -3731,6 +3732,7 @@ public class AppOpsService extends IAppOpsService.Stub {
                     return new SyncNotedAppOp(mode, code, attributionTag, packageName);
                 }
             }
+
             if (DEBUG) Slog.d(TAG, "startOperation: allowing code " + code + " uid " + uid
                     + " package " + packageName + " restricted: " + isRestricted
                     + " flags: " + AppOpsManager.flagsToString(flags));
diff --git a/services/permission/java/com/android/server/permission/access/appop/AppOpService.kt b/services/permission/java/com/android/server/permission/access/appop/AppOpService.kt
index c0d988d0c46b..b0c7073d792b 100644
--- a/services/permission/java/com/android/server/permission/access/appop/AppOpService.kt
+++ b/services/permission/java/com/android/server/permission/access/appop/AppOpService.kt
@@ -20,6 +20,7 @@ import android.app.AppOpsManager
 import android.companion.virtual.VirtualDeviceManager
 import android.os.Handler
 import android.os.UserHandle
+import android.permission.PermissionManager
 import android.permission.flags.Flags
 import android.util.ArrayMap
 import android.util.ArraySet
@@ -142,7 +143,7 @@ class AppOpService(private val service: AccessCheckingService) : AppOpsCheckingS
         }
     }
 
-    override fun getNonDefaultUidModes(uid: Int, persistentDeviceId: String): SparseIntArray {
+    override fun getNonDefaultUidModes(uid: Int, deviceId: String): SparseIntArray {
         val appId = UserHandle.getAppId(uid)
         val userId = UserHandle.getUserId(uid)
         service.getState {
@@ -150,7 +151,8 @@ class AppOpService(private val service: AccessCheckingService) : AppOpsCheckingS
                 with(appIdPolicy) { opNameMapToOpSparseArray(getAppOpModes(appId, userId)?.map) }
             if (Flags.runtimePermissionAppopsMappingEnabled()) {
                 runtimePermissionNameToAppOp.forEachIndexed { _, permissionName, appOpCode ->
-                    val mode = getUidModeFromPermissionState(appId, userId, permissionName)
+                    val mode =
+                        getUidModeFromPermissionState(appId, userId, permissionName, deviceId)
                     if (mode != AppOpsManager.opToDefaultMode(appOpCode)) {
                         modes[appOpCode] = mode
                     }
@@ -165,7 +167,7 @@ class AppOpService(private val service: AccessCheckingService) : AppOpsCheckingS
         return opNameMapToOpSparseArray(getPackageModes(packageName, userId))
     }
 
-    override fun getUidMode(uid: Int, persistentDeviceId: String, op: Int): Int {
+    override fun getUidMode(uid: Int, deviceId: String, op: Int): Int {
         val appId = UserHandle.getAppId(uid)
         val userId = UserHandle.getUserId(uid)
         val opName = AppOpsManager.opToPublicName(op)
@@ -174,7 +176,9 @@ class AppOpService(private val service: AccessCheckingService) : AppOpsCheckingS
         return if (!Flags.runtimePermissionAppopsMappingEnabled() || permissionName == null) {
             service.getState { with(appIdPolicy) { getAppOpMode(appId, userId, opName) } }
         } else {
-            service.getState { getUidModeFromPermissionState(appId, userId, permissionName) }
+            service.getState {
+                getUidModeFromPermissionState(appId, userId, permissionName, deviceId)
+            }
         }
     }
 
@@ -187,15 +191,31 @@ class AppOpService(private val service: AccessCheckingService) : AppOpsCheckingS
     private fun GetStateScope.getUidModeFromPermissionState(
         appId: Int,
         userId: Int,
-        permissionName: String
+        permissionName: String,
+        deviceId: String
     ): Int {
+        val checkDevicePermissionFlags =
+            deviceId != VirtualDeviceManager.PERSISTENT_DEVICE_ID_DEFAULT &&
+                permissionName in PermissionManager.DEVICE_AWARE_PERMISSIONS
         val permissionFlags =
-            with(permissionPolicy) { getPermissionFlags(appId, userId, permissionName) }
+            if (checkDevicePermissionFlags) {
+                with(devicePermissionPolicy) {
+                    getPermissionFlags(appId, deviceId, userId, permissionName)
+                }
+            } else {
+                with(permissionPolicy) { getPermissionFlags(appId, userId, permissionName) }
+            }
         val backgroundPermissionName = foregroundToBackgroundPermissionName[permissionName]
         val backgroundPermissionFlags =
             if (backgroundPermissionName != null) {
-                with(permissionPolicy) {
-                    getPermissionFlags(appId, userId, backgroundPermissionName)
+                if (checkDevicePermissionFlags) {
+                    with(devicePermissionPolicy) {
+                        getPermissionFlags(appId, deviceId, userId, backgroundPermissionName)
+                    }
+                } else {
+                    with(permissionPolicy) {
+                        getPermissionFlags(appId, userId, backgroundPermissionName)
+                    }
                 }
             } else {
                 PermissionFlags.RUNTIME_GRANTED
@@ -207,7 +227,7 @@ class AppOpService(private val service: AccessCheckingService) : AppOpsCheckingS
 
         val fullerPermissionName =
             PermissionService.getFullerPermission(permissionName) ?: return result
-        return getUidModeFromPermissionState(appId, userId, fullerPermissionName)
+        return getUidModeFromPermissionState(appId, userId, fullerPermissionName, deviceId)
     }
 
     private fun evaluateModeFromPermissionFlags(
@@ -224,7 +244,7 @@ class AppOpService(private val service: AccessCheckingService) : AppOpsCheckingS
             MODE_IGNORED
         }
 
-    override fun setUidMode(uid: Int, persistentDeviceId: String, code: Int, mode: Int): Boolean {
+    override fun setUidMode(uid: Int, deviceId: String, code: Int, mode: Int): Boolean {
         if (
             Flags.runtimePermissionAppopsMappingEnabled() && code in runtimeAppOpToPermissionNames
         ) {
@@ -308,7 +328,7 @@ class AppOpService(private val service: AccessCheckingService) : AppOpsCheckingS
         // and we have our own persistence.
     }
 
-    override fun getForegroundOps(uid: Int, persistentDeviceId: String): SparseBooleanArray {
+    override fun getForegroundOps(uid: Int, deviceId: String): SparseBooleanArray {
         return SparseBooleanArray().apply {
             getUidModes(uid)?.forEachIndexed { _, op, mode ->
                 if (mode == AppOpsManager.MODE_FOREGROUND) {
@@ -317,7 +337,7 @@ class AppOpService(private val service: AccessCheckingService) : AppOpsCheckingS
             }
             if (Flags.runtimePermissionAppopsMappingEnabled()) {
                 foregroundableOps.forEachIndexed { _, op, _ ->
-                    if (getUidMode(uid, persistentDeviceId, op) == AppOpsManager.MODE_FOREGROUND) {
+                    if (getUidMode(uid, deviceId, op) == AppOpsManager.MODE_FOREGROUND) {
                         this[op] = true
                     }
                 }
@@ -501,7 +521,7 @@ class AppOpService(private val service: AccessCheckingService) : AppOpsCheckingS
                         )
                     }
                 }
-                ?: runtimePermissionNameToAppOp[permissionName]?.let { appOpCode ->
+                    ?: runtimePermissionNameToAppOp[permissionName]?.let { appOpCode ->
                     addPendingChangedModeIfNeeded(
                         appId,
                         userId,
diff --git a/services/tests/servicestests/src/com/android/server/appop/AppOpsActiveWatcherTest.java b/services/tests/servicestests/src/com/android/server/appop/AppOpsActiveWatcherTest.java
index b487dc69ec6e..c970a3e34d12 100644
--- a/services/tests/servicestests/src/com/android/server/appop/AppOpsActiveWatcherTest.java
+++ b/services/tests/servicestests/src/com/android/server/appop/AppOpsActiveWatcherTest.java
@@ -16,7 +16,8 @@
 
 package com.android.server.appop;
 
-import static com.android.compatibility.common.util.SystemUtil.runWithShellPermissionIdentity;
+import static android.companion.virtual.VirtualDeviceParams.DEVICE_POLICY_CUSTOM;
+import static android.companion.virtual.VirtualDeviceParams.POLICY_TYPE_CAMERA;
 
 import static com.google.common.truth.Truth.assertThat;
 
@@ -31,6 +32,7 @@ import static org.mockito.Mockito.timeout;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 
+import android.Manifest;
 import android.app.AppOpsManager;
 import android.app.AppOpsManager.OnOpActiveChangedListener;
 import android.companion.virtual.VirtualDeviceManager;
@@ -38,7 +40,8 @@ import android.companion.virtual.VirtualDeviceParams;
 import android.content.AttributionSource;
 import android.content.Context;
 import android.os.Process;
-import android.virtualdevice.cts.common.FakeAssociationRule;
+import android.permission.PermissionManager;
+import android.virtualdevice.cts.common.VirtualDeviceRule;
 
 import androidx.test.InstrumentationRegistry;
 import androidx.test.filters.SmallTest;
@@ -49,7 +52,6 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 
 import java.util.List;
-import java.util.concurrent.atomic.AtomicInteger;
 
 /**
  * Tests app ops version upgrades
@@ -59,7 +61,13 @@ import java.util.concurrent.atomic.AtomicInteger;
 public class AppOpsActiveWatcherTest {
 
     @Rule
-    public FakeAssociationRule mFakeAssociationRule = new FakeAssociationRule();
+    public VirtualDeviceRule virtualDeviceRule =
+            VirtualDeviceRule.withAdditionalPermissions(
+                    Manifest.permission.GRANT_RUNTIME_PERMISSIONS,
+                    Manifest.permission.REVOKE_RUNTIME_PERMISSIONS,
+                    Manifest.permission.CREATE_VIRTUAL_DEVICE,
+                    Manifest.permission.GET_APP_OPS_STATS
+            );
     private static final long NOTIFICATION_TIMEOUT_MILLIS = 5000;
 
     @Test
@@ -145,20 +153,28 @@ public class AppOpsActiveWatcherTest {
 
     @Test
     public void testWatchActiveOpsForExternalDevice() {
-        final VirtualDeviceManager virtualDeviceManager = getContext().getSystemService(
-                VirtualDeviceManager.class);
-        AtomicInteger virtualDeviceId = new AtomicInteger();
-        runWithShellPermissionIdentity(() -> {
-            final VirtualDeviceManager.VirtualDevice virtualDevice =
-                    virtualDeviceManager.createVirtualDevice(
-                            mFakeAssociationRule.getAssociationInfo().getId(),
-                            new VirtualDeviceParams.Builder().setName("virtual_device").build());
-            virtualDeviceId.set(virtualDevice.getDeviceId());
-        });
+        VirtualDeviceManager.VirtualDevice virtualDevice =
+                virtualDeviceRule.createManagedVirtualDevice(
+                        new VirtualDeviceParams.Builder()
+                                .setDevicePolicy(POLICY_TYPE_CAMERA, DEVICE_POLICY_CUSTOM)
+                                .build()
+                );
+
+        PermissionManager permissionManager =
+                getContext().getSystemService(PermissionManager.class);
+
+        // Unlike runtime permission being automatically granted to the default device, we need to
+        // grant camera permission to the external device first before we can start op.
+        permissionManager.grantRuntimePermission(
+                getContext().getOpPackageName(),
+                Manifest.permission.CAMERA,
+                virtualDevice.getPersistentDeviceId()
+        );
+
         final OnOpActiveChangedListener listener = mock(OnOpActiveChangedListener.class);
         AttributionSource attributionSource = new AttributionSource(Process.myUid(),
                 getContext().getOpPackageName(), getContext().getAttributionTag(),
-                virtualDeviceId.get());
+                virtualDevice.getDeviceId());
 
         final AppOpsManager appOpsManager = getContext().getSystemService(AppOpsManager.class);
         appOpsManager.startWatchingActive(new String[]{AppOpsManager.OPSTR_CAMERA,
@@ -171,7 +187,7 @@ public class AppOpsActiveWatcherTest {
         verify(listener, timeout(NOTIFICATION_TIMEOUT_MILLIS)
                 .times(1)).onOpActiveChanged(eq(AppOpsManager.OPSTR_CAMERA),
                 eq(Process.myUid()), eq(getContext().getOpPackageName()),
-                eq(getContext().getAttributionTag()), eq(virtualDeviceId.get()), eq(true),
+                eq(getContext().getAttributionTag()), eq(virtualDevice.getDeviceId()), eq(true),
                 eq(AppOpsManager.ATTRIBUTION_FLAGS_NONE),
                 eq(AppOpsManager.ATTRIBUTION_CHAIN_ID_NONE));
         verifyNoMoreInteractions(listener);
@@ -182,7 +198,7 @@ public class AppOpsActiveWatcherTest {
         verify(listener, timeout(NOTIFICATION_TIMEOUT_MILLIS)
                 .times(1)).onOpActiveChanged(eq(AppOpsManager.OPSTR_CAMERA),
                 eq(Process.myUid()), eq(getContext().getOpPackageName()),
-                eq(getContext().getAttributionTag()), eq(virtualDeviceId.get()), eq(false),
+                eq(getContext().getAttributionTag()), eq(virtualDevice.getDeviceId()), eq(false),
                 eq(AppOpsManager.ATTRIBUTION_FLAGS_NONE),
                 eq(AppOpsManager.ATTRIBUTION_CHAIN_ID_NONE));
         verifyNoMoreInteractions(listener);