From 807f4cfc80728313d04f95343e5aea14691aceb0 Mon Sep 17 00:00:00 2001 From: Evan Severson Date: Fri, 29 Apr 2022 14:13:29 -0700 Subject: [PATCH 1/3] Disallow privileged apps to bypass location restriction This bypass was originally allowed to let restricted users who can't use location to pair bluetooth devices. This isn't needed anymore with the bluetooth permissions. Test: Set up restricted profile and pair bluetooth Verify com.android.phone gets rejected Bug: 230861324 Bug: 231496105 Merged-In: Ib34c0b56ef52f5ee2deceb84b02cd0ff73d8181d Change-Id: Ib34c0b56ef52f5ee2deceb84b02cd0ff73d8181d --- core/java/android/app/AppOpsManager.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/java/android/app/AppOpsManager.java b/core/java/android/app/AppOpsManager.java index d932a29beca61..643df9240de91 100644 --- a/core/java/android/app/AppOpsManager.java +++ b/core/java/android/app/AppOpsManager.java @@ -2463,8 +2463,8 @@ public class AppOpsManager { * restriction} for a certain app-op. */ private static RestrictionBypass[] sOpAllowSystemRestrictionBypass = new RestrictionBypass[] { - new RestrictionBypass(true, false), //COARSE_LOCATION - new RestrictionBypass(true, false), //FINE_LOCATION + null, //COARSE_LOCATION + null, //FINE_LOCATION null, //GPS null, //VIBRATE null, //READ_CONTACTS From 1dddfe1f703cab6e159fafad45f51e8bad207dba Mon Sep 17 00:00:00 2001 From: Evan Severson Date: Thu, 5 May 2022 11:21:41 -0700 Subject: [PATCH 2/3] Allow system server uid to bypass location restriction Blocking system server from giving itself location restriction doesn't make much sense. Test: Disable, reboot, observe bootloop, apply patch, build, flash, observe successful boot Bug: 230861324 Bug: 231496105 Merged-In: Ic869da4847e4f39896861f3bf6e83f6f6c76ea62 Change-Id: Ic869da4847e4f39896861f3bf6e83f6f6c76ea62 --- core/java/android/app/AppOpsManager.java | 23 +++++++++++-------- .../android/server/appop/AppOpsService.java | 8 +++++-- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/core/java/android/app/AppOpsManager.java b/core/java/android/app/AppOpsManager.java index 643df9240de91..fc89e1395073b 100644 --- a/core/java/android/app/AppOpsManager.java +++ b/core/java/android/app/AppOpsManager.java @@ -2463,8 +2463,8 @@ public class AppOpsManager { * restriction} for a certain app-op. */ private static RestrictionBypass[] sOpAllowSystemRestrictionBypass = new RestrictionBypass[] { - null, //COARSE_LOCATION - null, //FINE_LOCATION + new RestrictionBypass(true, false, false), //COARSE_LOCATION + new RestrictionBypass(true, false, false), //FINE_LOCATION null, //GPS null, //VIBRATE null, //READ_CONTACTS @@ -2473,7 +2473,7 @@ public class AppOpsManager { null, //WRITE_CALL_LOG null, //READ_CALENDAR null, //WRITE_CALENDAR - new RestrictionBypass(true, false), //WIFI_SCAN + new RestrictionBypass(false, true, false), //WIFI_SCAN null, //POST_NOTIFICATION null, //NEIGHBORING_CELLS null, //CALL_PHONE @@ -2487,10 +2487,10 @@ public class AppOpsManager { null, //READ_ICC_SMS null, //WRITE_ICC_SMS null, //WRITE_SETTINGS - new RestrictionBypass(true, false), //SYSTEM_ALERT_WINDOW + new RestrictionBypass(false, true, false), //SYSTEM_ALERT_WINDOW null, //ACCESS_NOTIFICATIONS null, //CAMERA - new RestrictionBypass(false, true), //RECORD_AUDIO + new RestrictionBypass(false, false, true), //RECORD_AUDIO null, //PLAY_AUDIO null, //READ_CLIPBOARD null, //WRITE_CLIPBOARD @@ -2508,7 +2508,7 @@ public class AppOpsManager { null, //MONITOR_HIGH_POWER_LOCATION null, //GET_USAGE_STATS null, //MUTE_MICROPHONE - new RestrictionBypass(true, false), //TOAST_WINDOW + new RestrictionBypass(false, true, false), //TOAST_WINDOW null, //PROJECT_MEDIA null, //ACTIVATE_VPN null, //WALLPAPER @@ -2540,7 +2540,7 @@ public class AppOpsManager { null, // ACCEPT_HANDOVER null, // MANAGE_IPSEC_HANDOVERS null, // START_FOREGROUND - new RestrictionBypass(true, false), // BLUETOOTH_SCAN + new RestrictionBypass(false, true, false), // BLUETOOTH_SCAN null, // USE_BIOMETRIC null, // ACTIVITY_RECOGNITION null, // SMS_FINANCIAL_TRANSACTIONS @@ -3105,6 +3105,9 @@ public class AppOpsManager { * @hide */ public static class RestrictionBypass { + /** Does the app need to be system uid to bypass the restriction */ + public boolean isSystemUid; + /** Does the app need to be privileged to bypass the restriction */ public boolean isPrivileged; @@ -3114,12 +3117,14 @@ public class AppOpsManager { */ public boolean isRecordAudioRestrictionExcept; - public RestrictionBypass(boolean isPrivileged, boolean isRecordAudioRestrictionExcept) { + public RestrictionBypass(boolean isSystemUid, boolean isPrivileged, + boolean isRecordAudioRestrictionExcept) { + this.isSystemUid = isSystemUid; this.isPrivileged = isPrivileged; this.isRecordAudioRestrictionExcept = isRecordAudioRestrictionExcept; } - public static RestrictionBypass UNRESTRICTED = new RestrictionBypass(true, true); + public static RestrictionBypass UNRESTRICTED = new RestrictionBypass(false, true, true); } /** diff --git a/services/core/java/com/android/server/appop/AppOpsService.java b/services/core/java/com/android/server/appop/AppOpsService.java index 6d29c379d1b15..9ef33c7c2f61c 100644 --- a/services/core/java/com/android/server/appop/AppOpsService.java +++ b/services/core/java/com/android/server/appop/AppOpsService.java @@ -4502,8 +4502,9 @@ public class AppOpsService extends IAppOpsService.Stub { * @return The restriction matching the package */ private RestrictionBypass getBypassforPackage(@NonNull AndroidPackage pkg) { - return new RestrictionBypass(pkg.isPrivileged(), mContext.checkPermission( - android.Manifest.permission.EXEMPT_FROM_AUDIO_RECORD_RESTRICTIONS, -1, pkg.getUid()) + return new RestrictionBypass(pkg.getUid() == Process.SYSTEM_UID, pkg.isPrivileged(), + mContext.checkPermission(android.Manifest.permission + .EXEMPT_FROM_AUDIO_RECORD_RESTRICTIONS, -1, pkg.getUid()) == PackageManager.PERMISSION_GRANTED); } @@ -4785,6 +4786,9 @@ public class AppOpsService extends IAppOpsService.Stub { if (opBypass != null) { // If we are the system, bypass user restrictions for certain codes synchronized (this) { + if (opBypass.isSystemUid && appBypass != null && appBypass.isSystemUid) { + return false; + } if (opBypass.isPrivileged && appBypass != null && appBypass.isPrivileged) { return false; } From 25f1b6a1ac5c71ebafe4b9235829aa3a79d1dd21 Mon Sep 17 00:00:00 2001 From: Evan Severson Date: Fri, 13 May 2022 14:30:38 -0700 Subject: [PATCH 3/3] Make CheckOp return allowed if any attr tag for a package is excluded checkOp doesn't support checking against an attribution tag, this causes some checkOps to fail when a noteOp is successful meaning that a preflight routine might fail before delivering data and doing the more precise check. This only affects when a user restriction is applied and there are excepted package+tag. Test: Checkop with test app Bug: 232502990 Bug: 231496105 Merged-In: Idcf5ac9a5401ad8089f5873da1f978fdf9258b5a Change-Id: Idcf5ac9a5401ad8089f5873da1f978fdf9258b5a (cherry picked from commit 61c2d0291bd5b9b39a1d7db7454b3d7c630e7de9) --- .../android/server/appop/AppOpsService.java | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/services/core/java/com/android/server/appop/AppOpsService.java b/services/core/java/com/android/server/appop/AppOpsService.java index 9ef33c7c2f61c..3808e0c93a38f 100644 --- a/services/core/java/com/android/server/appop/AppOpsService.java +++ b/services/core/java/com/android/server/appop/AppOpsService.java @@ -3242,7 +3242,7 @@ public class AppOpsService extends IAppOpsService.Stub { return AppOpsManager.MODE_IGNORED; } synchronized (this) { - if (isOpRestrictedLocked(uid, code, packageName, attributionTag, pvr.bypass)) { + if (isOpRestrictedLocked(uid, code, packageName, attributionTag, pvr.bypass, true)) { return AppOpsManager.MODE_IGNORED; } code = AppOpsManager.opToSwitch(code); @@ -3459,7 +3459,7 @@ public class AppOpsService extends IAppOpsService.Stub { final int switchCode = AppOpsManager.opToSwitch(code); final UidState uidState = ops.uidState; - if (isOpRestrictedLocked(uid, code, packageName, attributionTag, pvr.bypass)) { + if (isOpRestrictedLocked(uid, code, packageName, attributionTag, pvr.bypass, false)) { attributedOp.rejected(uidState.state, flags); scheduleOpNotedIfNeededLocked(code, uid, packageName, attributionTag, flags, AppOpsManager.MODE_IGNORED); @@ -3973,7 +3973,8 @@ public class AppOpsService extends IAppOpsService.Stub { final Op op = getOpLocked(ops, code, uid, true); final AttributedOp attributedOp = op.getOrCreateAttribution(op, attributionTag); final UidState uidState = ops.uidState; - isRestricted = isOpRestrictedLocked(uid, code, packageName, attributionTag, pvr.bypass); + isRestricted = isOpRestrictedLocked(uid, code, packageName, attributionTag, pvr.bypass, + false); final int switchCode = AppOpsManager.opToSwitch(code); // If there is a non-default per UID policy (we set UID op mode only if // non-default) it takes over, otherwise use the per package policy. @@ -4764,7 +4765,7 @@ public class AppOpsService extends IAppOpsService.Stub { } private boolean isOpRestrictedLocked(int uid, int code, String packageName, - String attributionTag, @Nullable RestrictionBypass appBypass) { + String attributionTag, @Nullable RestrictionBypass appBypass, boolean isCheckOp) { int restrictionSetCount = mOpGlobalRestrictions.size(); for (int i = 0; i < restrictionSetCount; i++) { @@ -4781,7 +4782,8 @@ public class AppOpsService extends IAppOpsService.Stub { // For each client, check that the given op is not restricted, or that the given // package is exempt from the restriction. ClientUserRestrictionState restrictionState = mOpUserRestrictions.valueAt(i); - if (restrictionState.hasRestriction(code, packageName, attributionTag, userHandle)) { + if (restrictionState.hasRestriction(code, packageName, attributionTag, userHandle, + isCheckOp)) { RestrictionBypass opBypass = opAllowSystemBypassRestriction(code); if (opBypass != null) { // If we are the system, bypass user restrictions for certain codes @@ -7141,7 +7143,7 @@ public class AppOpsService extends IAppOpsService.Stub { } public boolean hasRestriction(int restriction, String packageName, String attributionTag, - int userId) { + int userId, boolean isCheckOp) { if (perUserRestrictions == null) { return false; } @@ -7160,6 +7162,9 @@ public class AppOpsService extends IAppOpsService.Stub { return true; } + if (isCheckOp) { + return !perUserExclusions.includes(packageName); + } return !perUserExclusions.contains(packageName, attributionTag); } @@ -7326,7 +7331,8 @@ public class AppOpsService extends IAppOpsService.Stub { int numRestrictions = mOpUserRestrictions.size(); for (int i = 0; i < numRestrictions; i++) { if (mOpUserRestrictions.valueAt(i) - .hasRestriction(code, pkg, attributionTag, user.getIdentifier())) { + .hasRestriction(code, pkg, attributionTag, user.getIdentifier(), + false)) { number++; } }