From 66fc1f1941266997657231f6373664c69a8adb8d Mon Sep 17 00:00:00 2001 From: Alex Buynytskyy Date: Thu, 23 Sep 2021 15:34:51 -0700 Subject: [PATCH 1/6] Create XML parser only once. This greatly cuts memory usage: 2.5M -> 0.5M (see bug for traces). Bug: 200995209 Test: atest SystemConfigTest SystemConfigNamedActorTest Change-Id: I0b76dc2610afaad3e418ef3115c5e54a05ab334e Merged-In: I0b76dc2610afaad3e418ef3115c5e54a05ab334e --- .../java/com/android/server/SystemConfig.java | 49 ++++++++++--------- .../SystemConfigNamedActorTest.kt | 4 +- .../server/systemconfig/SystemConfigTest.java | 27 ++++++---- 3 files changed, 46 insertions(+), 34 deletions(-) diff --git a/core/java/com/android/server/SystemConfig.java b/core/java/com/android/server/SystemConfig.java index ae9d71610aaa5..946135fbb4437 100644 --- a/core/java/com/android/server/SystemConfig.java +++ b/core/java/com/android/server/SystemConfig.java @@ -509,12 +509,14 @@ public class SystemConfig { } private void readAllPermissions() { + final XmlPullParser parser = Xml.newPullParser(); + // Read configuration from system - readPermissions(Environment.buildPath( + readPermissions(parser, Environment.buildPath( Environment.getRootDirectory(), "etc", "sysconfig"), ALLOW_ALL); // Read configuration from the old permissions dir - readPermissions(Environment.buildPath( + readPermissions(parser, Environment.buildPath( Environment.getRootDirectory(), "etc", "permissions"), ALLOW_ALL); // Vendors are only allowed to customize these @@ -524,18 +526,18 @@ public class SystemConfig { // For backward compatibility vendorPermissionFlag |= (ALLOW_PERMISSIONS | ALLOW_APP_CONFIGS); } - readPermissions(Environment.buildPath( + readPermissions(parser, Environment.buildPath( Environment.getVendorDirectory(), "etc", "sysconfig"), vendorPermissionFlag); - readPermissions(Environment.buildPath( + readPermissions(parser, Environment.buildPath( Environment.getVendorDirectory(), "etc", "permissions"), vendorPermissionFlag); String vendorSkuProperty = SystemProperties.get(VENDOR_SKU_PROPERTY, ""); if (!vendorSkuProperty.isEmpty()) { String vendorSkuDir = "sku_" + vendorSkuProperty; - readPermissions(Environment.buildPath( + readPermissions(parser, Environment.buildPath( Environment.getVendorDirectory(), "etc", "sysconfig", vendorSkuDir), vendorPermissionFlag); - readPermissions(Environment.buildPath( + readPermissions(parser, Environment.buildPath( Environment.getVendorDirectory(), "etc", "permissions", vendorSkuDir), vendorPermissionFlag); } @@ -543,18 +545,18 @@ public class SystemConfig { // Allow ODM to customize system configs as much as Vendor, because /odm is another // vendor partition other than /vendor. int odmPermissionFlag = vendorPermissionFlag; - readPermissions(Environment.buildPath( + readPermissions(parser, Environment.buildPath( Environment.getOdmDirectory(), "etc", "sysconfig"), odmPermissionFlag); - readPermissions(Environment.buildPath( + readPermissions(parser, Environment.buildPath( Environment.getOdmDirectory(), "etc", "permissions"), odmPermissionFlag); String skuProperty = SystemProperties.get(SKU_PROPERTY, ""); if (!skuProperty.isEmpty()) { String skuDir = "sku_" + skuProperty; - readPermissions(Environment.buildPath( + readPermissions(parser, Environment.buildPath( Environment.getOdmDirectory(), "etc", "sysconfig", skuDir), odmPermissionFlag); - readPermissions(Environment.buildPath( + readPermissions(parser, Environment.buildPath( Environment.getOdmDirectory(), "etc", "permissions", skuDir), odmPermissionFlag); } @@ -562,9 +564,9 @@ public class SystemConfig { // Allow OEM to customize these int oemPermissionFlag = ALLOW_FEATURES | ALLOW_OEM_PERMISSIONS | ALLOW_ASSOCIATIONS | ALLOW_VENDOR_APEX; - readPermissions(Environment.buildPath( + readPermissions(parser, Environment.buildPath( Environment.getOemDirectory(), "etc", "sysconfig"), oemPermissionFlag); - readPermissions(Environment.buildPath( + readPermissions(parser, Environment.buildPath( Environment.getOemDirectory(), "etc", "permissions"), oemPermissionFlag); // Allow Product to customize these configs @@ -579,15 +581,15 @@ public class SystemConfig { // DEVICE_INITIAL_SDK_INT for the devices without product interface enforcement. productPermissionFlag = ALLOW_ALL; } - readPermissions(Environment.buildPath( + readPermissions(parser, Environment.buildPath( Environment.getProductDirectory(), "etc", "sysconfig"), productPermissionFlag); - readPermissions(Environment.buildPath( + readPermissions(parser, Environment.buildPath( Environment.getProductDirectory(), "etc", "permissions"), productPermissionFlag); // Allow /system_ext to customize all system configs - readPermissions(Environment.buildPath( + readPermissions(parser, Environment.buildPath( Environment.getSystemExtDirectory(), "etc", "sysconfig"), ALLOW_ALL); - readPermissions(Environment.buildPath( + readPermissions(parser, Environment.buildPath( Environment.getSystemExtDirectory(), "etc", "permissions"), ALLOW_ALL); // Skip loading configuration from apex if it is not a system process. @@ -601,12 +603,13 @@ public class SystemConfig { if (f.isFile() || f.getPath().contains("@")) { continue; } - readPermissions(Environment.buildPath(f, "etc", "permissions"), apexPermissionFlag); + readPermissions(parser, Environment.buildPath(f, "etc", "permissions"), + apexPermissionFlag); } } @VisibleForTesting - public void readPermissions(File libraryDir, int permissionFlag) { + public void readPermissions(final XmlPullParser parser, File libraryDir, int permissionFlag) { // Read permissions from given directory. if (!libraryDir.exists() || !libraryDir.isDirectory()) { if (permissionFlag == ALLOW_ALL) { @@ -641,12 +644,12 @@ public class SystemConfig { continue; } - readPermissionsFromXml(f, permissionFlag); + readPermissionsFromXml(parser, f, permissionFlag); } // Read platform permissions last so it will take precedence if (platformFile != null) { - readPermissionsFromXml(platformFile, permissionFlag); + readPermissionsFromXml(parser, platformFile, permissionFlag); } } @@ -655,8 +658,9 @@ public class SystemConfig { + permFile + " at " + parser.getPositionDescription()); } - private void readPermissionsFromXml(File permFile, int permissionFlag) { - FileReader permReader = null; + private void readPermissionsFromXml(final XmlPullParser parser, File permFile, + int permissionFlag) { + final FileReader permReader; try { permReader = new FileReader(permFile); } catch (FileNotFoundException e) { @@ -668,7 +672,6 @@ public class SystemConfig { final boolean lowRam = ActivityManager.isLowRamDeviceStatic(); try { - XmlPullParser parser = Xml.newPullParser(); parser.setInput(permReader); int type; diff --git a/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigNamedActorTest.kt b/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigNamedActorTest.kt index b7199d4a24435..150822bdff6be 100644 --- a/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigNamedActorTest.kt +++ b/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigNamedActorTest.kt @@ -17,6 +17,7 @@ package com.android.server.systemconfig import android.content.Context +import android.util.Xml import androidx.test.InstrumentationRegistry import com.android.server.SystemConfig import com.google.common.truth.Truth.assertThat @@ -227,6 +228,7 @@ class SystemConfigNamedActorTest { .writeText(this.trimIndent()) private fun assertPermissions() = SystemConfig(false).apply { - readPermissions(tempFolder.root, 0) + val parser = Xml.newPullParser() + readPermissions(parser, tempFolder.root, 0) }. let { assertThat(it.namedActors) } } diff --git a/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigTest.java b/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigTest.java index 5eb21a58c38e8..cc531607b4932 100644 --- a/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigTest.java +++ b/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigTest.java @@ -25,6 +25,7 @@ import android.platform.test.annotations.Presubmit; import android.util.ArrayMap; import android.util.ArraySet; import android.util.Log; +import android.util.Xml; import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; @@ -36,6 +37,7 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; import org.junit.runner.RunWith; +import org.xmlpull.v1.XmlPullParser; import java.io.BufferedWriter; import java.io.File; @@ -76,6 +78,11 @@ public class SystemConfigTest { } } + private void readPermissions(File libraryDir, int permissionFlag) { + final XmlPullParser parser = Xml.newPullParser(); + mSysConfig.readPermissions(parser, libraryDir, permissionFlag); + } + /** * Tests that readPermissions works correctly for the tag: install-in-user-type */ @@ -134,8 +141,8 @@ public class SystemConfigTest { // Also, make a third file, but with the name folder1/permFile2.xml, to prove no conflicts. createTempFile(folder1, "permFile2.xml", contents3); - mSysConfig.readPermissions(folder1, /* No permission needed anyway */ 0); - mSysConfig.readPermissions(folder2, /* No permission needed anyway */ 0); + readPermissions(folder1, /* No permission needed anyway */ 0); + readPermissions(folder2, /* No permission needed anyway */ 0); Map> actualWhite = mSysConfig.getAndClearPackageToUserTypeWhitelist(); Map> actualBlack = mSysConfig.getAndClearPackageToUserTypeBlacklist(); @@ -165,7 +172,7 @@ public class SystemConfigTest { final File folder = createTempSubfolder("folder"); createTempFile(folder, "component-override.xml", contents); - mSysConfig.readPermissions(folder, /* No permission needed anyway */ 0); + readPermissions(folder, /* No permission needed anyway */ 0); final ArrayMap packageOneExpected = new ArrayMap<>(); packageOneExpected.put("com.android.package1.Full", true); @@ -197,7 +204,7 @@ public class SystemConfigTest { final File folder = createTempSubfolder("folder"); createTempFile(folder, "staged-installer-whitelist.xml", contents); - mSysConfig.readPermissions(folder, /* Grant all permission flags */ ~0); + readPermissions(folder, /* Grant all permission flags */ ~0); assertThat(mSysConfig.getWhitelistedStagedInstallers()) .containsExactly("com.android.package1"); @@ -215,7 +222,7 @@ public class SystemConfigTest { final File folder = createTempSubfolder("folder"); createTempFile(folder, "staged-installer-whitelist.xml", contents); - mSysConfig.readPermissions(folder, /* Grant all permission flags */ ~0); + readPermissions(folder, /* Grant all permission flags */ ~0); assertThat(mSysConfig.getWhitelistedStagedInstallers()) .containsExactly("com.android.package1"); @@ -238,7 +245,7 @@ public class SystemConfigTest { IllegalStateException e = expectThrows( IllegalStateException.class, - () -> mSysConfig.readPermissions(folder, /* Grant all permission flags */ ~0)); + () -> readPermissions(folder, /* Grant all permission flags */ ~0)); assertThat(e).hasMessageThat().contains("Multiple modules installers"); } @@ -257,7 +264,7 @@ public class SystemConfigTest { final File folder = createTempSubfolder("folder"); createTempFile(folder, "staged-installer-whitelist.xml", contents); - mSysConfig.readPermissions(folder, /* Grant all but ALLOW_APP_CONFIGS flag */ ~0x08); + readPermissions(folder, /* Grant all but ALLOW_APP_CONFIGS flag */ ~0x08); assertThat(mSysConfig.getWhitelistedStagedInstallers()).isEmpty(); } @@ -277,7 +284,7 @@ public class SystemConfigTest { final File folder = createTempSubfolder("folder"); createTempFile(folder, "vendor-apex-allowlist.xml", contents); - mSysConfig.readPermissions(folder, /* Grant all permission flags */ ~0); + readPermissions(folder, /* Grant all permission flags */ ~0); assertThat(mSysConfig.getAllowedVendorApexes()) .containsExactly("com.android.apex1", "com.installer"); @@ -297,7 +304,7 @@ public class SystemConfigTest { final File folder = createTempSubfolder("folder"); createTempFile(folder, "vendor-apex-allowlist.xml", contents); - mSysConfig.readPermissions(folder, /* Grant all permission flags */ ~0); + readPermissions(folder, /* Grant all permission flags */ ~0); assertThat(mSysConfig.getAllowedVendorApexes()).isEmpty(); } @@ -317,7 +324,7 @@ public class SystemConfigTest { final File folder = createTempSubfolder("folder"); createTempFile(folder, "vendor-apex-allowlist.xml", contents); - mSysConfig.readPermissions(folder, /* Grant all but ALLOW_VENDOR_APEX flag */ ~0x400); + readPermissions(folder, /* Grant all but ALLOW_VENDOR_APEX flag */ ~0x400); assertThat(mSysConfig.getAllowedVendorApexes()).isEmpty(); } From 5c228ca3e97f330b5722f6a51d281fa477633731 Mon Sep 17 00:00:00 2001 From: Pedro Loureiro Date: Fri, 30 Jul 2021 17:27:00 +0000 Subject: [PATCH 2/6] Parse new xml attributes used for updatable shared libraries Attributes added to the `library` tag used in AndroidManifest.xml. They allow to easily and transparently include/exclude a library from apps for compatibility purposes. Bug: 191978330 Test: atest com.android.server.pm.parsing.library.ApexSharedLibraryUpdaterTest com.android.server.systemconfig.SystemConfigTest Change-Id: Ibdde742a05fd670a9aaee5ee77ae25b9c0801f53 Merged-In: Ibdde742a05fd670a9aaee5ee77ae25b9c0801f53 --- .../java/com/android/server/SystemConfig.java | 87 +++++- .../library/ApexSharedLibraryUpdater.java | 67 +++++ .../library/PackageBackwardCompatibility.java | 11 + .../library/ApexSharedLibraryUpdaterTest.java | 281 ++++++++++++++++++ .../PackageBackwardCompatibilityTest.java | 18 ++ .../server/systemconfig/SystemConfigTest.java | 166 +++++++++++ 6 files changed, 623 insertions(+), 7 deletions(-) create mode 100644 services/core/java/com/android/server/pm/parsing/library/ApexSharedLibraryUpdater.java create mode 100644 services/tests/servicestests/src/com/android/server/pm/parsing/library/ApexSharedLibraryUpdaterTest.java diff --git a/core/java/com/android/server/SystemConfig.java b/core/java/com/android/server/SystemConfig.java index 946135fbb4437..be5dc0039ac6c 100644 --- a/core/java/com/android/server/SystemConfig.java +++ b/core/java/com/android/server/SystemConfig.java @@ -112,17 +112,74 @@ public class SystemConfig { public final String name; public final String filename; public final String[] dependencies; + + /** + * SDK version this library was added to the BOOTCLASSPATH. + * + *

At the SDK level specified in this field and higher, the apps' uses-library tags for + * this library will be ignored, since the library is always available on BOOTCLASSPATH. + * + *

0 means not specified. + */ + public final int onBootclasspathSince; + + /** + * SDK version this library was removed from the BOOTCLASSPATH. + * + *

At the SDK level specified in this field and higher, this library needs to be + * explicitly added by apps. For compatibility reasons, when an app + * targets an SDK less than the value of this attribute, this library is automatically + * added. + * + *

0 means not specified. + */ + public final int onBootclasspathBefore; + + /** + * Declares whether this library can be safely ignored from tags. + * + *

This can happen if the library initially had to be explicitly depended-on using that + * tag but has since been moved to the BOOTCLASSPATH which means now is always available + * and the tag is no longer required. + */ + public final boolean canBeSafelyIgnored; + public final boolean isNative; - SharedLibraryEntry(String name, String filename, String[] dependencies) { - this(name, filename, dependencies, false /* isNative */); + + @VisibleForTesting + public SharedLibraryEntry(String name, String filename, String[] dependencies, + boolean isNative) { + this(name, filename, dependencies, 0 /* onBootclasspathSince */, + 0 /* onBootclasspathBefore */, isNative); } - SharedLibraryEntry(String name, String filename, String[] dependencies, boolean isNative) { + @VisibleForTesting + public SharedLibraryEntry(String name, String filename, String[] dependencies, + int onBootclasspathSince, int onBootclassPathBefore) { + this(name, filename, dependencies, onBootclasspathSince, onBootclassPathBefore, + false /* isNative */); + } + + SharedLibraryEntry(String name, String filename, String[] dependencies, + int onBootclasspathSince, int onBootclasspathBefore, boolean isNative) { this.name = name; this.filename = filename; this.dependencies = dependencies; + this.onBootclasspathSince = onBootclasspathSince; + this.onBootclasspathBefore = onBootclasspathBefore; this.isNative = isNative; + + canBeSafelyIgnored = this.onBootclasspathSince != 0 + && isSdkAtLeast(this.onBootclasspathSince); + } + + private static boolean isSdkAtLeast(int level) { + if ("REL".equals(Build.VERSION.CODENAME)) { + return Build.VERSION.SDK_INT >= level; + } + return level == Build.VERSION_CODES.CUR_DEVELOPMENT + || Build.VERSION.SDK_INT >= level; } } @@ -792,11 +849,17 @@ public class SystemConfig { XmlUtils.skipCurrentTag(parser); } } break; + case "updatable-library": + // "updatable-library" is meant to behave exactly like "library" case "library": { if (allowLibs) { String lname = parser.getAttributeValue(null, "name"); String lfile = parser.getAttributeValue(null, "file"); String ldependency = parser.getAttributeValue(null, "dependency"); + int minDeviceSdk = XmlUtils.readIntAttribute(parser, "min-device-sdk", + 0); + int maxDeviceSdk = XmlUtils.readIntAttribute(parser, "max-device-sdk", + 0); if (lname == null) { Slog.w(TAG, "<" + name + "> without name in " + permFile + " at " + parser.getPositionDescription()); @@ -804,10 +867,20 @@ public class SystemConfig { Slog.w(TAG, "<" + name + "> without file in " + permFile + " at " + parser.getPositionDescription()); } else { - //Log.i(TAG, "Got library " + lname + " in " + lfile); - SharedLibraryEntry entry = new SharedLibraryEntry(lname, lfile, - ldependency == null ? new String[0] : ldependency.split(":")); - mSharedLibraries.put(lname, entry); + boolean allowedMinSdk = minDeviceSdk <= Build.VERSION.SDK_INT; + boolean allowedMaxSdk = + maxDeviceSdk == 0 || maxDeviceSdk >= Build.VERSION.SDK_INT; + if (allowedMinSdk && allowedMaxSdk) { + int bcpSince = XmlUtils.readIntAttribute(parser, + "on-bootclasspath-since", 0); + int bcpBefore = XmlUtils.readIntAttribute(parser, + "on-bootclasspath-before", 0); + SharedLibraryEntry entry = new SharedLibraryEntry(lname, lfile, + ldependency == null + ? new String[0] : ldependency.split(":"), + bcpSince, bcpBefore); + mSharedLibraries.put(lname, entry); + } } } else { logNotAllowedInPartition(name, permFile, parser); diff --git a/services/core/java/com/android/server/pm/parsing/library/ApexSharedLibraryUpdater.java b/services/core/java/com/android/server/pm/parsing/library/ApexSharedLibraryUpdater.java new file mode 100644 index 0000000000000..0418afbf29ee9 --- /dev/null +++ b/services/core/java/com/android/server/pm/parsing/library/ApexSharedLibraryUpdater.java @@ -0,0 +1,67 @@ +/* + * 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 com.android.server.pm.parsing.library; + +import android.util.ArrayMap; + +import com.android.internal.annotations.VisibleForTesting; +import com.android.server.SystemConfig; +import com.android.server.pm.parsing.pkg.ParsedPackage; + +/** + * Updates packages to add or remove dependencies on shared libraries as per attributes + * in the library declaration + * + * @hide + */ +@VisibleForTesting +public class ApexSharedLibraryUpdater extends PackageSharedLibraryUpdater { + + /** + * ArrayMap like the one you find in {@link SystemConfig}. The keys are the library names. + */ + private final ArrayMap mSharedLibraries; + + public ApexSharedLibraryUpdater( + ArrayMap sharedLibraries) { + mSharedLibraries = sharedLibraries; + } + + @Override + public void updatePackage(ParsedPackage parsedPackage, boolean isUpdatedSystemApp) { + final int builtInLibCount = mSharedLibraries.size(); + for (int i = 0; i < builtInLibCount; i++) { + updateSharedLibraryForPackage(mSharedLibraries.valueAt(i), parsedPackage); + } + } + + private void updateSharedLibraryForPackage(SystemConfig.SharedLibraryEntry entry, + ParsedPackage parsedPackage) { + if (entry.onBootclasspathBefore != 0 + && parsedPackage.getTargetSdkVersion() < entry.onBootclasspathBefore) { + // this package targets an API where this library was in the BCP, so add + // the library transparently in case the package is using it + prefixRequiredLibrary(parsedPackage, entry.name); + } + + if (entry.canBeSafelyIgnored) { + // the library is now present in the BCP and always available; we don't need to add + // it a second time + removeLibrary(parsedPackage, entry.name); + } + } +} diff --git a/services/core/java/com/android/server/pm/parsing/library/PackageBackwardCompatibility.java b/services/core/java/com/android/server/pm/parsing/library/PackageBackwardCompatibility.java index 8a8a302734b10..d81e7d05fd731 100644 --- a/services/core/java/com/android/server/pm/parsing/library/PackageBackwardCompatibility.java +++ b/services/core/java/com/android/server/pm/parsing/library/PackageBackwardCompatibility.java @@ -25,6 +25,7 @@ import android.content.pm.PackageParser; import android.util.Log; import com.android.internal.annotations.VisibleForTesting; +import com.android.server.SystemConfig; import com.android.server.pm.parsing.pkg.ParsedPackage; import java.util.ArrayList; @@ -63,6 +64,11 @@ public class PackageBackwardCompatibility extends PackageSharedLibraryUpdater { boolean bootClassPathContainsATB = !addUpdaterForAndroidTestBase(packageUpdaters); + // ApexSharedLibraryUpdater should be the last one, to allow modifications introduced by + // mainline after dessert release. + packageUpdaters.add(new ApexSharedLibraryUpdater( + SystemConfig.getInstance().getSharedLibraries())); + PackageSharedLibraryUpdater[] updaterArray = packageUpdaters .toArray(new PackageSharedLibraryUpdater[0]); INSTANCE = new PackageBackwardCompatibility( @@ -106,6 +112,11 @@ public class PackageBackwardCompatibility extends PackageSharedLibraryUpdater { private final PackageSharedLibraryUpdater[] mPackageUpdaters; + @VisibleForTesting + PackageSharedLibraryUpdater[] getPackageUpdaters() { + return mPackageUpdaters; + } + private PackageBackwardCompatibility( boolean bootClassPathContainsATB, PackageSharedLibraryUpdater[] packageUpdaters) { this.mBootClassPathContainsATB = bootClassPathContainsATB; diff --git a/services/tests/servicestests/src/com/android/server/pm/parsing/library/ApexSharedLibraryUpdaterTest.java b/services/tests/servicestests/src/com/android/server/pm/parsing/library/ApexSharedLibraryUpdaterTest.java new file mode 100644 index 0000000000000..1d9ea4b6028c0 --- /dev/null +++ b/services/tests/servicestests/src/com/android/server/pm/parsing/library/ApexSharedLibraryUpdaterTest.java @@ -0,0 +1,281 @@ +/* + * 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 com.android.server.pm.parsing.library; + +import android.os.Build; +import android.platform.test.annotations.Presubmit; +import android.util.ArrayMap; + +import androidx.test.filters.SmallTest; + +import com.android.server.SystemConfig; +import com.android.server.pm.parsing.pkg.AndroidPackage; +import com.android.server.pm.parsing.pkg.PackageImpl; +import com.android.server.pm.parsing.pkg.ParsedPackage; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + + +/** + * Test for {@link ApexSharedLibraryUpdater} + */ +@Presubmit +@SmallTest +@RunWith(JUnit4.class) +public class ApexSharedLibraryUpdaterTest extends PackageSharedLibraryUpdaterTest { + + private final ArrayMap mSharedLibraries = + new ArrayMap<>(8); + + @Before + public void setUp() throws Exception { + installSharedLibraries(); + } + + private void installSharedLibraries() throws Exception { + mSharedLibraries.clear(); + insertLibrary("foo", 0, 0); + insertLibrary("fooBcpSince30", 30, 0); + insertLibrary("fooBcpBefore30", 0, 30); + insertLibrary("fooFromFuture", Build.VERSION.SDK_INT + 2, 0); + } + + private void insertLibrary(String libraryName, int onBootclasspathSince, + int onBootclasspathBefore) { + mSharedLibraries.put(libraryName, new SystemConfig.SharedLibraryEntry( + libraryName, + "foo.jar", + new String[0] /* dependencies */, + onBootclasspathSince, + onBootclasspathBefore + ) + ); + } + + @Test + public void testRegularAppOnRPlus() { + // platform Q should have changes (tested below) + + // these should have no changes + checkNoChanges(Build.VERSION_CODES.R); + checkNoChanges(Build.VERSION_CODES.S); + checkNoChanges(Build.VERSION_CODES.TIRAMISU); + checkNoChanges(Build.VERSION_CODES.CUR_DEVELOPMENT); + } + + private void checkNoChanges(int targetSdkVersion) { + ParsedPackage before = ((ParsedPackage) PackageImpl.forTesting(PACKAGE_NAME) + .setTargetSdkVersion(targetSdkVersion) + .hideAsParsed()); + + AndroidPackage after = ((ParsedPackage) PackageImpl.forTesting(PACKAGE_NAME) + .setTargetSdkVersion(targetSdkVersion) + .hideAsParsed()) + .hideAsFinal(); + + checkBackwardsCompatibility(before, after); + } + + @Test + public void testBcpSince30Applied() { + ParsedPackage before = ((ParsedPackage) PackageImpl.forTesting(PACKAGE_NAME) + .setTargetSdkVersion(Build.VERSION_CODES.R) + .addUsesLibrary("fooBcpSince30") + .hideAsParsed()); + + AndroidPackage after = ((ParsedPackage) PackageImpl.forTesting(PACKAGE_NAME) + .setTargetSdkVersion(Build.VERSION_CODES.R) + .hideAsParsed()) + .hideAsFinal(); + + // note: target sdk is not what matters in this logic. It's the system SDK + // should be removed because on 30+ (R+) it is implicit + + checkBackwardsCompatibility(before, after); + } + + @Test + public void testBcpSince11kNotAppliedWithoutLibrary() { + ParsedPackage before = ((ParsedPackage) PackageImpl.forTesting(PACKAGE_NAME) + .setTargetSdkVersion(Build.VERSION_CODES.R) + .hideAsParsed()); + + AndroidPackage after = ((ParsedPackage) PackageImpl.forTesting(PACKAGE_NAME) + .setTargetSdkVersion(Build.VERSION_CODES.R) + .hideAsParsed()) + .hideAsFinal(); + + // note: target sdk is not what matters in this logic. It's the system SDK + // nothing should change because the implicit from is only from a future platform release + checkBackwardsCompatibility(before, after); + } + + @Test + public void testBcpSince11kNotAppliedWithLibrary() { + ParsedPackage before = ((ParsedPackage) PackageImpl.forTesting(PACKAGE_NAME) + .setTargetSdkVersion(Build.VERSION_CODES.R) + .addUsesLibrary("fooFromFuture") + .hideAsParsed()); + + AndroidPackage after = ((ParsedPackage) PackageImpl.forTesting(PACKAGE_NAME) + .setTargetSdkVersion(Build.VERSION_CODES.R) + .addUsesLibrary("fooFromFuture") + .hideAsParsed()) + .hideAsFinal(); + + // note: target sdk is not what matters in this logic. It's the system SDK + // nothing should change because the implicit from is only from a future platform release + checkBackwardsCompatibility(before, after); + } + + @Test + public void testBcpBefore30NotApplied() { + ParsedPackage before = ((ParsedPackage) PackageImpl.forTesting(PACKAGE_NAME) + .setTargetSdkVersion(Build.VERSION_CODES.R) + .hideAsParsed()); + + AndroidPackage after = ((ParsedPackage) PackageImpl.forTesting(PACKAGE_NAME) + .setTargetSdkVersion(Build.VERSION_CODES.R) + .hideAsParsed()) + .hideAsFinal(); + + // should not be affected because it is still in the BCP in 30 / R + checkBackwardsCompatibility(before, after); + } + + @Test + public void testBcpBefore30Applied() { + ParsedPackage before = ((ParsedPackage) PackageImpl.forTesting(PACKAGE_NAME) + .setTargetSdkVersion(Build.VERSION_CODES.Q) + .hideAsParsed()); + + AndroidPackage after = ((ParsedPackage) PackageImpl.forTesting(PACKAGE_NAME) + .setTargetSdkVersion(Build.VERSION_CODES.Q) + .addUsesLibrary("fooBcpBefore30") + .hideAsParsed()) + .hideAsFinal(); + + // should be present because this was in BCP in 29 / Q + checkBackwardsCompatibility(before, after); + } + + /** + * Test a library that was first removed from the BCP [to a mainline module] and later was + * moved back to the BCP via a mainline module update. All of this happening before the current + * SDK. + */ + @Test + public void testBcpRemovedThenAddedPast() { + insertLibrary("fooBcpRemovedThenAdded", 30, 28); + + ParsedPackage before = ((ParsedPackage) PackageImpl.forTesting(PACKAGE_NAME) + .setTargetSdkVersion(Build.VERSION_CODES.N) + .hideAsParsed()); + + AndroidPackage after = ((ParsedPackage) PackageImpl.forTesting(PACKAGE_NAME) + .setTargetSdkVersion(Build.VERSION_CODES.N) + .addUsesLibrary("fooBcpBefore30") + .hideAsParsed()) + .hideAsFinal(); + + // the library is now in the BOOTCLASSPATH (for the second time) so it doesn't need to be + // listed + checkBackwardsCompatibility(before, after); + } + + /** + * Test a library that was first removed from the BCP [to a mainline module] and later was + * moved back to the BCP via a mainline module update. The first part happening before the + * current SDK and the second part after. + */ + @Test + public void testBcpRemovedThenAddedMiddle_targetQ() { + insertLibrary("fooBcpRemovedThenAdded", Build.VERSION.SDK_INT + 1, 30); + + ParsedPackage before = ((ParsedPackage) PackageImpl.forTesting(PACKAGE_NAME) + .setTargetSdkVersion(Build.VERSION_CODES.Q) + .hideAsParsed()); + + AndroidPackage after = ((ParsedPackage) PackageImpl.forTesting(PACKAGE_NAME) + .setTargetSdkVersion(Build.VERSION_CODES.Q) + .addUsesLibrary("fooBcpRemovedThenAdded") + .addUsesLibrary("fooBcpBefore30") + .hideAsParsed()) + .hideAsFinal(); + + // in this example, we are at the point where the library is not in the BOOTCLASSPATH. + // Because the app targets Q / 29 (when this library was in the BCP) then we need to add it + checkBackwardsCompatibility(before, after); + } + + /** + * Test a library that was first removed from the BCP [to a mainline module] and later was + * moved back to the BCP via a mainline module update. The first part happening before the + * current SDK and the second part after. + */ + @Test + public void testBcpRemovedThenAddedMiddle_targetR() { + insertLibrary("fooBcpRemovedThenAdded", Build.VERSION.SDK_INT + 1, 30); + + ParsedPackage before = ((ParsedPackage) PackageImpl.forTesting(PACKAGE_NAME) + .setTargetSdkVersion(Build.VERSION_CODES.R) + .hideAsParsed()); + + AndroidPackage after = ((ParsedPackage) PackageImpl.forTesting(PACKAGE_NAME) + .setTargetSdkVersion(Build.VERSION_CODES.R) + .hideAsParsed()) + .hideAsFinal(); + + // in this example, we are at the point where the library is not in the BOOTCLASSPATH. + // Because the app targets R/30 (when this library was removed from the BCP) then we don't + //need to add it + checkBackwardsCompatibility(before, after); + } + + /** + * Test a library that was first removed from the BCP [to a mainline module] and later was + * moved back to the BCP via a mainline module update. The first part happening before the + * current SDK and the second part after. + */ + @Test + public void testBcpRemovedThenAddedMiddle_targetR_usingLib() { + insertLibrary("fooBcpRemovedThenAdded", Build.VERSION.SDK_INT + 1, 30); + + ParsedPackage before = ((ParsedPackage) PackageImpl.forTesting(PACKAGE_NAME) + .setTargetSdkVersion(Build.VERSION_CODES.R) + .addUsesLibrary("fooBcpRemovedThenAdded") + .hideAsParsed()); + + AndroidPackage after = ((ParsedPackage) PackageImpl.forTesting(PACKAGE_NAME) + .setTargetSdkVersion(Build.VERSION_CODES.R) + .addUsesLibrary("fooBcpRemovedThenAdded") + .hideAsParsed()) + .hideAsFinal(); + + // in this example, we are at the point where the library is not in the BOOTCLASSPATH. + // Because the app wants to use the library, it needs to be present + checkBackwardsCompatibility(before, after); + } + + private void checkBackwardsCompatibility(ParsedPackage before, AndroidPackage after) { + checkBackwardsCompatibility(before, after, + () -> new ApexSharedLibraryUpdater(mSharedLibraries)); + } +} diff --git a/services/tests/servicestests/src/com/android/server/pm/parsing/library/PackageBackwardCompatibilityTest.java b/services/tests/servicestests/src/com/android/server/pm/parsing/library/PackageBackwardCompatibilityTest.java index 9768f176ea859..5bcd0f6bb0293 100644 --- a/services/tests/servicestests/src/com/android/server/pm/parsing/library/PackageBackwardCompatibilityTest.java +++ b/services/tests/servicestests/src/com/android/server/pm/parsing/library/PackageBackwardCompatibilityTest.java @@ -21,6 +21,8 @@ import static com.android.server.pm.parsing.library.SharedLibraryNames.ANDROID_T import static com.android.server.pm.parsing.library.SharedLibraryNames.ANDROID_TEST_RUNNER; import static com.android.server.pm.parsing.library.SharedLibraryNames.ORG_APACHE_HTTP_LEGACY; +import static com.google.common.truth.Truth.assertThat; + import android.content.pm.parsing.ParsingPackage; import android.os.Build; import android.platform.test.annotations.Presubmit; @@ -182,6 +184,22 @@ public class PackageBackwardCompatibilityTest extends PackageSharedLibraryUpdate checkBackwardsCompatibility(before, ((ParsedPackage) after.hideAsParsed()).hideAsFinal()); } + /** + * Ensures that ApexSharedLibraryUpdater is the last updater in the list of package updaters + * used by PackageBackwardCompatibility. + * + * This is required so mainline can add and remove libraries installed by the platform updaters. + */ + @Test + public void testApexPackageUpdaterOrdering() { + PackageBackwardCompatibility instance = + (PackageBackwardCompatibility) PackageBackwardCompatibility.getInstance(); + PackageSharedLibraryUpdater[] updaterArray = instance.getPackageUpdaters(); + + PackageSharedLibraryUpdater lastUpdater = updaterArray[updaterArray.length - 1]; + assertThat(lastUpdater).isInstanceOf(ApexSharedLibraryUpdater.class); + } + private void checkBackwardsCompatibility(ParsedPackage before, AndroidPackage after) { checkBackwardsCompatibility(before, after, PackageBackwardCompatibility::getInstance); } diff --git a/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigTest.java b/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigTest.java index cc531607b4932..4dcd633b45604 100644 --- a/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigTest.java +++ b/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigTest.java @@ -21,6 +21,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; import static org.testng.Assert.expectThrows; +import android.os.Build; import android.platform.test.annotations.Presubmit; import android.util.ArrayMap; import android.util.ArraySet; @@ -329,6 +330,164 @@ public class SystemConfigTest { assertThat(mSysConfig.getAllowedVendorApexes()).isEmpty(); } + /** + * Tests that readPermissions works correctly for a library with on-bootclasspath-before + * and on-bootclasspath-since. + */ + @Test + public void readPermissions_allowLibs_parsesSimpleLibrary() throws IOException { + String contents = + "\n" + + " \n\n" + + " "; + parseSharedLibraries(contents); + assertFooIsOnlySharedLibrary(); + SystemConfig.SharedLibraryEntry entry = mSysConfig.getSharedLibraries().get("foo"); + assertThat(entry.onBootclasspathBefore).isEqualTo(10); + assertThat(entry.onBootclasspathSince).isEqualTo(20); + } + + /** + * Tests that readPermissions works correctly for a library using the new + * {@code updatable-library} tag. + */ + @Test + public void readPermissions_allowLibs_parsesUpdatableLibrary() throws IOException { + String contents = + "\n" + + " \n\n" + + " "; + parseSharedLibraries(contents); + assertFooIsOnlySharedLibrary(); + SystemConfig.SharedLibraryEntry entry = mSysConfig.getSharedLibraries().get("foo"); + assertThat(entry.onBootclasspathBefore).isEqualTo(10); + assertThat(entry.onBootclasspathSince).isEqualTo(20); + } + + /** + * Tests that readPermissions for a library with {@code min-device-sdk} lower than the current + * SDK results in the library being added to the shared libraries. + */ + @Test + public void readPermissions_allowLibs_allowsOldMinSdk() throws IOException { + String contents = + "\n" + + " \n\n" + + " "; + parseSharedLibraries(contents); + assertFooIsOnlySharedLibrary(); + } + + /** + * Tests that readPermissions for a library with {@code min-device-sdk} equal to the current + * SDK results in the library being added to the shared libraries. + */ + @Test + public void readPermissions_allowLibs_allowsCurrentMinSdk() throws IOException { + String contents = + "\n" + + " \n\n" + + " "; + parseSharedLibraries(contents); + assertFooIsOnlySharedLibrary(); + } + + /** + * Tests that readPermissions for a library with {@code min-device-sdk} greater than the current + * SDK results in the library being ignored. + */ + @Test + public void readPermissions_allowLibs_ignoresMinSdkInFuture() throws IOException { + String contents = + "\n" + + " \n\n" + + " "; + parseSharedLibraries(contents); + assertThat(mSysConfig.getSharedLibraries()).isEmpty(); + } + + /** + * Tests that readPermissions for a library with {@code max-device-sdk} less than the current + * SDK results in the library being ignored. + */ + @Test + public void readPermissions_allowLibs_ignoredOldMaxSdk() throws IOException { + String contents = + "\n" + + " \n\n" + + " "; + parseSharedLibraries(contents); + assertThat(mSysConfig.getSharedLibraries()).isEmpty(); + } + + /** + * Tests that readPermissions for a library with {@code max-device-sdk} equal to the current + * SDK results in the library being added to the shared libraries. + */ + @Test + public void readPermissions_allowLibs_allowsCurrentMaxSdk() throws IOException { + String contents = + "\n" + + " \n\n" + + " "; + parseSharedLibraries(contents); + assertFooIsOnlySharedLibrary(); + } + + /** + * Tests that readPermissions for a library with {@code max-device-sdk} greater than the current + * SDK results in the library being added to the shared libraries. + */ + @Test + public void readPermissions_allowLibs_allowsMaxSdkInFuture() throws IOException { + String contents = + "\n" + + " \n\n" + + " "; + parseSharedLibraries(contents); + assertFooIsOnlySharedLibrary(); + } + + private void parseSharedLibraries(String contents) throws IOException { + File folder = createTempSubfolder("permissions_folder"); + createTempFile(folder, "permissions.xml", contents); + readPermissions(folder, /* permissionFlag = ALLOW_LIBS */ 0x02); + } + /** * Creates folderName/fileName in the mTemporaryFolder and fills it with the contents. * @@ -366,4 +525,11 @@ public class SystemConfigTest { return folder; } + + private void assertFooIsOnlySharedLibrary() { + assertThat(mSysConfig.getSharedLibraries().size()).isEqualTo(1); + SystemConfig.SharedLibraryEntry entry = mSysConfig.getSharedLibraries().get("foo"); + assertThat(entry.name).isEqualTo("foo"); + assertThat(entry.filename).isEqualTo("foo.jar"); + } } From 1df9a1e8f5b1c6697f249e7b4b5b5b0fa29a8233 Mon Sep 17 00:00:00 2001 From: Pedro Loureiro Date: Wed, 22 Dec 2021 19:42:44 +0000 Subject: [PATCH 3/6] Rename updatable-library to apex-library Mostly for consistency with the names used in related changes. Test: atest com.android.server.pm.parsing.library.ApexSharedLibraryUpdaterTest com.android.server.systemconfig.SystemConfigTest Bug: 191978330 Change-Id: Ic4ccc0fdca100b576e28bc0918d378cabae9ce61 Merged-In: Ic4ccc0fdca100b576e28bc0918d378cabae9ce61 --- core/java/com/android/server/SystemConfig.java | 4 ++-- .../src/com/android/server/systemconfig/SystemConfigTest.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/java/com/android/server/SystemConfig.java b/core/java/com/android/server/SystemConfig.java index be5dc0039ac6c..6c0a2888e8980 100644 --- a/core/java/com/android/server/SystemConfig.java +++ b/core/java/com/android/server/SystemConfig.java @@ -849,8 +849,8 @@ public class SystemConfig { XmlUtils.skipCurrentTag(parser); } } break; - case "updatable-library": - // "updatable-library" is meant to behave exactly like "library" + case "apex-library": + // "apex-library" is meant to behave exactly like "library" case "library": { if (allowLibs) { String lname = parser.getAttributeValue(null, "name"); diff --git a/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigTest.java b/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigTest.java index 4dcd633b45604..9623907add6aa 100644 --- a/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigTest.java +++ b/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigTest.java @@ -354,13 +354,13 @@ public class SystemConfigTest { /** * Tests that readPermissions works correctly for a library using the new - * {@code updatable-library} tag. + * {@code apex-library} tag. */ @Test public void readPermissions_allowLibs_parsesUpdatableLibrary() throws IOException { String contents = "\n" - + " Date: Tue, 7 Dec 2021 15:06:12 +0800 Subject: [PATCH 4/6] Ignore prebuilt shared library if it doesn't exist on device Bug: 191232777 Test: atest PackageManagerTest Test: atest SystemConfigTest Change-Id: I756e2c909af6ad0dcf8f1857ba398cfe07862b29 Merged-In: I756e2c909af6ad0dcf8f1857ba398cfe07862b29 --- .../java/com/android/server/SystemConfig.java | 16 +++++++++++- .../server/systemconfig/SystemConfigTest.java | 25 +++++++++++-------- 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/core/java/com/android/server/SystemConfig.java b/core/java/com/android/server/SystemConfig.java index 6c0a2888e8980..39f17e510a1cf 100644 --- a/core/java/com/android/server/SystemConfig.java +++ b/core/java/com/android/server/SystemConfig.java @@ -870,7 +870,8 @@ public class SystemConfig { boolean allowedMinSdk = minDeviceSdk <= Build.VERSION.SDK_INT; boolean allowedMaxSdk = maxDeviceSdk == 0 || maxDeviceSdk >= Build.VERSION.SDK_INT; - if (allowedMinSdk && allowedMaxSdk) { + final boolean exists = new File(lfile).exists(); + if (allowedMinSdk && allowedMaxSdk && exists) { int bcpSince = XmlUtils.readIntAttribute(parser, "on-bootclasspath-since", 0); int bcpBefore = XmlUtils.readIntAttribute(parser, @@ -880,6 +881,19 @@ public class SystemConfig { ? new String[0] : ldependency.split(":"), bcpSince, bcpBefore); mSharedLibraries.put(lname, entry); + } else { + final StringBuilder msg = new StringBuilder( + "Ignore shared library ").append(lname).append(":"); + if (!allowedMinSdk) { + msg.append(" min-device-sdk=").append(minDeviceSdk); + } + if (!allowedMaxSdk) { + msg.append(" max-device-sdk=").append(maxDeviceSdk); + } + if (!exists) { + msg.append(" ").append(lfile).append(" does not exist"); + } + Slog.i(TAG, msg.toString()); } } } else { diff --git a/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigTest.java b/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigTest.java index 9623907add6aa..eeaf781dd3071 100644 --- a/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigTest.java +++ b/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigTest.java @@ -62,12 +62,15 @@ public class SystemConfigTest { private static final String LOG_TAG = "SystemConfigTest"; private SystemConfig mSysConfig; + private File mFooJar; @Rule public TemporaryFolder mTemporaryFolder = new TemporaryFolder(); @Before public void setUp() throws Exception { mSysConfig = new SystemConfigTestClass(); + mFooJar = createTempFile( + mTemporaryFolder.getRoot().getCanonicalFile(), "foo.jar", "JAR"); } /** @@ -340,7 +343,7 @@ public class SystemConfigTest { "\n" + " \n\n" @@ -362,7 +365,7 @@ public class SystemConfigTest { "\n" + " \n\n" @@ -384,7 +387,7 @@ public class SystemConfigTest { "\n" + " \n\n" + " "; @@ -402,7 +405,7 @@ public class SystemConfigTest { "\n" + " \n\n" + " "; @@ -420,7 +423,7 @@ public class SystemConfigTest { "\n" + " \n\n" + " "; @@ -438,7 +441,7 @@ public class SystemConfigTest { "\n" + " \n\n" + " "; @@ -456,7 +459,7 @@ public class SystemConfigTest { "\n" + " \n\n" + " "; @@ -474,7 +477,7 @@ public class SystemConfigTest { "\n" + " \n\n" + " "; @@ -507,7 +510,7 @@ public class SystemConfigTest { * @param folder pre-existing subdirectory of mTemporaryFolder to put the file * @param fileName name of the file (e.g. filename.xml) to create * @param contents contents to write to the file - * @return the folder containing the newly created file (not the file itself!) + * @return the newly created file */ private File createTempFile(File folder, String fileName, String contents) throws IOException { @@ -523,13 +526,13 @@ public class SystemConfigTest { Log.d(LOG_TAG, input.nextLine()); } - return folder; + return file; } private void assertFooIsOnlySharedLibrary() { assertThat(mSysConfig.getSharedLibraries().size()).isEqualTo(1); SystemConfig.SharedLibraryEntry entry = mSysConfig.getSharedLibraries().get("foo"); assertThat(entry.name).isEqualTo("foo"); - assertThat(entry.filename).isEqualTo("foo.jar"); + assertThat(entry.filename).isEqualTo(mFooJar.toString()); } } From 5765fa9fb81e14850e605d84b67c631e0448f840 Mon Sep 17 00:00:00 2001 From: Andrei Onea Date: Thu, 16 Dec 2021 17:22:47 +0000 Subject: [PATCH 5/6] Add test for parsing apex allowlists Test: atest FrameworksServicesTests:SystemConfigTest Bug: 190375768 Change-Id: Ia530a7b5b62774660c410ca8a9f49b18ff9b9b57 --- .../java/com/android/server/SystemConfig.java | 17 ++++-- .../server/systemconfig/SystemConfigTest.java | 56 +++++++++++++++++-- 2 files changed, 62 insertions(+), 11 deletions(-) diff --git a/core/java/com/android/server/SystemConfig.java b/core/java/com/android/server/SystemConfig.java index 39f17e510a1cf..f364cb27888dc 100644 --- a/core/java/com/android/server/SystemConfig.java +++ b/core/java/com/android/server/SystemConfig.java @@ -1175,7 +1175,8 @@ public class SystemConfig { readPrivAppPermissions(parser, mSystemExtPrivAppPermissions, mSystemExtPrivAppDenyPermissions); } else if (apex) { - readApexPrivAppPermissions(parser, permFile); + readApexPrivAppPermissions(parser, permFile, + Environment.getApexDirectory().toPath()); } else { readPrivAppPermissions(parser, mPrivAppPermissions, mPrivAppDenyPermissions); @@ -1735,8 +1736,7 @@ public class SystemConfig { /** * Returns the module name for a file in the apex module's partition. */ - private String getApexModuleNameFromFilePath(Path path) { - final Path apexDirectoryPath = Environment.getApexDirectory().toPath(); + private String getApexModuleNameFromFilePath(Path path, Path apexDirectoryPath) { if (!path.startsWith(apexDirectoryPath)) { throw new IllegalArgumentException("File " + path + " is not part of an APEX."); } @@ -1748,9 +1748,14 @@ public class SystemConfig { return path.getName(apexDirectoryPath.getNameCount()).toString(); } - private void readApexPrivAppPermissions(XmlPullParser parser, File permFile) - throws IOException, XmlPullParserException { - final String moduleName = getApexModuleNameFromFilePath(permFile.toPath()); + /** + * Reads the contents of the privileged permission allowlist stored inside an APEX. + */ + @VisibleForTesting + public void readApexPrivAppPermissions(XmlPullParser parser, File permFile, + Path apexDirectoryPath) throws IOException, XmlPullParserException { + final String moduleName = + getApexModuleNameFromFilePath(permFile.toPath(), apexDirectoryPath); final ArrayMap> privAppPermissions; if (mApexPrivAppPermissions.containsKey(moduleName)) { privAppPermissions = mApexPrivAppPermissions.get(moduleName); diff --git a/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigTest.java b/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigTest.java index eeaf781dd3071..ca756f4778d50 100644 --- a/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigTest.java +++ b/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigTest.java @@ -39,9 +39,11 @@ import org.junit.Test; import org.junit.rules.TemporaryFolder; import org.junit.runner.RunWith; import org.xmlpull.v1.XmlPullParser; +import org.xmlpull.v1.XmlPullParserException; import java.io.BufferedWriter; import java.io.File; +import java.io.FileReader; import java.io.FileWriter; import java.io.IOException; import java.util.Arrays; @@ -137,13 +139,14 @@ public class SystemConfigTest { new ArraySet<>(Arrays.asList("GUEST", "PROFILE"))); final File folder1 = createTempSubfolder("folder1"); - createTempFile(folder1, "permFile1.xml", contents1); + createTempFile(folder1, "permissionFile1.xml", contents1); final File folder2 = createTempSubfolder("folder2"); - createTempFile(folder2, "permFile2.xml", contents2); + createTempFile(folder2, "permissionFile2.xml", contents2); - // Also, make a third file, but with the name folder1/permFile2.xml, to prove no conflicts. - createTempFile(folder1, "permFile2.xml", contents3); + // Also, make a third file, but with the name folder1/permissionFile2.xml, to prove no + // conflicts. + createTempFile(folder1, "permissionFile2.xml", contents3); readPermissions(folder1, /* No permission needed anyway */ 0); readPermissions(folder2, /* No permission needed anyway */ 0); @@ -333,6 +336,30 @@ public class SystemConfigTest { assertThat(mSysConfig.getAllowedVendorApexes()).isEmpty(); } + @Test + public void readApexPrivAppPermissions_addAllPermissions() + throws Exception { + final String contents = + "" + + "" + + "" + + ""; + File apexDir = createTempSubfolder("apex"); + File permissionFile = createTempFile( + createTempSubfolder("apex/com.android.my_module/etc/permissions"), + "permissions.xml", contents); + XmlPullParser parser = readXmlUntilStartTag(permissionFile); + + mSysConfig.readApexPrivAppPermissions(parser, permissionFile, apexDir.toPath()); + + assertThat(mSysConfig.getApexPrivAppPermissions("com.android.my_module", + "com.android.apk_in_apex")) + .containsExactly("android.permission.FOO"); + assertThat(mSysConfig.getApexPrivAppDenyPermissions("com.android.my_module", + "com.android.apk_in_apex")) + .containsExactly("android.permission.BAR"); + } + /** * Tests that readPermissions works correctly for a library with on-bootclasspath-before * and on-bootclasspath-since. @@ -491,6 +518,25 @@ public class SystemConfigTest { readPermissions(folder, /* permissionFlag = ALLOW_LIBS */ 0x02); } + /** + * Create an {@link XmlPullParser} for {@param permissionFile} and begin parsing it until + * reaching the root tag. + */ + private XmlPullParser readXmlUntilStartTag(File permissionFile) + throws IOException, XmlPullParserException { + FileReader permReader = new FileReader(permissionFile); + XmlPullParser parser = Xml.newPullParser(); + parser.setInput(permReader); + int type; + do { + type = parser.next(); + } while (type != parser.START_TAG && type != parser.END_DOCUMENT); + if (type != parser.START_TAG) { + throw new XmlPullParserException("No start tag found"); + } + return parser; + } + /** * Creates folderName/fileName in the mTemporaryFolder and fills it with the contents. * @@ -500,7 +546,7 @@ public class SystemConfigTest { private File createTempSubfolder(String folderName) throws IOException { File folder = new File(mTemporaryFolder.getRoot(), folderName); - folder.mkdir(); + folder.mkdirs(); return folder; } From 64a27bd856adf6ad126163926393f248ed050ea8 Mon Sep 17 00:00:00 2001 From: Andrei Onea Date: Thu, 16 Dec 2021 17:30:25 +0000 Subject: [PATCH 6/6] Ignore vendor apex priv-app permission allowlists Apexes contain the allowlists for privileged permissions used by their respective apk's, however vendor (partner) apexes should be forbidden from using this mechanism. Test: atest FrameworksServicesTests:SystemConfigTest Bug: 190375768 Change-Id: I34bf2a80fb66f2b2a732234111a338e3af1e919b --- .../java/com/android/server/SystemConfig.java | 16 +++++ .../server/systemconfig/SystemConfigTest.java | 61 +++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/core/java/com/android/server/SystemConfig.java b/core/java/com/android/server/SystemConfig.java index f364cb27888dc..fb0b7fdd6ce3d 100644 --- a/core/java/com/android/server/SystemConfig.java +++ b/core/java/com/android/server/SystemConfig.java @@ -663,6 +663,7 @@ public class SystemConfig { readPermissions(parser, Environment.buildPath(f, "etc", "permissions"), apexPermissionFlag); } + pruneVendorApexPrivappAllowlists(); } @VisibleForTesting @@ -1526,6 +1527,21 @@ public class SystemConfig { } } + /** + * Prunes out any privileged permission allowlists bundled in vendor apexes. + */ + @VisibleForTesting + public void pruneVendorApexPrivappAllowlists() { + for (String moduleName: mAllowedVendorApexes.keySet()) { + if (mApexPrivAppPermissions.containsKey(moduleName) + || mApexPrivAppDenyPermissions.containsKey(moduleName)) { + Slog.w(TAG, moduleName + " is a vendor apex, ignore its priv-app allowlist"); + mApexPrivAppPermissions.remove(moduleName); + mApexPrivAppDenyPermissions.remove(moduleName); + } + } + } + private void readInstallInUserType(XmlPullParser parser, Map> doInstallMap, Map> nonInstallMap) diff --git a/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigTest.java b/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigTest.java index ca756f4778d50..bfdffc0e65677 100644 --- a/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigTest.java +++ b/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigTest.java @@ -360,6 +360,67 @@ public class SystemConfigTest { .containsExactly("android.permission.BAR"); } + @Test + public void pruneVendorApexPrivappAllowlists_removeVendor() + throws Exception { + File apexDir = createTempSubfolder("apex"); + + // Read non-vendor apex permission allowlists + final String allowlistNonVendorContents = + "" + + "" + + "" + + ""; + File nonVendorPermDir = + createTempSubfolder("apex/com.android.non_vendor/etc/permissions"); + File nonVendorPermissionFile = + createTempFile(nonVendorPermDir, "permissions.xml", allowlistNonVendorContents); + XmlPullParser nonVendorParser = readXmlUntilStartTag(nonVendorPermissionFile); + mSysConfig.readApexPrivAppPermissions(nonVendorParser, nonVendorPermissionFile, + apexDir.toPath()); + + // Read vendor apex permission allowlists + final String allowlistVendorContents = + "" + + "" + + "" + + ""; + File vendorPermissionFile = + createTempFile(createTempSubfolder("apex/com.android.vendor/etc/permissions"), + "permissions.xml", allowlistNonVendorContents); + XmlPullParser vendorParser = readXmlUntilStartTag(vendorPermissionFile); + mSysConfig.readApexPrivAppPermissions(vendorParser, vendorPermissionFile, + apexDir.toPath()); + + // Read allowed vendor apex list + final String allowedVendorContents = + "\n" + + " \n" + + ""; + final File allowedVendorFolder = createTempSubfolder("folder"); + createTempFile(allowedVendorFolder, "vendor-apex-allowlist.xml", allowedVendorContents); + readPermissions(allowedVendorFolder, /* Grant all permission flags */ ~0); + + // Finally, prune non-vendor allowlists. + // There is no guarantee in which order the above reads will be done, however pruning + // will always happen last. + mSysConfig.pruneVendorApexPrivappAllowlists(); + + assertThat(mSysConfig.getApexPrivAppPermissions("com.android.non_vendor", + "com.android.apk_in_non_vendor_apex")) + .containsExactly("android.permission.FOO"); + assertThat(mSysConfig.getApexPrivAppDenyPermissions("com.android.non_vendor", + "com.android.apk_in_non_vendor_apex")) + .containsExactly("android.permission.BAR"); + assertThat(mSysConfig.getApexPrivAppPermissions("com.android.vendor", + "com.android.apk_in_vendor_apex")) + .isNull(); + assertThat(mSysConfig.getApexPrivAppDenyPermissions("com.android.vendor", + "com.android.apk_in_vendor_apex")) + .isNull(); + } + /** * Tests that readPermissions works correctly for a library with on-bootclasspath-before * and on-bootclasspath-since.