From 3a4059f7595ed40117e93d504c1112391b34ba50 Mon Sep 17 00:00:00 2001
From: Songchun Fan <schfan@google.com>
Date: Thu, 8 Apr 2021 01:22:49 +0000
Subject: [PATCH] [SettingsProvider] allow test_only apps access @hide keys w/o
 @Readable

+ Also improve error message

BUG: 183436286
BUG: 183709745
Test: atest android.appsecurity.cts.ReadableSettingsFieldsTest
Change-Id: I17224d213d707f6f359aa17f1b745bf508208de8
---
 core/java/android/provider/Settings.java       | 18 +++++++++++-------
 .../providers/settings/SettingsProvider.java   | 11 +++++++----
 2 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/core/java/android/provider/Settings.java b/core/java/android/provider/Settings.java
index c97f097e02c32..f3c37ff827af4 100644
--- a/core/java/android/provider/Settings.java
+++ b/core/java/android/provider/Settings.java
@@ -2790,13 +2790,12 @@ public final class Settings {
             // Settings.Global and is not annotated as @Readable.
             // Notice that a key string that is not defined in any of the Settings.* classes will
             // still be regarded as readable.
-            // TODO(b/175024829): provide a register method.
-            if (!Settings.isInSystemServer() && !isSystemOrPrivilegedApp()
+            if (!isCallerExemptFromReadableRestriction()
                     && mAllFields.contains(name) && !mReadableFields.contains(name)) {
                 throw new SecurityException(
-                        "Settings key: <" + name + "> is not readable. From S+, new public "
-                        + "settings keys need to be annotated with @Readable unless they are "
-                        + "annotated with @hide.");
+                        "Settings key: <" + name + "> is not readable. From S+, settings keys "
+                                + "annotated with @hide are restricted to system_server and system "
+                                + "apps only, unless they are annotated with @Readable.");
             }
             final boolean isSelf = (userHandle == UserHandle.myUserId());
             int currentGeneration = -1;
@@ -2972,7 +2971,10 @@ public final class Settings {
             }
         }
 
-        private static boolean isSystemOrPrivilegedApp() {
+        private static boolean isCallerExemptFromReadableRestriction() {
+            if (Settings.isInSystemServer()) {
+                return true;
+            }
             if (UserHandle.getAppId(Binder.getCallingUid()) < Process.FIRST_APPLICATION_UID) {
                 return true;
             }
@@ -2981,7 +2983,9 @@ public final class Settings {
                 return false;
             }
             final ApplicationInfo applicationInfo = application.getApplicationInfo();
-            return applicationInfo.isSystemApp() || applicationInfo.isPrivilegedApp()
+            final boolean isTestOnly =
+                    (applicationInfo.flags & ApplicationInfo.FLAG_TEST_ONLY) != 0;
+            return isTestOnly || applicationInfo.isSystemApp() || applicationInfo.isPrivilegedApp()
                     || applicationInfo.isSignedWithPlatformKey();
         }
 
diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java
index 081f3f6737021..91667c4e88c74 100644
--- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java
+++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java
@@ -1941,7 +1941,10 @@ public class SettingsProvider extends ContentProvider {
         if (ai.isSystemApp() || ai.isSignedWithPlatformKey()) {
             return;
         }
-        checkReadableAnnotation(settingsType, settingName);
+        if ((ai.flags & ApplicationInfo.FLAG_TEST_ONLY) == 0) {
+            // Skip checking readable annotations for test_only apps
+            checkReadableAnnotation(settingsType, settingName);
+        }
         if (!ai.isInstantApp()) {
             return;
         }
@@ -1983,9 +1986,9 @@ public class SettingsProvider extends ContentProvider {
 
         if (allFields.contains(settingName) && !readableFields.contains(settingName)) {
             throw new SecurityException(
-                    "Settings key: <" + settingName + "> is not readable. From S+, new public "
-                            + "settings keys need to be annotated with @Readable unless they are "
-                            + "annotated with @hide.");
+                    "Settings key: <" + settingName + "> is not readable. From S+, settings keys "
+                            + "annotated with @hide are restricted to system_server and system "
+                            + "apps only, unless they are annotated with @Readable.");
         }
     }