From d493ca93ceeade6a419b677575a4682950f6e85d Mon Sep 17 00:00:00 2001 From: Fabian Kozynski Date: Wed, 4 Sep 2019 16:53:37 -0400 Subject: [PATCH 1/5] Use UnlockMethodCache#canSkipBouncer in user switcher KeyguardMonitor#canSkipBouncer was not updated properly when the phone was unlocked using fingerprint. This CL removes that method and changes UserSwitcherController to query UnlockMethodCache directly, as it was KeyguardMonitor's only client for that method. Test: manual unlocking with FP and with pattern Test: no automated test yet Bug: 140486529 Merged-In: Idbff4fbabca962c632ff5d78b25418c0502db9a7 Change-Id: Idbff4fbabca962c632ff5d78b25418c0502db9a7 (cherry picked from commit d2eb34b689eaa8a9b064b4e69773083c723e3756) --- .../statusbar/policy/KeyguardMonitor.java | 1 - .../statusbar/policy/KeyguardMonitorImpl.java | 26 ------------------- .../policy/UserSwitcherController.java | 7 +++-- .../utils/leaks/FakeKeyguardMonitor.java | 5 ---- 4 files changed, 5 insertions(+), 34 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/policy/KeyguardMonitor.java b/packages/SystemUI/src/com/android/systemui/statusbar/policy/KeyguardMonitor.java index 01498e6bd54da..6fc265e6f983c 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/policy/KeyguardMonitor.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/policy/KeyguardMonitor.java @@ -19,7 +19,6 @@ public interface KeyguardMonitor extends CallbackController { boolean isSecure(); - boolean canSkipBouncer(); boolean isShowing(); boolean isOccluded(); boolean isKeyguardFadingAway(); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/policy/KeyguardMonitorImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/policy/KeyguardMonitorImpl.java index b53ff0e45cea7..2b08d68f1072e 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/policy/KeyguardMonitorImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/policy/KeyguardMonitorImpl.java @@ -17,13 +17,11 @@ package com.android.systemui.statusbar.policy; import android.annotation.NonNull; -import android.app.ActivityManager; import android.content.Context; import com.android.internal.util.Preconditions; import com.android.keyguard.KeyguardUpdateMonitor; import com.android.keyguard.KeyguardUpdateMonitorCallback; -import com.android.systemui.settings.CurrentUserTracker; import java.util.ArrayList; @@ -39,14 +37,11 @@ public class KeyguardMonitorImpl extends KeyguardUpdateMonitorCallback private final ArrayList mCallbacks = new ArrayList<>(); private final Context mContext; - private final CurrentUserTracker mUserTracker; private final KeyguardUpdateMonitor mKeyguardUpdateMonitor; - private int mCurrentUser; private boolean mShowing; private boolean mSecure; private boolean mOccluded; - private boolean mCanSkipBouncer; private boolean mListening; private boolean mKeyguardFadingAway; @@ -61,13 +56,6 @@ public class KeyguardMonitorImpl extends KeyguardUpdateMonitorCallback public KeyguardMonitorImpl(Context context) { mContext = context; mKeyguardUpdateMonitor = KeyguardUpdateMonitor.getInstance(mContext); - mUserTracker = new CurrentUserTracker(mContext) { - @Override - public void onUserSwitched(int newUserId) { - mCurrentUser = newUserId; - updateCanSkipBouncerState(); - } - }; } @Override @@ -76,10 +64,7 @@ public void addCallback(@NonNull Callback callback) { mCallbacks.add(callback); if (mCallbacks.size() != 0 && !mListening) { mListening = true; - mCurrentUser = ActivityManager.getCurrentUser(); - updateCanSkipBouncerState(); mKeyguardUpdateMonitor.registerCallback(this); - mUserTracker.startTracking(); } } @@ -89,7 +74,6 @@ public void removeCallback(@NonNull Callback callback) { if (mCallbacks.remove(callback) && mCallbacks.size() == 0 && mListening) { mListening = false; mKeyguardUpdateMonitor.removeCallback(this); - mUserTracker.stopTracking(); } } @@ -108,11 +92,6 @@ public boolean isOccluded() { return mOccluded; } - @Override - public boolean canSkipBouncer() { - return mCanSkipBouncer; - } - public void notifyKeyguardState(boolean showing, boolean secure, boolean occluded) { if (mShowing == showing && mSecure == secure && mOccluded == occluded) return; mShowing = showing; @@ -123,7 +102,6 @@ public void notifyKeyguardState(boolean showing, boolean secure, boolean occlude @Override public void onTrustChanged(int userId) { - updateCanSkipBouncerState(); notifyKeyguardChanged(); } @@ -131,10 +109,6 @@ public boolean isDeviceInteractive() { return mKeyguardUpdateMonitor.isDeviceInteractive(); } - private void updateCanSkipBouncerState() { - mCanSkipBouncer = mKeyguardUpdateMonitor.getUserCanSkipBouncer(mCurrentUser); - } - private void notifyKeyguardChanged() { // Copy the list to allow removal during callback. new ArrayList<>(mCallbacks).forEach(Callback::onKeyguardShowingChanged); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/policy/UserSwitcherController.java b/packages/SystemUI/src/com/android/systemui/statusbar/policy/UserSwitcherController.java index 395add76dda49..35e3923f285b3 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/policy/UserSwitcherController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/policy/UserSwitcherController.java @@ -61,6 +61,7 @@ import com.android.systemui.plugins.qs.DetailAdapter; import com.android.systemui.qs.tiles.UserDetailView; import com.android.systemui.statusbar.phone.SystemUIDialog; +import com.android.systemui.statusbar.phone.UnlockMethodCache; import java.io.FileDescriptor; import java.io.PrintWriter; @@ -595,17 +596,19 @@ public static abstract class BaseUserAdapter extends BaseAdapter { final UserSwitcherController mController; private final KeyguardMonitor mKeyguardMonitor; + private final UnlockMethodCache mUnlockMethodCache; protected BaseUserAdapter(UserSwitcherController controller) { mController = controller; mKeyguardMonitor = controller.mKeyguardMonitor; + mUnlockMethodCache = UnlockMethodCache.getInstance(controller.mContext); controller.addAdapter(new WeakReference<>(this)); } public int getUserCount() { boolean secureKeyguardShowing = mKeyguardMonitor.isShowing() && mKeyguardMonitor.isSecure() - && !mKeyguardMonitor.canSkipBouncer(); + && !mUnlockMethodCache.canSkipBouncer(); if (!secureKeyguardShowing) { return mController.getUsers().size(); } @@ -627,7 +630,7 @@ public int getUserCount() { public int getCount() { boolean secureKeyguardShowing = mKeyguardMonitor.isShowing() && mKeyguardMonitor.isSecure() - && !mKeyguardMonitor.canSkipBouncer(); + && !mUnlockMethodCache.canSkipBouncer(); if (!secureKeyguardShowing) { return mController.getUsers().size(); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/utils/leaks/FakeKeyguardMonitor.java b/packages/SystemUI/tests/src/com/android/systemui/utils/leaks/FakeKeyguardMonitor.java index 95c7a4d09f928..2fb0e0e7caf8c 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/utils/leaks/FakeKeyguardMonitor.java +++ b/packages/SystemUI/tests/src/com/android/systemui/utils/leaks/FakeKeyguardMonitor.java @@ -80,9 +80,4 @@ public long getKeyguardFadingAwayDelay() { public long calculateGoingToFullShadeDelay() { return 0; } - - @Override - public boolean canSkipBouncer() { - return false; - } } From 57c5b256b4eb0c1ba7c789c966bac1eb95e1bbef Mon Sep 17 00:00:00 2001 From: Jungshik Shin Date: Sun, 15 Sep 2019 22:57:31 -0700 Subject: [PATCH 2/5] Use NotoSansMyanmar (pure Unicode) fonts Revert Myanmar fonts to those on Android P. Instead NotoSansMyanmar*-ZawDecode (Unicode-Zawgyi hybrid) fonts, go back to NotoSansMyanmar and NotoSansMyanmarUI (pure Unicode) fonts. Bug: 141019225 Change-Id: Ib2494b9b5cb148f598e69271c5676e7104f66ae3 (cherry picked from commit 0aeafabef3c8d35a06fa0e08db7bca6f0226895f) --- data/fonts/fonts.xml | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/data/fonts/fonts.xml b/data/fonts/fonts.xml index 072beae8baf7e..c6920977f6b9c 100644 --- a/data/fonts/fonts.xml +++ b/data/fonts/fonts.xml @@ -323,14 +323,16 @@ NotoSansLaoUI-Bold.ttf - NotoSansMyanmar-Regular-ZawDecode.ttf - NotoSansMyanmar-Bold-ZawDecode.ttf + NotoSansMyanmar-Regular.otf + NotoSansMyanmar-Medium.otf + NotoSansMyanmar-Bold.otf NotoSerifMyanmar-Regular.otf NotoSerifMyanmar-Bold.otf - NotoSansMyanmarUI-Regular-ZawDecode.ttf - NotoSansMyanmarUI-Bold-ZawDecode.ttf + NotoSansMyanmarUI-Regular.otf + NotoSansMyanmarUI-Medium.otf + NotoSansMyanmarUI-Bold.otf NotoSansThaana-Regular.ttf From 659b148a95eee652f83e1e4f4fc42e4c2e0fd388 Mon Sep 17 00:00:00 2001 From: Svet Ganov Date: Mon, 23 Sep 2019 21:32:08 -0700 Subject: [PATCH 3/5] Update PermissionChecker usages to avoid unnecessary attribution. We had accidental usages of the PermissionChecker for cases where no private data was provided to the app but the checkPermission API on the latter also did blame data access on the app. The PermissionChecker was designed to handle IPC calls and not for generic API checks. To avoid future accidental incorrect PermissionChecker usages this change renames the existing APIs of the latter to clearly indicate that they should be used for data delivery and also adds sibling methods for doing the same permission checks for preflight purposes. Also the documentation is improved to furhter assist developers. In addition, this change fixes accidental permission checker usages that blame when they should not by using the new preflight flavor of the permission check APIs. Test: atest com.android.settingslib.location.RecentLocationAppsTest atest CtsPermissionTestCases added: LocationAccessCheckTest#notificationOnlyForAccessesSinceFeatureWasEnabled added: LocationAccessCheckTest#noNotificationIfFeatureDisabled added: LocationAccessCheckTest#noNotificationIfBlamerNotSystemOrLocationProvider added: LocationAccessCheckTest#testOpeningLocationSettingsDoesNotTriggerAccess bug:141028068 Merged-In: I65c71569d0dd8a40bc6fecabb22c5373dd6e806e Change-Id: I65c71569d0dd8a40bc6fecabb22c5373dd6e806e (cherry picked from commit 7fe065eb660cfe8ae54f8cd1e1e615f47d7b311c) --- .../android/content/PermissionChecker.java | 296 ++++++++++++++++-- .../android/speech/RecognitionService.java | 24 +- .../location/RecentLocationAccesses.java | 5 +- .../location/RecentLocationApps.java | 6 +- .../server/role/RoleManagerService.java | 3 +- .../DevicePolicyManagerService.java | 11 +- 6 files changed, 297 insertions(+), 48 deletions(-) diff --git a/core/java/android/content/PermissionChecker.java b/core/java/android/content/PermissionChecker.java index 6fe6e991fb1e7..95286e4bc1dbf 100644 --- a/core/java/android/content/PermissionChecker.java +++ b/core/java/android/content/PermissionChecker.java @@ -50,6 +50,19 @@ * permission model for which the user had disabled the "permission" * which is achieved by disallowing the corresponding app op. *

+ *

+ * This class has two types of methods and you should be careful which + * type to call based on whether permission protected data is being + * passed to the app or you are just checking whether the app holds a + * permission. The reason is that a permission check requires checking + * the runtime permission and if it is granted checking the corresponding + * app op as for apps not supporting the runtime mode we never revoke + * permissions but disable app ops. Since there are two types of app op + * checks, one that does not leave a record an action was performed and + * another the does, one needs to call the preflight flavor of the checks + * named xxxForPreflight only if no private data is being delivered but + * a permission check is what is needed and the xxxForDataDelivery where + * the permission check is right before private data delivery. * * @hide */ @@ -63,6 +76,9 @@ public final class PermissionChecker { /** Permission result: The permission is denied because the app op is not allowed. */ public static final int PERMISSION_DENIED_APP_OP = PackageManager.PERMISSION_DENIED - 1; + /** Constant when the PID for which we check permissions is unknown. */ + public static final int PID_UNKNOWN = -1; + /** @hide */ @IntDef({PERMISSION_GRANTED, PERMISSION_DENIED, @@ -78,47 +94,127 @@ private PermissionChecker() { * Checks whether a given package in a UID and PID has a given permission * and whether the app op that corresponds to this permission is allowed. * + * NOTE: Use this method only for permission checks at the + * point where you will deliver the permission protected data to clients. + * + *

For example, if an app registers a location listener it should have the location + * permission but no data is actually sent to the app at the moment of registration + * and you should use {@link #checkPermissionForPreflight(Context, String, int, int, String)} + * to determine if the app has or may have location permission (if app has only foreground + * location the grant state depends on the app's fg/gb state) and this check will not + * leave a trace that permission protected data was delivered. When you are about to + * deliver the location data to a registered listener you should use this method which + * will evaluate the permission access based on the current fg/bg state of the app and + * leave a record that the data was accessed. + * * @param context Context for accessing resources. * @param permission The permission to check. - * @param pid The process id for which to check. + * @param pid The process id for which to check. Use {@link #PID_UNKNOWN} if the PID + * is not known. * @param uid The uid for which to check. * @param packageName The package name for which to check. If null the * the first package for the calling UID will be used. * @return The permission check result which is either {@link #PERMISSION_GRANTED} * or {@link #PERMISSION_DENIED} or {@link #PERMISSION_DENIED_APP_OP}. + * + * @see #checkPermissionForPreflight(Context, String, int, int, String) */ @PermissionResult - public static int checkPermission(@NonNull Context context, @NonNull String permission, - int pid, int uid, @Nullable String packageName) { - if (context.checkPermission(permission, pid, uid) == PackageManager.PERMISSION_DENIED) { - return PERMISSION_DENIED; - } - - AppOpsManager appOpsManager = context.getSystemService(AppOpsManager.class); - String op = appOpsManager.permissionToOp(permission); - if (op == null) { - return PERMISSION_GRANTED; - } - - if (packageName == null) { - String[] packageNames = context.getPackageManager().getPackagesForUid(uid); - if (packageNames == null || packageNames.length <= 0) { - return PERMISSION_DENIED; - } - packageName = packageNames[0]; - } + public static int checkPermissionForDataDelivery(@NonNull Context context, + @NonNull String permission, int pid, int uid, @Nullable String packageName) { + return checkPermissionCommon(context, permission, pid, uid, packageName, + true /*forDataDelivery*/); + } - if (appOpsManager.noteProxyOpNoThrow(op, packageName, uid) != AppOpsManager.MODE_ALLOWED) { - return PERMISSION_DENIED_APP_OP; - } + /** + * Checks whether a given package in a UID and PID has a given permission + * and whether the app op that corresponds to this permission is allowed. + * + * NOTE: Use this method only for permission checks at the + * preflight point where you will not deliver the permission protected data + * to clients but schedule permission data delivery, apps register listeners, + * etc. + * + *

For example, if an app registers a location listener it should have the location + * permission but no data is actually sent to the app at the moment of registration + * and you should use this method to determine if the app has or may have location + * permission (if app has only foreground location the grant state depends on the app's + * fg/gb state) and this check will not leave a trace that permission protected data + * was delivered. When you are about to deliver the location data to a registered + * listener you should use {@link #checkPermissionForDataDelivery(Context, String, + * int, int, String)} which will evaluate the permission access based on the current + * fg/bg state of the app and leave a record that the data was accessed. + * + * @param context Context for accessing resources. + * @param permission The permission to check. + * @param pid The process id for which to check. + * @param uid The uid for which to check. + * @param packageName The package name for which to check. If null the + * the first package for the calling UID will be used. + * @return The permission check result which is either {@link #PERMISSION_GRANTED} + * or {@link #PERMISSION_DENIED} or {@link #PERMISSION_DENIED_APP_OP}. + * + * @see #checkPermissionForDataDelivery(Context, String, int, int, String) + */ + @PermissionResult + public static int checkPermissionForPreflight(@NonNull Context context, + @NonNull String permission, int pid, int uid, @Nullable String packageName) { + return checkPermissionCommon(context, permission, pid, uid, packageName, + false /*forDataDelivery*/); + } - return PERMISSION_GRANTED; + /** + * Checks whether your app has a given permission and whether the app op + * that corresponds to this permission is allowed. + * + * NOTE: Use this method only for permission checks at the + * point where you will deliver the permission protected data to clients. + * + *

For example, if an app registers a location listener it should have the location + * permission but no data is actually sent to the app at the moment of registration + * and you should use {@link #checkSelfPermissionForPreflight(Context, String)} + * to determine if the app has or may have location permission (if app has only foreground + * location the grant state depends on the app's fg/gb state) and this check will not + * leave a trace that permission protected data was delivered. When you are about to + * deliver the location data to a registered listener you should use this method + * which will evaluate the permission access based on the current fg/bg state of the + * app and leave a record that the data was accessed. + * + *

This API assumes the the {@link Binder#getCallingUid()} is the same as + * {@link Process#myUid()}. + * + * @param context Context for accessing resources. + * @param permission The permission to check. + * @return The permission check result which is either {@link #PERMISSION_GRANTED} + * or {@link #PERMISSION_DENIED} or {@link #PERMISSION_DENIED_APP_OP}. + * + * @see #checkSelfPermissionForPreflight(Context, String) + */ + @PermissionResult + public static int checkSelfPermissionForDataDelivery(@NonNull Context context, + @NonNull String permission) { + return checkPermissionForDataDelivery(context, permission, Process.myPid(), + Process.myUid(), context.getPackageName()); } /** * Checks whether your app has a given permission and whether the app op * that corresponds to this permission is allowed. * + * NOTE: Use this method only for permission checks at the + * preflight point where you will not deliver the permission protected data + * to clients but schedule permission data delivery, apps register listeners, + * etc. + * + *

For example, if an app registers a location listener it should have the location + * permission but no data is actually sent to the app at the moment of registration + * and you should use this method to determine if the app has or may have location + * permission (if app has only foreground location the grant state depends on the + * app's fg/gb state) and this check will not leave a trace that permission protected + * data was delivered. When you are about to deliver the location data to a registered + * listener you should use this method which will evaluate the permission access based + * on the current fg/bg state of the app and leave a record that the data was accessed. + * *

This API assumes the the {@link Binder#getCallingUid()} is the same as * {@link Process#myUid()}. * @@ -126,11 +222,13 @@ public static int checkPermission(@NonNull Context context, @NonNull String perm * @param permission The permission to check. * @return The permission check result which is either {@link #PERMISSION_GRANTED} * or {@link #PERMISSION_DENIED} or {@link #PERMISSION_DENIED_APP_OP}. + * + * @see #checkSelfPermissionForDataDelivery(Context, String) */ @PermissionResult - public static int checkSelfPermission(@NonNull Context context, + public static int checkSelfPermissionForPreflight(@NonNull Context context, @NonNull String permission) { - return checkPermission(context, permission, Process.myPid(), + return checkPermissionForPreflight(context, permission, Process.myPid(), Process.myUid(), context.getPackageName()); } @@ -138,20 +236,106 @@ public static int checkSelfPermission(@NonNull Context context, * Checks whether the IPC you are handling has a given permission and whether * the app op that corresponds to this permission is allowed. * + * NOTE: Use this method only for permission checks at the + * point where you will deliver the permission protected data to clients. + * + *

For example, if an app registers a location listener it should have the location + * permission but no data is actually sent to the app at the moment of registration + * and you should use {@link #checkCallingPermissionForPreflight(Context, String, String)} + * to determine if the app has or may have location permission (if app has only foreground + * location the grant state depends on the app's fg/gb state) and this check will not + * leave a trace that permission protected data was delivered. When you are about to + * deliver the location data to a registered listener you should use this method which + * will evaluate the permission access based on the current fg/bg state of the app and + * leave a record that the data was accessed. + * * @param context Context for accessing resources. * @param permission The permission to check. * @param packageName The package name making the IPC. If null the * the first package for the calling UID will be used. * @return The permission check result which is either {@link #PERMISSION_GRANTED} * or {@link #PERMISSION_DENIED} or {@link #PERMISSION_DENIED_APP_OP}. + * + * @see #checkCallingPermissionForPreflight(Context, String, String) */ @PermissionResult - public static int checkCallingPermission(@NonNull Context context, + public static int checkCallingPermissionForDataDelivery(@NonNull Context context, @NonNull String permission, @Nullable String packageName) { if (Binder.getCallingPid() == Process.myPid()) { return PERMISSION_DENIED; } - return checkPermission(context, permission, Binder.getCallingPid(), + return checkPermissionForDataDelivery(context, permission, Binder.getCallingPid(), + Binder.getCallingUid(), packageName); + } + + /** + * Checks whether the IPC you are handling has a given permission and whether + * the app op that corresponds to this permission is allowed. + * + * NOTE: Use this method only for permission checks at the + * preflight point where you will not deliver the permission protected data + * to clients but schedule permission data delivery, apps register listeners, + * etc. + * + *

For example, if an app registers a location listener it should have the location + * permission but no data is actually sent to the app at the moment of registration + * and you should use this method to determine if the app has or may have location + * permission (if app has only foreground location the grant state depends on the app's + * fg/gb state) and this check will not leave a trace that permission protected data + * was delivered. When you are about to deliver the location data to a registered + * listener you should use {@link #checkCallingOrSelfPermissionForDataDelivery(Context, + * String)} which will evaluate the permission access based on the current fg/bg state + * of the app and leave a record that the data was accessed. + * + * @param context Context for accessing resources. + * @param permission The permission to check. + * @param packageName The package name making the IPC. If null the + * the first package for the calling UID will be used. + * @return The permission check result which is either {@link #PERMISSION_GRANTED} + * or {@link #PERMISSION_DENIED} or {@link #PERMISSION_DENIED_APP_OP}. + * + * @see #checkCallingPermissionForDataDelivery(Context, String, String) + */ + @PermissionResult + public static int checkCallingPermissionForPreflight(@NonNull Context context, + @NonNull String permission, @Nullable String packageName) { + if (Binder.getCallingPid() == Process.myPid()) { + return PERMISSION_DENIED; + } + return checkPermissionForPreflight(context, permission, Binder.getCallingPid(), + Binder.getCallingUid(), packageName); + } + + /** + * Checks whether the IPC you are handling or your app has a given permission + * and whether the app op that corresponds to this permission is allowed. + * + * NOTE: Use this method only for permission checks at the + * point where you will deliver the permission protected data to clients. + * + *

For example, if an app registers a location listener it should have the location + * permission but no data is actually sent to the app at the moment of registration + * and you should use {@link #checkCallingOrSelfPermissionForPreflight(Context, String)} + * to determine if the app has or may have location permission (if app has only foreground + * location the grant state depends on the app's fg/gb state) and this check will not + * leave a trace that permission protected data was delivered. When you are about to + * deliver the location data to a registered listener you should use this method which + * will evaluate the permission access based on the current fg/bg state of the app and + * leave a record that the data was accessed. + * + * @param context Context for accessing resources. + * @param permission The permission to check. + * @return The permission check result which is either {@link #PERMISSION_GRANTED} + * or {@link #PERMISSION_DENIED} or {@link #PERMISSION_DENIED_APP_OP}. + * + * @see #checkCallingOrSelfPermissionForPreflight(Context, String) + */ + @PermissionResult + public static int checkCallingOrSelfPermissionForDataDelivery(@NonNull Context context, + @NonNull String permission) { + String packageName = (Binder.getCallingPid() == Process.myPid()) + ? context.getPackageName() : null; + return checkPermissionForDataDelivery(context, permission, Binder.getCallingPid(), Binder.getCallingUid(), packageName); } @@ -159,17 +343,69 @@ public static int checkCallingPermission(@NonNull Context context, * Checks whether the IPC you are handling or your app has a given permission * and whether the app op that corresponds to this permission is allowed. * + * NOTE: Use this method only for permission checks at the + * preflight point where you will not deliver the permission protected data + * to clients but schedule permission data delivery, apps register listeners, + * etc. + * + *

For example, if an app registers a location listener it should have the location + * permission but no data is actually sent to the app at the moment of registration + * and you should use this method to determine if the app has or may have location + * permission (if app has only foreground location the grant state depends on the + * app's fg/gb state) and this check will not leave a trace that permission protected + * data was delivered. When you are about to deliver the location data to a registered + * listener you should use {@link #checkCallingOrSelfPermissionForDataDelivery(Context, + * String)} which will evaluate the permission access based on the current fg/bg state + * of the app and leave a record that the data was accessed. + * * @param context Context for accessing resources. * @param permission The permission to check. * @return The permission check result which is either {@link #PERMISSION_GRANTED} * or {@link #PERMISSION_DENIED} or {@link #PERMISSION_DENIED_APP_OP}. + * + * @see #checkCallingOrSelfPermissionForDataDelivery(Context, String) */ @PermissionResult - public static int checkCallingOrSelfPermission(@NonNull Context context, + public static int checkCallingOrSelfPermissionForPreflight(@NonNull Context context, @NonNull String permission) { String packageName = (Binder.getCallingPid() == Process.myPid()) ? context.getPackageName() : null; - return checkPermission(context, permission, Binder.getCallingPid(), + return checkPermissionForPreflight(context, permission, Binder.getCallingPid(), Binder.getCallingUid(), packageName); } + + private static int checkPermissionCommon(@NonNull Context context, @NonNull String permission, + int pid, int uid, @Nullable String packageName, boolean forDataDelivery) { + if (context.checkPermission(permission, pid, uid) == PackageManager.PERMISSION_DENIED) { + return PERMISSION_DENIED; + } + + AppOpsManager appOpsManager = context.getSystemService(AppOpsManager.class); + String op = appOpsManager.permissionToOp(permission); + if (op == null) { + return PERMISSION_GRANTED; + } + + if (packageName == null) { + String[] packageNames = context.getPackageManager().getPackagesForUid(uid); + if (packageNames == null || packageNames.length <= 0) { + return PERMISSION_DENIED; + } + packageName = packageNames[0]; + } + + if (forDataDelivery) { + if (appOpsManager.noteProxyOpNoThrow(op, packageName, uid) + != AppOpsManager.MODE_ALLOWED) { + return PERMISSION_DENIED_APP_OP; + } + } else { + final int mode = appOpsManager.unsafeCheckOpRawNoThrow(op, uid, packageName); + if (mode != AppOpsManager.MODE_ALLOWED && mode != AppOpsManager.MODE_FOREGROUND) { + return PERMISSION_DENIED_APP_OP; + } + } + + return PERMISSION_GRANTED; + } } diff --git a/core/java/android/speech/RecognitionService.java b/core/java/android/speech/RecognitionService.java index 70dfef574ca53..fb13c1f5dde3b 100644 --- a/core/java/android/speech/RecognitionService.java +++ b/core/java/android/speech/RecognitionService.java @@ -170,13 +170,23 @@ public StartListeningArgs(Intent intent, IRecognitionListener listener, int call * Checks whether the caller has sufficient permissions * * @param listener to send the error message to in case of error + * @param forDataDelivery If the permission check is for delivering the sensitive data. * @return {@code true} if the caller has enough permissions, {@code false} otherwise */ - private boolean checkPermissions(IRecognitionListener listener) { + private boolean checkPermissions(IRecognitionListener listener, boolean forDataDelivery) { if (DBG) Log.d(TAG, "checkPermissions"); - if (PermissionChecker.checkCallingOrSelfPermission(this, - android.Manifest.permission.RECORD_AUDIO) == PermissionChecker.PERMISSION_GRANTED) { - return true; + if (forDataDelivery) { + if (PermissionChecker.checkCallingOrSelfPermissionForDataDelivery(this, + android.Manifest.permission.RECORD_AUDIO) + == PermissionChecker.PERMISSION_GRANTED) { + return true; + } + } else { + if (PermissionChecker.checkCallingOrSelfPermissionForPreflight(this, + android.Manifest.permission.RECORD_AUDIO) + == PermissionChecker.PERMISSION_GRANTED) { + return true; + } } try { Log.e(TAG, "call for recognition service without RECORD_AUDIO permissions"); @@ -342,7 +352,7 @@ public RecognitionServiceBinder(RecognitionService service) { public void startListening(Intent recognizerIntent, IRecognitionListener listener) { if (DBG) Log.d(TAG, "startListening called by:" + listener.asBinder()); final RecognitionService service = mServiceRef.get(); - if (service != null && service.checkPermissions(listener)) { + if (service != null && service.checkPermissions(listener, true /*forDataDelivery*/)) { service.mHandler.sendMessage(Message.obtain(service.mHandler, MSG_START_LISTENING, service.new StartListeningArgs( recognizerIntent, listener, Binder.getCallingUid()))); @@ -353,7 +363,7 @@ MSG_START_LISTENING, service.new StartListeningArgs( public void stopListening(IRecognitionListener listener) { if (DBG) Log.d(TAG, "stopListening called by:" + listener.asBinder()); final RecognitionService service = mServiceRef.get(); - if (service != null && service.checkPermissions(listener)) { + if (service != null && service.checkPermissions(listener, false /*forDataDelivery*/)) { service.mHandler.sendMessage(Message.obtain(service.mHandler, MSG_STOP_LISTENING, listener)); } @@ -363,7 +373,7 @@ public void stopListening(IRecognitionListener listener) { public void cancel(IRecognitionListener listener) { if (DBG) Log.d(TAG, "cancel called by:" + listener.asBinder()); final RecognitionService service = mServiceRef.get(); - if (service != null && service.checkPermissions(listener)) { + if (service != null && service.checkPermissions(listener, false /*forDataDelivery*/)) { service.mHandler.sendMessage(Message.obtain(service.mHandler, MSG_CANCEL, listener)); } diff --git a/packages/SettingsLib/src/com/android/settingslib/location/RecentLocationAccesses.java b/packages/SettingsLib/src/com/android/settingslib/location/RecentLocationAccesses.java index ea39317fb045b..81ca9eaf8e36b 100644 --- a/packages/SettingsLib/src/com/android/settingslib/location/RecentLocationAccesses.java +++ b/packages/SettingsLib/src/com/android/settingslib/location/RecentLocationAccesses.java @@ -111,8 +111,9 @@ public List getAppList() { for (int op : LOCATION_OPS) { final String permission = AppOpsManager.opToPermission(op); final int permissionFlags = pm.getPermissionFlags(permission, packageName, user); - if (PermissionChecker.checkPermission(mContext, permission, -1, uid, packageName) - == PermissionChecker.PERMISSION_GRANTED) { + if (PermissionChecker.checkPermissionForPreflight(mContext, permission, + PermissionChecker.PID_UNKNOWN, uid, packageName) + == PermissionChecker.PERMISSION_GRANTED) { if ((permissionFlags & PackageManager.FLAG_PERMISSION_USER_SENSITIVE_WHEN_GRANTED) == 0) { showApp = false; diff --git a/packages/SettingsLib/src/com/android/settingslib/location/RecentLocationApps.java b/packages/SettingsLib/src/com/android/settingslib/location/RecentLocationApps.java index 6fd874989c358..c623909a48e73 100644 --- a/packages/SettingsLib/src/com/android/settingslib/location/RecentLocationApps.java +++ b/packages/SettingsLib/src/com/android/settingslib/location/RecentLocationApps.java @@ -106,9 +106,9 @@ public List getAppList(boolean showSystemApps) { final String permission = AppOpsManager.opToPermission(op); final int permissionFlags = pm.getPermissionFlags(permission, packageName, user); - if (PermissionChecker.checkPermission(mContext, permission, -1, uid, - packageName) - == PermissionChecker.PERMISSION_GRANTED) { + if (PermissionChecker.checkPermissionForPreflight(mContext, permission, + PermissionChecker.PID_UNKNOWN, uid, packageName) + == PermissionChecker.PERMISSION_GRANTED) { if ((permissionFlags & PackageManager.FLAG_PERMISSION_USER_SENSITIVE_WHEN_GRANTED) == 0) { diff --git a/services/core/java/com/android/server/role/RoleManagerService.java b/services/core/java/com/android/server/role/RoleManagerService.java index c6a1867fa1e98..369e7fcd82af6 100644 --- a/services/core/java/com/android/server/role/RoleManagerService.java +++ b/services/core/java/com/android/server/role/RoleManagerService.java @@ -49,7 +49,6 @@ import android.os.ShellCallback; import android.os.UserHandle; import android.os.UserManagerInternal; -import android.provider.Telephony; import android.service.sms.FinancialSmsService; import android.telephony.IFinancialSmsCallback; import android.text.TextUtils; @@ -681,7 +680,7 @@ protected void dump(@NonNull FileDescriptor fd, @NonNull PrintWriter fout, @Override public void getSmsMessagesForFinancialApp( String callingPkg, Bundle params, IFinancialSmsCallback callback) { - int mode = PermissionChecker.checkCallingOrSelfPermission( + int mode = PermissionChecker.checkCallingOrSelfPermissionForDataDelivery( getContext(), AppOpsManager.OPSTR_SMS_FINANCIAL_TRANSACTIONS); diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index e5518d05e9c5a..37931be4eb10f 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -11830,10 +11830,13 @@ public int getPermissionGrantState(ComponentName admin, String callerPackage, try { int uid = packageManager.getPackageUidAsUser(packageName, user.getIdentifier()); - - // TODO: Prevent noting the app-op - granted = PermissionChecker.checkPermission(mContext, permission, -1, - uid, packageName); + if (PermissionChecker.checkPermissionForPreflight(mContext, permission, + PermissionChecker.PID_UNKNOWN, uid, packageName) + != PermissionChecker.PERMISSION_GRANTED) { + granted = PackageManager.PERMISSION_DENIED; + } else { + granted = PackageManager.PERMISSION_GRANTED; + } } catch (NameNotFoundException e) { throw new RemoteException( "Cannot check if " + permission + "is a runtime permission", e, From c8674c92fa75a0f592de34132318fb4e7cd676e8 Mon Sep 17 00:00:00 2001 From: Beverly Date: Tue, 3 Sep 2019 17:00:51 -0400 Subject: [PATCH 4/5] Fix zen alarms only mode check Also fixes setting the consolidated zen policy logging. Fixes: 140329813 Test: android.app.cts.NotificationManagerTest#testTotalSilenceOnlyMuteStreams Test: android.app.cts.NotificationManagerTest#testAlarmsOnlyMuteStreams Change-Id: I43d503ac23d7b0b141930d77cb76f1f589b22525 (cherry picked from commit dcc5cfc56111676d687c8c29d1ceb6914dc1d8ce) (cherry picked from commit 51563b5ffe99c1c17a38b35b70b77faf2d1de95b) --- core/java/android/service/notification/ZenModeConfig.java | 2 +- .../core/java/com/android/server/notification/ZenLog.java | 1 + .../java/com/android/server/notification/ZenModeHelper.java | 5 ++--- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/java/android/service/notification/ZenModeConfig.java b/core/java/android/service/notification/ZenModeConfig.java index cb7d41bdf21a2..93bcdbffd1330 100644 --- a/core/java/android/service/notification/ZenModeConfig.java +++ b/core/java/android/service/notification/ZenModeConfig.java @@ -1639,7 +1639,7 @@ public static class ZenRule implements Parcelable { @UnsupportedAppUsage public String name; // required for automatic @UnsupportedAppUsage - public int zenMode; + public int zenMode; // ie: Global.ZEN_MODE_IMPORTANT_INTERRUPTIONS @UnsupportedAppUsage public Uri conditionId; // required for automatic public Condition condition; // optional diff --git a/services/core/java/com/android/server/notification/ZenLog.java b/services/core/java/com/android/server/notification/ZenLog.java index c6af7566a8bda..8f05636eed9c8 100644 --- a/services/core/java/com/android/server/notification/ZenLog.java +++ b/services/core/java/com/android/server/notification/ZenLog.java @@ -179,6 +179,7 @@ private static String typeToString(int type) { case TYPE_SUPPRESSOR_CHANGED: return "suppressor_changed"; case TYPE_LISTENER_HINTS_CHANGED: return "listener_hints_changed"; case TYPE_SET_NOTIFICATION_POLICY: return "set_notification_policy"; + case TYPE_SET_CONSOLIDATED_ZEN_POLICY: return "set_consolidated_policy"; default: return "unknown"; } } diff --git a/services/core/java/com/android/server/notification/ZenModeHelper.java b/services/core/java/com/android/server/notification/ZenModeHelper.java index f81015dae4685..ebc41916d034e 100644 --- a/services/core/java/com/android/server/notification/ZenModeHelper.java +++ b/services/core/java/com/android/server/notification/ZenModeHelper.java @@ -942,12 +942,11 @@ private int computeZenMode() { } private void applyCustomPolicy(ZenPolicy policy, ZenRule rule) { - if (rule.zenMode == NotificationManager.INTERRUPTION_FILTER_NONE) { + if (rule.zenMode == Global.ZEN_MODE_NO_INTERRUPTIONS) { policy.apply(new ZenPolicy.Builder() .disallowAllSounds() .build()); - } else if (rule.zenMode - == NotificationManager.INTERRUPTION_FILTER_ALARMS) { + } else if (rule.zenMode == Global.ZEN_MODE_ALARMS) { policy.apply(new ZenPolicy.Builder() .disallowAllSounds() .allowAlarms(true) From f9bc93b250121f9167e07b4740ac07d6d11fcee9 Mon Sep 17 00:00:00 2001 From: "Philip P. Moltmann" Date: Fri, 13 Sep 2019 15:12:34 -0700 Subject: [PATCH 5/5] [DO NOT MERGE] Split access-media-storage from read-external-storage And also pre-grant it to all apps that currently get any storage permission pre-granted Test: atest SplitPermissionTest m -j gts && gts-tradefed run commandAndExit gts-dev -m GtsPermissionTestCases --test=com.google.android.permission.gts.DefaultPermissionGrantPolicyTest#testDefaultGrantsWithRemoteExceptions Manual testing: All combinations of - App targetSdk = 28 and 29 (and 22 for extra credit) - App having the tag for ACCESS_MEDIA_LOCATION or not - Upgrade from P->Q-QPR and from vanilla Q->Q-QPR Further upgrade of targetSdk from 28->29 while on Q-QPR ==> All permission behavior should make sense. Sometimes there are weird, but expected behaviors. Hence we need to collect the results and then look at the unexpected ones. See SplitPermissionTest for some tests I added for the location-background permission which was split from the fine/coarse-location permissions Fixes: 141048840,140961754 Change-Id: Ib9f50d25c002036f13cf2d42fc4d1b214f20920c (cherry picked from commit ac7b10c135bb148edcad1aad8e19c733d333f769) (cherry picked from commit f3ff750f2998f43124ab59a3c0926b37b2a50dbd) --- core/java/android/app/AppOpsManager.java | 17 ++++++++++++++++- data/etc/platform.xml | 4 ++++ .../DefaultPermissionGrantPolicy.java | 1 + 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/core/java/android/app/AppOpsManager.java b/core/java/android/app/AppOpsManager.java index fb72e651cebd4..f3e4892afdff7 100644 --- a/core/java/android/app/AppOpsManager.java +++ b/core/java/android/app/AppOpsManager.java @@ -834,9 +834,12 @@ public static String flagsToString(@OpFlags int flags) { public static final int OP_ACCESS_ACCESSIBILITY = 88; /** @hide Read the device identifiers (IMEI / MEID, IMSI, SIM / Build serial) */ public static final int OP_READ_DEVICE_IDENTIFIERS = 89; + /** @hide Read location metadata from media */ + public static final int OP_ACCESS_MEDIA_LOCATION = 90; + /** @hide */ @UnsupportedAppUsage - public static final int _NUM_OP = 90; + public static final int _NUM_OP = 91; /** Access to coarse location information. */ public static final String OPSTR_COARSE_LOCATION = "android:coarse_location"; @@ -1107,6 +1110,9 @@ public static String flagsToString(@OpFlags int flags) { @TestApi @SystemApi public static final String OPSTR_LEGACY_STORAGE = "android:legacy_storage"; + /** @hide Read location metadata from media */ + public static final String OPSTR_ACCESS_MEDIA_LOCATION = "android:access_media_location"; + /** @hide Interact with accessibility. */ @SystemApi public static final String OPSTR_ACCESS_ACCESSIBILITY = "android:access_accessibility"; @@ -1134,6 +1140,7 @@ public static String flagsToString(@OpFlags int flags) { // Storage OP_READ_EXTERNAL_STORAGE, OP_WRITE_EXTERNAL_STORAGE, + OP_ACCESS_MEDIA_LOCATION, // Location OP_COARSE_LOCATION, OP_FINE_LOCATION, @@ -1273,6 +1280,7 @@ public static String flagsToString(@OpFlags int flags) { OP_LEGACY_STORAGE, // LEGACY_STORAGE OP_ACCESS_ACCESSIBILITY, // ACCESS_ACCESSIBILITY OP_READ_DEVICE_IDENTIFIERS, // READ_DEVICE_IDENTIFIERS + OP_ACCESS_MEDIA_LOCATION, // ACCESS_MEDIA_LOCATION }; /** @@ -1369,6 +1377,7 @@ public static String flagsToString(@OpFlags int flags) { OPSTR_LEGACY_STORAGE, OPSTR_ACCESS_ACCESSIBILITY, OPSTR_READ_DEVICE_IDENTIFIERS, + OPSTR_ACCESS_MEDIA_LOCATION, }; /** @@ -1466,6 +1475,7 @@ public static String flagsToString(@OpFlags int flags) { "LEGACY_STORAGE", "ACCESS_ACCESSIBILITY", "READ_DEVICE_IDENTIFIERS", + "ACCESS_MEDIA_LOCATION", }; /** @@ -1564,6 +1574,7 @@ public static String flagsToString(@OpFlags int flags) { null, // no permission for OP_LEGACY_STORAGE null, // no permission for OP_ACCESS_ACCESSIBILITY null, // no direct permission for OP_READ_DEVICE_IDENTIFIERS + Manifest.permission.ACCESS_MEDIA_LOCATION, }; /** @@ -1662,6 +1673,7 @@ public static String flagsToString(@OpFlags int flags) { null, // LEGACY_STORAGE null, // ACCESS_ACCESSIBILITY null, // READ_DEVICE_IDENTIFIERS + null, // ACCESS_MEDIA_LOCATION }; /** @@ -1759,6 +1771,7 @@ public static String flagsToString(@OpFlags int flags) { false, // LEGACY_STORAGE false, // ACCESS_ACCESSIBILITY false, // READ_DEVICE_IDENTIFIERS + false, // ACCESS_MEDIA_LOCATION }; /** @@ -1855,6 +1868,7 @@ public static String flagsToString(@OpFlags int flags) { AppOpsManager.MODE_DEFAULT, // LEGACY_STORAGE AppOpsManager.MODE_ALLOWED, // ACCESS_ACCESSIBILITY AppOpsManager.MODE_ERRORED, // READ_DEVICE_IDENTIFIERS + AppOpsManager.MODE_ALLOWED, // ALLOW_MEDIA_LOCATION }; /** @@ -1955,6 +1969,7 @@ public static String flagsToString(@OpFlags int flags) { false, // LEGACY_STORAGE false, // ACCESS_ACCESSIBILITY false, // READ_DEVICE_IDENTIFIERS + false, // ACCESS_MEDIA_LOCATION }; /** diff --git a/data/etc/platform.xml b/data/etc/platform.xml index 233f82640a203..65f784dbee832 100644 --- a/data/etc/platform.xml +++ b/data/etc/platform.xml @@ -205,6 +205,10 @@ targetSdk="29"> + + + diff --git a/services/core/java/com/android/server/pm/permission/DefaultPermissionGrantPolicy.java b/services/core/java/com/android/server/pm/permission/DefaultPermissionGrantPolicy.java index 4550446f88c5d..ecf66861a4014 100644 --- a/services/core/java/com/android/server/pm/permission/DefaultPermissionGrantPolicy.java +++ b/services/core/java/com/android/server/pm/permission/DefaultPermissionGrantPolicy.java @@ -190,6 +190,7 @@ public final class DefaultPermissionGrantPolicy { static { STORAGE_PERMISSIONS.add(Manifest.permission.READ_EXTERNAL_STORAGE); STORAGE_PERMISSIONS.add(Manifest.permission.WRITE_EXTERNAL_STORAGE); + STORAGE_PERMISSIONS.add(Manifest.permission.ACCESS_MEDIA_LOCATION); } private static final int MSG_READ_DEFAULT_PERMISSION_EXCEPTIONS = 1;