diff options
author | 2024-09-19 23:25:21 +0000 | |
---|---|---|
committer | 2024-10-15 22:04:30 +0000 | |
commit | 2a90b8fa79c96539f18e108fc97b653af7b2beda (patch) | |
tree | 6c83ce196b3e28779f706ff1797dfa236a306e3e | |
parent | 35bb4587b0f6435dfcdb4b7937506239bbd2c4f6 (diff) |
Integrate system feature codegen into SystemConfig
Add initial hooks from SystemConfig to query against system features
that are compiled into the framework via build flags. The behavior
is as follows:
* System features compiled as available will always be exposed
* System features compiled as unavailable will never be exposed
* Runtime parsing of system features won't override compiled features
Currently, we log a warning if there's a conflict between the set of
compiled and runtime system features, but this will likely be elevated
to an exception after additional hardening and development.
The current codegen is a no-op with respect to this behavior, as
RELEASE_USE_SYSTEM_FEATURE_BUILD_FLAGS is flagged as off.
A follow-up change will integrate RoSystemFeature queries into:
* ApplicationPackageManager - To bypass binder calls when possible
* SystemServer - For various hasSystemFeature checks
Bug: 203143243
Test: atest FrameworksServicesTests
Flag: build.RELEASE_USE_SYSTEM_FEATURE_BUILD_FLAGS
Change-Id: I1fcd2f2f86dc5605f33182e88fc98a21b57bf812
10 files changed, 293 insertions, 28 deletions
diff --git a/core/java/Android.bp b/core/java/Android.bp index fae411d495ca..ee71a9d4d573 100644 --- a/core/java/Android.bp +++ b/core/java/Android.bp @@ -19,6 +19,7 @@ filegroup { srcs: [ "**/*.java", "**/*.aidl", + ":systemfeatures-gen-srcs", ":framework-nfc-non-updatable-sources", ], visibility: ["//frameworks/base"], @@ -597,3 +598,29 @@ java_library { } // protolog end + +// Whether to enable read-only system feature codegen. +gen_readonly_feature_apis = select(release_flag("RELEASE_USE_SYSTEM_FEATURE_BUILD_FLAGS"), { + true: "true", + false: "false", + default: "false", +}) + +// Generates com.android.internal.pm.RoSystemFeatures, optionally compiling in +// details about fixed system features defined by build flags. When disabled, +// the APIs are simply passthrough stubs with no meaningful side effects. +genrule { + name: "systemfeatures-gen-srcs", + cmd: "$(location systemfeatures-gen-tool) com.android.internal.pm.RoSystemFeatures " + + // --readonly=false (default) makes the codegen an effective no-op passthrough API. + " --readonly=" + gen_readonly_feature_apis + + // For now, only export "android.hardware.type.*" system features APIs. + // TODO(b/203143243): Use an intermediate soong var that aggregates all declared + // RELEASE_SYSTEM_FEATURE_* declarations into a single arg. + " --feature-apis=AUTOMOTIVE,WATCH,TELEVISION,EMBEDDED,PC" + + " > $(out)", + out: [ + "RoSystemFeatures.java", + ], + tools: ["systemfeatures-gen-tool"], +} diff --git a/services/core/java/com/android/server/SystemConfig.java b/services/core/java/com/android/server/SystemConfig.java index d80e40c5898a..8b619a40f19b 100644 --- a/services/core/java/com/android/server/SystemConfig.java +++ b/services/core/java/com/android/server/SystemConfig.java @@ -46,6 +46,7 @@ import android.util.TimingsTraceLog; import android.util.Xml; import com.android.internal.annotations.VisibleForTesting; +import com.android.internal.pm.RoSystemFeatures; import com.android.internal.util.XmlUtils; import com.android.modules.utils.build.UnboundedSdkLevel; import com.android.server.pm.permission.PermissionAllowlist; @@ -212,6 +213,30 @@ public class SystemConfig { } } + /** + * Utility class for testing interaction with compile-time defined system features. + * @hide + */ + @VisibleForTesting + public static class Injector { + /** Whether a system feature is defined as enabled and available at compile-time. */ + public boolean isReadOnlySystemEnabledFeature(String featureName, int version) { + return Boolean.TRUE.equals(RoSystemFeatures.maybeHasFeature(featureName, version)); + } + + /** Whether a system feature is defined as disabled and unavailable at compile-time. */ + public boolean isReadOnlySystemDisabledFeature(String featureName, int version) { + return Boolean.FALSE.equals(RoSystemFeatures.maybeHasFeature(featureName, version)); + } + + /** The full set of system features defined as compile-time enabled and available. */ + public ArrayMap<String, FeatureInfo> getReadOnlySystemEnabledFeatures() { + return RoSystemFeatures.getReadOnlySystemEnabledFeatures(); + } + } + + private final Injector mInjector; + // These are the built-in shared libraries that were read from the // system configuration files. Keys are the library names; values are // the individual entries that contain information such as filename @@ -220,7 +245,7 @@ public class SystemConfig { // These are the features this devices supports that were read from the // system configuration files. - final ArrayMap<String, FeatureInfo> mAvailableFeatures = new ArrayMap<>(); + final ArrayMap<String, FeatureInfo> mAvailableFeatures; // These are the features which this device doesn't support; the OEM // partition uses these to opt-out of features from the system image. @@ -602,12 +627,26 @@ public class SystemConfig { public ArrayMap<String, Integer> getOemDefinedUids() { return mOemDefinedUids; } + /** * Only use for testing. Do NOT use in production code. * @param readPermissions false to create an empty SystemConfig; true to read the permissions. */ @VisibleForTesting public SystemConfig(boolean readPermissions) { + this(readPermissions, new Injector()); + } + + /** + * Only use for testing. Do NOT use in production code. + * @param readPermissions false to create an empty SystemConfig; true to read the permissions. + * @param injector Additional dependency injection for testing. + */ + @VisibleForTesting + public SystemConfig(boolean readPermissions, Injector injector) { + mInjector = injector; + mAvailableFeatures = mInjector.getReadOnlySystemEnabledFeatures(); + if (readPermissions) { Slog.w(TAG, "Constructing a test SystemConfig"); readAllPermissions(); @@ -617,6 +656,9 @@ public class SystemConfig { } SystemConfig() { + mInjector = new Injector(); + mAvailableFeatures = mInjector.getReadOnlySystemEnabledFeatures(); + TimingsTraceLog log = new TimingsTraceLog(TAG, Trace.TRACE_TAG_SYSTEM_SERVER); log.traceBegin("readAllPermissions"); try { @@ -1777,6 +1819,10 @@ public class SystemConfig { } private void addFeature(String name, int version) { + if (mInjector.isReadOnlySystemDisabledFeature(name, version)) { + Slog.w(TAG, "Skipping feature addition for compile-time disabled feature: " + name); + return; + } FeatureInfo fi = mAvailableFeatures.get(name); if (fi == null) { fi = new FeatureInfo(); @@ -1789,6 +1835,10 @@ public class SystemConfig { } private void removeFeature(String name) { + if (mInjector.isReadOnlySystemEnabledFeature(name, /*version=*/0)) { + Slog.w(TAG, "Skipping feature removal for compile-time enabled feature: " + name); + return; + } if (mAvailableFeatures.remove(name) != null) { Slog.d(TAG, "Removed unavailable feature " + name); } diff --git a/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigReadOnlyFeaturesTest.kt b/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigReadOnlyFeaturesTest.kt new file mode 100644 index 000000000000..22d894a5a739 --- /dev/null +++ b/services/tests/servicestests/src/com/android/server/systemconfig/SystemConfigReadOnlyFeaturesTest.kt @@ -0,0 +1,167 @@ +/* + * Copyright (C) 2024 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.systemconfig + +import android.content.Context +import android.content.pm.FeatureInfo +import android.util.ArrayMap +import android.util.Xml + +import androidx.test.filters.SmallTest +import androidx.test.platform.app.InstrumentationRegistry +import com.android.server.SystemConfig +import com.google.common.truth.Truth.assertThat +import org.junit.runner.RunWith +import org.junit.Rule + +import org.junit.Test +import org.junit.rules.TemporaryFolder +import org.junit.runners.JUnit4 + +@SmallTest +@RunWith(JUnit4::class) +class SystemConfigReadOnlyFeaturesTest { + + companion object { + private const val FEATURE_ONE = "feature.test.1" + private const val FEATURE_TWO = "feature.test.2" + private const val FEATURE_RUNTIME_AVAILABLE_TEMPLATE = + """ + <permissions> + <feature name="%s" /> + </permissions> + """ + private const val FEATURE_RUNTIME_DISABLED_TEMPLATE = + """ + <permissions> + <Disabled-feature name="%s" /> + </permissions> + """ + + fun featureInfo(featureName: String) = FeatureInfo().apply { name = featureName } + } + + private val context: Context = InstrumentationRegistry.getInstrumentation().context + + @get:Rule + val tempFolder = TemporaryFolder(context.filesDir) + + private val injector = TestInjector() + + private var uniqueCounter = 0 + + @Test + fun empty() { + assertFeatures().isEmpty() + } + + @Test + fun readOnlyEnabled() { + addReadOnlyEnabledFeature(FEATURE_ONE) + addReadOnlyEnabledFeature(FEATURE_TWO) + + assertFeatures().containsAtLeast(FEATURE_ONE, FEATURE_TWO) + } + + @Test + fun readOnlyAndRuntimeEnabled() { + addReadOnlyEnabledFeature(FEATURE_ONE) + addRuntimeEnabledFeature(FEATURE_TWO) + + // No issues with matching availability. + assertFeatures().containsAtLeast(FEATURE_ONE, FEATURE_TWO) + } + + @Test + fun readOnlyEnabledRuntimeDisabled() { + addReadOnlyEnabledFeature(FEATURE_ONE) + addRuntimeDisabledFeature(FEATURE_ONE) + + // Read-only feature availability should take precedence. + assertFeatures().contains(FEATURE_ONE) + } + + @Test + fun readOnlyDisabled() { + addReadOnlyDisabledFeature(FEATURE_ONE) + + assertFeatures().doesNotContain(FEATURE_ONE) + } + + @Test + fun readOnlyAndRuntimeDisabled() { + addReadOnlyDisabledFeature(FEATURE_ONE) + addRuntimeDisabledFeature(FEATURE_ONE) + + // No issues with matching (un)availability. + assertFeatures().doesNotContain(FEATURE_ONE) + } + + @Test + fun readOnlyDisabledRuntimeEnabled() { + addReadOnlyDisabledFeature(FEATURE_ONE) + addRuntimeEnabledFeature(FEATURE_ONE) + addRuntimeEnabledFeature(FEATURE_TWO) + + // Read-only feature (un)availability should take precedence. + assertFeatures().doesNotContain(FEATURE_ONE) + assertFeatures().contains(FEATURE_TWO) + } + + fun addReadOnlyEnabledFeature(featureName: String) { + injector.readOnlyEnabledFeatures[featureName] = featureInfo(featureName) + } + + fun addReadOnlyDisabledFeature(featureName: String) { + injector.readOnlyDisabledFeatures.add(featureName) + } + + fun addRuntimeEnabledFeature(featureName: String) { + FEATURE_RUNTIME_AVAILABLE_TEMPLATE.format(featureName).write() + } + + fun addRuntimeDisabledFeature(featureName: String) { + FEATURE_RUNTIME_DISABLED_TEMPLATE.format(featureName).write() + } + + private fun String.write() = tempFolder.root.resolve("${uniqueCounter++}.xml") + .writeText(this.trimIndent()) + + private fun assertFeatures() = assertThat(availableFeatures().keys) + + private fun availableFeatures() = SystemConfig(false, injector).apply { + val parser = Xml.newPullParser() + readPermissions(parser, tempFolder.root, /*Grant all permission flags*/ 0.inv()) + }.let { it.availableFeatures } + + internal class TestInjector() : SystemConfig.Injector() { + val readOnlyEnabledFeatures = ArrayMap<String, FeatureInfo>() + val readOnlyDisabledFeatures = mutableSetOf<String>() + + override fun isReadOnlySystemEnabledFeature(featureName: String, version: Int): Boolean { + return readOnlyEnabledFeatures.containsKey(featureName) + } + + override fun isReadOnlySystemDisabledFeature(featureName: String, version: Int): Boolean { + return readOnlyDisabledFeatures.contains(featureName) + } + + override fun getReadOnlySystemEnabledFeatures(): ArrayMap<String, FeatureInfo> { + return readOnlyEnabledFeatures + } + } +} diff --git a/tools/systemfeatures/src/com/android/systemfeatures/SystemFeaturesGenerator.kt b/tools/systemfeatures/src/com/android/systemfeatures/SystemFeaturesGenerator.kt index cba521e639cb..196b5e7c02ab 100644 --- a/tools/systemfeatures/src/com/android/systemfeatures/SystemFeaturesGenerator.kt +++ b/tools/systemfeatures/src/com/android/systemfeatures/SystemFeaturesGenerator.kt @@ -22,8 +22,6 @@ import com.squareup.javapoet.JavaFile import com.squareup.javapoet.MethodSpec import com.squareup.javapoet.ParameterizedTypeName import com.squareup.javapoet.TypeSpec -import java.util.HashMap -import java.util.Map import javax.lang.model.element.Modifier /* @@ -52,7 +50,7 @@ import javax.lang.model.element.Modifier * public static boolean hasFeatureAutomotive(Context context); * public static boolean hasFeatureLeanback(Context context); * public static Boolean maybeHasFeature(String feature, int version); - * public static ArrayMap<String, FeatureInfo> getCompileTimeAvailableFeatures(); + * public static ArrayMap<String, FeatureInfo> getReadOnlySystemEnabledFeatures(); * } * </pre> */ @@ -63,6 +61,7 @@ object SystemFeaturesGenerator { private val PACKAGEMANAGER_CLASS = ClassName.get("android.content.pm", "PackageManager") private val CONTEXT_CLASS = ClassName.get("android.content", "Context") private val FEATUREINFO_CLASS = ClassName.get("android.content.pm", "FeatureInfo") + private val ARRAYMAP_CLASS = ClassName.get("android.util", "ArrayMap") private val ASSUME_TRUE_CLASS = ClassName.get("com.android.aconfig.annotations", "AssumeTrueForR8") private val ASSUME_FALSE_CLASS = @@ -291,19 +290,19 @@ object SystemFeaturesGenerator { features: Collection<FeatureInfo>, ) { val methodBuilder = - MethodSpec.methodBuilder("getCompileTimeAvailableFeatures") + MethodSpec.methodBuilder("getReadOnlySystemEnabledFeatures") .addModifiers(Modifier.PUBLIC, Modifier.STATIC) .addAnnotation(ClassName.get("android.annotation", "NonNull")) .addJavadoc("Gets features marked as available at compile-time, keyed by name." + "\n\n@hide") .returns(ParameterizedTypeName.get( - ClassName.get(Map::class.java), + ARRAYMAP_CLASS, ClassName.get(String::class.java), FEATUREINFO_CLASS)) val availableFeatures = features.filter { it.readonly && it.version != null } - methodBuilder.addStatement("Map<String, FeatureInfo> features = new \$T<>(\$L)", - HashMap::class.java, availableFeatures.size) + methodBuilder.addStatement("\$T<String, FeatureInfo> features = new \$T<>(\$L)", + ARRAYMAP_CLASS, ARRAYMAP_CLASS, availableFeatures.size) if (!availableFeatures.isEmpty()) { methodBuilder.addStatement("FeatureInfo fi = new FeatureInfo()") } diff --git a/tools/systemfeatures/tests/golden/RoFeatures.java.gen b/tools/systemfeatures/tests/golden/RoFeatures.java.gen index edbfc4237547..ee97b26159de 100644 --- a/tools/systemfeatures/tests/golden/RoFeatures.java.gen +++ b/tools/systemfeatures/tests/golden/RoFeatures.java.gen @@ -13,10 +13,9 @@ import android.annotation.Nullable; import android.content.Context; import android.content.pm.FeatureInfo; import android.content.pm.PackageManager; +import android.util.ArrayMap; import com.android.aconfig.annotations.AssumeFalseForR8; import com.android.aconfig.annotations.AssumeTrueForR8; -import java.util.HashMap; -import java.util.Map; /** * @hide @@ -94,8 +93,8 @@ public final class RoFeatures { * @hide */ @NonNull - public static Map<String, FeatureInfo> getCompileTimeAvailableFeatures() { - Map<String, FeatureInfo> features = new HashMap<>(2); + public static ArrayMap<String, FeatureInfo> getReadOnlySystemEnabledFeatures() { + ArrayMap<String, FeatureInfo> features = new ArrayMap<>(2); FeatureInfo fi = new FeatureInfo(); fi.name = PackageManager.FEATURE_WATCH; fi.version = 1; diff --git a/tools/systemfeatures/tests/golden/RoNoFeatures.java.gen b/tools/systemfeatures/tests/golden/RoNoFeatures.java.gen index bf7a00679fa6..40c7db7ff1df 100644 --- a/tools/systemfeatures/tests/golden/RoNoFeatures.java.gen +++ b/tools/systemfeatures/tests/golden/RoNoFeatures.java.gen @@ -9,8 +9,7 @@ import android.annotation.Nullable; import android.content.Context; import android.content.pm.FeatureInfo; import android.content.pm.PackageManager; -import java.util.HashMap; -import java.util.Map; +import android.util.ArrayMap; /** * @hide @@ -43,8 +42,8 @@ public final class RoNoFeatures { * @hide */ @NonNull - public static Map<String, FeatureInfo> getCompileTimeAvailableFeatures() { - Map<String, FeatureInfo> features = new HashMap<>(0); + public static ArrayMap<String, FeatureInfo> getReadOnlySystemEnabledFeatures() { + ArrayMap<String, FeatureInfo> features = new ArrayMap<>(0); return features; } } diff --git a/tools/systemfeatures/tests/golden/RwFeatures.java.gen b/tools/systemfeatures/tests/golden/RwFeatures.java.gen index b20b228f9814..7bf89614b92d 100644 --- a/tools/systemfeatures/tests/golden/RwFeatures.java.gen +++ b/tools/systemfeatures/tests/golden/RwFeatures.java.gen @@ -12,8 +12,7 @@ import android.annotation.Nullable; import android.content.Context; import android.content.pm.FeatureInfo; import android.content.pm.PackageManager; -import java.util.HashMap; -import java.util.Map; +import android.util.ArrayMap; /** * @hide @@ -73,8 +72,8 @@ public final class RwFeatures { * @hide */ @NonNull - public static Map<String, FeatureInfo> getCompileTimeAvailableFeatures() { - Map<String, FeatureInfo> features = new HashMap<>(0); + public static ArrayMap<String, FeatureInfo> getReadOnlySystemEnabledFeatures() { + ArrayMap<String, FeatureInfo> features = new ArrayMap<>(0); return features; } } diff --git a/tools/systemfeatures/tests/golden/RwNoFeatures.java.gen b/tools/systemfeatures/tests/golden/RwNoFeatures.java.gen index d91f5b62d8d4..eb7ec63f1d7d 100644 --- a/tools/systemfeatures/tests/golden/RwNoFeatures.java.gen +++ b/tools/systemfeatures/tests/golden/RwNoFeatures.java.gen @@ -7,8 +7,7 @@ import android.annotation.NonNull; import android.annotation.Nullable; import android.content.Context; import android.content.pm.FeatureInfo; -import java.util.HashMap; -import java.util.Map; +import android.util.ArrayMap; /** * @hide @@ -32,8 +31,8 @@ public final class RwNoFeatures { * @hide */ @NonNull - public static Map<String, FeatureInfo> getCompileTimeAvailableFeatures() { - Map<String, FeatureInfo> features = new HashMap<>(0); + public static ArrayMap<String, FeatureInfo> getReadOnlySystemEnabledFeatures() { + ArrayMap<String, FeatureInfo> features = new ArrayMap<>(0); return features; } } diff --git a/tools/systemfeatures/tests/src/ArrayMap.java b/tools/systemfeatures/tests/src/ArrayMap.java new file mode 100644 index 000000000000..a5ed9b088896 --- /dev/null +++ b/tools/systemfeatures/tests/src/ArrayMap.java @@ -0,0 +1,26 @@ +/* + * Copyright (C) 2024 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.util; + +import java.util.HashMap; + +/** Stub for testing. */ +public final class ArrayMap<K, V> extends HashMap<K, V> { + public ArrayMap(int capacity) { + super(capacity); + } +} diff --git a/tools/systemfeatures/tests/src/SystemFeaturesGeneratorTest.java b/tools/systemfeatures/tests/src/SystemFeaturesGeneratorTest.java index 39f8fc44fe23..ed3f5c94ba79 100644 --- a/tools/systemfeatures/tests/src/SystemFeaturesGeneratorTest.java +++ b/tools/systemfeatures/tests/src/SystemFeaturesGeneratorTest.java @@ -60,7 +60,7 @@ public class SystemFeaturesGeneratorTest { assertThat(RwNoFeatures.maybeHasFeature(PackageManager.FEATURE_VULKAN, 0)).isNull(); assertThat(RwNoFeatures.maybeHasFeature(PackageManager.FEATURE_AUTO, 0)).isNull(); assertThat(RwNoFeatures.maybeHasFeature("com.arbitrary.feature", 0)).isNull(); - assertThat(RwNoFeatures.getCompileTimeAvailableFeatures()).isEmpty(); + assertThat(RwNoFeatures.getReadOnlySystemEnabledFeatures()).isEmpty(); } @Test @@ -72,7 +72,7 @@ public class SystemFeaturesGeneratorTest { assertThat(RoNoFeatures.maybeHasFeature(PackageManager.FEATURE_VULKAN, 0)).isNull(); assertThat(RoNoFeatures.maybeHasFeature(PackageManager.FEATURE_AUTO, 0)).isNull(); assertThat(RoNoFeatures.maybeHasFeature("com.arbitrary.feature", 0)).isNull(); - assertThat(RoNoFeatures.getCompileTimeAvailableFeatures()).isEmpty(); + assertThat(RoNoFeatures.getReadOnlySystemEnabledFeatures()).isEmpty(); // Also ensure we fall back to the PackageManager for feature APIs without an accompanying // versioned feature definition. @@ -106,7 +106,7 @@ public class SystemFeaturesGeneratorTest { assertThat(RwFeatures.maybeHasFeature(PackageManager.FEATURE_VULKAN, 0)).isNull(); assertThat(RwFeatures.maybeHasFeature(PackageManager.FEATURE_AUTO, 0)).isNull(); assertThat(RwFeatures.maybeHasFeature("com.arbitrary.feature", 0)).isNull(); - assertThat(RwFeatures.getCompileTimeAvailableFeatures()).isEmpty(); + assertThat(RwFeatures.getReadOnlySystemEnabledFeatures()).isEmpty(); } @Test @@ -163,7 +163,7 @@ public class SystemFeaturesGeneratorTest { assertThat(RoFeatures.maybeHasFeature("com.arbitrary.feature", 100)).isNull(); assertThat(RoFeatures.maybeHasFeature("", 0)).isNull(); - Map<String, FeatureInfo> compiledFeatures = RoFeatures.getCompileTimeAvailableFeatures(); + Map<String, FeatureInfo> compiledFeatures = RoFeatures.getReadOnlySystemEnabledFeatures(); assertThat(compiledFeatures.keySet()) .containsExactly(PackageManager.FEATURE_WATCH, PackageManager.FEATURE_WIFI); assertThat(compiledFeatures.get(PackageManager.FEATURE_WATCH).version).isEqualTo(1); |