From 9408f5804de69cc6da5108f7c548b47f3dc32564 Mon Sep 17 00:00:00 2001 From: "Philip P. Moltmann" Date: Wed, 10 Apr 2019 16:58:24 -0700 Subject: [PATCH] Use PermissionPolicySve for fg perm app-ops This avoids having to call into app-ops manager while holding the package manager lock Fixes: 124731615, 123436301 Test: atest CtsPermissionTestCases:android.permission.cts.SplitPermissionTest Change-Id: Ie7d5302b4265ed012e2303f3c6e6c7219ff3afa9 --- .../permission/PermissionManagerService.java | 193 +++--------------- .../policy/PermissionPolicyService.java | 71 +++++++ 2 files changed, 96 insertions(+), 168 deletions(-) diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java index c75a462d5bc15..dd63e3ca290e0 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -18,12 +18,6 @@ package com.android.server.pm.permission; import static android.Manifest.permission.READ_EXTERNAL_STORAGE; import static android.Manifest.permission.WRITE_EXTERNAL_STORAGE; -import static android.app.AppOpsManager.MODE_ALLOWED; -import static android.app.AppOpsManager.MODE_ERRORED; -import static android.app.AppOpsManager.MODE_FOREGROUND; -import static android.app.AppOpsManager.OP_NONE; -import static android.app.AppOpsManager.permissionToOp; -import static android.app.AppOpsManager.permissionToOpCode; import static android.content.pm.PackageManager.FLAGS_PERMISSION_RESTRICTION_ANY_EXEMPT; import static android.content.pm.PackageManager.FLAG_PERMISSION_APPLY_RESTRICTION; import static android.content.pm.PackageManager.FLAG_PERMISSION_GRANTED_BY_DEFAULT; @@ -40,8 +34,6 @@ import static android.content.pm.PackageManager.FLAG_PERMISSION_WHITELIST_UPGRAD import static android.content.pm.PackageManager.MASK_PERMISSION_FLAGS_ALL; import static android.content.pm.PackageManager.RESTRICTED_PERMISSIONS_ENABLED; import static android.os.Trace.TRACE_TAG_PACKAGE_MANAGER; -import static android.os.UserHandle.getAppId; -import static android.os.UserHandle.getUid; import static com.android.server.pm.PackageManagerService.DEBUG_INSTALL; import static com.android.server.pm.PackageManagerService.DEBUG_PACKAGE_SCANNING; @@ -56,8 +48,6 @@ import android.Manifest; import android.annotation.NonNull; import android.annotation.Nullable; import android.annotation.UserIdInt; -import android.app.AppOpsManager; -import android.app.AppOpsManagerInternal; import android.content.Context; import android.content.pm.PackageManager; import android.content.pm.PackageManager.PermissionWhitelistFlags; @@ -1309,9 +1299,6 @@ public class PermissionManagerService { updatedUserIds); updatedUserIds = setInitialGrantForNewImplicitPermissionsLocked(origPermissions, permissionsState, pkg, updatedUserIds); - - // TODO: Move to PermissionPolicyService - setAppOpsLocked(permissionsState, pkg); } // Persist the runtime permissions state for users with changes. If permissions @@ -1326,23 +1313,6 @@ public class PermissionManagerService { } } - /** - * Set app op for a app-op related to a permission. - * - * @param permission The permission the app-op belongs to - * @param pkg The package the permission belongs to - * @param userId The user to be changed - * @param mode The new mode to set - */ - private void setAppOpMode(@NonNull String permission, @NonNull PackageParser.Package pkg, - @UserIdInt int userId, int mode) { - AppOpsManagerInternal appOpsInternal = LocalServices.getService( - AppOpsManagerInternal.class); - - appOpsInternal.setUidMode(permissionToOpCode(permission), - getUid(userId, getAppId(pkg.applicationInfo.uid)), mode); - } - /** * Revoke permissions that are not implicit anymore and that have * {@link PackageManager#FLAG_PERMISSION_REVOKE_WHEN_REQUESTED} set. @@ -1357,8 +1327,6 @@ public class PermissionManagerService { private @NonNull int[] revokePermissionsNoLongerImplicitLocked( @NonNull PermissionsState ps, @NonNull PackageParser.Package pkg, @NonNull int[] updatedUserIds) { - AppOpsManager appOpsManager = mContext.getSystemService(AppOpsManager.class); - String pkgName = pkg.packageName; boolean supportsRuntimePermissions = pkg.applicationInfo.targetSdkVersion >= Build.VERSION_CODES.M; @@ -1390,23 +1358,6 @@ public class PermissionManagerService { } flagsToRemove |= USER_PERMISSION_FLAGS; - - List fgPerms = mBackgroundPermissions.get(permission); - if (fgPerms != null) { - int numFgPerms = fgPerms.size(); - for (int fgPermNum = 0; fgPermNum < numFgPerms; fgPermNum++) { - String fgPerm = fgPerms.get(fgPermNum); - - int mode = appOpsManager.unsafeCheckOpRaw( - permissionToOp(fgPerm), - getUid(userId, getAppId(pkg.applicationInfo.uid)), - pkgName); - - if (mode == MODE_ALLOWED) { - setAppOpMode(fgPerm, pkg, userId, MODE_FOREGROUND); - } - } - } } ps.updatePermissionFlags(bp, userId, flagsToRemove, 0); @@ -1438,91 +1389,39 @@ public class PermissionManagerService { @NonNull ArraySet sourcePerms, @NonNull String newPerm, @NonNull PermissionsState ps, @NonNull PackageParser.Package pkg, @UserIdInt int userId) { - AppOpsManagerInternal appOpsManager = LocalServices.getService(AppOpsManagerInternal.class); String pkgName = pkg.packageName; + boolean isGranted = false; + int flags = 0; - if (pkg.applicationInfo.targetSdkVersion < Build.VERSION_CODES.M) { - if (permissionToOp(newPerm) != null) { - int mostLenientSourceMode = MODE_ERRORED; - int flags = 0; - - // Find most lenient source permission state. - int numSourcePerms = sourcePerms.size(); - for (int i = 0; i < numSourcePerms; i++) { - String sourcePerm = sourcePerms.valueAt(i); - - if (ps.hasRuntimePermission(sourcePerm, userId)) { - int sourceOp = permissionToOpCode(sourcePerm); - - if (sourceOp != OP_NONE) { - int mode = appOpsManager.checkOperationUnchecked(sourceOp, - getUid(userId, getAppId(pkg.applicationInfo.uid)), pkgName); - - if (mode == MODE_FOREGROUND || mode == MODE_ERRORED) { - Log.wtf(TAG, "split permission" + sourcePerm + " has app-op state " - + AppOpsManager.MODE_NAMES[mode]); - - continue; - } - - // Leniency order: allowed < ignored < default - if (mode < mostLenientSourceMode) { - mostLenientSourceMode = mode; - flags = ps.getPermissionFlags(sourcePerm, userId); - } else if (mode == mostLenientSourceMode) { - flags |= ps.getPermissionFlags(sourcePerm, userId); - } - } - } + int numSourcePerm = sourcePerms.size(); + for (int i = 0; i < numSourcePerm; i++) { + String sourcePerm = sourcePerms.valueAt(i); + if ((ps.hasRuntimePermission(sourcePerm, userId)) + || ps.hasInstallPermission(sourcePerm)) { + if (!isGranted) { + flags = 0; } - if (mostLenientSourceMode != MODE_ERRORED) { - if (DEBUG_PERMISSIONS) { - Slog.i(TAG, newPerm + " inherits app-ops state " + mostLenientSourceMode - + " from " + sourcePerms + " for " + pkgName); - } - - setAppOpMode(newPerm, pkg, userId, mostLenientSourceMode); - - // Add permission flags - ps.updatePermissionFlags(mSettings.getPermission(newPerm), userId, flags, - flags); - } - } - } else { - boolean isGranted = false; - int flags = 0; - - int numSourcePerm = sourcePerms.size(); - for (int i = 0; i < numSourcePerm; i++) { - String sourcePerm = sourcePerms.valueAt(i); - if ((ps.hasRuntimePermission(sourcePerm, userId)) - || ps.hasInstallPermission(sourcePerm)) { - if (!isGranted) { - flags = 0; - } - - isGranted = true; + isGranted = true; + flags |= ps.getPermissionFlags(sourcePerm, userId); + } else { + if (!isGranted) { flags |= ps.getPermissionFlags(sourcePerm, userId); - } else { - if (!isGranted) { - flags |= ps.getPermissionFlags(sourcePerm, userId); - } } } - - if (isGranted) { - if (DEBUG_PERMISSIONS) { - Slog.i(TAG, newPerm + " inherits runtime perm grant from " + sourcePerms - + " for " + pkgName); - } - - ps.grantRuntimePermission(mSettings.getPermissionLocked(newPerm), userId); - } - - // Add permission flags - ps.updatePermissionFlags(mSettings.getPermission(newPerm), userId, flags, flags); } + + if (isGranted) { + if (DEBUG_PERMISSIONS) { + Slog.i(TAG, newPerm + " inherits runtime perm grant from " + sourcePerms + + " for " + pkgName); + } + + ps.grantRuntimePermission(mSettings.getPermissionLocked(newPerm), userId); + } + + // Add permission flags + ps.updatePermissionFlags(mSettings.getPermission(newPerm), userId, flags, flags); } /** @@ -1632,48 +1531,6 @@ public class PermissionManagerService { return updatedUserIds; } - /** - * Fix app-op modes for runtime permissions. - * - * @param permsState The state of the permissions of the package - * @param pkg The package information - */ - private void setAppOpsLocked(@NonNull PermissionsState permsState, - @NonNull PackageParser.Package pkg) { - for (int userId : UserManagerService.getInstance().getUserIds()) { - int numPerms = pkg.requestedPermissions.size(); - for (int i = 0; i < numPerms; i++) { - String permission = pkg.requestedPermissions.get(i); - - // For pre-M apps the runtime permission do not store the state - if (pkg.applicationInfo.targetSdkVersion < Build.VERSION_CODES.M) { - continue; - } - - PermissionState state = permsState.getRuntimePermissionState(permission, userId); - if (state == null) { - continue; - } - - // Adjust app-op mods for foreground/background permissions. If an package used to - // have both fg and bg permission granted and it lost the bg permission during an - // upgrade the app-op mode should get downgraded to foreground. - if (state.isGranted()) { - BasePermission bp = mSettings.getPermission(permission); - - if (bp != null && bp.perm != null && bp.perm.info != null - && bp.perm.info.backgroundPermission != null) { - PermissionState bgState = permsState.getRuntimePermissionState( - bp.perm.info.backgroundPermission, userId); - - setAppOpMode(permission, pkg, userId, bgState != null && bgState.isGranted() - ? MODE_ALLOWED : MODE_FOREGROUND); - } - } - } - } - } - private boolean isNewPlatformPermissionForPackage(String perm, PackageParser.Package pkg) { boolean allowed = false; final int NP = PackageParser.NEW_PERMISSIONS.length; diff --git a/services/core/java/com/android/server/policy/PermissionPolicyService.java b/services/core/java/com/android/server/policy/PermissionPolicyService.java index 3bdcbab869ae0..1fd8b711d3482 100644 --- a/services/core/java/com/android/server/policy/PermissionPolicyService.java +++ b/services/core/java/com/android/server/policy/PermissionPolicyService.java @@ -31,6 +31,7 @@ import android.content.pm.PackageManagerInternal; import android.content.pm.PackageManagerInternal.PackageListObserver; import android.content.pm.PackageParser; import android.content.pm.PermissionInfo; +import android.os.Build; import android.os.Process; import android.os.UserHandle; import android.permission.PermissionControllerManager; @@ -234,6 +235,7 @@ public final class PermissionPolicyService extends SystemService { * @see #syncRestrictedOps */ private final @NonNull ArrayList mOpsToRestrict = new ArrayList<>(); + /** * All ops that need to be unrestricted * @@ -241,6 +243,13 @@ public final class PermissionPolicyService extends SystemService { */ private final @NonNull ArrayList mOpsToUnrestrict = new ArrayList<>(); + /** + * All foreground permissions + * + * @see #syncOpsOfFgPermissions() + */ + private final @NonNull ArrayList mFgPermOps = new ArrayList<>(); + PermissionToOpSynchroniser(@NonNull Context context) { mContext = context; mPackageManager = context.getPackageManager(); @@ -339,11 +348,42 @@ public final class PermissionPolicyService extends SystemService { } } + /** + * Set app ops that belong to restricted permissions. + * + *

This processed ops previously added by {@link #addOpIfRestricted} + */ + private void syncOpsOfFgPermissions() { + int numFgPermOps = mFgPermOps.size(); + for (int i = 0; i < numFgPermOps; i++) { + FgPermission perm = mFgPermOps.get(i); + + if (mPackageManager.checkPermission(perm.fgPermissionName, perm.packageName) + == PackageManager.PERMISSION_GRANTED) { + if (mPackageManager.checkPermission(perm.bgPermissionName, perm.packageName) + == PackageManager.PERMISSION_GRANTED) { + mAppOpsManager.setUidMode( + AppOpsManager.permissionToOpCode(perm.fgPermissionName), perm.uid, + AppOpsManager.MODE_ALLOWED); + } else { + mAppOpsManager.setUidMode( + AppOpsManager.permissionToOpCode(perm.fgPermissionName), perm.uid, + AppOpsManager.MODE_FOREGROUND); + } + } else { + mAppOpsManager.setUidMode( + AppOpsManager.permissionToOpCode(perm.fgPermissionName), perm.uid, + AppOpsManager.MODE_IGNORED); + } + } + } + /** * Synchronize all previously {@link #addPackage added} packages. */ void syncPackages() { syncRestrictedOps(); + syncOpsOfFgPermissions(); } /** @@ -380,6 +420,21 @@ public final class PermissionPolicyService extends SystemService { } } + private void addOpIfFgPermissions(@NonNull PermissionInfo permissionInfo, + @NonNull PackageInfo pkg) { + if (pkg.applicationInfo.targetSdkVersion < Build.VERSION_CODES.M) { + // Pre-M apps do not store their fg/bg state in the permissions + return; + } + + if (permissionInfo.backgroundPermission == null) { + return; + } + + mFgPermOps.add(new FgPermission(pkg.applicationInfo.uid, pkg.packageName, + permissionInfo.name, permissionInfo.backgroundPermission)); + } + /** * Add a package for {@link #syncPackages() processing} later. * @@ -415,6 +470,7 @@ public final class PermissionPolicyService extends SystemService { } addOpIfRestricted(permissionInfo, pkg); + addOpIfFgPermissions(permissionInfo, pkg); } } @@ -451,5 +507,20 @@ public final class PermissionPolicyService extends SystemService { this.code = code; } } + + private class FgPermission { + final int uid; + final @NonNull String packageName; + final @NonNull String fgPermissionName; + final @NonNull String bgPermissionName; + + private FgPermission(int uid, @NonNull String packageName, + @NonNull String fgPermissionName, @NonNull String bgPermissionName) { + this.uid = uid; + this.packageName = packageName; + this.fgPermissionName = fgPermissionName; + this.bgPermissionName = bgPermissionName; + } + } } }