diff options
| author | 2018-12-10 13:47:59 +0000 | |
|---|---|---|
| committer | 2018-12-11 11:53:10 +0000 | |
| commit | 40ba44348274ac61ac18ef8417d99bb1cfbe759f (patch) | |
| tree | 2f273caf2723794dfeeceb5fca103f78ccb3a0ae | |
| parent | d8ea250a631af2ec854a963edb61603c14a97d41 (diff) | |
Add key-value mappers for improved safety.
This allows strings to be embedded into the config, which get mapped to a
specific value when the config is read. Also add string to integer mappings
for the API enforcement policy.
Doing this improves safety, as there are no brittle integer values to be
kepy in sync across mutliple SDK versions, and between the tools that
generate the config and the device. Relying on integer values runs the risk
of an unintended policy being applied at runtime. The main risk now is of
just generating invalid config which is rejected on the device.
Also fix some niggles with null JSON values while we're here.
Bug: 110509075
Test: atest SignedConfigTest
Change-Id: Iec689c42066d60ca569805bc260ddd0b05890da3
3 files changed, 184 insertions, 35 deletions
diff --git a/services/core/java/com/android/server/signedconfig/SignedConfig.java b/services/core/java/com/android/server/signedconfig/SignedConfig.java index a3f452c27d13..e6bb800045c8 100644 --- a/services/core/java/com/android/server/signedconfig/SignedConfig.java +++ b/services/core/java/com/android/server/signedconfig/SignedConfig.java @@ -48,7 +48,7 @@ public class SignedConfig { private static final String CONFIG_KEY_VALUE = "value"; /** - * Represents config values targetting to an SDK range. + * Represents config values targeting an SDK range. */ public static class PerSdkConfig { public final int minSdk; @@ -90,11 +90,24 @@ public class SignedConfig { /** * Parse configuration from an APK. * - * @param config config as read from the APK metadata. + * @param config Config string as read from the APK metadata. + * @param allowedKeys Set of allowed keys in the config. Any key/value mapping for a key not in + * this set will result in an {@link InvalidConfigException} being thrown. + * @param keyValueMappers Mappings for values per key. The keys in the top level map should be + * a subset of {@code allowedKeys}. The keys in the inner map indicate + * the set of allowed values for that keys value. This map will be + * applied to the value in the configuration. This is intended to allow + * enum-like values to be encoded as strings in the configuration, and + * mapped back to integers when the configuration is parsed. + * + * <p>Any config key with a value that does not appear in the + * corresponding map will result in an {@link InvalidConfigException} + * being thrown. * @return Parsed configuration. * @throws InvalidConfigException If there's a problem parsing the config. */ - public static SignedConfig parse(String config, Set<String> allowedKeys) + public static SignedConfig parse(String config, Set<String> allowedKeys, + Map<String, Map<String, String>> keyValueMappers) throws InvalidConfigException { try { JSONObject json = new JSONObject(config); @@ -103,7 +116,8 @@ public class SignedConfig { JSONArray perSdkConfig = json.getJSONArray(KEY_CONFIG); List<PerSdkConfig> parsedConfigs = new ArrayList<>(); for (int i = 0; i < perSdkConfig.length(); ++i) { - parsedConfigs.add(parsePerSdkConfig(perSdkConfig.getJSONObject(i), allowedKeys)); + parsedConfigs.add(parsePerSdkConfig(perSdkConfig.getJSONObject(i), allowedKeys, + keyValueMappers)); } return new SignedConfig(version, parsedConfigs); @@ -113,8 +127,17 @@ public class SignedConfig { } + private static CharSequence quoted(Object s) { + if (s == null) { + return "null"; + } else { + return "\"" + s + "\""; + } + } + @VisibleForTesting - static PerSdkConfig parsePerSdkConfig(JSONObject json, Set<String> allowedKeys) + static PerSdkConfig parsePerSdkConfig(JSONObject json, Set<String> allowedKeys, + Map<String, Map<String, String>> keyValueMappers) throws JSONException, InvalidConfigException { int minSdk = json.getInt(CONFIG_KEY_MIN_SDK); int maxSdk = json.getInt(CONFIG_KEY_MAX_SDK); @@ -123,12 +146,23 @@ public class SignedConfig { for (int i = 0; i < valueArray.length(); ++i) { JSONObject keyValuePair = valueArray.getJSONObject(i); String key = keyValuePair.getString(CONFIG_KEY_KEY); - String value = keyValuePair.has(CONFIG_KEY_VALUE) - ? keyValuePair.getString(CONFIG_KEY_VALUE) + Object valueObject = keyValuePair.has(CONFIG_KEY_VALUE) + ? keyValuePair.get(CONFIG_KEY_VALUE) : null; + String value = valueObject == JSONObject.NULL || valueObject == null + ? null + : valueObject.toString(); if (!allowedKeys.contains(key)) { throw new InvalidConfigException("Config key " + key + " is not allowed"); } + if (keyValueMappers.containsKey(key)) { + Map<String, String> mapper = keyValueMappers.get(key); + if (!mapper.containsKey(value)) { + throw new InvalidConfigException( + "Config key " + key + " contains unsupported value " + quoted(value)); + } + value = mapper.get(value); + } values.put(key, value); } return new PerSdkConfig(minSdk, maxSdk, values); diff --git a/services/core/java/com/android/server/signedconfig/SignedConfigApplicator.java b/services/core/java/com/android/server/signedconfig/SignedConfigApplicator.java index 2312f5f0a2a2..e4d799a2e3b7 100644 --- a/services/core/java/com/android/server/signedconfig/SignedConfigApplicator.java +++ b/services/core/java/com/android/server/signedconfig/SignedConfigApplicator.java @@ -17,8 +17,10 @@ package com.android.server.signedconfig; import android.content.Context; +import android.content.pm.ApplicationInfo; import android.os.Build; import android.provider.Settings; +import android.util.ArrayMap; import android.util.ArraySet; import android.util.Slog; @@ -37,6 +39,30 @@ class SignedConfigApplicator { Settings.Global.HIDDEN_API_BLACKLIST_EXEMPTIONS ))); + private static final Map<String, String> HIDDEN_API_POLICY_KEY_MAP = makeMap( + "DEFAULT", String.valueOf(ApplicationInfo.HIDDEN_API_ENFORCEMENT_DEFAULT), + "DISABLED", String.valueOf(ApplicationInfo.HIDDEN_API_ENFORCEMENT_DISABLED), + "JUST_WARN", String.valueOf(ApplicationInfo.HIDDEN_API_ENFORCEMENT_JUST_WARN), + "ENABLED", String.valueOf(ApplicationInfo.HIDDEN_API_ENFORCEMENT_ENABLED) + ); + + private static final Map<String, Map<String, String>> KEY_VALUE_MAPPERS = makeMap( + Settings.Global.HIDDEN_API_POLICY, HIDDEN_API_POLICY_KEY_MAP + ); + + private static <K, V> Map<K, V> makeMap(Object... keyValuePairs) { + if (keyValuePairs.length % 2 != 0) { + throw new IllegalArgumentException(); + } + final int len = keyValuePairs.length / 2; + ArrayMap<K, V> m = new ArrayMap<>(len); + for (int i = 0; i < len; ++i) { + m.put((K) keyValuePairs[i * 2], (V) keyValuePairs[(i * 2) + 1]); + } + return Collections.unmodifiableMap(m); + + } + private final Context mContext; private final String mSourcePackage; @@ -75,7 +101,7 @@ class SignedConfigApplicator { } SignedConfig config; try { - config = SignedConfig.parse(configStr, ALLOWED_KEYS); + config = SignedConfig.parse(configStr, ALLOWED_KEYS, KEY_VALUE_MAPPERS); } catch (InvalidConfigException e) { Slog.e(TAG, "Failed to parse config from package " + mSourcePackage, e); return; diff --git a/services/tests/servicestests/src/com/android/server/signedconfig/SignedConfigTest.java b/services/tests/servicestests/src/com/android/server/signedconfig/SignedConfigTest.java index a9d4519104ba..70fadd101a91 100644 --- a/services/tests/servicestests/src/com/android/server/signedconfig/SignedConfigTest.java +++ b/services/tests/servicestests/src/com/android/server/signedconfig/SignedConfigTest.java @@ -20,8 +20,11 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.fail; +import static java.util.Collections.emptyMap; import static java.util.Collections.emptySet; +import android.util.ArrayMap; + import androidx.test.runner.AndroidJUnit4; import com.google.common.collect.Sets; @@ -33,6 +36,7 @@ import org.junit.runner.RunWith; import java.util.Arrays; import java.util.Collections; +import java.util.Map; import java.util.Set; @@ -46,10 +50,25 @@ public class SignedConfigTest { return Sets.newHashSet(values); } + private static <K, V> Map<K, V> mapOf(Object... keyValuePairs) { + if (keyValuePairs.length % 2 != 0) { + throw new IllegalArgumentException(); + } + final int len = keyValuePairs.length / 2; + ArrayMap<K, V> m = new ArrayMap<>(len); + for (int i = 0; i < len; ++i) { + m.put((K) keyValuePairs[i * 2], (V) keyValuePairs[(i * 2) + 1]); + } + return Collections.unmodifiableMap(m); + + } + + @Test public void testParsePerSdkConfigSdkMinMax() throws JSONException, InvalidConfigException { JSONObject json = new JSONObject("{\"minSdk\":2, \"maxSdk\": 3, \"values\": []}"); - SignedConfig.PerSdkConfig config = SignedConfig.parsePerSdkConfig(json, emptySet()); + SignedConfig.PerSdkConfig config = SignedConfig.parsePerSdkConfig(json, emptySet(), + emptyMap()); assertThat(config.minSdk).isEqualTo(2); assertThat(config.maxSdk).isEqualTo(3); } @@ -58,7 +77,7 @@ public class SignedConfigTest { public void testParsePerSdkConfigNoMinSdk() throws JSONException { JSONObject json = new JSONObject("{\"maxSdk\": 3, \"values\": []}"); try { - SignedConfig.parsePerSdkConfig(json, emptySet()); + SignedConfig.parsePerSdkConfig(json, emptySet(), emptyMap()); fail("Expected InvalidConfigException or JSONException"); } catch (JSONException | InvalidConfigException e) { // expected @@ -69,7 +88,7 @@ public class SignedConfigTest { public void testParsePerSdkConfigNoMaxSdk() throws JSONException { JSONObject json = new JSONObject("{\"minSdk\": 1, \"values\": []}"); try { - SignedConfig.parsePerSdkConfig(json, emptySet()); + SignedConfig.parsePerSdkConfig(json, emptySet(), emptyMap()); fail("Expected InvalidConfigException or JSONException"); } catch (JSONException | InvalidConfigException e) { // expected @@ -80,7 +99,7 @@ public class SignedConfigTest { public void testParsePerSdkConfigNoValues() throws JSONException { JSONObject json = new JSONObject("{\"minSdk\": 1, \"maxSdk\": 3}"); try { - SignedConfig.parsePerSdkConfig(json, emptySet()); + SignedConfig.parsePerSdkConfig(json, emptySet(), emptyMap()); fail("Expected InvalidConfigException or JSONException"); } catch (JSONException | InvalidConfigException e) { // expected @@ -88,10 +107,10 @@ public class SignedConfigTest { } @Test - public void testParsePerSdkConfigSdkNullMinSdk() throws JSONException, InvalidConfigException { + public void testParsePerSdkConfigSdkNullMinSdk() throws JSONException { JSONObject json = new JSONObject("{\"minSdk\":null, \"maxSdk\": 3, \"values\": []}"); try { - SignedConfig.parsePerSdkConfig(json, emptySet()); + SignedConfig.parsePerSdkConfig(json, emptySet(), emptyMap()); fail("Expected InvalidConfigException or JSONException"); } catch (JSONException | InvalidConfigException e) { // expected @@ -99,10 +118,10 @@ public class SignedConfigTest { } @Test - public void testParsePerSdkConfigSdkNullMaxSdk() throws JSONException, InvalidConfigException { + public void testParsePerSdkConfigSdkNullMaxSdk() throws JSONException { JSONObject json = new JSONObject("{\"minSdk\":1, \"maxSdk\": null, \"values\": []}"); try { - SignedConfig.parsePerSdkConfig(json, emptySet()); + SignedConfig.parsePerSdkConfig(json, emptySet(), emptyMap()); fail("Expected InvalidConfigException or JSONException"); } catch (JSONException | InvalidConfigException e) { // expected @@ -113,7 +132,7 @@ public class SignedConfigTest { public void testParsePerSdkConfigNullValues() throws JSONException { JSONObject json = new JSONObject("{\"minSdk\": 1, \"maxSdk\": 3, \"values\": null}"); try { - SignedConfig.parsePerSdkConfig(json, emptySet()); + SignedConfig.parsePerSdkConfig(json, emptySet(), emptyMap()); fail("Expected InvalidConfigException or JSONException"); } catch (JSONException | InvalidConfigException e) { // expected @@ -124,7 +143,8 @@ public class SignedConfigTest { public void testParsePerSdkConfigZeroValues() throws JSONException, InvalidConfigException { JSONObject json = new JSONObject("{\"minSdk\": 1, \"maxSdk\": 3, \"values\": []}"); - SignedConfig.PerSdkConfig config = SignedConfig.parsePerSdkConfig(json, setOf("a", "b")); + SignedConfig.PerSdkConfig config = SignedConfig.parsePerSdkConfig(json, setOf("a", "b"), + emptyMap()); assertThat(config.values).hasSize(0); } @@ -133,18 +153,30 @@ public class SignedConfigTest { throws JSONException, InvalidConfigException { JSONObject json = new JSONObject( "{\"minSdk\": 1, \"maxSdk\": 1, \"values\": [{\"key\":\"a\", \"value\": \"1\"}]}"); - SignedConfig.PerSdkConfig config = SignedConfig.parsePerSdkConfig(json, setOf("a", "b")); + SignedConfig.PerSdkConfig config = SignedConfig.parsePerSdkConfig(json, setOf("a", "b"), + emptyMap()); assertThat(config.values).containsExactly("a", "1"); } @Test + public void testParsePerSdkConfigSingleKeyNullValue() + throws JSONException, InvalidConfigException { + JSONObject json = new JSONObject( + "{\"minSdk\": 1, \"maxSdk\": 1, \"values\": [{\"key\":\"a\", \"value\": null}]}"); + SignedConfig.PerSdkConfig config = SignedConfig.parsePerSdkConfig(json, setOf("a", "b"), + emptyMap()); + assertThat(config.values.keySet()).containsExactly("a"); + assertThat(config.values.get("a")).isNull(); + } + + @Test public void testParsePerSdkConfigMultiKeys() throws JSONException, InvalidConfigException { JSONObject json = new JSONObject( "{\"minSdk\": 1, \"maxSdk\": 1, \"values\": [{\"key\":\"a\", \"value\": \"1\"}, " + "{\"key\":\"c\", \"value\": \"2\"}]}"); SignedConfig.PerSdkConfig config = SignedConfig.parsePerSdkConfig( - json, setOf("a", "b", "c")); + json, setOf("a", "b", "c"), emptyMap()); assertThat(config.values).containsExactly("a", "1", "c", "2"); } @@ -153,7 +185,7 @@ public class SignedConfigTest { JSONObject json = new JSONObject( "{\"minSdk\": 1, \"maxSdk\": 1, \"values\": [{\"key\":\"a\", \"value\": \"1\"}]}"); try { - SignedConfig.parsePerSdkConfig(json, setOf("b")); + SignedConfig.parsePerSdkConfig(json, setOf("b"), emptyMap()); fail("Expected InvalidConfigException or JSONException"); } catch (JSONException | InvalidConfigException e) { // expected @@ -161,11 +193,67 @@ public class SignedConfigTest { } @Test + public void testParsePerSdkConfigSingleKeyWithMap() + throws JSONException, InvalidConfigException { + JSONObject json = new JSONObject( + "{\"minSdk\": 1, \"maxSdk\": 1, \"values\": [{\"key\":\"a\", \"value\": \"1\"}]}"); + SignedConfig.PerSdkConfig config = SignedConfig.parsePerSdkConfig(json, setOf("a"), + mapOf("a", mapOf("1", "one"))); + assertThat(config.values).containsExactly("a", "one"); + } + + @Test + public void testParsePerSdkConfigSingleKeyWithMapInvalidValue() throws JSONException { + JSONObject json = new JSONObject( + "{\"minSdk\": 1, \"maxSdk\": 1, \"values\": [{\"key\":\"a\", \"value\": \"2\"}]}"); + try { + SignedConfig.parsePerSdkConfig(json, setOf("b"), mapOf("a", mapOf("1", "one"))); + fail("Expected InvalidConfigException"); + } catch (InvalidConfigException e) { + // expected + } + } + + @Test + public void testParsePerSdkConfigMultiKeysWithMap() + throws JSONException, InvalidConfigException { + JSONObject json = new JSONObject( + "{\"minSdk\": 1, \"maxSdk\": 1, \"values\": [{\"key\":\"a\", \"value\": \"1\"}," + + "{\"key\":\"b\", \"value\": \"1\"}]}"); + SignedConfig.PerSdkConfig config = SignedConfig.parsePerSdkConfig(json, setOf("a", "b"), + mapOf("a", mapOf("1", "one"))); + assertThat(config.values).containsExactly("a", "one", "b", "1"); + } + + @Test + public void testParsePerSdkConfigSingleKeyWithMapToNull() + throws JSONException, InvalidConfigException { + JSONObject json = new JSONObject( + "{\"minSdk\": 1, \"maxSdk\": 1, \"values\": [{\"key\":\"a\", \"value\": \"1\"}]}"); + SignedConfig.PerSdkConfig config = SignedConfig.parsePerSdkConfig(json, setOf("a"), + mapOf("a", mapOf("1", null))); + assertThat(config.values).containsExactly("a", null); + } + + @Test + public void testParsePerSdkConfigSingleKeyWithMapFromNull() + throws JSONException, InvalidConfigException { + assertThat(mapOf(null, "allitnil")).containsExactly(null, "allitnil"); + assertThat(mapOf(null, "allitnil").containsKey(null)).isTrue(); + JSONObject json = new JSONObject( + "{\"minSdk\": 1, \"maxSdk\": 1, \"values\": [{\"key\":\"a\", \"value\": null}]}"); + SignedConfig.PerSdkConfig config = SignedConfig.parsePerSdkConfig(json, setOf("a"), + mapOf("a", mapOf(null, "allitnil"))); + assertThat(config.values).containsExactly("a", "allitnil"); + } + + @Test public void testParsePerSdkConfigSingleKeyNoValue() throws JSONException, InvalidConfigException { JSONObject json = new JSONObject( "{\"minSdk\": 1, \"maxSdk\": 1, \"values\": [{\"key\":\"a\"}]}"); - SignedConfig.PerSdkConfig config = SignedConfig.parsePerSdkConfig(json, setOf("a", "b")); + SignedConfig.PerSdkConfig config = SignedConfig.parsePerSdkConfig(json, setOf("a", "b"), + emptyMap()); assertThat(config.values).containsExactly("a", null); } @@ -173,7 +261,7 @@ public class SignedConfigTest { public void testParsePerSdkConfigValuesInvalid() throws JSONException { JSONObject json = new JSONObject("{\"minSdk\": 1, \"maxSdk\": 1, \"values\": \"foo\"}"); try { - SignedConfig.parsePerSdkConfig(json, emptySet()); + SignedConfig.parsePerSdkConfig(json, emptySet(), emptyMap()); fail("Expected InvalidConfigException or JSONException"); } catch (JSONException | InvalidConfigException e) { // expected @@ -184,7 +272,7 @@ public class SignedConfigTest { public void testParsePerSdkConfigConfigEntryInvalid() throws JSONException { JSONObject json = new JSONObject("{\"minSdk\": 1, \"maxSdk\": 1, \"values\": [1, 2]}"); try { - SignedConfig.parsePerSdkConfig(json, emptySet()); + SignedConfig.parsePerSdkConfig(json, emptySet(), emptyMap()); fail("Expected InvalidConfigException or JSONException"); } catch (JSONException | InvalidConfigException e) { // expected @@ -195,7 +283,7 @@ public class SignedConfigTest { public void testParsePerSdkConfigConfigEntryNull() throws JSONException { JSONObject json = new JSONObject("{\"minSdk\": 1, \"maxSdk\": 1, \"values\": [null]}"); try { - SignedConfig.parsePerSdkConfig(json, emptySet()); + SignedConfig.parsePerSdkConfig(json, emptySet(), emptyMap()); fail("Expected InvalidConfigException or JSONException"); } catch (JSONException | InvalidConfigException e) { // expected @@ -205,14 +293,15 @@ public class SignedConfigTest { @Test public void testParseVersion() throws InvalidConfigException { SignedConfig config = SignedConfig.parse( - "{\"version\": 1, \"config\": []}", emptySet()); + "{\"version\": 1, \"config\": []}", emptySet(), emptyMap()); assertThat(config.version).isEqualTo(1); } @Test public void testParseVersionInvalid() { try { - SignedConfig.parse("{\"version\": \"notanint\", \"config\": []}", emptySet()); + SignedConfig.parse("{\"version\": \"notanint\", \"config\": []}", emptySet(), + emptyMap()); fail("Expected InvalidConfigException"); } catch (InvalidConfigException e) { //expected @@ -222,7 +311,7 @@ public class SignedConfigTest { @Test public void testParseNoVersion() { try { - SignedConfig.parse("{\"config\": []}", emptySet()); + SignedConfig.parse("{\"config\": []}", emptySet(), emptyMap()); fail("Expected InvalidConfigException"); } catch (InvalidConfigException e) { //expected @@ -232,7 +321,7 @@ public class SignedConfigTest { @Test public void testParseNoConfig() { try { - SignedConfig.parse("{\"version\": 1}", emptySet()); + SignedConfig.parse("{\"version\": 1}", emptySet(), emptyMap()); fail("Expected InvalidConfigException"); } catch (InvalidConfigException e) { //expected @@ -242,7 +331,7 @@ public class SignedConfigTest { @Test public void testParseConfigNull() { try { - SignedConfig.parse("{\"version\": 1, \"config\": null}", emptySet()); + SignedConfig.parse("{\"version\": 1, \"config\": null}", emptySet(), emptyMap()); fail("Expected InvalidConfigException"); } catch (InvalidConfigException e) { //expected @@ -252,7 +341,7 @@ public class SignedConfigTest { @Test public void testParseVersionNull() { try { - SignedConfig.parse("{\"version\": null, \"config\": []}", emptySet()); + SignedConfig.parse("{\"version\": null, \"config\": []}", emptySet(), emptyMap()); fail("Expected InvalidConfigException"); } catch (InvalidConfigException e) { //expected @@ -262,7 +351,7 @@ public class SignedConfigTest { @Test public void testParseConfigInvalidEntry() { try { - SignedConfig.parse("{\"version\": 1, \"config\": [{}]}", emptySet()); + SignedConfig.parse("{\"version\": 1, \"config\": [{}]}", emptySet(), emptyMap()); fail("Expected InvalidConfigException"); } catch (InvalidConfigException e) { //expected @@ -273,7 +362,7 @@ public class SignedConfigTest { public void testParseSdkConfigSingle() throws InvalidConfigException { SignedConfig config = SignedConfig.parse( "{\"version\": 1, \"config\":[{\"minSdk\": 1, \"maxSdk\": 1, \"values\": []}]}", - emptySet()); + emptySet(), emptyMap()); assertThat(config.perSdkConfig).hasSize(1); } @@ -281,7 +370,8 @@ public class SignedConfigTest { public void testParseSdkConfigMultiple() throws InvalidConfigException { SignedConfig config = SignedConfig.parse( "{\"version\": 1, \"config\":[{\"minSdk\": 1, \"maxSdk\": 1, \"values\": []}, " - + "{\"minSdk\": 2, \"maxSdk\": 2, \"values\": []}]}", emptySet()); + + "{\"minSdk\": 2, \"maxSdk\": 2, \"values\": []}]}", emptySet(), + emptyMap()); assertThat(config.perSdkConfig).hasSize(2); } @@ -314,7 +404,6 @@ public class SignedConfigTest { SignedConfig config = new SignedConfig(0, Arrays.asList(sdk13, sdk46)); assertThat(config.getMatchingConfig(2)).isEqualTo(sdk13); } - @Test public void testGetMatchingConfigNoMatch() { SignedConfig.PerSdkConfig sdk1 = new SignedConfig.PerSdkConfig( |