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
diff --git a/core/api/test-current.txt b/core/api/test-current.txt
index 1383096..6dab6b7 100644
--- a/core/api/test-current.txt
+++ b/core/api/test-current.txt
@@ -263,6 +263,7 @@
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 7ae514a..947ceac 100644
--- a/core/java/android/app/AppOpsManager.java
+++ b/core/java/android/app/AppOpsManager.java
@@ -8790,6 +8790,18 @@
* 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 @@
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 1bd93e4..6f6e862 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 @@
}
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 @@
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 @@
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 @@
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 @@
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 c0d988d..b0c7073 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.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 @@
}
}
- 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
// 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 @@
}
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 @@
)
}
}
- ?: 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 b487dc6..c970a3e 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.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.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.runner.RunWith;
import java.util.List;
-import java.util.concurrent.atomic.AtomicInteger;
/**
* Tests app ops version upgrades
@@ -59,7 +61,13 @@
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 @@
@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 @@
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 @@
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);
