From 74065c8903861004cefb5db7688f21737cf43980 Mon Sep 17 00:00:00 2001 From: "Philip P. Moltmann" Date: Wed, 15 May 2019 10:46:32 -0700 Subject: [PATCH] Apps can go from isolated to non-isolated storage But not the other way round. Before we used OP_LEGACY_STORAGE==default as marker that the op has not been set yet. Hence we ignore this condition when changing to "allow", but not when changing to "ignore". Test: atest RestrictedPermissionsTest Fixes: 131747080 Change-Id: I0cff8b66155cae69b77aed2d8f20fe43c5064a1f --- .../permission/PermissionManagerService.java | 23 ++++++++++++++ .../policy/PermissionPolicyService.java | 30 +++++++++++++++---- 2 files changed, 47 insertions(+), 6 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 2b7691b87de94..2a1aee8a441c1 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -1347,6 +1347,7 @@ public class PermissionManagerService { updatedUserIds); updatedUserIds = setInitialGrantForNewImplicitPermissionsLocked(origPermissions, permissionsState, pkg, newImplicitPermissions, updatedUserIds); + updatedUserIds = checkIfLegacyStorageOpsNeedToBeUpdated(pkg, replace, updatedUserIds); } // Persist the runtime permissions state for users with changes. If permissions @@ -1472,6 +1473,28 @@ public class PermissionManagerService { ps.updatePermissionFlags(mSettings.getPermission(newPerm), userId, flags, flags); } + /** + * When the app has requested legacy storage we might need to update + * {@link android.app.AppOpsManager#OP_LEGACY_STORAGE}. Hence force an update in + * {@link com.android.server.policy.PermissionPolicyService#synchronizePackagePermissionsAndAppOpsForUser(Context, String, int)} + * + * @param pkg The package for which the permissions are updated + * @param replace If the app is being replaced + * @param updatedUserIds The ids of the users that already changed. + * + * @return The ids of the users that are changed + */ + private @NonNull int[] checkIfLegacyStorageOpsNeedToBeUpdated( + @NonNull PackageParser.Package pkg, boolean replace, @NonNull int[] updatedUserIds) { + if (replace && pkg.applicationInfo.hasRequestedLegacyExternalStorage() && ( + pkg.requestedPermissions.contains(READ_EXTERNAL_STORAGE) + || pkg.requestedPermissions.contains(WRITE_EXTERNAL_STORAGE))) { + return UserManagerService.getInstance().getUserIds(); + } + + return updatedUserIds; + } + /** * Set the state of a implicit permission that is seen for the first time. * diff --git a/services/core/java/com/android/server/policy/PermissionPolicyService.java b/services/core/java/com/android/server/policy/PermissionPolicyService.java index 570a15310089b..475b54816aa14 100644 --- a/services/core/java/com/android/server/policy/PermissionPolicyService.java +++ b/services/core/java/com/android/server/policy/PermissionPolicyService.java @@ -229,6 +229,15 @@ public final class PermissionPolicyService extends SystemService { * * @see #syncRestrictedOps */ + private final @NonNull ArrayList mOpsToAllowIfDefault = new ArrayList<>(); + + /** + * All ops that need to be flipped to allow. + * + * Currently, only used by the restricted permissions logic. + * + * @see #syncRestrictedOps + */ private final @NonNull ArrayList mOpsToAllow = new ArrayList<>(); /** @@ -238,7 +247,7 @@ public final class PermissionPolicyService extends SystemService { * * @see #syncRestrictedOps */ - private final @NonNull ArrayList mOpsToIgnore = new ArrayList<>(); + private final @NonNull ArrayList mOpsToIgnoreIfDefault = new ArrayList<>(); /** * All foreground permissions @@ -262,11 +271,16 @@ public final class PermissionPolicyService extends SystemService { final int allowCount = mOpsToAllow.size(); for (int i = 0; i < allowCount; i++) { final OpToUnrestrict op = mOpsToAllow.get(i); + setUidModeAllowed(op.code, op.uid); + } + final int allowIfDefaultCount = mOpsToAllowIfDefault.size(); + for (int i = 0; i < allowIfDefaultCount; i++) { + final OpToUnrestrict op = mOpsToAllowIfDefault.get(i); setUidModeAllowedIfDefault(op.code, op.uid, op.packageName); } - final int ignoreCount = mOpsToIgnore.size(); - for (int i = 0; i < ignoreCount; i++) { - final OpToUnrestrict op = mOpsToIgnore.get(i); + final int ignoreIfDefaultCount = mOpsToIgnoreIfDefault.size(); + for (int i = 0; i < ignoreIfDefaultCount; i++) { + final OpToUnrestrict op = mOpsToIgnoreIfDefault.get(i); setUidModeIgnoredIfDefault(op.code, op.uid, op.packageName); } final int defaultCount = mOpsToDefault.size(); @@ -341,7 +355,7 @@ public final class PermissionPolicyService extends SystemService { if (applyRestriction) { mOpsToDefault.add(new OpToRestrict(uid, opCode)); } else { - mOpsToAllow.add(new OpToUnrestrict(uid, pkg.packageName, opCode)); + mOpsToAllowIfDefault.add(new OpToUnrestrict(uid, pkg.packageName, opCode)); } } else if (permissionInfo.isSoftRestricted()) { // Storage uses a special app op to decide the mount state and @@ -356,7 +370,7 @@ public final class PermissionPolicyService extends SystemService { mOpsToAllow.add(new OpToUnrestrict(uid, pkg.packageName, AppOpsManager.OP_LEGACY_STORAGE)); } else { - mOpsToIgnore.add(new OpToUnrestrict(uid, pkg.packageName, + mOpsToIgnoreIfDefault.add(new OpToUnrestrict(uid, pkg.packageName, AppOpsManager.OP_LEGACY_STORAGE)); } } @@ -421,6 +435,10 @@ public final class PermissionPolicyService extends SystemService { setUidModeIfDefault(opCode, uid, AppOpsManager.MODE_ALLOWED, packageName); } + private void setUidModeAllowed(int opCode, int uid) { + mAppOpsManager.setUidMode(opCode, uid, AppOpsManager.MODE_ALLOWED); + } + private void setUidModeIgnoredIfDefault(int opCode, int uid, @NonNull String packageName) { setUidModeIfDefault(opCode, uid, AppOpsManager.MODE_IGNORED, packageName); }