Don't include restricted images in incident reports unless they're specifically mentioned in the IncidentReportArgs

Test: adb shell incident -p EXPLICIT -s com.google.android.incident.gts/.ReportReadyReceiver 3025
Test: adb shell incident -p EXPLICIT -s com.google.android.incident.gts/.ReportReadyReceiver
Bug: 123543706
Change-Id: I2c55831b73338f68196838ee529e595f566e657f

5 files changed, 46 insertions, 21 deletions

diff --git a/cmds/incidentd/src/PrivacyFilter.cpp b/cmds/incidentd/src/PrivacyFilter.cpp
index e8fa4f4ee58e..14ddec18cf1a 100644
--- a/cmds/incidentd/src/PrivacyFilter.cpp
+++ b/cmds/incidentd/src/PrivacyFilter.cpp
@@ -18,6 +18,10 @@
 
 #include "PrivacyFilter.h"
 
+#include "incidentd_util.h"
+#include "proto_util.h"
+#include "Section.h"
+
 #include <android-base/file.h>
 #include <android/util/ProtoFileReader.h>
 #include <android/util/protobuf.h>
@@ -25,8 +29,6 @@
 
 #include "cipher/IncidentKeyStore.h"
 #include "cipher/ProtoEncryption.h"
-#include "incidentd_util.h"
-#include "proto_util.h"
 
 namespace android {
 namespace os {
@@ -220,6 +222,11 @@ status_t FieldStripper::strip(const uint8_t privacyPolicy) {
 status_t FieldStripper::writeData(int fd) {
     status_t err = NO_ERROR;
     sp<ProtoReader> reader = mData;
+    if (mData == nullptr) {
+        // There had been an error processing the data. We won't write anything,
+        // but we also won't return an error, because errors are fatal.
+        return NO_ERROR;
+    }
     while (reader->readBuffer() != NULL) {
         err = WriteFully(fd, reader->readBuffer(), reader->currentToRead()) ? NO_ERROR : -errno;
         reader->move(reader->currentToRead());
@@ -316,7 +323,7 @@ status_t PrivacyFilter::writeData(const FdBuffer& buffer, uint8_t bufferLevel, s
             if (err != NO_ERROR) {
                 // We can't successfully strip this data.  We will skip
                 // the rest of this section.
-                return err;
+                return NO_ERROR;
             }
         }
 
@@ -367,8 +374,8 @@ status_t filter_and_write_report(int to, int from, uint8_t bufferLevel,
         uint64_t fieldTag = reader->readRawVarint();
         uint32_t fieldId = read_field_id(fieldTag);
         uint8_t wireType = read_wire_type(fieldTag);
-        if (wireType == WIRE_TYPE_LENGTH_DELIMITED && args.containsSection(fieldId)) {
-            VLOG("Read section %d", fieldId);
+        if (wireType == WIRE_TYPE_LENGTH_DELIMITED
+                && args.containsSection(fieldId, section_requires_specific_mention(fieldId))) {
             // We need this field, but we need to strip it to the level provided in args.
             PrivacyFilter filter(fieldId, get_privacy_of_section(fieldId));
             filter.addFd(new ReadbackFilterFd(args.getPrivacyPolicy(), to));
diff --git a/cmds/incidentd/src/Reporter.cpp b/cmds/incidentd/src/Reporter.cpp
index 322b97293a26..00a31e0de43f 100644
--- a/cmds/incidentd/src/Reporter.cpp
+++ b/cmds/incidentd/src/Reporter.cpp
@@ -66,22 +66,18 @@ IncidentMetadata_Destination privacy_policy_to_dest(uint8_t privacyPolicy) {
     }
 }
 
-void poo_make_metadata(IncidentMetadata* result, const IncidentMetadata& full,
-        int64_t reportId, int32_t privacyPolicy, const IncidentReportArgs& args) {
-    result->set_report_id(reportId);
-    result->set_dest(privacy_policy_to_dest(privacyPolicy));
 
-    size_t sectionCount = full.sections_size();
-    for (int sectionIndex = 0; sectionIndex < sectionCount; sectionIndex++) {
-        const IncidentMetadata::SectionStats& sectionStats = full.sections(sectionIndex);
-        if (args.containsSection(sectionStats.id())) {
-            *result->add_sections() = sectionStats;
-        }
-    }
+static bool contains_section(const IncidentReportArgs& args, int sectionId) {
+    return args.containsSection(sectionId, section_requires_specific_mention(sectionId));
+}
+
+static bool contains_section(const sp<ReportRequest>& args, int sectionId) {
+    return args->containsSection(sectionId);
 }
 
 // ARGS must have a containsSection(int) method
-template <typename ARGS> void make_metadata(IncidentMetadata* result, const IncidentMetadata& full,
+template <typename ARGS>
+void make_metadata(IncidentMetadata* result, const IncidentMetadata& full,
         int64_t reportId, int32_t privacyPolicy, ARGS args) {
     result->set_report_id(reportId);
     result->set_dest(privacy_policy_to_dest(privacyPolicy));
@@ -89,7 +85,7 @@ template <typename ARGS> void make_metadata(IncidentMetadata* result, const Inci
     size_t sectionCount = full.sections_size();
     for (int sectionIndex = 0; sectionIndex < sectionCount; sectionIndex++) {
         const IncidentMetadata::SectionStats& sectionStats = full.sections(sectionIndex);
-        if (args->containsSection(sectionStats.id())) {
+        if (contains_section(args, sectionStats.id())) {
             *result->add_sections() = sectionStats;
         }
     }
@@ -160,6 +156,10 @@ bool ReportRequest::ok() {
     return mFd >= 0 && mStatus == NO_ERROR;
 }
 
+bool ReportRequest::containsSection(int sectionId) const {
+    return args.containsSection(sectionId, section_requires_specific_mention(sectionId));
+}
+
 void ReportRequest::closeFd() {
     if (mIsStreaming && mFd >= 0) {
         close(mFd);
@@ -441,7 +441,9 @@ status_t ReportWriter::writeSection(const FdBuffer& buffer) {
 
     // Add the fds for the streamed requests
     mBatch->forEachStreamingRequest([&filter, this](const sp<ReportRequest>& request) {
-        if (request->ok() && request->args.containsSection(mCurrentSectionId)) {
+        if (request->ok()
+                && request->args.containsSection(mCurrentSectionId,
+                    section_requires_specific_mention(mCurrentSectionId))) {
             filter.addFd(new StreamingFilterFd(request->args.getPrivacyPolicy(),
                         request->getFd(), request));
         }
@@ -619,7 +621,7 @@ DONE:
         mBatch->getCombinedPersistedArgs(&combinedArgs);
         IncidentMetadata persistedMetadata;
         make_metadata(&persistedMetadata, metadata, mPersistedFile->getTimestampNs(),
-                persistedPrivacyPolicy, &combinedArgs);
+                persistedPrivacyPolicy, combinedArgs);
         mPersistedFile->setMetadata(persistedMetadata);
 
         mPersistedFile->markCompleted();
diff --git a/cmds/incidentd/src/Reporter.h b/cmds/incidentd/src/Reporter.h
index e7a474fde40c..5179f489ffb8 100644
--- a/cmds/incidentd/src/Reporter.h
+++ b/cmds/incidentd/src/Reporter.h
@@ -58,7 +58,7 @@ public:
 
     bool ok();  // returns true if the request is ok for write.
 
-    bool containsSection(int sectionId) const { return args.containsSection(sectionId); }
+    bool containsSection(int sectionId) const;
 
     sp<IIncidentReportStatusListener> getListener() { return mListener; }
 
diff --git a/cmds/incidentd/src/Section.cpp b/cmds/incidentd/src/Section.cpp
index 935a7c43fe90..85c5a20174c3 100644
--- a/cmds/incidentd/src/Section.cpp
+++ b/cmds/incidentd/src/Section.cpp
@@ -63,6 +63,15 @@ static pid_t fork_execute_incident_helper(const int id, Fpipe* p2cPipe, Fpipe* c
     return fork_execute_cmd(const_cast<char**>(ihArgs), p2cPipe, c2pPipe);
 }
 
+bool section_requires_specific_mention(int sectionId) {
+    switch (sectionId) {
+        case 3025: // restricted_images
+            return true;
+        default:
+            return false;
+    }
+}
+
 // ================================================================================
 Section::Section(int i, int64_t timeoutMs)
     : id(i),
diff --git a/cmds/incidentd/src/Section.h b/cmds/incidentd/src/Section.h
index cfe7e1648ad8..c9b80563a609 100644
--- a/cmds/incidentd/src/Section.h
+++ b/cmds/incidentd/src/Section.h
@@ -171,6 +171,13 @@ private:
     std::string mType;
 };
 
+
+/**
+ * These sections will not be generated when doing an 'all' report, either
+ * for size, speed of collection, or privacy.
+ */
+bool section_requires_specific_mention(int sectionId);
+
 }  // namespace incidentd
 }  // namespace os
 }  // namespace android