Revert^3 "Trace Redaction - Reject invalid traces" This reverts commit 719510ee9ccdf29748623245a56b315bade35c32. Reason for revert: The test calling system trace is providing an invalid trace, resulting in the test failing now that the trace is verified before redacting. Change-Id: Id4c849e2c24421258be22b5861bbc7c430c527fc
diff --git a/Android.bp b/Android.bp index 9aa955d..54b1f20 100644 --- a/Android.bp +++ b/Android.bp
@@ -13589,7 +13589,6 @@ "src/trace_redaction/suspend_resume.cc", "src/trace_redaction/trace_redaction_framework.cc", "src/trace_redaction/trace_redactor.cc", - "src/trace_redaction/verify_integrity.cc", ], }
diff --git a/src/trace_redaction/BUILD.gn b/src/trace_redaction/BUILD.gn index ee876b0..b4aecb6 100644 --- a/src/trace_redaction/BUILD.gn +++ b/src/trace_redaction/BUILD.gn
@@ -81,8 +81,6 @@ "trace_redaction_framework.h", "trace_redactor.cc", "trace_redactor.h", - "verify_integrity.cc", - "verify_integrity.h", ] deps = [ "../../gn:default_deps",
diff --git a/src/trace_redaction/main.cc b/src/trace_redaction/main.cc index 798d1a0..78bf3d0 100644 --- a/src/trace_redaction/main.cc +++ b/src/trace_redaction/main.cc
@@ -39,7 +39,6 @@ #include "src/trace_redaction/suspend_resume.h" #include "src/trace_redaction/trace_redaction_framework.h" #include "src/trace_redaction/trace_redactor.h" -#include "src/trace_redaction/verify_integrity.h" namespace perfetto::trace_redaction { @@ -49,12 +48,6 @@ std::string_view package_name) { TraceRedactor redactor; - // VerifyIntegrity breaks the CollectPrimitive pattern. Instead of writing to - // the context, its job is to read trace packets and return errors if any - // packet does not look "correct". This primitive is added first in an effort - // to detect and react to bad input before other collectors run. - redactor.emplace_collect<VerifyIntegrity>(); - // Add all collectors. redactor.emplace_collect<FindPackageUid>(); redactor.emplace_collect<CollectTimelineEvents>();
diff --git a/src/trace_redaction/verify_integrity.cc b/src/trace_redaction/verify_integrity.cc deleted file mode 100644 index a2d58ce..0000000 --- a/src/trace_redaction/verify_integrity.cc +++ /dev/null
@@ -1,103 +0,0 @@ -/* - * Copyright (C) 2024 The Android Open Source Projectf - * - * Licensed under the Apache License, Version 2.0 (the "License"); - - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include "src/trace_redaction/verify_integrity.h" - -#include "protos/perfetto/trace/ftrace/ftrace_event.pbzero.h" -#include "protos/perfetto/trace/ftrace/ftrace_event_bundle.pbzero.h" -#include "protos/perfetto/trace/trace_packet.pbzero.h" -#include "src/trace_processor/util/status_macros.h" - -namespace perfetto::trace_redaction { -namespace { -// All constants are from -// "system/core/libcutils/include/private/android_filesystem_config.h" -// -// AID 1000 == system (those are probably frame_timeline packets you will see -// those on) AID 9999 is nobody == traced/traced_probes -constexpr int32_t kAidSystem = 1000; -constexpr int32_t kAidNobody = 9999; -} // namespace - -base::Status VerifyIntegrity::Collect( - const protos::pbzero::TracePacket::Decoder& packet, - Context*) const { - if (!packet.has_trusted_uid()) { - return base::ErrStatus( - "VerifyIntegrity: missing field (TracePacket.trusted_uid)."); - } - - if (packet.trusted_uid() != kAidSystem && - packet.trusted_uid() != kAidNobody) { - return base::ErrStatus( - "VerifyIntegrity: invalid field value (TracePacket.trusted_uid)."); - } - - if (packet.has_ftrace_events()) { - protos::pbzero::FtraceEventBundle::Decoder ftrace_events( - packet.ftrace_events()); - - // The other clocks in ftrace are only used on very old kernel versions. No - // device with V should have such an old version. As a failsafe though, - // check that the ftrace_clock field is unset to ensure no invalid - // timestamps get by. - if (ftrace_events.has_ftrace_clock()) { - return base::ErrStatus( - "VerifyIntegrity: unexpected field " - "(FtraceEventBundle::kFtraceClockFieldNumber)."); - } - - // Every ftrace event bundle should have a CPU field. This is necessary for - // switch/waking redaction to work. - if (!ftrace_events.has_cpu()) { - return base::ErrStatus( - "VerifyIntegrity: missing field " - "(FtraceEventBundle::kCpuFieldNumber)."); - } - - RETURN_IF_ERROR(VerifyFtraceEventsTime(ftrace_events)); - } - - return base::OkStatus(); -} - -base::Status VerifyIntegrity::VerifyFtraceEventsTime( - const protos::pbzero::FtraceEventBundle::Decoder& bundle) const { - // If a bundle has ftrace events, the events will contain the time stamps. - // However, there are no ftrace events, the timestamp will be in the bundle. - if (!bundle.has_event() && !bundle.has_ftrace_timestamp()) { - return base::ErrStatus( - "VerifyIntegrity: missing field " - "(FtraceEventBundle::kFtraceTimestampFieldNumber)."); - } - - for (auto event_buffer = bundle.event(); event_buffer; ++event_buffer) { - protos::pbzero::FtraceEvent::Decoder event(*event_buffer); - - if (!protozero::ProtoDecoder(*event_buffer) - .FindField(protos::pbzero::FtraceEvent::kTimestampFieldNumber) - .valid()) { - return base::ErrStatus( - "VerifyIntegrity: missing field " - "(FtraceEvent::kTimestampFieldNumber)"); - } - } - - return base::OkStatus(); -} - -} // namespace perfetto::trace_redaction
diff --git a/src/trace_redaction/verify_integrity.h b/src/trace_redaction/verify_integrity.h deleted file mode 100644 index 6548ce5..0000000 --- a/src/trace_redaction/verify_integrity.h +++ /dev/null
@@ -1,42 +0,0 @@ -/* - * Copyright (C) 2024 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include "protos/perfetto/trace/ftrace/ftrace_event_bundle.pbzero.h" -#include "src/trace_redaction/trace_redaction_framework.h" - -#include "protos/perfetto/trace/trace_packet.pbzero.h" - -#ifndef SRC_TRACE_REDACTION_VERIFY_INTEGRITY_H_ -#define SRC_TRACE_REDACTION_VERIFY_INTEGRITY_H_ - -namespace perfetto::trace_redaction { - -// This breaks the normal collect primitive pattern. Rather than collecting -// information, it looks at packets and returns an error if the packet violates -// any requirements. -class VerifyIntegrity : public CollectPrimitive { - public: - base::Status Collect(const protos::pbzero::TracePacket::Decoder& packet, - Context* context) const override; - - private: - base::Status VerifyFtraceEventsTime( - const protos::pbzero::FtraceEventBundle::Decoder& bundle) const; -}; - -} // namespace perfetto::trace_redaction - -#endif // SRC_TRACE_REDACTION_VERIFY_INTEGRITY_H_