From 63ab14cb507d5e960f3ba82c0b83019ff4f4aba0 Mon Sep 17 00:00:00 2001 From: Hai Zhang Date: Mon, 12 Oct 2020 12:29:27 -0700 Subject: [PATCH] Refactor BasePermission.generatePermissionInfo(). Into one single method with target SDK version passed in, and move out group filtering logic to where it should be. Bug: 158736025 Test: presubmit Change-Id: Id0fcab76181692e4849d2b833a858097249f449b --- .../server/pm/permission/BasePermission.java | 51 ++++++++-------- .../permission/PermissionManagerService.java | 60 +++++++------------ 2 files changed, 49 insertions(+), 62 deletions(-) diff --git a/services/core/java/com/android/server/pm/permission/BasePermission.java b/services/core/java/com/android/server/pm/permission/BasePermission.java index c086017401581..fedbed2f7a01a 100644 --- a/services/core/java/com/android/server/pm/permission/BasePermission.java +++ b/services/core/java/com/android/server/pm/permission/BasePermission.java @@ -32,6 +32,7 @@ import android.annotation.Nullable; import android.content.pm.PackageManagerInternal; import android.content.pm.PermissionInfo; import android.content.pm.parsing.component.ParsedPermission; +import android.os.Build; import android.os.UserHandle; import android.util.Log; import android.util.Slog; @@ -445,36 +446,38 @@ public final class BasePermission { return null; } - public @Nullable PermissionInfo generatePermissionInfo(@NonNull String groupName, int flags) { - if (groupName == null) { - if (perm == null || perm.getGroup() == null) { - return generatePermissionInfo(protectionLevel, flags); - } - } else { - if (perm != null && groupName.equals(perm.getGroup())) { - return PackageInfoUtils.generatePermissionInfo(perm, flags); - } - } - return null; + @Nullable + public String getGroup() { + return perm != null ? perm.getGroup() : null; } - public @NonNull PermissionInfo generatePermissionInfo(int adjustedProtectionLevel, int flags) { + @NonNull + public PermissionInfo generatePermissionInfo(int flags) { + return generatePermissionInfo(flags, Build.VERSION_CODES.CUR_DEVELOPMENT); + } + + @NonNull + public PermissionInfo generatePermissionInfo(int flags, int targetSdkVersion) { PermissionInfo permissionInfo; if (perm != null) { - final boolean protectionLevelChanged = protectionLevel != adjustedProtectionLevel; permissionInfo = PackageInfoUtils.generatePermissionInfo(perm, flags); - if (protectionLevelChanged) { - // if we return different protection level, don't use the cached info - permissionInfo = new PermissionInfo(permissionInfo); - permissionInfo.protectionLevel = adjustedProtectionLevel; - } - return permissionInfo; + } else { + permissionInfo = new PermissionInfo(); + permissionInfo.name = name; + permissionInfo.packageName = sourcePackageName; + permissionInfo.nonLocalizedLabel = name; + } + if (targetSdkVersion >= Build.VERSION_CODES.O) { + permissionInfo.protectionLevel = protectionLevel; + } else { + final int protection = protectionLevel & PermissionInfo.PROTECTION_MASK_BASE; + if (protection == PermissionInfo.PROTECTION_SIGNATURE) { + // Signature permission's protection flags are always reported. + permissionInfo.protectionLevel = protectionLevel; + } else { + permissionInfo.protectionLevel = protection; + } } - permissionInfo = new PermissionInfo(); - permissionInfo.name = name; - permissionInfo.packageName = sourcePackageName; - permissionInfo.nonLocalizedLabel = name; - permissionInfo.protectionLevel = protectionLevel; return permissionInfo; } 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 25e1848816d17..45872a833469a 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -544,24 +544,37 @@ public class PermissionManagerService extends IPermissionManager.Stub { @Override @Nullable - public PermissionInfo getPermissionInfo(String permName, String packageName, + public PermissionInfo getPermissionInfo(@NonNull String permName, @NonNull String opPackageName, @PermissionInfoFlags int flags) { final int callingUid = getCallingUid(); if (mPackageManagerInt.getInstantAppPackageName(callingUid) != null) { return null; } - final AndroidPackage pkg = mPackageManagerInt.getPackage(packageName); + final AndroidPackage opPackage = mPackageManagerInt.getPackage(opPackageName); + final int targetSdkVersion = getPermissionInfoCallingTargetSdkVersion(opPackage, + callingUid); synchronized (mLock) { final BasePermission bp = mSettings.getPermissionLocked(permName); if (bp == null) { return null; } - final int adjustedProtectionLevel = adjustPermissionProtectionFlagsLocked( - bp.getProtectionLevel(), pkg, callingUid); - return bp.generatePermissionInfo(adjustedProtectionLevel, flags); + return bp.generatePermissionInfo(flags, targetSdkVersion); } } + private int getPermissionInfoCallingTargetSdkVersion(@Nullable AndroidPackage pkg, int uid) { + final int appId = UserHandle.getAppId(uid); + if (appId == Process.ROOT_UID || appId == Process.SYSTEM_UID + || appId == Process.SHELL_UID) { + // System sees all flags. + return Build.VERSION_CODES.CUR_DEVELOPMENT; + } + if (pkg == null) { + return Build.VERSION_CODES.CUR_DEVELOPMENT; + } + return pkg.getTargetSdkVersion(); + } + @Override @Nullable public ParceledListSlice queryPermissionsByGroup(String groupName, @@ -576,9 +589,8 @@ public class PermissionManagerService extends IPermissionManager.Stub { } final ArrayList out = new ArrayList(10); for (BasePermission bp : mSettings.mPermissions.values()) { - final PermissionInfo pi = bp.generatePermissionInfo(groupName, flags); - if (pi != null) { - out.add(pi); + if (Objects.equals(bp.getGroup(), groupName)) { + out.add(bp.generatePermissionInfo(flags)); } } return new ParceledListSlice<>(out); @@ -2235,32 +2247,6 @@ public class PermissionManagerService extends IPermissionManager.Stub { } } - private int adjustPermissionProtectionFlagsLocked(int protectionLevel, - @Nullable AndroidPackage pkg, int uid) { - // Signature permission flags area always reported - final int protectionLevelMasked = protectionLevel - & (PermissionInfo.PROTECTION_NORMAL - | PermissionInfo.PROTECTION_DANGEROUS - | PermissionInfo.PROTECTION_SIGNATURE); - if (protectionLevelMasked == PermissionInfo.PROTECTION_SIGNATURE) { - return protectionLevel; - } - // System sees all flags. - final int appId = UserHandle.getAppId(uid); - if (appId == Process.SYSTEM_UID || appId == Process.ROOT_UID - || appId == Process.SHELL_UID) { - return protectionLevel; - } - if (pkg == null) { - return protectionLevel; - } - if (pkg.getTargetSdkVersion() < Build.VERSION_CODES.O) { - return protectionLevelMasked; - } - // Apps that target O see flags for all protection levels. - return protectionLevel; - } - /** * 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 @@ -4903,8 +4889,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { BasePermission bp = mSettings.mPermissions.valueAt(i); if (bp.perm != null && bp.perm.getProtection() == protection) { - matchingPermissions.add( - PackageInfoUtils.generatePermissionInfo(bp.perm, 0)); + matchingPermissions.add(bp.generatePermissionInfo(0)); } } } @@ -4925,8 +4910,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { if (bp.perm != null && (bp.perm.getProtectionFlags() & protectionFlags) == protectionFlags) { - matchingPermissions.add( - PackageInfoUtils.generatePermissionInfo(bp.perm, 0)); + matchingPermissions.add(bp.generatePermissionInfo(0)); } } }