Delay appop revocation when only capability is lost
On proc state changes, if the proc state doesn't change the uid state
over the restriction threshold and if no new capabilities are added then
delay the uid state commit. This fixes cases such as going from
UID_STATE_TOP -> UID_STATE_FOREGROUND, we shouldn't be revoking appop
access right away.
Flag: android.permission.flags.delay_uid_state_changes_from_capability_updates
Test: AppOpsUidStateTrackerTest
Bug: 347891382
Change-Id: Ia23c0f5b79fa194539151ffbd04a36891e06361b
diff --git a/core/java/android/permission/flags.aconfig b/core/java/android/permission/flags.aconfig
index bca5bcc..4bc0b3a 100644
--- a/core/java/android/permission/flags.aconfig
+++ b/core/java/android/permission/flags.aconfig
@@ -266,3 +266,14 @@
description: "This fixed read-only flag is used to enable replacing permission BODY_SENSORS (and BODY_SENSORS_BACKGROUND) with granular health permission READ_HEART_RATE (and READ_HEALTH_DATA_IN_BACKGROUND)"
bug: "364638912"
}
+
+flag {
+ name: "delay_uid_state_changes_from_capability_updates"
+ is_fixed_read_only: true
+ namespace: "permissions"
+ description: "If proc state is decreasing over the restriction threshold and capability is changed, delay if no new capabilities are added"
+ bug: "308573169"
+ metadata {
+ purpose: PURPOSE_BUGFIX
+ }
+}
diff --git a/services/core/java/com/android/server/appop/AppOpsUidStateTrackerImpl.java b/services/core/java/com/android/server/appop/AppOpsUidStateTrackerImpl.java
index 03c8156..ed41f2e 100644
--- a/services/core/java/com/android/server/appop/AppOpsUidStateTrackerImpl.java
+++ b/services/core/java/com/android/server/appop/AppOpsUidStateTrackerImpl.java
@@ -36,6 +36,7 @@
import static android.app.AppOpsManager.UID_STATE_MAX_LAST_NON_RESTRICTED;
import static android.app.AppOpsManager.UID_STATE_NONEXISTENT;
import static android.app.AppOpsManager.UID_STATE_TOP;
+import static android.permission.flags.Flags.delayUidStateChangesFromCapabilityUpdates;
import static android.permission.flags.Flags.finishRunningOpsForKilledPackages;
import static com.android.server.appop.AppOpsUidStateTracker.processStateToUidState;
@@ -236,20 +237,26 @@
mPendingUidStates.put(uid, uidState);
mPendingCapability.put(uid, capability);
+ boolean hasLostCapability = (prevCapability & ~capability) != 0;
+
if (procState == PROCESS_STATE_NONEXISTENT) {
mPendingGone.put(uid, true);
commitUidPendingState(uid);
- } else if (uidState < prevUidState
- || (uidState <= UID_STATE_MAX_LAST_NON_RESTRICTED
- && prevUidState > UID_STATE_MAX_LAST_NON_RESTRICTED)) {
+ } else if (uidState < prevUidState) {
// We are moving to a more important state, or the new state may be in the
// foreground and the old state is in the background, then always do it
// immediately.
commitUidPendingState(uid);
- } else if (uidState == prevUidState && capability != prevCapability) {
+ } else if (delayUidStateChangesFromCapabilityUpdates()
+ && uidState == prevUidState && !hasLostCapability) {
+ // No change on process state, but process capability hasn't decreased.
+ commitUidPendingState(uid);
+ } else if (!delayUidStateChangesFromCapabilityUpdates()
+ && uidState == prevUidState && capability != prevCapability) {
// No change on process state, but process capability has changed.
commitUidPendingState(uid);
- } else if (uidState <= UID_STATE_MAX_LAST_NON_RESTRICTED) {
+ } else if (uidState <= UID_STATE_MAX_LAST_NON_RESTRICTED
+ && (!delayUidStateChangesFromCapabilityUpdates() || !hasLostCapability)) {
// We are moving to a less important state, but it doesn't cross the restriction
// threshold.
commitUidPendingState(uid);
diff --git a/services/tests/mockingservicestests/src/com/android/server/appop/AppOpsUidStateTrackerTest.java b/services/tests/mockingservicestests/src/com/android/server/appop/AppOpsUidStateTrackerTest.java
index 1731590..026e72f 100644
--- a/services/tests/mockingservicestests/src/com/android/server/appop/AppOpsUidStateTrackerTest.java
+++ b/services/tests/mockingservicestests/src/com/android/server/appop/AppOpsUidStateTrackerTest.java
@@ -31,12 +31,14 @@
import static android.app.AppOpsManager.UID_STATE_FOREGROUND_SERVICE;
import static android.app.AppOpsManager.UID_STATE_MAX_LAST_NON_RESTRICTED;
import static android.app.AppOpsManager.UID_STATE_TOP;
+import static android.permission.flags.Flags.delayUidStateChangesFromCapabilityUpdates;
import static com.android.server.appop.AppOpsUidStateTracker.processStateToUidState;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assume.assumeTrue;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.eq;
@@ -325,6 +327,10 @@
.backgroundState()
.update();
+ assertEquals(MODE_IGNORED, mIntf.evalMode(UID, OP_RECORD_AUDIO, MODE_FOREGROUND));
+ assertEquals(MODE_IGNORED,
+ mIntf.evalMode(UID, OP_RECEIVE_EXPLICIT_USER_INTERACTION_AUDIO, MODE_FOREGROUND));
+
procStateBuilder(UID)
.backgroundState()
.microphoneCapability()
@@ -342,10 +348,23 @@
.microphoneCapability()
.update();
+ assertEquals(MODE_ALLOWED, mIntf.evalMode(UID, OP_RECORD_AUDIO, MODE_FOREGROUND));
+ assertEquals(MODE_ALLOWED,
+ mIntf.evalMode(UID, OP_RECEIVE_EXPLICIT_USER_INTERACTION_AUDIO, MODE_FOREGROUND));
+
procStateBuilder(UID)
.backgroundState()
.update();
+ if (delayUidStateChangesFromCapabilityUpdates()) {
+ mClock.advanceTime(mConstants.BG_STATE_SETTLE_TIME - 1);
+ assertEquals(MODE_ALLOWED, mIntf.evalMode(UID, OP_RECORD_AUDIO, MODE_FOREGROUND));
+ assertEquals(MODE_ALLOWED,
+ mIntf.evalMode(UID, OP_RECEIVE_EXPLICIT_USER_INTERACTION_AUDIO,
+ MODE_FOREGROUND));
+
+ mClock.advanceTime(1);
+ }
assertEquals(MODE_IGNORED, mIntf.evalMode(UID, OP_RECORD_AUDIO, MODE_FOREGROUND));
assertEquals(MODE_IGNORED,
mIntf.evalMode(UID, OP_RECEIVE_EXPLICIT_USER_INTERACTION_AUDIO, MODE_FOREGROUND));
@@ -357,6 +376,8 @@
.backgroundState()
.update();
+ assertEquals(MODE_IGNORED, mIntf.evalMode(UID, OP_CAMERA, MODE_FOREGROUND));
+
procStateBuilder(UID)
.backgroundState()
.cameraCapability()
@@ -372,10 +393,18 @@
.cameraCapability()
.update();
+ assertEquals(MODE_ALLOWED, mIntf.evalMode(UID, OP_CAMERA, MODE_FOREGROUND));
+
procStateBuilder(UID)
.backgroundState()
.update();
+ if (delayUidStateChangesFromCapabilityUpdates()) {
+ mClock.advanceTime(mConstants.BG_STATE_SETTLE_TIME - 1);
+ assertEquals(MODE_ALLOWED, mIntf.evalMode(UID, OP_CAMERA, MODE_FOREGROUND));
+
+ mClock.advanceTime(1);
+ }
assertEquals(MODE_IGNORED, mIntf.evalMode(UID, OP_CAMERA, MODE_FOREGROUND));
}
@@ -385,6 +414,9 @@
.backgroundState()
.update();
+ assertEquals(MODE_IGNORED, mIntf.evalMode(UID, OP_COARSE_LOCATION, MODE_FOREGROUND));
+ assertEquals(MODE_IGNORED, mIntf.evalMode(UID, OP_FINE_LOCATION, MODE_FOREGROUND));
+
procStateBuilder(UID)
.backgroundState()
.locationCapability()
@@ -401,15 +433,55 @@
.locationCapability()
.update();
+ assertEquals(MODE_ALLOWED, mIntf.evalMode(UID, OP_COARSE_LOCATION, MODE_FOREGROUND));
+ assertEquals(MODE_ALLOWED, mIntf.evalMode(UID, OP_FINE_LOCATION, MODE_FOREGROUND));
+
procStateBuilder(UID)
.backgroundState()
.update();
+ if (delayUidStateChangesFromCapabilityUpdates()) {
+ mClock.advanceTime(mConstants.BG_STATE_SETTLE_TIME - 1);
+ assertEquals(MODE_ALLOWED, mIntf.evalMode(UID, OP_COARSE_LOCATION, MODE_FOREGROUND));
+ assertEquals(MODE_ALLOWED, mIntf.evalMode(UID, OP_FINE_LOCATION, MODE_FOREGROUND));
+
+ mClock.advanceTime(1);
+ }
assertEquals(MODE_IGNORED, mIntf.evalMode(UID, OP_COARSE_LOCATION, MODE_FOREGROUND));
assertEquals(MODE_IGNORED, mIntf.evalMode(UID, OP_FINE_LOCATION, MODE_FOREGROUND));
}
@Test
+ public void testProcStateChangesAndStaysUnrestrictedAndCapabilityRemoved() {
+ assumeTrue(delayUidStateChangesFromCapabilityUpdates());
+
+ procStateBuilder(UID)
+ .topState()
+ .microphoneCapability()
+ .cameraCapability()
+ .locationCapability()
+ .update();
+
+ assertEquals(MODE_ALLOWED, mIntf.evalMode(UID, OP_RECORD_AUDIO, MODE_FOREGROUND));
+ assertEquals(MODE_ALLOWED, mIntf.evalMode(UID, OP_CAMERA, MODE_FOREGROUND));
+ assertEquals(MODE_ALLOWED, mIntf.evalMode(UID, OP_COARSE_LOCATION, MODE_FOREGROUND));
+
+ procStateBuilder(UID)
+ .foregroundState()
+ .update();
+
+ mClock.advanceTime(mConstants.TOP_STATE_SETTLE_TIME - 1);
+ assertEquals(MODE_ALLOWED, mIntf.evalMode(UID, OP_RECORD_AUDIO, MODE_FOREGROUND));
+ assertEquals(MODE_ALLOWED, mIntf.evalMode(UID, OP_CAMERA, MODE_FOREGROUND));
+ assertEquals(MODE_ALLOWED, mIntf.evalMode(UID, OP_COARSE_LOCATION, MODE_FOREGROUND));
+
+ mClock.advanceTime(1);
+ assertEquals(MODE_IGNORED, mIntf.evalMode(UID, OP_RECORD_AUDIO, MODE_FOREGROUND));
+ assertEquals(MODE_IGNORED, mIntf.evalMode(UID, OP_CAMERA, MODE_FOREGROUND));
+ assertEquals(MODE_IGNORED, mIntf.evalMode(UID, OP_COARSE_LOCATION, MODE_FOREGROUND));
+ }
+
+ @Test
public void testVisibleAppWidget() {
procStateBuilder(UID)
.backgroundState()