Update contents of a telephony bug report.

Much of the current info is not suitable to share with carriers from
devices running user builds, so we strip much of it out.

We also show a progress notification for connectivity reports now to
satisfy system health requirements for a user-visible indicator.

Bug: 146521742
Test: take bug report on user build
Test: atest dumpstate_test
Change-Id: I21182e75dd85936c979e8b983aa03cb3b02420b7
diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp
index 4d98520..7cfd4d1 100644
--- a/cmds/dumpstate/dumpstate.cpp
+++ b/cmds/dumpstate/dumpstate.cpp
@@ -890,6 +890,14 @@
                CommandOptions::WithTimeoutInMs(timeout_ms).Build());
 }
 
+static void DoRadioLogcat() {
+    unsigned long timeout_ms = logcat_timeout({"radio"});
+    RunCommand(
+        "RADIO LOG",
+        {"logcat", "-b", "radio", "-v", "threadtime", "-v", "printable", "-v", "uid", "-d", "*:v"},
+        CommandOptions::WithTimeoutInMs(timeout_ms).Build(), true /* verbose_duration */);
+}
+
 static void DoLogcat() {
     unsigned long timeout_ms;
     // DumpFile("EVENT LOG TAGS", "/etc/event-log-tags");
@@ -908,11 +916,7 @@
         "STATS LOG",
         {"logcat", "-b", "stats", "-v", "threadtime", "-v", "printable", "-v", "uid", "-d", "*:v"},
         CommandOptions::WithTimeoutInMs(timeout_ms).Build(), true /* verbose_duration */);
-    timeout_ms = logcat_timeout({"radio"});
-    RunCommand(
-        "RADIO LOG",
-        {"logcat", "-b", "radio", "-v", "threadtime", "-v", "printable", "-v", "uid", "-d", "*:v"},
-        CommandOptions::WithTimeoutInMs(timeout_ms).Build(), true /* verbose_duration */);
+    DoRadioLogcat();
 
     RunCommand("LOG STATISTICS", {"logcat", "-b", "all", "-S"});
 
@@ -1599,8 +1603,10 @@
     return status;
 }
 
-// This method collects common dumpsys for telephony and wifi
-static void DumpstateRadioCommon() {
+// This method collects common dumpsys for telephony and wifi. Typically, wifi
+// reports are fine to include all information, but telephony reports on user
+// builds need to strip some content (see DumpstateTelephonyOnly).
+static void DumpstateRadioCommon(bool include_sensitive_info = true) {
     DumpIpTablesAsRoot();
 
     ds.AddDir(LOGPERSIST_DATA_DIR, false);
@@ -1609,26 +1615,51 @@
         return;
     }
 
-    do_dmesg();
-    DoLogcat();
+    // We need to be picky about some stuff for telephony reports on user builds.
+    if (!include_sensitive_info) {
+        // Only dump the radio log buffer (other buffers and dumps contain too much unrelated info).
+        DoRadioLogcat();
+    } else {
+        // Contains various system properties and process startup info.
+        do_dmesg();
+        // Logs other than the radio buffer may contain package/component names and potential PII.
+        DoLogcat();
+        // Too broad for connectivity problems.
+        DoKmsg();
+        // Contains unrelated hardware info (camera, NFC, biometrics, ...).
+        DumpHals();
+    }
+
     DumpPacketStats();
-    DoKmsg();
     DumpIpAddrAndRules();
     dump_route_tables();
-    DumpHals();
-
     RunDumpsys("NETWORK DIAGNOSTICS", {"connectivity", "--diag"},
                CommandOptions::WithTimeout(10).Build());
 }
 
-// This method collects dumpsys for telephony debugging only
+// We use "telephony" here for legacy reasons, though this now really means "connectivity" (cellular
+// + wifi + networking). This method collects dumpsys for connectivity debugging only. General rules
+// for what can be included on user builds: all reported information MUST directly relate to
+// connectivity debugging or customer support and MUST NOT contain unrelated personally identifiable
+// information. This information MUST NOT identify user-installed packages (UIDs are OK, package
+// names are not), and MUST NOT contain logs of user application traffic.
+// TODO(b/148168577) rename this and other related fields/methods to "connectivity" instead.
 static void DumpstateTelephonyOnly() {
     DurationReporter duration_reporter("DUMPSTATE");
+
     const CommandOptions DUMPSYS_COMPONENTS_OPTIONS = CommandOptions::WithTimeout(60).Build();
 
-    DumpstateRadioCommon();
+    const bool include_sensitive_info = !PropertiesHelper::IsUserBuild();
 
-    RunCommand("SYSTEM PROPERTIES", {"getprop"});
+    DumpstateRadioCommon(include_sensitive_info);
+
+    if (include_sensitive_info) {
+        // Contains too much unrelated PII, and given the unstructured nature of sysprops, we can't
+        // really cherrypick all of the connectivity-related ones. Apps generally have no business
+        // reading these anyway, and there should be APIs to supply the info in a more app-friendly
+        // way.
+        RunCommand("SYSTEM PROPERTIES", {"getprop"});
+    }
 
     printf("========================================================\n");
     printf("== Android Framework Services\n");
@@ -1636,15 +1667,28 @@
 
     RunDumpsys("DUMPSYS", {"connectivity"}, CommandOptions::WithTimeout(90).Build(),
                SEC_TO_MSEC(10));
-    RunDumpsys("DUMPSYS", {"connmetrics"}, CommandOptions::WithTimeout(90).Build(),
-               SEC_TO_MSEC(10));
-    RunDumpsys("DUMPSYS", {"netd"}, CommandOptions::WithTimeout(90).Build(), SEC_TO_MSEC(10));
+    // TODO(b/146521742) build out an argument to include bound services here for user builds
     RunDumpsys("DUMPSYS", {"carrier_config"}, CommandOptions::WithTimeout(90).Build(),
                SEC_TO_MSEC(10));
     RunDumpsys("DUMPSYS", {"wifi"}, CommandOptions::WithTimeout(90).Build(),
                SEC_TO_MSEC(10));
-    RunDumpsys("BATTERYSTATS", {"batterystats"}, CommandOptions::WithTimeout(90).Build(),
+    RunDumpsys("DUMPSYS", {"netpolicy"}, CommandOptions::WithTimeout(90).Build(), SEC_TO_MSEC(10));
+    RunDumpsys("DUMPSYS", {"network_management"}, CommandOptions::WithTimeout(90).Build(),
                SEC_TO_MSEC(10));
+    if (include_sensitive_info) {
+        // Contains raw IP addresses, omit from reports on user builds.
+        RunDumpsys("DUMPSYS", {"netd"}, CommandOptions::WithTimeout(90).Build(), SEC_TO_MSEC(10));
+        // Contains raw destination IP/MAC addresses, omit from reports on user builds.
+        RunDumpsys("DUMPSYS", {"connmetrics"}, CommandOptions::WithTimeout(90).Build(),
+                   SEC_TO_MSEC(10));
+        // Contains package/component names, omit from reports on user builds.
+        RunDumpsys("BATTERYSTATS", {"batterystats"}, CommandOptions::WithTimeout(90).Build(),
+                   SEC_TO_MSEC(10));
+        // Contains package names, but should be relatively simple to remove them (also contains
+        // UIDs already), omit from reports on user builds.
+        RunDumpsys("BATTERYSTATS", {"deviceidle"}, CommandOptions::WithTimeout(90).Build(),
+                   SEC_TO_MSEC(10));
+    }
 
     printf("========================================================\n");
     printf("== Running Application Services\n");
@@ -1652,18 +1696,24 @@
 
     RunDumpsys("TELEPHONY SERVICES", {"activity", "service", "TelephonyDebugService"});
 
-    printf("========================================================\n");
-    printf("== Running Application Services (non-platform)\n");
-    printf("========================================================\n");
+    if (include_sensitive_info) {
+        printf("========================================================\n");
+        printf("== Running Application Services (non-platform)\n");
+        printf("========================================================\n");
 
-    RunDumpsys("APP SERVICES NON-PLATFORM", {"activity", "service", "all-non-platform"},
-            DUMPSYS_COMPONENTS_OPTIONS);
+        // Contains package/component names and potential PII, omit from reports on user builds.
+        // To get dumps of the active CarrierService(s) on user builds, we supply an argument to the
+        // carrier_config dumpsys instead.
+        RunDumpsys("APP SERVICES NON-PLATFORM", {"activity", "service", "all-non-platform"},
+                   DUMPSYS_COMPONENTS_OPTIONS);
 
-    printf("========================================================\n");
-    printf("== Checkins\n");
-    printf("========================================================\n");
+        printf("========================================================\n");
+        printf("== Checkins\n");
+        printf("========================================================\n");
 
-    RunDumpsys("CHECKIN BATTERYSTATS", {"batterystats", "-c"});
+        // Contains package/component names, omit from reports on user builds.
+        RunDumpsys("CHECKIN BATTERYSTATS", {"batterystats", "-c"});
+    }
 
     printf("========================================================\n");
     printf("== dumpstate: done (id %d)\n", ds.id_);
@@ -2275,6 +2325,7 @@
             break;
         case Dumpstate::BugreportMode::BUGREPORT_TELEPHONY:
             options->telephony_only = true;
+            options->do_progress_updates = true;
             options->do_fb = false;
             options->do_broadcast = true;
             break;