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

3 files changed, 95 insertions, 5 deletions

diff --git a/core/java/android/permission/flags.aconfig b/core/java/android/permission/flags.aconfig
index bca5bcc99c7e..4bc0b3a27b4f 100644
--- a/core/java/android/permission/flags.aconfig
+++ b/core/java/android/permission/flags.aconfig
@@ -266,3 +266,14 @@ flag {
     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 03c81560be89..ed41f2e881f8 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_FOREGROUND_SERVICE;
 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 @@ class AppOpsUidStateTrackerImpl implements AppOpsUidStateTracker {
             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 1731590be3c9..026e72f117b4 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;
 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 @@ public class AppOpsUidStateTrackerTest {
                 .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 @@ public class AppOpsUidStateTrackerTest {
                 .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 @@ public class AppOpsUidStateTrackerTest {
                 .backgroundState()
                 .update();
 
+        assertEquals(MODE_IGNORED, mIntf.evalMode(UID, OP_CAMERA, MODE_FOREGROUND));
+
         procStateBuilder(UID)
                 .backgroundState()
                 .cameraCapability()
@@ -372,10 +393,18 @@ public class AppOpsUidStateTrackerTest {
                 .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 @@ public class AppOpsUidStateTrackerTest {
                 .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 @@ public class AppOpsUidStateTrackerTest {
                 .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()