From a57db58f24569ed1788db9f006a80fac2f9f9a10 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Fri, 19 Mar 2021 10:38:59 -0600 Subject: [PATCH] Surface "neverForLocation" through public API. Since developers can declare the "neverForLocation" flag in their manifest as public API, we should also offer a way to inspect the value that we parsed from the manifest. We do this by surfacing it through the existing PackageInfo.requestedPermissionsFlags field. This also means we can remove the PackageManagerInternal API, since interested parties can now check PackageInfo directly. Fix a potential security issue by only accepting flags from manifest when the application is targeting a modern enough SDK. Bug: 181812281 Test: atest CtsContentTestCases:PackageManagerTest Test: atest FrameworksServicesTests:PackageParserTest Test: atest com.android.server.pm.parsing Change-Id: I877768c06ee15281f3334794034f4af563e74569 --- core/api/current.txt | 2 ++ core/java/android/content/pm/PackageInfo.java | 14 +++++++++-- .../parsing/PackageInfoWithoutStateUtils.java | 23 ++++++++++++++----- .../component/ParsedUsesPermission.java | 4 +++- core/res/res/values/attrs_manifest.xml | 5 ++-- core/res/res/values/public.xml | 1 + .../content/pm/PackageManagerInternal.java | 8 ------- .../server/pm/PackageManagerService.java | 23 ------------------- 8 files changed, 38 insertions(+), 42 deletions(-) diff --git a/core/api/current.txt b/core/api/current.txt index ba5f623960254..cb20f4b5e48a3 100644 --- a/core/api/current.txt +++ b/core/api/current.txt @@ -1581,6 +1581,7 @@ package android { field public static final int useLevel = 16843167; // 0x101019f field public static final int userVisible = 16843409; // 0x1010291 field public static final int usesCleartextTraffic = 16844012; // 0x10104ec + field public static final int usesPermissionFlags = 16844356; // 0x1010644 field public static final int value = 16842788; // 0x1010024 field public static final int valueFrom = 16843486; // 0x10102de field public static final int valueTo = 16843487; // 0x10102df @@ -12168,6 +12169,7 @@ package android.content.pm { field public static final int INSTALL_LOCATION_INTERNAL_ONLY = 1; // 0x1 field public static final int INSTALL_LOCATION_PREFER_EXTERNAL = 2; // 0x2 field public static final int REQUESTED_PERMISSION_GRANTED = 2; // 0x2 + field public static final int REQUESTED_PERMISSION_NEVER_FOR_LOCATION = 65536; // 0x10000 field public android.content.pm.ActivityInfo[] activities; field public android.content.pm.ApplicationInfo applicationInfo; field @Nullable public android.content.pm.Attribution[] attributions; diff --git a/core/java/android/content/pm/PackageInfo.java b/core/java/android/content/pm/PackageInfo.java index e8ef077fedc85..0462a4b2b6819 100644 --- a/core/java/android/content/pm/PackageInfo.java +++ b/core/java/android/content/pm/PackageInfo.java @@ -232,13 +232,23 @@ public class PackageInfo implements Parcelable { * * @removed We do not support required permissions. */ - public static final int REQUESTED_PERMISSION_REQUIRED = 1<<0; + public static final int REQUESTED_PERMISSION_REQUIRED = 0x00000001; /** * Flag for {@link #requestedPermissionsFlags}: the requested permission * is currently granted to the application. */ - public static final int REQUESTED_PERMISSION_GRANTED = 1<<1; + public static final int REQUESTED_PERMISSION_GRANTED = 0x00000002; + + /** + * Flag for {@link #requestedPermissionsFlags}: the requested permission has + * declared {@code neverForLocation} in their manifest as a strong assertion + * by a developer that they will never use this permission to derive the + * physical location of the device, regardless of + * {@link android.Manifest.permission#ACCESS_FINE_LOCATION} and/or + * {@link android.Manifest.permission#ACCESS_COARSE_LOCATION} being granted. + */ + public static final int REQUESTED_PERMISSION_NEVER_FOR_LOCATION = 0x00010000; /** * Array of all signatures read from the package file. This is only filled diff --git a/core/java/android/content/pm/parsing/PackageInfoWithoutStateUtils.java b/core/java/android/content/pm/parsing/PackageInfoWithoutStateUtils.java index fdd2c2ab83e38..e0052da9468bb 100644 --- a/core/java/android/content/pm/parsing/PackageInfoWithoutStateUtils.java +++ b/core/java/android/content/pm/parsing/PackageInfoWithoutStateUtils.java @@ -52,6 +52,7 @@ import android.content.pm.parsing.component.ParsedPermission; import android.content.pm.parsing.component.ParsedPermissionGroup; import android.content.pm.parsing.component.ParsedProvider; import android.content.pm.parsing.component.ParsedService; +import android.content.pm.parsing.component.ParsedUsesPermission; import android.os.Environment; import android.os.UserHandle; @@ -61,6 +62,7 @@ import libcore.util.EmptyArray; import java.io.File; import java.util.Collections; +import java.util.List; import java.util.Set; /** @hide **/ @@ -264,17 +266,26 @@ public class PackageInfoWithoutStateUtils { flags); } } - size = pkg.getRequestedPermissions().size(); + final List usesPermissions = pkg.getUsesPermissions(); + size = usesPermissions.size(); if (size > 0) { pi.requestedPermissions = new String[size]; pi.requestedPermissionsFlags = new int[size]; for (int i = 0; i < size; i++) { - final String perm = pkg.getRequestedPermissions().get(i); - pi.requestedPermissions[i] = perm; + final ParsedUsesPermission usesPermission = usesPermissions.get(i); + pi.requestedPermissions[i] = usesPermission.name; // The notion of required permissions is deprecated but for compatibility. - pi.requestedPermissionsFlags[i] |= PackageInfo.REQUESTED_PERMISSION_REQUIRED; - if (grantedPermissions != null && grantedPermissions.contains(perm)) { - pi.requestedPermissionsFlags[i] |= PackageInfo.REQUESTED_PERMISSION_GRANTED; + pi.requestedPermissionsFlags[i] |= + PackageInfo.REQUESTED_PERMISSION_REQUIRED; + if (grantedPermissions != null + && grantedPermissions.contains(usesPermission.name)) { + pi.requestedPermissionsFlags[i] |= + PackageInfo.REQUESTED_PERMISSION_GRANTED; + } + if ((usesPermission.usesPermissionFlags + & ParsedUsesPermission.FLAG_NEVER_FOR_LOCATION) != 0) { + pi.requestedPermissionsFlags[i] |= + PackageInfo.REQUESTED_PERMISSION_NEVER_FOR_LOCATION; } } } diff --git a/core/java/android/content/pm/parsing/component/ParsedUsesPermission.java b/core/java/android/content/pm/parsing/component/ParsedUsesPermission.java index b9c2e366c2d54..adf8da0fbe251 100644 --- a/core/java/android/content/pm/parsing/component/ParsedUsesPermission.java +++ b/core/java/android/content/pm/parsing/component/ParsedUsesPermission.java @@ -20,6 +20,7 @@ import static android.content.pm.parsing.ParsingPackageImpl.sForInternedString; import android.annotation.IntDef; import android.annotation.NonNull; +import android.content.pm.PackageInfo; import android.os.Parcel; import android.os.Parcelable; @@ -44,7 +45,8 @@ public class ParsedUsesPermission implements Parcelable { * to derive the physical location of the device, regardless of * ACCESS_FINE_LOCATION and/or ACCESS_COARSE_LOCATION being granted. */ - public static final int FLAG_NEVER_FOR_LOCATION = 0x1; + public static final int FLAG_NEVER_FOR_LOCATION = + PackageInfo.REQUESTED_PERMISSION_NEVER_FOR_LOCATION; /** @hide */ @Retention(RetentionPolicy.SOURCE) diff --git a/core/res/res/values/attrs_manifest.xml b/core/res/res/values/attrs_manifest.xml index 0318be788ce58..140163e476007 100644 --- a/core/res/res/values/attrs_manifest.xml +++ b/core/res/res/values/attrs_manifest.xml @@ -2063,13 +2063,14 @@ requested. If it does support the feature, it will be as if the manifest didn't request it at all. --> - + - + diff --git a/core/res/res/values/public.xml b/core/res/res/values/public.xml index 33cc89d2ab6e1..3e5fad8a34b4c 100644 --- a/core/res/res/values/public.xml +++ b/core/res/res/values/public.xml @@ -3091,6 +3091,7 @@ + diff --git a/services/core/java/android/content/pm/PackageManagerInternal.java b/services/core/java/android/content/pm/PackageManagerInternal.java index b089014b6e1ec..c2957780c9d78 100644 --- a/services/core/java/android/content/pm/PackageManagerInternal.java +++ b/services/core/java/android/content/pm/PackageManagerInternal.java @@ -1141,12 +1141,4 @@ public abstract class PackageManagerInternal { */ public abstract boolean isPackageFrozen( @NonNull String packageName, int callingUid, int userId); - - /** - * Returns true if the given {@code packageName} has declared the - * {@code neverForLocation} flag in the {@code uses-permission} manifest tag - * where they request the given {@code permissionName}. - */ - public abstract boolean isPackageUsesPermissionNeverForLocation(@NonNull String packageName, - @NonNull String permissionName); } diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index ff042c257f6ae..43eeb2a5eb572 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -27324,31 +27324,8 @@ public class PackageManagerService extends IPackageManager.Stub return PackageManagerService.this.getPackageStartability( packageName, callingUid, userId) == PACKAGE_STARTABILITY_FROZEN; } - - @Override - public boolean isPackageUsesPermissionNeverForLocation(@NonNull String packageName, - @NonNull String permissionName) { - Objects.requireNonNull(packageName); - Objects.requireNonNull(permissionName); - final AndroidPackage pkg; - synchronized (mLock) { - pkg = mPackages.get(packageName); - } - if (pkg == null) return false; - final List usesPermissions = pkg.getUsesPermissions(); - final int size = usesPermissions.size(); - for (int i = 0; i < size; i++) { - final ParsedUsesPermission usesPermission = usesPermissions.get(i); - if (Objects.equals(usesPermission.name, permissionName)) { - return (usesPermission.usesPermissionFlags - & ParsedUsesPermission.FLAG_NEVER_FOR_LOCATION) != 0; - } - } - return false; - } } - @GuardedBy("mLock") private SparseArray getAppsWithSharedUserIdsLocked() { final SparseArray sharedUserIds = new SparseArray<>();