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
diff --git a/cmds/incidentd/src/PrivacyFilter.cpp b/cmds/incidentd/src/PrivacyFilter.cpp
index e8fa4f4..14ddec1 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::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 @@
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 @@
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 322b972..00a31e0 100644
--- a/cmds/incidentd/src/Reporter.cpp
+++ b/cmds/incidentd/src/Reporter.cpp
@@ -66,22 +66,18 @@
}
}
-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 @@
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 @@
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 @@
// 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 @@
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 e7a474f..5179f48 100644
--- a/cmds/incidentd/src/Reporter.h
+++ b/cmds/incidentd/src/Reporter.h
@@ -58,7 +58,7 @@
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 935a7c4..85c5a20 100644
--- a/cmds/incidentd/src/Section.cpp
+++ b/cmds/incidentd/src/Section.cpp
@@ -63,6 +63,15 @@
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 cfe7e16..c9b8056 100644
--- a/cmds/incidentd/src/Section.h
+++ b/cmds/incidentd/src/Section.h
@@ -171,6 +171,13 @@
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
diff --git a/libs/incident/include/android/os/IncidentReportArgs.h b/libs/incident/include/android/os/IncidentReportArgs.h
index 4391a9b..94b4ad6 100644
--- a/libs/incident/include/android/os/IncidentReportArgs.h
+++ b/libs/incident/include/android/os/IncidentReportArgs.h
@@ -53,7 +53,7 @@
void addHeader(const vector<uint8_t>& headerProto);
inline bool all() const { return mAll; }
- bool containsSection(int section) const;
+ bool containsSection(int section, bool specific) const;
inline int getPrivacyPolicy() const { return mPrivacyPolicy; }
inline const set<int>& sections() const { return mSections; }
inline const string& receiverPkg() const { return mReceiverPkg; }
diff --git a/libs/incident/src/IncidentReportArgs.cpp b/libs/incident/src/IncidentReportArgs.cpp
index 4268638..9d8a983 100644
--- a/libs/incident/src/IncidentReportArgs.cpp
+++ b/libs/incident/src/IncidentReportArgs.cpp
@@ -194,9 +194,13 @@
}
bool
-IncidentReportArgs::containsSection(int section) const
+IncidentReportArgs::containsSection(int section, bool specific) const
{
- return mAll || mSections.find(section) != mSections.end();
+ if (specific) {
+ return mSections.find(section) != mSections.end();
+ } else {
+ return mAll || mSections.find(section) != mSections.end();
+ }
}
void