diff options
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 ae9d71610aaa..946135fbb443 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 b7199d4a2443..150822bdff6b 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 5eb21a58c38e..cc531607b493 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<String, Set<String>> actualWhite = mSysConfig.getAndClearPackageToUserTypeWhitelist(); Map<String, Set<String>> 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<String, Boolean> 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(); } |