From e31c33ea3586531ca99dd4c6d68a34ce07c1cebb Mon Sep 17 00:00:00 2001
From: Hao Ke <haok@google.com>
Date: Mon, 22 Apr 2024 15:13:58 +0000
Subject: Fix READ/WRITE operation access issues on Restricted appOps.

Problems were identified around read and write access to the restricted appOps, this change includes:

- Filter out restricted appOps status for unprivileged readers.
- Allow additional privileged appOps permission holder reading restricted appOps status.

Bug: 336273802
Bug: 336323279
Test: Local test see b/336273802#comment3
Test: atest AppOpsTest#testRestrictedSettingsOpsRead
Change-Id: I09008b365e36b2c20c9a1fe5a1d52699ddb17d35
Merged-In: I09008b365e36b2c20c9a1fe5a1d52699ddb17d35
---
 core/java/android/app/AppOpInfo.java               |  2 +-
 core/java/android/app/AppOpsManager.java           |  2 +-
 .../com/android/server/appop/AppOpsService.java    | 31 ++++++++++++++++++----
 3 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/core/java/android/app/AppOpInfo.java b/core/java/android/app/AppOpInfo.java
index 5268ec42e21c..a0f0ccaec58c 100644
--- a/core/java/android/app/AppOpInfo.java
+++ b/core/java/android/app/AppOpInfo.java
@@ -88,7 +88,7 @@ class AppOpInfo {
 
     /**
      * This specifies whether each option is only allowed to be read
-     * by apps with manage appops permission.
+     * by apps with privileged appops permission.
      */
     public final boolean restrictRead;
 
diff --git a/core/java/android/app/AppOpsManager.java b/core/java/android/app/AppOpsManager.java
index ccd83f756730..2ec54535cdb9 100644
--- a/core/java/android/app/AppOpsManager.java
+++ b/core/java/android/app/AppOpsManager.java
@@ -2985,7 +2985,7 @@ public class AppOpsManager {
     }
 
     /**
-     * Retrieve whether the op can be read by apps with manage appops permission.
+     * Retrieve whether the op can be read by apps with privileged appops permission.
      * @hide
      */
     public static boolean opRestrictsRead(int op) {
diff --git a/services/core/java/com/android/server/appop/AppOpsService.java b/services/core/java/com/android/server/appop/AppOpsService.java
index 33655f748230..e2388e2918ab 100644
--- a/services/core/java/com/android/server/appop/AppOpsService.java
+++ b/services/core/java/com/android/server/appop/AppOpsService.java
@@ -1430,16 +1430,26 @@ public class AppOpsService extends IAppOpsService.Stub {
 
     private ArrayList<AppOpsManager.OpEntry> collectOps(Ops pkgOps, int[] ops) {
         ArrayList<AppOpsManager.OpEntry> resOps = null;
+        boolean shouldReturnRestrictedAppOps = mContext.checkPermission(
+                Manifest.permission.GET_APP_OPS_STATS,
+                Binder.getCallingPid(), Binder.getCallingUid())
+                == PackageManager.PERMISSION_GRANTED;
         if (ops == null) {
             resOps = new ArrayList<>();
-            for (int j=0; j<pkgOps.size(); j++) {
+            for (int j = 0; j < pkgOps.size(); j++) {
                 Op curOp = pkgOps.valueAt(j);
+                if (opRestrictsRead(curOp.op) && !shouldReturnRestrictedAppOps) {
+                    continue;
+                }
                 resOps.add(getOpEntryForResult(curOp));
             }
         } else {
-            for (int j=0; j<ops.length; j++) {
+            for (int j = 0; j < ops.length; j++) {
                 Op curOp = pkgOps.get(ops[j]);
                 if (curOp != null) {
+                    if (opRestrictsRead(curOp.op) && !shouldReturnRestrictedAppOps) {
+                        continue;
+                    }
                     if (resOps == null) {
                         resOps = new ArrayList<>();
                     }
@@ -3615,10 +3625,21 @@ public class AppOpsService extends IAppOpsService.Stub {
 
     private void verifyIncomingOp(int op) {
         if (op >= 0 && op < AppOpsManager._NUM_OP) {
-            // Enforce manage appops permission if it's a restricted read op.
+            // Enforce privileged appops permission if it's a restricted read op.
             if (opRestrictsRead(op)) {
-                mContext.enforcePermission(Manifest.permission.MANAGE_APPOPS,
-                        Binder.getCallingPid(), Binder.getCallingUid(), "verifyIncomingOp");
+                if (!(mContext.checkPermission(Manifest.permission.MANAGE_APPOPS,
+                        Binder.getCallingPid(), Binder.getCallingUid())
+                        == PackageManager.PERMISSION_GRANTED || mContext.checkPermission(
+                        Manifest.permission.GET_APP_OPS_STATS,
+                        Binder.getCallingPid(), Binder.getCallingUid())
+                        == PackageManager.PERMISSION_GRANTED || mContext.checkPermission(
+                        Manifest.permission.MANAGE_APP_OPS_MODES,
+                        Binder.getCallingPid(), Binder.getCallingUid())
+                        == PackageManager.PERMISSION_GRANTED)) {
+                    throw new SecurityException("verifyIncomingOp: uid " + Binder.getCallingUid()
+                            + " does not have any of {MANAGE_APPOPS, GET_APP_OPS_STATS, "
+                            + "MANAGE_APP_OPS_MODES}");
+                }
             }
             return;
         }
-- 
cgit v1.2.3-59-g8ed1b