diff --git a/core/java/android/provider/Settings.java b/core/java/android/provider/Settings.java index 3aa534e79965d..9bcde71d56f24 100644 --- a/core/java/android/provider/Settings.java +++ b/core/java/android/provider/Settings.java @@ -252,6 +252,9 @@ public final class Settings { /** @hide */ public static final String EXTRA_NETWORK_TEMPLATE = "network_template"; + /** @hide */ + public static final String KEY_CONFIG_SET_RETURN = "config_set_return"; + /** * An int extra specifying a subscription ID. * @@ -2504,13 +2507,14 @@ public final class Settings { args.putString(CALL_METHOD_PREFIX_KEY, prefix); args.putSerializable(CALL_METHOD_FLAGS_KEY, keyValues); IContentProvider cp = mProviderHolder.getProvider(cr); - cp.call(cr.getPackageName(), cr.getFeatureId(), mProviderHolder.mUri.getAuthority(), + Bundle bundle = cp.call(cr.getPackageName(), cr.getFeatureId(), + mProviderHolder.mUri.getAuthority(), mCallSetAllCommand, null, args); + return bundle.getBoolean(KEY_CONFIG_SET_RETURN); } catch (RemoteException e) { // Not supported by the remote side return false; } - return true; } @UnsupportedAppUsage @@ -13985,14 +13989,18 @@ public final class Settings { */ @RequiresPermission(Manifest.permission.WRITE_DEVICE_CONFIG) static boolean setStrings(@NonNull ContentResolver resolver, @NonNull String namespace, - @NonNull Map keyValues) { + @NonNull Map keyValues) throws DeviceConfig.BadConfigException { HashMap compositeKeyValueMap = new HashMap<>(keyValues.keySet().size()); for (Map.Entry entry : keyValues.entrySet()) { compositeKeyValueMap.put( createCompositeName(namespace, entry.getKey()), entry.getValue()); } - return sNameValueCache.setStringsForPrefix(resolver, createPrefix(namespace), - compositeKeyValueMap); + // If can't set given configuration that means it's bad + if (!sNameValueCache.setStringsForPrefix(resolver, createPrefix(namespace), + compositeKeyValueMap)) { + throw new DeviceConfig.BadConfigException(); + } + return true; } /** diff --git a/core/tests/coretests/src/android/provider/DeviceConfigTest.java b/core/tests/coretests/src/android/provider/DeviceConfigTest.java index ae835e4a1c73f..84c42dbc14e76 100644 --- a/core/tests/coretests/src/android/provider/DeviceConfigTest.java +++ b/core/tests/coretests/src/android/provider/DeviceConfigTest.java @@ -20,6 +20,8 @@ import static android.provider.DeviceConfig.Properties; import static com.google.common.truth.Truth.assertThat; +import static org.testng.Assert.assertThrows; + import android.content.ContentResolver; import android.os.Bundle; import android.platform.test.annotations.Presubmit; @@ -44,9 +46,11 @@ public class DeviceConfigTest { private static final String KEY = "key1"; private static final String KEY2 = "key2"; private static final String KEY3 = "key3"; + private static final String KEY4 = "key4"; private static final String VALUE = "value1"; private static final String VALUE2 = "value2"; private static final String VALUE3 = "value3"; + private static final String NULL_VALUE = "null"; @After public void cleanUp() { @@ -561,6 +565,78 @@ public class DeviceConfigTest { assertThat(properties.getFloat("key5", 0f)).isEqualTo(floatValue); } + @Test + public void banNamespaceProperties() throws DeviceConfig.BadConfigException { + // Given namespace will be permanently banned, thus it needs to be different every time + final String namespaceToBan = NAMESPACE + System.currentTimeMillis(); + Properties properties = new Properties.Builder(namespaceToBan).setString(KEY, VALUE) + .setString(KEY4, NULL_VALUE).build(); + // Set namespace properties + DeviceConfig.setProperties(properties); + // Ban namespace with related properties + DeviceConfig.resetToDefaults(Settings.RESET_MODE_PACKAGE_DEFAULTS, namespaceToBan); + // Verify given namespace properties are banned + assertThrows(DeviceConfig.BadConfigException.class, + () -> DeviceConfig.setProperties(properties)); + // Modify properties and verify we can set them + Properties modifiedProperties = new Properties.Builder(namespaceToBan).setString(KEY, VALUE) + .setString(KEY4, NULL_VALUE).setString(KEY2, VALUE2).build(); + DeviceConfig.setProperties(modifiedProperties); + modifiedProperties = DeviceConfig.getProperties(namespaceToBan); + assertThat(modifiedProperties.getKeyset()).containsExactly(KEY, KEY2, KEY4); + assertThat(modifiedProperties.getString(KEY, DEFAULT_VALUE)).isEqualTo(VALUE); + assertThat(modifiedProperties.getString(KEY2, DEFAULT_VALUE)).isEqualTo(VALUE2); + // Since value is null DEFAULT_VALUE should be returned + assertThat(modifiedProperties.getString(KEY4, DEFAULT_VALUE)).isEqualTo(DEFAULT_VALUE); + } + + @Test + public void banEntireDeviceConfig() throws DeviceConfig.BadConfigException { + // Given namespaces will be permanently banned, thus they need to be different every time + final String namespaceToBan1 = NAMESPACE + System.currentTimeMillis(); + final String namespaceToBan2 = NAMESPACE + System.currentTimeMillis() + 1; + + // Set namespaces properties + Properties properties1 = new Properties.Builder(namespaceToBan1).setString(KEY, VALUE) + .setString(KEY4, NULL_VALUE).build(); + DeviceConfig.setProperties(properties1); + Properties properties2 = new Properties.Builder(namespaceToBan2).setString(KEY2, VALUE2) + .setString(KEY4, NULL_VALUE).build(); + DeviceConfig.setProperties(properties2); + + // Ban entire DeviceConfig + DeviceConfig.resetToDefaults(Settings.RESET_MODE_PACKAGE_DEFAULTS, null); + + // Verify given namespace properties are banned + assertThrows(DeviceConfig.BadConfigException.class, + () -> DeviceConfig.setProperties(properties1)); + assertThrows(DeviceConfig.BadConfigException.class, + () -> DeviceConfig.setProperties(properties2)); + + // Modify properties and verify we can set them + Properties modifiedProperties1 = new Properties.Builder(namespaceToBan1).setString(KEY, + VALUE) + .setString(KEY4, NULL_VALUE).setString(KEY2, VALUE2).build(); + DeviceConfig.setProperties(modifiedProperties1); + modifiedProperties1 = DeviceConfig.getProperties(namespaceToBan1); + assertThat(modifiedProperties1.getKeyset()).containsExactly(KEY, KEY2, KEY4); + assertThat(modifiedProperties1.getString(KEY, DEFAULT_VALUE)).isEqualTo(VALUE); + assertThat(modifiedProperties1.getString(KEY2, DEFAULT_VALUE)).isEqualTo(VALUE2); + // Since value is null DEFAULT_VALUE should be returned + assertThat(modifiedProperties1.getString(KEY4, DEFAULT_VALUE)).isEqualTo(DEFAULT_VALUE); + + Properties modifiedProperties2 = new Properties.Builder(namespaceToBan2).setString(KEY, + VALUE) + .setString(KEY4, NULL_VALUE).setString(KEY2, VALUE2).build(); + DeviceConfig.setProperties(modifiedProperties1); + modifiedProperties2 = DeviceConfig.getProperties(namespaceToBan1); + assertThat(modifiedProperties2.getKeyset()).containsExactly(KEY, KEY2, KEY4); + assertThat(modifiedProperties2.getString(KEY, DEFAULT_VALUE)).isEqualTo(VALUE); + assertThat(modifiedProperties2.getString(KEY2, DEFAULT_VALUE)).isEqualTo(VALUE2); + // Since value is null DEFAULT_VALUE should be returned + assertThat(modifiedProperties2.getString(KEY4, DEFAULT_VALUE)).isEqualTo(DEFAULT_VALUE); + } + // TODO(mpape): resolve b/142727848 and re-enable listener tests // @Test // public void onPropertiesChangedListener_setPropertyCallback() throws InterruptedException { diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java index 4309c8030dbd5..1e0c1d877f67d 100644 --- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java +++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java @@ -387,8 +387,10 @@ public class SettingsProvider extends ContentProvider { case Settings.CALL_METHOD_SET_ALL_CONFIG: { String prefix = getSettingPrefix(args); Map flags = getSettingFlags(args); - setAllConfigSettings(prefix, flags); - break; + Bundle result = new Bundle(); + result.putBoolean(Settings.KEY_CONFIG_SET_RETURN, + setAllConfigSettings(prefix, flags)); + return result; } case Settings.CALL_METHOD_RESET_CONFIG: { @@ -2666,20 +2668,28 @@ public class SettingsProvider extends ContentProvider { return success; } + /** + * Set Settings using consumed keyValues, returns true if the keyValues can be set, false + * otherwise. + */ public boolean setSettingsLocked(int type, int userId, String prefix, Map keyValues, String packageName) { final int key = makeKey(type, userId); SettingsState settingsState = peekSettingsStateLocked(key); if (settingsState != null) { + if (SETTINGS_TYPE_CONFIG == type && settingsState.isNewConfigBannedLocked(prefix, + keyValues)) { + return false; + } List changedSettings = settingsState.setSettingsLocked(prefix, keyValues, packageName); if (!changedSettings.isEmpty()) { notifyForConfigSettingsChangeLocked(key, prefix, changedSettings); } } - - return settingsState != null; + // keyValues aren't banned and can be set + return true; } public boolean deleteSettingLocked(int type, int userId, String name, boolean forceNotify, @@ -2739,7 +2749,8 @@ public class SettingsProvider extends ContentProvider { public void resetSettingsLocked(int type, int userId, String packageName, int mode, String tag) { - resetSettingsLocked(type, userId, packageName, mode, tag, null); + resetSettingsLocked(type, userId, packageName, mode, tag, /*prefix=*/ + null); } public void resetSettingsLocked(int type, int userId, String packageName, int mode, @@ -2750,6 +2761,7 @@ public class SettingsProvider extends ContentProvider { return; } + banConfigurationIfNecessary(type, prefix, settingsState); switch (mode) { case Settings.RESET_MODE_PACKAGE_DEFAULTS: { for (String name : settingsState.getSettingNamesLocked()) { @@ -3173,6 +3185,34 @@ public class SettingsProvider extends ContentProvider { return getTypeFromKey(key) == SETTINGS_TYPE_SSAID; } + private boolean shouldBan(int type) { + if (SETTINGS_TYPE_CONFIG != type) { + return false; + } + final int callingUid = Binder.getCallingUid(); + final int appId = UserHandle.getAppId(callingUid); + + // Only non-shell resets should result in namespace banning + return appId != SHELL_UID; + } + + private void banConfigurationIfNecessary(int type, @Nullable String prefix, + SettingsState settingsState) { + // Banning should be performed only for Settings.Config and for non-shell reset calls + if (!shouldBan(type)) { + return; + } + if (prefix != null) { + settingsState.banConfigurationLocked(prefix, getAllConfigFlags(prefix)); + } else { + Set configPrefixes = settingsState.getAllConfigPrefixesLocked(); + for (String configPrefix : configPrefixes) { + settingsState.banConfigurationLocked(configPrefix, + getAllConfigFlags(configPrefix)); + } + } + } + private File getSettingsFile(int key) { if (isConfigSettingsKey(key)) { final int userId = getUserIdFromKey(key); diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java index 086b20facb469..5b1b5305865eb 100644 --- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java +++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java @@ -65,9 +65,12 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; +import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Set; /** * This class contains the state for one type of settings. It is responsible @@ -109,6 +112,11 @@ final class SettingsState { private static final String ATTR_ID = "id"; private static final String ATTR_NAME = "name"; + private static final String TAG_NAMESPACE_HASHES = "namespaceHashes"; + private static final String TAG_NAMESPACE_HASH = "namespaceHash"; + private static final String ATTR_NAMESPACE = "namespace"; + private static final String ATTR_BANNED_HASH = "bannedHash"; + /** * Non-binary value will be written in this attributes. */ @@ -158,6 +166,9 @@ final class SettingsState { @GuardedBy("mLock") private final ArrayMap mSettings = new ArrayMap<>(); + @GuardedBy("mLock") + private final ArrayMap mNamespaceBannedHashes = new ArrayMap<>(); + @GuardedBy("mLock") private final ArrayMap mPackageToMemoryUsage; @@ -418,6 +429,41 @@ final class SettingsState { return true; } + @GuardedBy("mLock") + public boolean isNewConfigBannedLocked(String prefix, Map keyValues) { + // Replaces old style "null" String values with actual null's. This is done to simulate + // what will happen to String "null" values when they are written to Settings. This needs to + // be done here make sure that config hash computed during is banned check matches the + // one computed during banning when values are already stored. + keyValues = removeNullValueOldStyle(keyValues); + String bannedHash = mNamespaceBannedHashes.get(prefix); + if (bannedHash == null) { + return false; + } + return bannedHash.equals(hashCode(keyValues)); + } + + @GuardedBy("mLock") + public void banConfigurationLocked(String prefix, Map keyValues) { + if (prefix == null || keyValues.isEmpty()) { + return; + } + // The write is intentionally not scheduled here, banned hashes should and will be written + // when the related setting changes are written + mNamespaceBannedHashes.put(prefix, hashCode(keyValues)); + } + + @GuardedBy("mLock") + public Set getAllConfigPrefixesLocked() { + Set prefixSet = new HashSet<>(); + final int settingsCount = mSettings.size(); + for (int i = 0; i < settingsCount; i++) { + String name = mSettings.keyAt(i); + prefixSet.add(name.split("/")[0] + "/"); + } + return prefixSet; + } + // The settings provider must hold its lock when calling here. // Returns the list of keys which changed (added, updated, or deleted). @GuardedBy("mLock") @@ -710,10 +756,12 @@ final class SettingsState { boolean wroteState = false; final int version; final ArrayMap settings; + final ArrayMap namespaceBannedHashes; synchronized (mLock) { version = mVersion; settings = new ArrayMap<>(mSettings); + namespaceBannedHashes = new ArrayMap<>(mNamespaceBannedHashes); mDirty = false; mWriteScheduled = false; } @@ -756,8 +804,19 @@ final class SettingsState { + setting.getValue()); } } - serializer.endTag(null, TAG_SETTINGS); + + serializer.startTag(null, TAG_NAMESPACE_HASHES); + for (int i = 0; i < namespaceBannedHashes.size(); i++) { + String namespace = namespaceBannedHashes.keyAt(i); + String bannedHash = namespaceBannedHashes.get(namespace); + writeSingleNamespaceHash(serializer, namespace, bannedHash); + if (DEBUG_PERSISTENCE) { + Slog.i(LOG_TAG, "[PERSISTED] namespace=" + namespace + + ", bannedHash=" + bannedHash); + } + } + serializer.endTag(null, TAG_NAMESPACE_HASHES); serializer.endDocument(); destination.finishWrite(out); @@ -869,6 +928,21 @@ final class SettingsState { } } + private static void writeSingleNamespaceHash(XmlSerializer serializer, String namespace, + String bannedHashCode) throws IOException { + if (namespace == null || bannedHashCode == null) { + return; + } + serializer.startTag(null, TAG_NAMESPACE_HASH); + serializer.attribute(null, ATTR_NAMESPACE, namespace); + serializer.attribute(null, ATTR_BANNED_HASH, bannedHashCode); + serializer.endTag(null, TAG_NAMESPACE_HASH); + } + + private static String hashCode(Map keyValues) { + return Integer.toString(keyValues.hashCode()); + } + private String getValueAttribute(XmlPullParser parser, String attr, String base64Attr) { if (mVersion >= SETTINGS_VERSION_NEW_ENCODING) { final String value = parser.getAttributeValue(null, attr); @@ -939,6 +1013,8 @@ final class SettingsState { String tagName = parser.getName(); if (tagName.equals(TAG_SETTINGS)) { parseSettingsLocked(parser); + } else if (tagName.equals(TAG_NAMESPACE_HASHES)) { + parseNamespaceHash(parser); } } } @@ -982,6 +1058,37 @@ final class SettingsState { } } + @GuardedBy("mLock") + private void parseNamespaceHash(XmlPullParser parser) + throws IOException, XmlPullParserException { + + final int outerDepth = parser.getDepth(); + int type; + while ((type = parser.next()) != XmlPullParser.END_DOCUMENT + && (type != XmlPullParser.END_TAG || parser.getDepth() > outerDepth)) { + if (type == XmlPullParser.END_TAG || type == XmlPullParser.TEXT) { + continue; + } + + if (parser.getName().equals(TAG_NAMESPACE_HASH)) { + String namespace = parser.getAttributeValue(null, ATTR_NAMESPACE); + String bannedHashCode = parser.getAttributeValue(null, ATTR_BANNED_HASH); + mNamespaceBannedHashes.put(namespace, bannedHashCode); + } + } + } + + private static Map removeNullValueOldStyle(Map keyValues) { + Iterator> it = keyValues.entrySet().iterator(); + while (it.hasNext()) { + Map.Entry keyValueEntry = it.next(); + if (NULL_VALUE_OLD_STYLE.equals(keyValueEntry.getValue())) { + keyValueEntry.setValue(null); + } + } + return keyValues; + } + private final class MyHandler extends Handler { public static final int MSG_PERSIST_SETTINGS = 1;