From bb8fb09eb94c58396ca3cfaeefe6d82a7ac7544b Mon Sep 17 00:00:00 2001 From: Jackal Guo Date: Mon, 26 Apr 2021 17:45:15 +0800 Subject: [PATCH] Refactor PackageParser (11/n) Replace PackageParser.NewPermissionInfo by the independent one in order to move away from PackageParser. Bug: 174723245 Test: atest -p core/java/android/content/pm/parsing \ services/core/java/com/android/server/pm/permission \ services/tests/servicestests/src/com/android/server/pm/permission Change-Id: I2398ee87516c813a86f5e9308ae43434b841fae5 --- .../pm/parsing/ParsingPackageImpl.java | 1 + .../pm/parsing/ParsingPackageUtils.java | 33 +++-------- .../CompatibilityPermissionInfo.java | 57 +++++++++++++++++++ .../permission/PermissionManagerService.java | 16 +++--- services/tests/servicestests/Android.bp | 1 + .../android/server/pm/PackageParserTest.java | 57 +++++++++++++++++++ .../test-apps/PackageParserApp/Android.bp | 14 +++++ .../PackageParserApp/AndroidManifestApp5.xml | 28 +++++++++ 8 files changed, 174 insertions(+), 33 deletions(-) create mode 100644 core/java/android/content/pm/permission/CompatibilityPermissionInfo.java create mode 100644 services/tests/servicestests/test-apps/PackageParserApp/AndroidManifestApp5.xml diff --git a/core/java/android/content/pm/parsing/ParsingPackageImpl.java b/core/java/android/content/pm/parsing/ParsingPackageImpl.java index 3cf59a04dde04..0da66140c22aa 100644 --- a/core/java/android/content/pm/parsing/ParsingPackageImpl.java +++ b/core/java/android/content/pm/parsing/ParsingPackageImpl.java @@ -723,6 +723,7 @@ public class ParsingPackageImpl implements ParsingPackage, Parcelable { @Override public ParsingPackageImpl addImplicitPermission(String permission) { + addUsesPermission(new ParsedUsesPermission(permission, 0 /*usesPermissionFlags*/)); this.implicitPermissions = CollectionUtils.add(this.implicitPermissions, TextUtils.safeIntern(permission)); return this; diff --git a/core/java/android/content/pm/parsing/ParsingPackageUtils.java b/core/java/android/content/pm/parsing/ParsingPackageUtils.java index 0ef85834e7880..41d8ee6642806 100644 --- a/core/java/android/content/pm/parsing/ParsingPackageUtils.java +++ b/core/java/android/content/pm/parsing/ParsingPackageUtils.java @@ -74,6 +74,7 @@ import android.content.pm.parsing.result.ParseInput; import android.content.pm.parsing.result.ParseInput.DeferredError; import android.content.pm.parsing.result.ParseResult; import android.content.pm.parsing.result.ParseTypeImpl; +import android.content.pm.permission.CompatibilityPermissionInfo; import android.content.pm.split.DefaultSplitAssetLoader; import android.content.pm.split.SplitAssetDependencyLoader; import android.content.pm.split.SplitAssetLoader; @@ -904,7 +905,7 @@ public class ParsingPackageUtils { ); } - convertNewPermissions(pkg); + convertCompatPermissions(pkg); convertSplitPermissions(pkg); @@ -2790,31 +2791,16 @@ public class ParsingPackageUtils { } } - private static void convertNewPermissions(ParsingPackage pkg) { - final int NP = PackageParser.NEW_PERMISSIONS.length; - StringBuilder newPermsMsg = null; - for (int ip = 0; ip < NP; ip++) { - final PackageParser.NewPermissionInfo npi - = PackageParser.NEW_PERMISSIONS[ip]; - if (pkg.getTargetSdkVersion() >= npi.sdkVersion) { + private static void convertCompatPermissions(ParsingPackage pkg) { + for (int i = 0, size = CompatibilityPermissionInfo.COMPAT_PERMS.length; i < size; i++) { + final CompatibilityPermissionInfo info = CompatibilityPermissionInfo.COMPAT_PERMS[i]; + if (pkg.getTargetSdkVersion() >= info.sdkVersion) { break; } - if (!pkg.getRequestedPermissions().contains(npi.name)) { - if (newPermsMsg == null) { - newPermsMsg = new StringBuilder(128); - newPermsMsg.append(pkg.getPackageName()); - newPermsMsg.append(": compat added "); - } else { - newPermsMsg.append(' '); - } - newPermsMsg.append(npi.name); - pkg.addUsesPermission(new ParsedUsesPermission(npi.name, 0)) - .addImplicitPermission(npi.name); + if (!pkg.getRequestedPermissions().contains(info.name)) { + pkg.addImplicitPermission(info.name); } } - if (newPermsMsg != null) { - Slog.i(TAG, newPermsMsg.toString()); - } } private void convertSplitPermissions(ParsingPackage pkg) { @@ -2830,8 +2816,7 @@ public class ParsingPackageUtils { for (int in = 0; in < newPerms.size(); in++) { final String perm = newPerms.get(in); if (!requestedPermissions.contains(perm)) { - pkg.addUsesPermission(new ParsedUsesPermission(perm, 0)) - .addImplicitPermission(perm); + pkg.addImplicitPermission(perm); } } } diff --git a/core/java/android/content/pm/permission/CompatibilityPermissionInfo.java b/core/java/android/content/pm/permission/CompatibilityPermissionInfo.java new file mode 100644 index 0000000000000..9198b95a6f71b --- /dev/null +++ b/core/java/android/content/pm/permission/CompatibilityPermissionInfo.java @@ -0,0 +1,57 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.content.pm.permission; + +import android.Manifest; +import android.content.pm.parsing.component.ParsedUsesPermission; + +/** + * Implements compatibility support for permissions, and old applications + * will be automatically granted it. + * + * Compatibility permissions are permissions that are automatically granted to + * packages that target an SDK prior to when the permission was introduced. + * Sometimes the platform makes breaking behaviour changes and hides the legacy + * behaviour behind a permission. In these instances, we ensure applications + * targeting older platform versions are implicitly granted the correct set of + * permissions. + * + * @hide + */ +public class CompatibilityPermissionInfo extends ParsedUsesPermission { + + public final int sdkVersion; + + /** + * List of new permissions that have been added since 1.0. + * + * @hide + */ + public static final CompatibilityPermissionInfo[] COMPAT_PERMS = + new CompatibilityPermissionInfo[]{ + new CompatibilityPermissionInfo(Manifest.permission.WRITE_EXTERNAL_STORAGE, + android.os.Build.VERSION_CODES.DONUT, 0 /*usesPermissionFlags*/), + new CompatibilityPermissionInfo(Manifest.permission.READ_PHONE_STATE, + android.os.Build.VERSION_CODES.DONUT, 0 /*usesPermissionFlags*/) + }; + + private CompatibilityPermissionInfo(String name, int sdkVersion, + @UsesPermissionFlags int usesPermissionFlags) { + super(name, usesPermissionFlags); + this.sdkVersion = sdkVersion; + } +} 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 5523511193938..57c0ecd8525f4 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -80,13 +80,13 @@ import android.content.pm.PackageManager.PermissionGroupInfoFlags; import android.content.pm.PackageManager.PermissionInfoFlags; import android.content.pm.PackageManager.PermissionWhitelistFlags; import android.content.pm.PackageManagerInternal; -import android.content.pm.PackageParser; import android.content.pm.ParceledListSlice; import android.content.pm.PermissionGroupInfo; import android.content.pm.PermissionInfo; import android.content.pm.SigningDetails; import android.content.pm.parsing.component.ParsedPermission; import android.content.pm.parsing.component.ParsedPermissionGroup; +import android.content.pm.permission.CompatibilityPermissionInfo; import android.content.pm.permission.SplitPermissionInfoParcelable; import android.metrics.LogMaker; import android.os.AsyncTask; @@ -2818,7 +2818,7 @@ public class PermissionManagerService extends IPermissionManager.Stub { // Except... if this is a permission that was added // to the platform (note: need to only do this when // updating the platform). - if (!isNewPlatformPermissionForPackage(perm, pkg)) { + if (!isCompatPlatformPermissionForPackage(perm, pkg)) { shouldGrantNormalPermission = false; } } @@ -3354,14 +3354,12 @@ public class PermissionManagerService extends IPermissionManager.Stub { return result; } - private static boolean isNewPlatformPermissionForPackage(String perm, AndroidPackage pkg) { + private static boolean isCompatPlatformPermissionForPackage(String perm, AndroidPackage pkg) { boolean allowed = false; - final int NP = PackageParser.NEW_PERMISSIONS.length; - for (int ip=0; ip compatPermissions = + Arrays.stream(COMPAT_PERMS).map(p -> p.name).collect(toList()); + assertWithMessage( + "Compatibility permissions shouldn't be added into uses permissions.") + .that(pkg.getUsesPermissions().stream().map(p -> p.name).collect(toList())) + .containsNoneIn(compatPermissions); + assertWithMessage( + "Compatibility permissions shouldn't be added into requested permissions.") + .that(pkg.getRequestedPermissions()).containsNoneIn(compatPermissions); + assertWithMessage( + "Compatibility permissions shouldn't be added into implicit permissions.") + .that(pkg.getImplicitPermissions()).containsNoneIn(compatPermissions); + } finally { + testFile.delete(); + } + } + + @Test + public void testParseLegacyPackageHasCompatPermissions() throws Exception { + final File testFile = extractFile(TEST_APP5_APK); + try { + final ParsedPackage pkg = new TestPackageParser2() + .parsePackage(testFile, 0 /*flags*/, false /*useCaches*/); + assertWithMessage( + "Compatibility permissions should be added into uses permissions.") + .that(Arrays.stream(COMPAT_PERMS).map(p -> p.name) + .allMatch(pkg.getUsesPermissions().stream().map(p -> p.name) + .collect(toList())::contains)) + .isTrue(); + assertWithMessage( + "Compatibility permissions should be added into requested permissions.") + .that(Arrays.stream(COMPAT_PERMS).map(p -> p.name) + .allMatch(pkg.getRequestedPermissions()::contains)) + .isTrue(); + assertWithMessage( + "Compatibility permissions should be added into implicit permissions.") + .that(Arrays.stream(COMPAT_PERMS).map(p -> p.name) + .allMatch(pkg.getImplicitPermissions()::contains)) + .isTrue(); + } finally { + testFile.delete(); + } + } + /** * A trivial subclass of package parser that only caches the package name, and throws away * all other information. diff --git a/services/tests/servicestests/test-apps/PackageParserApp/Android.bp b/services/tests/servicestests/test-apps/PackageParserApp/Android.bp index b11c85c008ad1..c611e38b068aa 100644 --- a/services/tests/servicestests/test-apps/PackageParserApp/Android.bp +++ b/services/tests/servicestests/test-apps/PackageParserApp/Android.bp @@ -74,3 +74,17 @@ android_test_helper_app { resource_dirs: ["res"], manifest: "AndroidManifestApp4.xml", } + +android_test_helper_app { + name: "PackageParserTestApp5", + sdk_version: "current", + srcs: ["**/*.java"], + dex_preopt: { + enabled: false, + }, + optimize: { + enabled: false, + }, + resource_dirs: ["res"], + manifest: "AndroidManifestApp5.xml", +} diff --git a/services/tests/servicestests/test-apps/PackageParserApp/AndroidManifestApp5.xml b/services/tests/servicestests/test-apps/PackageParserApp/AndroidManifestApp5.xml new file mode 100644 index 0000000000000..cc6caad8e2e3f --- /dev/null +++ b/services/tests/servicestests/test-apps/PackageParserApp/AndroidManifestApp5.xml @@ -0,0 +1,28 @@ + + + + + + + + + + + + \ No newline at end of file