Restricts notified app ops based on flags
AppOps that are received by SystemUI and notified to listeners of
AppOpsControllerImpl are filtered based on the permission flag
PackageManager.FLAG_PERMISSION_USER_SENSITIVE_WHEN_GRANTED.
As calls to obtain this flag require an IPC, three things are done to
mitigate impact:
* PermissionFlagsCache keeps track of requested flags and will update
those (in a background thread), when a change is notified for a given
uid.
* Calls to getActiveAppOps/getActiveAppOpsForUser should be made from a
background thread.
* notifySubscribers is always called in the background thread.
Bug: 160966908
Test: atest PermissionFlagsCacheTest AppOpsControllerTest
Change-Id: I871094c32ce5ec940d779626333caa0ca500a4e3
Merged-In: I871094c32ce5ec940d779626333caa0ca500a4e3
diff --git a/data/etc/com.android.systemui.xml b/data/etc/com.android.systemui.xml
index a5a2221..ada8b00 100644
--- a/data/etc/com.android.systemui.xml
+++ b/data/etc/com.android.systemui.xml
@@ -39,6 +39,7 @@
<permission name="android.permission.MODIFY_PHONE_STATE"/>
<permission name="android.permission.MOUNT_UNMOUNT_FILESYSTEMS"/>
<permission name="android.permission.OBSERVE_NETWORK_POLICY"/>
+ <permission name="android.permission.OBSERVE_GRANT_REVOKE_PERMISSIONS" />
<permission name="android.permission.OVERRIDE_WIFI_CONFIG"/>
<permission name="android.permission.PACKAGE_USAGE_STATS" />
<permission name="android.permission.READ_DREAM_STATE"/>
diff --git a/packages/SystemUI/AndroidManifest.xml b/packages/SystemUI/AndroidManifest.xml
index a7ef5e6..46cb795 100644
--- a/packages/SystemUI/AndroidManifest.xml
+++ b/packages/SystemUI/AndroidManifest.xml
@@ -239,6 +239,7 @@
<!-- Listen app op changes -->
<uses-permission android:name="android.permission.WATCH_APPOPS" />
+ <uses-permission android:name="android.permission.OBSERVE_GRANT_REVOKE_PERMISSIONS" />
<!-- to read and change hvac values in a car -->
<uses-permission android:name="android.car.permission.CONTROL_CAR_CLIMATE" />
diff --git a/packages/SystemUI/src/com/android/systemui/appops/AppOpsControllerImpl.java b/packages/SystemUI/src/com/android/systemui/appops/AppOpsControllerImpl.java
index fc7cc7e..ce8e285 100644
--- a/packages/SystemUI/src/com/android/systemui/appops/AppOpsControllerImpl.java
+++ b/packages/SystemUI/src/com/android/systemui/appops/AppOpsControllerImpl.java
@@ -18,6 +18,7 @@
import android.app.AppOpsManager;
import android.content.Context;
+import android.content.pm.PackageManager;
import android.os.Handler;
import android.os.Looper;
import android.os.UserHandle;
@@ -25,6 +26,8 @@
import android.util.ArraySet;
import android.util.Log;
+import androidx.annotation.WorkerThread;
+
import com.android.internal.annotations.GuardedBy;
import com.android.internal.annotations.VisibleForTesting;
import com.android.systemui.Dumpable;
@@ -62,6 +65,7 @@
private H mBGHandler;
private final List<AppOpsController.Callback> mCallbacks = new ArrayList<>();
private final ArrayMap<Integer, Set<Callback>> mCallbacksByCode = new ArrayMap<>();
+ private final PermissionFlagsCache mFlagsCache;
private boolean mListening;
@GuardedBy("mActiveItems")
@@ -81,8 +85,11 @@
public AppOpsControllerImpl(
Context context,
@Background Looper bgLooper,
- DumpManager dumpManager) {
+ DumpManager dumpManager,
+ PermissionFlagsCache cache
+ ) {
mAppOps = (AppOpsManager) context.getSystemService(Context.APP_OPS_SERVICE);
+ mFlagsCache = cache;
mBGHandler = new H(bgLooper);
final int numOps = OPS.length;
for (int i = 0; i < numOps; i++) {
@@ -229,10 +236,66 @@
}
/**
+ * Does the app-op code refer to a user sensitive permission for the specified user id
+ * and package. Only user sensitive permission should be shown to the user by default.
+ *
+ * @param appOpCode The code of the app-op.
+ * @param uid The uid of the user.
+ * @param packageName The name of the package.
+ *
+ * @return {@code true} iff the app-op item is user sensitive
+ */
+ private boolean isUserSensitive(int appOpCode, int uid, String packageName) {
+ String permission = AppOpsManager.opToPermission(appOpCode);
+ if (permission == null) {
+ return false;
+ }
+ int permFlags = mFlagsCache.getPermissionFlags(permission,
+ packageName, uid);
+ return (permFlags & PackageManager.FLAG_PERMISSION_USER_SENSITIVE_WHEN_GRANTED) != 0;
+ }
+
+ /**
+ * Does the app-op item refer to an operation that should be shown to the user.
+ * Only specficic ops (like SYSTEM_ALERT_WINDOW) or ops that refer to user sensitive
+ * permission should be shown to the user by default.
+ *
+ * @param item The item
+ *
+ * @return {@code true} iff the app-op item should be shown to the user
+ */
+ private boolean isUserVisible(AppOpItem item) {
+ return isUserVisible(item.getCode(), item.getUid(), item.getPackageName());
+ }
+
+
+ /**
+ * Does the app-op, uid and package name, refer to an operation that should be shown to the
+ * user. Only specficic ops (like {@link AppOpsManager.OP_SYSTEM_ALERT_WINDOW}) or
+ * ops that refer to user sensitive permission should be shown to the user by default.
+ *
+ * @param item The item
+ *
+ * @return {@code true} iff the app-op for should be shown to the user
+ */
+ private boolean isUserVisible(int appOpCode, int uid, String packageName) {
+ // currently OP_SYSTEM_ALERT_WINDOW does not correspond to a platform permission
+ // which may be user senstive, so for now always show it to the user.
+ if (appOpCode == AppOpsManager.OP_SYSTEM_ALERT_WINDOW) {
+ return true;
+ }
+
+ return isUserSensitive(appOpCode, uid, packageName);
+ }
+
+ /**
* Returns a copy of the list containing all the active AppOps that the controller tracks.
*
+ * Call from a worker thread as it may perform long operations.
+ *
* @return List of active AppOps information
*/
+ @WorkerThread
public List<AppOpItem> getActiveAppOps() {
return getActiveAppOpsForUser(UserHandle.USER_ALL);
}
@@ -241,10 +304,13 @@
* Returns a copy of the list containing all the active AppOps that the controller tracks, for
* a given user id.
*
+ * Call from a worker thread as it may perform long operations.
+ *
* @param userId User id to track, can be {@link UserHandle#USER_ALL}
*
* @return List of active AppOps information for that user id
*/
+ @WorkerThread
public List<AppOpItem> getActiveAppOpsForUser(int userId) {
List<AppOpItem> list = new ArrayList<>();
synchronized (mActiveItems) {
@@ -252,7 +318,8 @@
for (int i = 0; i < numActiveItems; i++) {
AppOpItem item = mActiveItems.get(i);
if ((userId == UserHandle.USER_ALL
- || UserHandle.getUserId(item.getUid()) == userId)) {
+ || UserHandle.getUserId(item.getUid()) == userId)
+ && isUserVisible(item)) {
list.add(item);
}
}
@@ -262,7 +329,8 @@
for (int i = 0; i < numNotedItems; i++) {
AppOpItem item = mNotedItems.get(i);
if ((userId == UserHandle.USER_ALL
- || UserHandle.getUserId(item.getUid()) == userId)) {
+ || UserHandle.getUserId(item.getUid()) == userId)
+ && isUserVisible(item)) {
list.add(item);
}
}
@@ -310,7 +378,7 @@
}
private void notifySuscribers(int code, int uid, String packageName, boolean active) {
- if (mCallbacksByCode.containsKey(code)) {
+ if (mCallbacksByCode.containsKey(code) && isUserVisible(code, uid, packageName)) {
if (DEBUG) Log.d(TAG, "Notifying of change in package " + packageName);
for (Callback cb: mCallbacksByCode.get(code)) {
cb.onActiveStateChanged(code, uid, packageName, active);
diff --git a/packages/SystemUI/src/com/android/systemui/appops/PermissionFlagsCache.kt b/packages/SystemUI/src/com/android/systemui/appops/PermissionFlagsCache.kt
new file mode 100644
index 0000000..9248b4f8
--- /dev/null
+++ b/packages/SystemUI/src/com/android/systemui/appops/PermissionFlagsCache.kt
@@ -0,0 +1,85 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+
+package com.android.systemui.appops
+
+import android.content.pm.PackageManager
+import android.os.UserHandle
+import androidx.annotation.WorkerThread
+import com.android.systemui.dagger.qualifiers.Background
+import java.util.concurrent.Executor
+import javax.inject.Inject
+import javax.inject.Singleton
+
+private data class PermissionFlagKey(
+ val permission: String,
+ val packageName: String,
+ val uid: Int
+)
+
+/**
+ * Cache for PackageManager's PermissionFlags.
+ *
+ * After a specific `{permission, package, uid}` has been requested, updates to it will be tracked,
+ * and changes to the uid will trigger new requests (in the background).
+ */
+@Singleton
+class PermissionFlagsCache @Inject constructor(
+ private val packageManager: PackageManager,
+ @Background private val executor: Executor
+) : PackageManager.OnPermissionsChangedListener {
+
+ private val permissionFlagsCache =
+ mutableMapOf<Int, MutableMap<PermissionFlagKey, Int>>()
+ private var listening = false
+
+ override fun onPermissionsChanged(uid: Int) {
+ executor.execute {
+ // Only track those that we've seen before
+ val keys = permissionFlagsCache.get(uid)
+ if (keys != null) {
+ keys.mapValuesTo(keys) {
+ getFlags(it.key)
+ }
+ }
+ }
+ }
+
+ /**
+ * Retrieve permission flags from cache or PackageManager. There parameters will be passed
+ * directly to [PackageManager].
+ *
+ * Calls to this method should be done from a background thread.
+ */
+ @WorkerThread
+ fun getPermissionFlags(permission: String, packageName: String, uid: Int): Int {
+ if (!listening) {
+ listening = true
+ packageManager.addOnPermissionsChangeListener(this)
+ }
+ val key = PermissionFlagKey(permission, packageName, uid)
+ return permissionFlagsCache.getOrPut(uid, { mutableMapOf() }).get(key) ?: run {
+ getFlags(key).also {
+ permissionFlagsCache.get(uid)?.put(key, it)
+ }
+ }
+ }
+
+ private fun getFlags(key: PermissionFlagKey): Int {
+ return packageManager.getPermissionFlags(key.permission, key.packageName,
+ UserHandle.getUserHandleForUid(key.uid))
+ }
+}
\ No newline at end of file
diff --git a/packages/SystemUI/tests/src/com/android/systemui/appops/AppOpsControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/appops/AppOpsControllerTest.java
index e0049d1..4fdc06e 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/appops/AppOpsControllerTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/appops/AppOpsControllerTest.java
@@ -26,11 +26,14 @@
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
import android.app.AppOpsManager;
+import android.content.pm.PackageManager;
import android.os.Looper;
import android.os.UserHandle;
import android.testing.AndroidTestingRunner;
@@ -56,6 +59,7 @@
private static final String TEST_PACKAGE_NAME = "test";
private static final int TEST_UID = UserHandle.getUid(0, 0);
private static final int TEST_UID_OTHER = UserHandle.getUid(1, 0);
+ private static final int TEST_UID_NON_USER_SENSITIVE = UserHandle.getUid(2, 0);
@Mock
private AppOpsManager mAppOpsManager;
@@ -65,6 +69,10 @@
private AppOpsControllerImpl.H mMockHandler;
@Mock
private DumpManager mDumpManager;
+ @Mock
+ private PermissionFlagsCache mFlagsCache;
+ @Mock
+ private PackageManager mPackageManager;
private AppOpsControllerImpl mController;
private TestableLooper mTestableLooper;
@@ -76,8 +84,22 @@
getContext().addMockSystemService(AppOpsManager.class, mAppOpsManager);
- mController =
- new AppOpsControllerImpl(mContext, mTestableLooper.getLooper(), mDumpManager);
+ // All permissions of TEST_UID and TEST_UID_OTHER are user sensitive. None of
+ // TEST_UID_NON_USER_SENSITIVE are user sensitive.
+ getContext().setMockPackageManager(mPackageManager);
+ when(mFlagsCache.getPermissionFlags(anyString(), anyString(), eq(TEST_UID))).thenReturn(
+ PackageManager.FLAG_PERMISSION_USER_SENSITIVE_WHEN_GRANTED);
+ when(mFlagsCache.getPermissionFlags(anyString(), anyString(), eq(TEST_UID_OTHER)))
+ .thenReturn(PackageManager.FLAG_PERMISSION_USER_SENSITIVE_WHEN_GRANTED);
+ when(mFlagsCache.getPermissionFlags(anyString(), anyString(),
+ eq(TEST_UID_NON_USER_SENSITIVE))).thenReturn(0);
+
+ mController = new AppOpsControllerImpl(
+ mContext,
+ mTestableLooper.getLooper(),
+ mDumpManager,
+ mFlagsCache
+ );
}
@Test
@@ -173,6 +195,26 @@
}
@Test
+ public void nonUserSensitiveOpsAreIgnored() {
+ mController.onOpActiveChanged(AppOpsManager.OP_RECORD_AUDIO,
+ TEST_UID_NON_USER_SENSITIVE, TEST_PACKAGE_NAME, true);
+ assertEquals(0, mController.getActiveAppOpsForUser(
+ UserHandle.getUserId(TEST_UID_NON_USER_SENSITIVE)).size());
+ }
+
+ @Test
+ public void nonUserSensitiveOpsNotNotified() {
+ mController.addCallback(new int[]{AppOpsManager.OP_RECORD_AUDIO}, mCallback);
+ mController.onOpActiveChanged(AppOpsManager.OP_RECORD_AUDIO,
+ TEST_UID_NON_USER_SENSITIVE, TEST_PACKAGE_NAME, true);
+
+ mTestableLooper.processAllMessages();
+
+ verify(mCallback, never())
+ .onActiveStateChanged(anyInt(), anyInt(), anyString(), anyBoolean());
+ }
+
+ @Test
public void opNotedScheduledForRemoval() {
mController.setBGHandler(mMockHandler);
mController.onOpNoted(AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME,
diff --git a/packages/SystemUI/tests/src/com/android/systemui/appops/PermissionFlagsCacheTest.kt b/packages/SystemUI/tests/src/com/android/systemui/appops/PermissionFlagsCacheTest.kt
new file mode 100644
index 0000000..0fb0ce0
--- /dev/null
+++ b/packages/SystemUI/tests/src/com/android/systemui/appops/PermissionFlagsCacheTest.kt
@@ -0,0 +1,145 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+
+package com.android.systemui.appops
+
+import android.content.pm.PackageManager
+import android.os.UserHandle
+import android.testing.AndroidTestingRunner
+import androidx.test.filters.SmallTest
+import com.android.systemui.SysuiTestCase
+import com.android.systemui.util.concurrency.FakeExecutor
+import com.android.systemui.util.time.FakeSystemClock
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNotEquals
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.mockito.ArgumentMatchers.any
+import org.mockito.ArgumentMatchers.anyString
+import org.mockito.Mock
+import org.mockito.Mockito.`when`
+import org.mockito.Mockito.never
+import org.mockito.Mockito.times
+import org.mockito.Mockito.verify
+import org.mockito.MockitoAnnotations
+
+@SmallTest
+@RunWith(AndroidTestingRunner::class)
+class PermissionFlagsCacheTest : SysuiTestCase() {
+
+ companion object {
+ const val TEST_PERMISSION = "test_permission"
+ const val TEST_PACKAGE = "test_package"
+ const val TEST_UID1 = 1000
+ const val TEST_UID2 = UserHandle.PER_USER_RANGE + 1000
+ }
+
+ @Mock
+ private lateinit var packageManager: PackageManager
+
+ private lateinit var executor: FakeExecutor
+ private lateinit var flagsCache: PermissionFlagsCache
+
+ @Before
+ fun setUp() {
+ MockitoAnnotations.initMocks(this)
+
+ executor = FakeExecutor(FakeSystemClock())
+
+ flagsCache = PermissionFlagsCache(packageManager, executor)
+ executor.runAllReady()
+ }
+
+ @Test
+ fun testNotListeningByDefault() {
+ verify(packageManager, never()).addOnPermissionsChangeListener(any())
+ }
+
+ @Test
+ fun testGetCorrectFlags() {
+ `when`(packageManager.getPermissionFlags(anyString(), anyString(), any())).thenReturn(0)
+ `when`(packageManager.getPermissionFlags(
+ TEST_PERMISSION,
+ TEST_PACKAGE,
+ UserHandle.getUserHandleForUid(TEST_UID1))
+ ).thenReturn(1)
+
+ assertEquals(1, flagsCache.getPermissionFlags(TEST_PERMISSION, TEST_PACKAGE, TEST_UID1))
+ assertNotEquals(1, flagsCache.getPermissionFlags(TEST_PERMISSION, TEST_PACKAGE, TEST_UID2))
+ }
+
+ @Test
+ fun testFlagIsCached() {
+ flagsCache.getPermissionFlags(TEST_PERMISSION, TEST_PACKAGE, TEST_UID1)
+
+ flagsCache.getPermissionFlags(TEST_PERMISSION, TEST_PACKAGE, TEST_UID1)
+
+ verify(packageManager, times(1)).getPermissionFlags(
+ TEST_PERMISSION,
+ TEST_PACKAGE,
+ UserHandle.getUserHandleForUid(TEST_UID1)
+ )
+ }
+
+ @Test
+ fun testListeningAfterFirstRequest() {
+ flagsCache.getPermissionFlags(TEST_PERMISSION, TEST_PACKAGE, TEST_UID1)
+
+ verify(packageManager).addOnPermissionsChangeListener(any())
+ }
+
+ @Test
+ fun testListeningOnlyOnce() {
+ flagsCache.getPermissionFlags(TEST_PERMISSION, TEST_PACKAGE, TEST_UID1)
+
+ flagsCache.getPermissionFlags(TEST_PERMISSION, TEST_PACKAGE, TEST_UID2)
+
+ verify(packageManager, times(1)).addOnPermissionsChangeListener(any())
+ }
+
+ @Test
+ fun testUpdateFlag() {
+ assertEquals(0, flagsCache.getPermissionFlags(TEST_PERMISSION, TEST_PACKAGE, TEST_UID1))
+
+ `when`(packageManager.getPermissionFlags(
+ TEST_PERMISSION,
+ TEST_PACKAGE,
+ UserHandle.getUserHandleForUid(TEST_UID1))
+ ).thenReturn(1)
+
+ flagsCache.onPermissionsChanged(TEST_UID1)
+
+ executor.runAllReady()
+
+ assertEquals(1, flagsCache.getPermissionFlags(TEST_PERMISSION, TEST_PACKAGE, TEST_UID1))
+ }
+
+ @Test
+ fun testUpdateFlag_notUpdatedIfUidHasNotBeenRequestedBefore() {
+ flagsCache.getPermissionFlags(TEST_PERMISSION, TEST_PACKAGE, TEST_UID1)
+
+ flagsCache.onPermissionsChanged(TEST_UID2)
+
+ executor.runAllReady()
+
+ verify(packageManager, never()).getPermissionFlags(
+ TEST_PERMISSION,
+ TEST_PACKAGE,
+ UserHandle.getUserHandleForUid(TEST_UID2)
+ )
+ }
+}
\ No newline at end of file
