Add READ_DEVICE_CONFIG permission check to DeviceConfig API.
Test: atest FrameworksCoreTests:DeviceConfigTest
atest FrameworksCoreTests:SettingsProviderTest
atest SettingsProviderTest:DeviceConfigServiceTest
Bug:117663715
Change-Id: I04226876ddf910945bf343d25fa3dd04ba8eab31
diff --git a/core/java/android/provider/DeviceConfig.java b/core/java/android/provider/DeviceConfig.java
index 4322a59..728d77e 100644
--- a/core/java/android/provider/DeviceConfig.java
+++ b/core/java/android/provider/DeviceConfig.java
@@ -27,6 +27,8 @@
import android.annotation.TestApi;
import android.app.ActivityThread;
import android.content.ContentResolver;
+import android.content.Context;
+import android.content.pm.PackageManager;
import android.database.ContentObserver;
import android.net.Uri;
import android.provider.Settings.ResetMode;
@@ -37,6 +39,7 @@
import com.android.internal.annotations.GuardedBy;
import com.android.internal.util.Preconditions;
+import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -257,6 +260,14 @@
public static final String NAMESPACE_TEXTCLASSIFIER = "textclassifier";
/**
+ * List of namespaces which can be read without READ_DEVICE_CONFIG permission
+ *
+ * @hide
+ */
+ @NonNull
+ private static final List<String> PUBLIC_NAMESPACES =
+ Arrays.asList(NAMESPACE_TEXTCLASSIFIER, NAMESPACE_RUNTIME);
+ /**
* Privacy related properties definitions.
*
* @hide
@@ -527,6 +538,8 @@
@NonNull String namespace,
@NonNull @CallbackExecutor Executor executor,
@NonNull OnPropertyChangedListener onPropertyChangedListener) {
+ enforceReadPermission(ActivityThread.currentApplication().getApplicationContext(),
+ namespace);
synchronized (sLock) {
Pair<String, Executor> oldNamespace = sSingleListeners.get(onPropertyChangedListener);
if (oldNamespace == null) {
@@ -566,6 +579,8 @@
@NonNull String namespace,
@NonNull @CallbackExecutor Executor executor,
@NonNull OnPropertiesChangedListener onPropertiesChangedListener) {
+ enforceReadPermission(ActivityThread.currentApplication().getApplicationContext(),
+ namespace);
synchronized (sLock) {
Pair<String, Executor> oldNamespace = sListeners.get(onPropertiesChangedListener);
if (oldNamespace == null) {
@@ -667,7 +682,7 @@
}
/**
- * Decrement the count used to represent th enumber of listeners subscribed to the given
+ * Decrement the count used to represent the number of listeners subscribed to the given
* namespace. If this is the final decrement call (i.e. decrementing from 1 to 0) for the given
* namespace, the ContentObserver that had been tracking it will be removed.
*
@@ -696,7 +711,14 @@
// pathSegments(0) is "config"
final String namespace = pathSegments.get(1);
final String name = pathSegments.get(2);
- final String value = getProperty(namespace, name);
+ final String value;
+ try {
+ value = getProperty(namespace, name);
+ } catch (SecurityException e) {
+ // Silently failing to not crash binder or listener threads.
+ Log.e(TAG, "OnPropertyChangedListener update failed: permission violation.");
+ return;
+ }
synchronized (sLock) {
// OnPropertiesChangedListeners
for (int i = 0; i < sListeners.size(); i++) {
@@ -730,6 +752,22 @@
}
}
+
+ /**
+ * Enforces READ_DEVICE_CONFIG permission if namespace is not one of public namespaces.
+ * @hide
+ */
+ public static void enforceReadPermission(Context context, String namespace) {
+ if (context.checkCallingOrSelfPermission(READ_DEVICE_CONFIG)
+ != PackageManager.PERMISSION_GRANTED) {
+ if (!PUBLIC_NAMESPACES.contains(namespace)) {
+ throw new SecurityException("Permission denial: reading from settings requires:"
+ + READ_DEVICE_CONFIG);
+ }
+ }
+ }
+
+
/**
* Interface for monitoring single property changes.
* <p>
diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java
index 7337cdb..e8d726e 100644
--- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java
+++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java
@@ -1074,8 +1074,7 @@
Slog.v(LOG_TAG, "getConfigSetting(" + name + ")");
}
- // TODO(b/117663715): Ensure the caller can access the setting.
- // enforceReadPermission(READ_DEVICE_CONFIG);
+ DeviceConfig.enforceReadPermission(getContext(), /*namespace=*/name.split("/")[0]);
// Get the value.
synchronized (mLock) {