From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Kevin Jeon <kevinjeon@google.com>
Date: Fri, 17 Feb 2023 20:17:54 +0000
Subject: [PATCH] Update Traceur to check admin user status

This change updates Traceur to check for admin user privileges wherever
a developer options check occurs. This is intended to address the case
in which developer options (a global setting not differentiated on
current user privileges) being enabled would allow guest users to open
Traceur through a 3P app and view its trace files. This would previously
be possible even when ADB debugging was disabled by the admin user.

Traceur now listens for user changes so that its document root
(containing traces) is enabled/disabled based on the new user's admin
status.

This change also includes a partial fix for a previous less-severe
security vulnerability (developer options checks; terminating ongoing
traces if the state of developer options changes). The entire fix is not
included because the full vulnerability did not exist in this branch.

Because Traceur is a platform app in R, a separate change to grant
MANAGE_USERS access in the privapp permissions allowlist is /not/
required (as in S).

Test: On a local CF build (cf_x86_64_phone-userdebug), explictly
      enable multi-user + apply aosp/1625022 and check that:
      - CtsIntentSignatureTestCases passes (b/270791503)
      - TraceurUiTests passes
      - Traceur cannot be opened through 'am start' on a guest account
      - Opening Files on a guest account no longer shows a System Traces
        folder (even if Traceur's onCreate is somehow called)
      - System tracing no longer appears in settings for guests
Bug: 262243665
Bug: 262244249
Bug: 204992293
Bug: 160155846
Ignore-AOSP-First: Internal-first security fix
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:0c0b30d30dfad851cdce35e1b9097b62fffabc5f)
Merged-In: I14ee42d18802e7869bae8cb437c4d0b65dbea999
Change-Id: I14ee42d18802e7869bae8cb437c4d0b65dbea999
---
 AndroidManifest.xml                           |  4 +++
 .../google/android/traceur/MainActivity.java  |  5 ++-
 .../android/traceur/MainTvActivity.java       |  5 ++-
 src/com/google/android/traceur/Receiver.java  | 33 ++++++++++++++-----
 .../android/traceur/SearchProvider.java       |  7 ++--
 .../android/traceur/StopTraceService.java     | 16 +++++++++
 .../android/traceur/StorageProvider.java      |  8 +++--
 .../google/android/traceur/TraceService.java  | 31 +++++++++++++++++
 8 files changed, 94 insertions(+), 15 deletions(-)

diff --git a/AndroidManifest.xml b/AndroidManifest.xml
index 88c7c50..4050cfd 100644
--- a/AndroidManifest.xml
+++ b/AndroidManifest.xml
@@ -29,6 +29,9 @@
     <!-- Used for brief periods where the trace service is foregrounded. -->
     <uses-permission android:name="android.permission.FOREGROUND_SERVICE" />
 
+    <!-- Used to check that the current user is an admin user. -->
+    <uses-permission android:name="android.permission.MANAGE_USERS" />
+
     <!-- Declare Android TV support. -->
     <uses-feature android:name="android.software.leanback" android:required="false" />
 
@@ -82,6 +85,7 @@
         <receiver android:name=".Receiver" android:permission="android.permission.DUMP">
             <intent-filter>
                 <action android:name="android.intent.action.BOOT_COMPLETED" />
+                <action android:name="android.intent.action.USER_FOREGROUND"/>
             </intent-filter>
         </receiver>
 
diff --git a/src/com/google/android/traceur/MainActivity.java b/src/com/google/android/traceur/MainActivity.java
index be14223..72e6aba 100644
--- a/src/com/google/android/traceur/MainActivity.java
+++ b/src/com/google/android/traceur/MainActivity.java
@@ -17,6 +17,7 @@ package com.android.traceur;
 
 import android.app.Activity;
 import android.os.Bundle;
+import android.os.UserManager;
 import android.provider.Settings;
 
 public class MainActivity extends Activity {
@@ -32,8 +33,10 @@ public class MainActivity extends Activity {
         boolean developerOptionsIsEnabled =
             Settings.Global.getInt(getApplicationContext().getContentResolver(),
                 Settings.Global.DEVELOPMENT_SETTINGS_ENABLED, 0) != 0;
+        boolean isAdminUser = getApplicationContext()
+                .getSystemService(UserManager.class).isAdminUser();
 
-        if (!developerOptionsIsEnabled) {
+        if (!developerOptionsIsEnabled || !isAdminUser) {
             finish();
         }
     }
diff --git a/src/com/google/android/traceur/MainTvActivity.java b/src/com/google/android/traceur/MainTvActivity.java
index 91f67c4..7459b7a 100644
--- a/src/com/google/android/traceur/MainTvActivity.java
+++ b/src/com/google/android/traceur/MainTvActivity.java
@@ -17,6 +17,7 @@ package com.android.traceur;
 
 import android.app.Activity;
 import android.os.Bundle;
+import android.os.UserManager;
 import android.provider.Settings;
 
 public class MainTvActivity extends Activity {
@@ -32,8 +33,10 @@ public class MainTvActivity extends Activity {
         boolean developerOptionsIsEnabled =
             Settings.Global.getInt(getApplicationContext().getContentResolver(),
                 Settings.Global.DEVELOPMENT_SETTINGS_ENABLED, 0) != 0;
+        boolean isAdminUser = getApplicationContext()
+                .getSystemService(UserManager.class).isAdminUser();
 
-        if (!developerOptionsIsEnabled) {
+        if (!developerOptionsIsEnabled || !isAdminUser) {
             finish();
         }
     }
diff --git a/src/com/google/android/traceur/Receiver.java b/src/com/google/android/traceur/Receiver.java
index 4f3415d..36b5ab5 100644
--- a/src/com/google/android/traceur/Receiver.java
+++ b/src/com/google/android/traceur/Receiver.java
@@ -32,6 +32,7 @@ import android.os.Build;
 import android.os.Handler;
 import android.os.RemoteException;
 import android.os.ServiceManager;
+import android.os.UserManager;
 import android.preference.PreferenceManager;
 import android.provider.Settings;
 import android.text.TextUtils;
@@ -79,6 +80,12 @@ public class Receiver extends BroadcastReceiver {
             // We know that Perfetto won't be tracing already at boot, so pass the
             // tracingIsOff argument to avoid the Perfetto check.
             updateTracing(context, /* assumeTracingIsOff= */ true);
+        } else if (Intent.ACTION_USER_FOREGROUND.equals(intent.getAction())) {
+            boolean developerOptionsEnabled = (1 ==
+                Settings.Global.getInt(context.getContentResolver(),
+                    Settings.Global.DEVELOPMENT_SETTINGS_ENABLED , 0));
+            boolean isAdminUser = context.getSystemService(UserManager.class).isAdminUser();
+            updateStorageProvider(context, developerOptionsEnabled && isAdminUser);
         } else if (STOP_ACTION.equals(intent.getAction())) {
             prefs.edit().putBoolean(context.getString(R.string.pref_key_tracing_on), false).commit();
             updateTracing(context);
@@ -194,14 +201,9 @@ public class Receiver extends BroadcastReceiver {
                         boolean developerOptionsEnabled = (1 ==
                             Settings.Global.getInt(context.getContentResolver(),
                                 Settings.Global.DEVELOPMENT_SETTINGS_ENABLED , 0));
-
-                        ComponentName name = new ComponentName(context,
-                            StorageProvider.class);
-                        context.getPackageManager().setComponentEnabledSetting(name,
-                           developerOptionsEnabled
-                                ? PackageManager.COMPONENT_ENABLED_STATE_ENABLED
-                                : PackageManager.COMPONENT_ENABLED_STATE_DISABLED,
-                            PackageManager.DONT_KILL_APP);
+                        boolean isAdminUser = context.getSystemService(UserManager.class)
+                                .isAdminUser();
+                        updateStorageProvider(context, developerOptionsEnabled && isAdminUser);
 
                         if (!developerOptionsEnabled) {
                             SharedPreferences prefs =
@@ -210,6 +212,10 @@ public class Receiver extends BroadcastReceiver {
                                 context.getString(R.string.pref_key_quick_setting), false)
                                 .commit();
                             updateQuickSettings(context);
+                            // Stop an ongoing trace if one exists.
+                            if (TraceUtils.isTracingOn()) {
+                                TraceService.stopTracingWithoutSaving(context);
+                            }
                         }
                     }
                 };
@@ -220,6 +226,17 @@ public class Receiver extends BroadcastReceiver {
         }
     }
 
+    // Enables/disables the System Traces storage component. enableProvider should be true iff
+    // developer options are enabled and the current user is an admin user.
+    static void updateStorageProvider(Context context, boolean enableProvider) {
+        ComponentName name = new ComponentName(context, StorageProvider.class);
+        context.getPackageManager().setComponentEnabledSetting(name,
+                enableProvider
+                        ? PackageManager.COMPONENT_ENABLED_STATE_ENABLED
+                        : PackageManager.COMPONENT_ENABLED_STATE_DISABLED,
+                PackageManager.DONT_KILL_APP);
+    }
+
     private static void postCategoryNotification(Context context, SharedPreferences prefs) {
         Intent sendIntent = new Intent(context, MainActivity.class);
 
diff --git a/src/com/google/android/traceur/SearchProvider.java b/src/com/google/android/traceur/SearchProvider.java
index 202d2b0..9098e89 100644
--- a/src/com/google/android/traceur/SearchProvider.java
+++ b/src/com/google/android/traceur/SearchProvider.java
@@ -31,6 +31,7 @@ import android.content.Context;
 import android.content.Intent;
 import android.database.Cursor;
 import android.database.MatrixCursor;
+import android.os.UserManager;
 import android.provider.SearchIndexablesProvider;
 import android.provider.Settings;
 
@@ -69,9 +70,11 @@ public class SearchProvider extends SearchIndexablesProvider {
         boolean developerOptionsIsEnabled =
             Settings.Global.getInt(getContext().getContentResolver(),
                 Settings.Global.DEVELOPMENT_SETTINGS_ENABLED, 0) != 0;
+        boolean isAdminUser = getContext().getSystemService(UserManager.class).isAdminUser();
 
-        // If developer options is not enabled, System Tracing shouldn't be searchable.
-        if (!developerOptionsIsEnabled) {
+        // System Tracing shouldn't be searchable if developer options are not enabled or if the
+        // user is not an admin.
+        if (!developerOptionsIsEnabled || !isAdminUser) {
             MatrixCursor cursor = new MatrixCursor(NON_INDEXABLES_KEYS_COLUMNS);
             Object[] row = new Object[] {getContext().getString(R.string.system_tracing)};
             cursor.addRow(row);
diff --git a/src/com/google/android/traceur/StopTraceService.java b/src/com/google/android/traceur/StopTraceService.java
index a8dd636..ed20906 100644
--- a/src/com/google/android/traceur/StopTraceService.java
+++ b/src/com/google/android/traceur/StopTraceService.java
@@ -20,7 +20,10 @@ package com.android.traceur;
 import android.content.Context;
 import android.content.Intent;
 import android.content.SharedPreferences;
+import android.os.UserManager;
 import android.preference.PreferenceManager;
+import android.provider.Settings;
+import android.util.EventLog;
 import android.util.Log;
 
 public class StopTraceService extends TraceService {
@@ -38,6 +41,19 @@ public class StopTraceService extends TraceService {
     @Override
     public void onHandleIntent(Intent intent) {
         Context context = getApplicationContext();
+        // Checks that developer options are enabled and the user is an admin before continuing.
+        boolean developerOptionsEnabled =
+                Settings.Global.getInt(context.getContentResolver(),
+                        Settings.Global.DEVELOPMENT_SETTINGS_ENABLED, 0) != 0;
+        if (!developerOptionsEnabled) {
+            // Refer to b/204992293.
+            EventLog.writeEvent(0x534e4554, "204992293", -1, "");
+            return;
+        }
+        boolean isAdminUser = context.getSystemService(UserManager.class).isAdminUser();
+        if (!isAdminUser) {
+            return;
+        }
         SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(context);
         boolean prefsTracingOn =
             prefs.getBoolean(context.getString(R.string.pref_key_tracing_on), false);
diff --git a/src/com/google/android/traceur/StorageProvider.java b/src/com/google/android/traceur/StorageProvider.java
index 1a92b61..e27b450 100644
--- a/src/com/google/android/traceur/StorageProvider.java
+++ b/src/com/google/android/traceur/StorageProvider.java
@@ -23,6 +23,7 @@ import android.os.Bundle;
 import android.os.FileUtils;
 import android.os.CancellationSignal;
 import android.os.ParcelFileDescriptor;
+import android.os.UserManager;
 import android.provider.DocumentsContract;
 import android.provider.DocumentsContract.Document;
 import android.provider.DocumentsContract.Root;
@@ -78,10 +79,11 @@ public class StorageProvider extends FileSystemProvider{
         boolean developerOptionsIsEnabled =
             Settings.Global.getInt(getContext().getContentResolver(),
                 Settings.Global.DEVELOPMENT_SETTINGS_ENABLED, 0) != 0;
+        boolean isAdminUser = getContext().getSystemService(UserManager.class).isAdminUser();
 
-        // If developer options is not enabled, return an empty root cursor.
-        // This removes the provider from the list entirely.
-        if (!developerOptionsIsEnabled) {
+        // If developer options is not enabled or the user is not an admin, return an empty root
+        // cursor. This removes the provider from the list entirely.
+        if (!developerOptionsIsEnabled || !isAdminUser) {
             return null;
         }
 
diff --git a/src/com/google/android/traceur/TraceService.java b/src/com/google/android/traceur/TraceService.java
index dc567b6..e3a3c3c 100644
--- a/src/com/google/android/traceur/TraceService.java
+++ b/src/com/google/android/traceur/TraceService.java
@@ -24,8 +24,12 @@ import android.app.PendingIntent;
 import android.app.Service;
 import android.content.Context;
 import android.content.Intent;
+import android.content.SharedPreferences;
 import android.content.pm.PackageManager;
+import android.os.UserManager;
 import android.preference.PreferenceManager;
+import android.provider.Settings;
+import android.util.EventLog;
 import android.util.Log;
 
 import java.io.File;
@@ -69,6 +73,20 @@ public class TraceService extends IntentService {
         context.startForegroundService(intent);
     }
 
+    // Silently stops a trace without saving it. This is intended to be called when tracing is no
+    // longer allowed, i.e. if developer options are turned off while tracing. The usual method of
+    // stopping a trace via intent, stopTracing(), will not work because intents cannot be received
+    // when developer options are disabled.
+    static void stopTracingWithoutSaving(final Context context) {
+        NotificationManager notificationManager =
+            context.getSystemService(NotificationManager.class);
+        notificationManager.cancel(TRACE_NOTIFICATION);
+        SharedPreferences prefs = PreferenceManager.getDefaultSharedPreferences(context);
+        prefs.edit().putBoolean(context.getString(
+            R.string.pref_key_tracing_on), false).commit();
+        TraceUtils.traceStop();
+    }
+
     public TraceService() {
         this("TraceService");
     }
@@ -81,6 +99,19 @@ public class TraceService extends IntentService {
     @Override
     public void onHandleIntent(Intent intent) {
         Context context = getApplicationContext();
+        // Checks that developer options are enabled and the user is an admin before continuing.
+        boolean developerOptionsEnabled =
+                Settings.Global.getInt(context.getContentResolver(),
+                        Settings.Global.DEVELOPMENT_SETTINGS_ENABLED, 0) != 0;
+        if (!developerOptionsEnabled) {
+            // Refer to b/204992293.
+            EventLog.writeEvent(0x534e4554, "204992293", -1, "");
+            return;
+        }
+        boolean isAdminUser = context.getSystemService(UserManager.class).isAdminUser();
+        if (!isAdminUser) {
+            return;
+        }
 
         if (intent.getAction().equals(INTENT_ACTION_START_TRACING)) {
             startTracingInternal(intent.getStringArrayListExtra(INTENT_EXTRA_TAGS),
