From fd6643e366577d1c2c8bf6f521dc9733e7ffdbd4 Mon Sep 17 00:00:00 2001 From: Jay Thomas Sullivan Date: Tue, 11 Oct 2022 16:31:30 -0700 Subject: [PATCH] Enforce ADJUST_RUNTIME_PERMISSIONS_POLICY In updatePermissionFlags, we're calling clearCallingIdentity. And, just after doing so, we're calling enforceCallingOrSelfPermission(ADJUST_RUNTIME_PERMISSIONS_POLICY). But, these two things don't really make sense together, because the former nullifies the latter. We could either: 1. Remove clearCallingIdentity but keep enforceCallingOrSelfPermission, or 2. Remove both For security, this CL goes with the first option. But, doing so means updatePermissionFlags now enforces ADJUST_RUNTIME_PERMISSIONS_POLICY. And this breaks some CTS tests. To address this, we have to add ADJUST_RUNTIME_PERMISSIONS_POLICY to the shell identity. Bug: 190694761 Test: atest ActivityPermissionRationaleTest Change-Id: I7031aebf69d9ec919334573b99eb6b7cb8be31d0 --- core/api/test-current.txt | 5 ++- core/res/AndroidManifest.xml | 6 ++-- packages/Shell/AndroidManifest.xml | 3 ++ .../PermissionManagerServiceImpl.java | 33 ++++++++----------- 4 files changed, 24 insertions(+), 23 deletions(-) diff --git a/core/api/test-current.txt b/core/api/test-current.txt index 9f333315451cc..54387c56a7cdb 100644 --- a/core/api/test-current.txt +++ b/core/api/test-current.txt @@ -4,6 +4,7 @@ package android { public static final class Manifest.permission { field public static final String ACCESS_NOTIFICATIONS = "android.permission.ACCESS_NOTIFICATIONS"; field public static final String ACTIVITY_EMBEDDING = "android.permission.ACTIVITY_EMBEDDING"; + field public static final String ADJUST_RUNTIME_PERMISSIONS_POLICY = "android.permission.ADJUST_RUNTIME_PERMISSIONS_POLICY"; field public static final String APPROVE_INCIDENT_REPORTS = "android.permission.APPROVE_INCIDENT_REPORTS"; field public static final String BACKGROUND_CAMERA = "android.permission.BACKGROUND_CAMERA"; field public static final String BIND_CELL_BROADCAST_SERVICE = "android.permission.BIND_CELL_BROADCAST_SERVICE"; @@ -17,11 +18,13 @@ package android { field public static final String CONTROL_DEVICE_STATE = "android.permission.CONTROL_DEVICE_STATE"; field public static final String FORCE_DEVICE_POLICY_MANAGER_LOGS = "android.permission.FORCE_DEVICE_POLICY_MANAGER_LOGS"; field public static final String FORCE_STOP_PACKAGES = "android.permission.FORCE_STOP_PACKAGES"; + field public static final String GRANT_RUNTIME_PERMISSIONS = "android.permission.GRANT_RUNTIME_PERMISSIONS"; field public static final String INSTALL_TEST_ONLY_PACKAGE = "android.permission.INSTALL_TEST_ONLY_PACKAGE"; field public static final String KEEP_UNINSTALLED_PACKAGES = "android.permission.KEEP_UNINSTALLED_PACKAGES"; field public static final String MAKE_UID_VISIBLE = "android.permission.MAKE_UID_VISIBLE"; field @Deprecated public static final String MANAGE_ACTIVITY_STACKS = "android.permission.MANAGE_ACTIVITY_STACKS"; field public static final String MANAGE_ACTIVITY_TASKS = "android.permission.MANAGE_ACTIVITY_TASKS"; + field public static final String MANAGE_APP_OPS_MODES = "android.permission.MANAGE_APP_OPS_MODES"; field public static final String MANAGE_CRATES = "android.permission.MANAGE_CRATES"; field public static final String MANAGE_NOTIFICATION_LISTENERS = "android.permission.MANAGE_NOTIFICATION_LISTENERS"; field public static final String MANAGE_ROLLBACKS = "android.permission.MANAGE_ROLLBACKS"; @@ -225,7 +228,7 @@ package android.app { method @RequiresPermission("android.permission.MANAGE_APPOPS") public void resetHistoryParameters(); method @RequiresPermission("android.permission.MANAGE_APPOPS") public void resetPackageOpsNoHistory(@NonNull String); method @RequiresPermission("android.permission.MANAGE_APPOPS") public void setHistoryParameters(int, long, int); - method @RequiresPermission("android.permission.MANAGE_APP_OPS_MODES") public void setMode(int, int, String, int); + method @RequiresPermission(android.Manifest.permission.MANAGE_APP_OPS_MODES) public void setMode(int, int, String, int); method public static int strOpToOp(@NonNull String); field public static final int ATTRIBUTION_CHAIN_ID_NONE = -1; // 0xffffffff field public static final int ATTRIBUTION_FLAGS_NONE = 0; // 0x0 diff --git a/core/res/AndroidManifest.xml b/core/res/AndroidManifest.xml index f0b1b2aa7e3df..b6c80b1481ac9 100644 --- a/core/res/AndroidManifest.xml +++ b/core/res/AndroidManifest.xml @@ -3928,7 +3928,7 @@ - - @@ -4766,7 +4766,7 @@ - diff --git a/packages/Shell/AndroidManifest.xml b/packages/Shell/AndroidManifest.xml index 6fe80875c65bc..0417fcdecd70b 100644 --- a/packages/Shell/AndroidManifest.xml +++ b/packages/Shell/AndroidManifest.xml @@ -708,6 +708,9 @@ + + + = Build.VERSION_CODES.Q) { - throw new IllegalArgumentException( - Manifest.permission.ADJUST_RUNTIME_PERMISSIONS_POLICY + " needs " - + " to be checked for packages targeting " - + Build.VERSION_CODES.Q + " or later when changing policy " - + "flags"); - } - overridePolicy = true; + if ((flagMask & FLAG_PERMISSION_POLICY_FIXED) != 0) { + if (checkAdjustPolicyFlagPermission) { + mContext.enforceCallingOrSelfPermission( + Manifest.permission.ADJUST_RUNTIME_PERMISSIONS_POLICY, + "Need " + Manifest.permission.ADJUST_RUNTIME_PERMISSIONS_POLICY + + " to change policy flags"); + } else if (mPackageManagerInt.getUidTargetSdkVersion(callingUid) + >= Build.VERSION_CODES.Q) { + throw new IllegalArgumentException( + Manifest.permission.ADJUST_RUNTIME_PERMISSIONS_POLICY + " needs " + + " to be checked for packages targeting " + + Build.VERSION_CODES.Q + " or later when changing policy " + + "flags"); } - } finally { - Binder.restoreCallingIdentity(callingIdentity); + overridePolicy = true; } }