From 09080dc177288035c4694690a0f2dcd752acb3ba Mon Sep 17 00:00:00 2001 From: Nate Myren Date: Mon, 30 Nov 2020 12:06:26 -0800 Subject: [PATCH] Revoke storage on SDK downgrade or new full storage request When an app downgrades from above API 28 to below, or when an app begins requesting full storage in api 29, revoke storage permissions Test: Atest StorageEscalationTest Bug: 171430330 Merged-In: Ia6bd5658b77a26ab15fea013ec875050457473e0 Change-Id: Ia6bd5658b77a26ab15fea013ec875050457473e0 --- .../server/pm/PackageManagerService.java | 1 + .../permission/PermissionManagerService.java | 62 +++++++++++++++++++ .../PermissionManagerServiceInternal.java | 11 ++++ 3 files changed, 74 insertions(+) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index e8b2f3c428bce..bb7f17d4524a4 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -12486,6 +12486,7 @@ public class PackageManagerService extends IPackageManager.Stub if (hasOldPkg) { mPermissionManager.revokeRuntimePermissionsIfGroupChanged(pkg, oldPkg, allPackageNames); + mPermissionManager.revokeStoragePermissionsIfScopeExpanded(pkg, oldPkg); } if (hasPermissionDefinitionChanges) { mPermissionManager.revokeRuntimePermissionsIfPermissionDefinitionChanged( 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 d864cfb75d51a..7f0796fc99ac0 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -206,6 +206,9 @@ public class PermissionManagerService extends IPermissionManager.Stub { private static final int USER_PERMISSION_FLAGS = FLAG_PERMISSION_USER_SET | FLAG_PERMISSION_USER_FIXED; + /** All storage permissions */ + private static final List STORAGE_PERMISSIONS = new ArrayList<>(); + /** If the permission of the value is granted, so is the key */ private static final Map FULLER_PERMISSION_MAP = new HashMap<>(); @@ -214,6 +217,9 @@ public class PermissionManagerService extends IPermissionManager.Stub { Manifest.permission.ACCESS_FINE_LOCATION); FULLER_PERMISSION_MAP.put(Manifest.permission.INTERACT_ACROSS_USERS, Manifest.permission.INTERACT_ACROSS_USERS_FULL); + STORAGE_PERMISSIONS.add(Manifest.permission.READ_EXTERNAL_STORAGE); + STORAGE_PERMISSIONS.add(Manifest.permission.WRITE_EXTERNAL_STORAGE); + STORAGE_PERMISSIONS.add(Manifest.permission.ACCESS_MEDIA_LOCATION); } /** Lock to protect internal data access */ @@ -2265,6 +2271,49 @@ public class PermissionManagerService extends IPermissionManager.Stub { return protectionLevel; } + /** + * If the app is updated, and has scoped storage permissions, then it is possible that the + * app updated in an attempt to get unscoped storage. If so, revoke all storage permissions. + * @param newPackage The new package that was installed + * @param oldPackage The old package that was updated + */ + private void revokeStoragePermissionsIfScopeExpanded( + @NonNull AndroidPackage newPackage, + @NonNull AndroidPackage oldPackage, + @NonNull PermissionCallback permissionCallback) { + boolean downgradedSdk = oldPackage.getTargetSdkVersion() >= Build.VERSION_CODES.Q + && newPackage.getTargetSdkVersion() < Build.VERSION_CODES.Q; + boolean upgradedSdk = oldPackage.getTargetSdkVersion() < Build.VERSION_CODES.Q + && newPackage.getTargetSdkVersion() >= Build.VERSION_CODES.Q; + boolean newlyRequestsLegacy = !upgradedSdk && !oldPackage.isRequestLegacyExternalStorage() + && newPackage.isRequestLegacyExternalStorage(); + + if (!newlyRequestsLegacy && !downgradedSdk) { + return; + } + + final int callingUid = Binder.getCallingUid(); + final int userId = UserHandle.getUserId(newPackage.getUid()); + int numRequestedPermissions = newPackage.getRequestedPermissions().size(); + for (int i = 0; i < numRequestedPermissions; i++) { + PermissionInfo permInfo = getPermissionInfo(newPackage.getRequestedPermissions().get(i), + newPackage.getPackageName(), 0); + if (permInfo == null || !STORAGE_PERMISSIONS.contains(permInfo.name)) { + continue; + } + + EventLog.writeEvent(0x534e4554, "171430330", newPackage.getUid(), + "Revoking permission " + permInfo.name + " from package " + + newPackage.getPackageName() + " as either the sdk downgraded " + + downgradedSdk + " or newly requested legacy full storage " + + newlyRequestsLegacy); + + revokeRuntimePermissionInternal(permInfo.name, newPackage.getPackageName(), + false, callingUid, userId, null, permissionCallback); + } + + } + /** * We might auto-grant permissions if any permission of the group is already granted. Hence if * the group of a granted permission changes we need to revoke it to avoid having permissions of @@ -4726,6 +4775,19 @@ public class PermissionManagerService extends IPermissionManager.Stub { @UserIdInt int userId) { return PermissionManagerService.this.isPermissionsReviewRequired(pkg, userId); } + /** + * If the app is updated, and has scoped storage permissions, then it is possible that the + * app updated in an attempt to get unscoped storage. If so, revoke all storage permissions. + * @param newPackage The new package that was installed + * @param oldPackage The old package that was updated + */ + public void revokeStoragePermissionsIfScopeExpanded( + @NonNull AndroidPackage newPackage, + @NonNull AndroidPackage oldPackage + ) { + PermissionManagerService.this.revokeStoragePermissionsIfScopeExpanded(newPackage, + oldPackage, mDefaultPermissionCallback); + } @Override public void revokeRuntimePermissionsIfGroupChanged( diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java index 31a65ba35f5eb..df0edfa16924b 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java @@ -265,6 +265,17 @@ public abstract class PermissionManagerServiceInternal extends PermissionManager @NonNull List permissionsToRevoke, @NonNull ArrayList allPackageNames); + /** + * If the app is updated, and has scoped storage permissions, then it is possible that the + * app updated in an attempt to get unscoped storage. If so, revoke all storage permissions. + * @param newPackage The new package that was installed + * @param oldPackage The old package that was updated + */ + public abstract void revokeStoragePermissionsIfScopeExpanded( + @NonNull AndroidPackage newPackage, + @NonNull AndroidPackage oldPackage + ); + /** * Add all permissions in the given package. *