From 980ab95c437366f988983db9d546cc284d90fd3a Mon Sep 17 00:00:00 2001
From: Nate Myren <ntmyren@google.com>
Date: Mon, 7 Mar 2022 11:35:16 -0800
Subject: [PATCH] Check if an app requests POST_NOTIFICATIONS before setting
 state

Ensure that we only set POST_NOTIFICATIONS if an app requests it. PermMs
throws an Exception when attempting to grant/revoke a permission an app
doesn't request.

Fixes: 219798259
Test: atest PermissionHelperTest
Change-Id: I51a8339ff2c943832c831ac968613035ea0ca2e9
---
 .../server/notification/PermissionHelper.java | 29 +++++++++++++++----
 .../notification/PermissionHelperTest.java    | 20 +++++++++++++
 2 files changed, 43 insertions(+), 6 deletions(-)

diff --git a/services/core/java/com/android/server/notification/PermissionHelper.java b/services/core/java/com/android/server/notification/PermissionHelper.java
index 6b9e374771ff4..e551f1056b24f 100644
--- a/services/core/java/com/android/server/notification/PermissionHelper.java
+++ b/services/core/java/com/android/server/notification/PermissionHelper.java
@@ -35,6 +35,7 @@ import android.util.ArrayMap;
 import android.util.Pair;
 import android.util.Slog;
 
+import com.android.internal.util.ArrayUtils;
 import com.android.server.pm.permission.PermissionManagerServiceInternal;
 
 import java.util.Collections;
@@ -178,13 +179,16 @@ public final class PermissionHelper {
             boolean userSet, boolean reviewRequired) {
         assertFlag();
         final long callingId = Binder.clearCallingIdentity();
-        // Do not change fixed permissions, and do not change non-user set permissions that are
-        // granted by default, or granted by role.
-        if (isPermissionFixed(packageName, userId)
-                || (isPermissionGrantedByDefaultOrRole(packageName, userId) && !userSet)) {
-            return;
-        }
         try {
+            // Do not change the permission if the package doesn't request it, do not change fixed
+            // permissions, and do not change non-user set permissions that are granted by default,
+            // or granted by role.
+            if (!packageRequestsNotificationPermission(packageName, userId)
+                    || isPermissionFixed(packageName, userId)
+                    || (isPermissionGrantedByDefaultOrRole(packageName, userId) && !userSet)) {
+                return;
+            }
+
             boolean currentlyGranted = mPmi.checkPermission(packageName, NOTIFICATION_PERMISSION,
                     userId) != PackageManager.PERMISSION_DENIED;
             if (grant && !reviewRequired && !currentlyGranted) {
@@ -278,6 +282,19 @@ public final class PermissionHelper {
         }
     }
 
+    private boolean packageRequestsNotificationPermission(String packageName,
+            @UserIdInt int userId) {
+        assertFlag();
+        try {
+            String[] permissions = mPackageManager.getPackageInfo(packageName, GET_PERMISSIONS,
+                    userId).requestedPermissions;
+            return ArrayUtils.contains(permissions, NOTIFICATION_PERMISSION);
+        } catch (RemoteException e) {
+            Slog.e(TAG, "Could not reach system server", e);
+        }
+        return false;
+    }
+
     private void assertFlag() {
         if (!mMigrationEnabled) {
             throw new IllegalStateException("Method called without checking flag value");
diff --git a/services/tests/uiservicestests/src/com/android/server/notification/PermissionHelperTest.java b/services/tests/uiservicestests/src/com/android/server/notification/PermissionHelperTest.java
index a24ba0d1e1c2a..50151bfb71917 100644
--- a/services/tests/uiservicestests/src/com/android/server/notification/PermissionHelperTest.java
+++ b/services/tests/uiservicestests/src/com/android/server/notification/PermissionHelperTest.java
@@ -89,6 +89,10 @@ public class PermissionHelperTest extends UiServiceTestCase {
     public void setUp() throws Exception {
         MockitoAnnotations.initMocks(this);
         mPermissionHelper = new PermissionHelper(mPmi, mPackageManager, mPermManager, true);
+        PackageInfo testPkgInfo = new PackageInfo();
+        testPkgInfo.requestedPermissions = new String[]{ Manifest.permission.POST_NOTIFICATIONS };
+        when(mPackageManager.getPackageInfo(anyString(), anyLong(), anyInt()))
+                .thenReturn(testPkgInfo);
     }
 
     // TODO (b/194833441): Remove when the migration is enabled
@@ -383,6 +387,22 @@ public class PermissionHelperTest extends UiServiceTestCase {
                 eq("pkg"), eq(Manifest.permission.POST_NOTIFICATIONS), eq(10), anyString());
     }
 
+    @Test
+    public void testSetNotificationPermission_doesntRequestNotChanged() throws Exception {
+        when(mPmi.checkPermission(anyString(), anyString(), anyInt()))
+                .thenReturn(PERMISSION_GRANTED);
+        PackageInfo testPkgInfo = new PackageInfo();
+        testPkgInfo.requestedPermissions = new String[]{ Manifest.permission.RECORD_AUDIO };
+        when(mPackageManager.getPackageInfo(anyString(), anyLong(), anyInt()))
+                .thenReturn(testPkgInfo);
+        mPermissionHelper.setNotificationPermission("pkg", 10, false, false);
+
+        verify(mPmi, never()).checkPermission(
+                eq("pkg"), eq(Manifest.permission.POST_NOTIFICATIONS), eq(10));
+        verify(mPermManager, never()).revokeRuntimePermission(
+                eq("pkg"), eq(Manifest.permission.POST_NOTIFICATIONS), eq(10), anyString());
+    }
+
     @Test
     public void testIsPermissionFixed() throws Exception {
         when(mPermManager.getPermissionFlags(anyString(),