From 9a033327c6382bdd78412ff9f60e512a19a4129a Mon Sep 17 00:00:00 2001 From: Xufei Tan Date: Fri, 31 Jan 2025 15:13:39 -0800 Subject: [PATCH] Make DebugString print debug output, enable debug markers for debug output Debug output is unstable and redacts fields marked directly with debug_redact, and indirectly via options. This is a breaking change for anyone who depends on DebugString output to be stable and/or redacting. See https://protobuf.dev/news/2024-12-04/ PiperOrigin-RevId: 721918779 --- src/google/protobuf/text_format.cc | 77 ++++++++++++++++++++- src/google/protobuf/text_format.h | 4 ++ src/google/protobuf/text_format_unittest.cc | 62 ++++++++++++----- 3 files changed, 123 insertions(+), 20 deletions(-) diff --git a/src/google/protobuf/text_format.cc b/src/google/protobuf/text_format.cc index 2524d49c7917d..966626d175549 100644 --- a/src/google/protobuf/text_format.cc +++ b/src/google/protobuf/text_format.cc @@ -21,10 +21,12 @@ #include #include #include +#include #include #include #include +#include "absl/base/macros.h" #include "absl/container/btree_set.h" #include "absl/log/absl_check.h" #include "absl/strings/ascii.h" @@ -36,6 +38,8 @@ #include "absl/strings/str_format.h" #include "absl/strings/str_join.h" #include "absl/strings/string_view.h" +#include "absl/time/clock.h" +#include "absl/time/time.h" #include "google/protobuf/any.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/descriptor.pb.h" @@ -99,7 +103,7 @@ const char kDebugStringSilentMarkerForDetection[] = "\t "; // Controls insertion of a marker making debug strings non-parseable, and // redacting annotated fields in Protobuf's DebugString APIs. -PROTOBUF_EXPORT std::atomic enable_debug_string_safe_format{false}; +PROTOBUF_EXPORT std::atomic enable_debug_string_safe_format{true}; int64_t GetRedactedFieldCount() { return num_redacted_field.load(std::memory_order_relaxed); @@ -2299,6 +2303,9 @@ bool TextFormat::Printer::Print(const Message& message, internal::FieldReporterLevel reporter) const { TextGenerator generator(output, insert_silent_marker_, initial_indent_level_); + internal::PrintTextMarker(&generator, redact_debug_string_, + randomize_debug_string_, single_line_mode_); + Print(message, &generator); @@ -3109,6 +3116,74 @@ bool TextFormat::Printer::TryRedactFieldValue( } return false; } + +class TextMarkerGenerator final { + public: + static TextMarkerGenerator CreateRandom(); + + void PrintMarker(TextFormat::BaseTextGenerator* generator, bool redact, + bool randomize, bool single_line_mode) const { + if (redact) { + generator->Print(redaction_marker_.data(), redaction_marker_.size()); + } + if (randomize) { + generator->Print(random_marker_.data(), random_marker_.size()); + } + if ((redact || randomize) && !single_line_mode) { + generator->PrintLiteral("\n"); + } + } + + private: + static constexpr absl::string_view kRedactionMarkers[] = { + "goo.gle/debugonly ", "goo.gle/nodeserialize ", "goo.gle/debugstr ", + "goo.gle/debugproto "}; + + static constexpr absl::string_view kRandomMarker = " "; + + static_assert(!kRandomMarker.empty(), "The random marker cannot be empty!"); + + constexpr TextMarkerGenerator(absl::string_view redaction_marker, + absl::string_view random_marker) + : redaction_marker_(redaction_marker), random_marker_(random_marker) {} + + absl::string_view redaction_marker_; + absl::string_view random_marker_; +}; + +TextMarkerGenerator TextMarkerGenerator::CreateRandom() { + // We avoid using sources backed by system entropy to allow the marker + // generator to work in sandboxed environments that have no access to syscalls + // such as getrandom or getpid. Note that this randomization has no security + // implications, it's only used to break code that attempts to deserialize + // debug strings. + std::mt19937_64 random{ + static_cast(absl::ToUnixMicros(absl::Now()))}; + + size_t redaction_marker_index = std::uniform_int_distribution{ + 0, ABSL_ARRAYSIZE(kRedactionMarkers) - 1}(random); + + size_t random_marker_size = + std::uniform_int_distribution{1, kRandomMarker.size()}(random); + + return TextMarkerGenerator(kRedactionMarkers[redaction_marker_index], + kRandomMarker.substr(0, random_marker_size)); +} + +const TextMarkerGenerator& GetGlobalTextMarkerGenerator() { + static const TextMarkerGenerator kTextMarkerGenerator = + TextMarkerGenerator::CreateRandom(); + return kTextMarkerGenerator; +} + +namespace internal { +void PrintTextMarker(TextFormat::BaseTextGenerator* generator, bool redact, + bool randomize, bool single_line_mode) { + GetGlobalTextMarkerGenerator().PrintMarker(generator, redact, randomize, + single_line_mode); +} +} // namespace internal + } // namespace protobuf } // namespace google diff --git a/src/google/protobuf/text_format.h b/src/google/protobuf/text_format.h index 58dded12b13c6..980919795520e 100644 --- a/src/google/protobuf/text_format.h +++ b/src/google/protobuf/text_format.h @@ -829,6 +829,10 @@ class PROTOBUF_EXPORT TextFormat { const T&... values); }; +namespace internal { +void PrintTextMarker(TextFormat::BaseTextGenerator* generator, bool redact, + bool randomize, bool single_line_mode); +} // namespace internal inline void TextFormat::RecordLocation(ParseInfoTree* info_tree, const FieldDescriptor* field, diff --git a/src/google/protobuf/text_format_unittest.cc b/src/google/protobuf/text_format_unittest.cc index c57333cf3afae..030d2379fef0b 100644 --- a/src/google/protobuf/text_format_unittest.cc +++ b/src/google/protobuf/text_format_unittest.cc @@ -76,7 +76,6 @@ class UnsetFieldsMetadataTextFormatTestUtil { // Can't use an anonymous namespace here due to brokenness of Tru64 compiler. namespace text_format_unittest { -using ::google::protobuf::internal::kDebugStringSilentMarker; using ::google::protobuf::internal::UnsetFieldsMetadataTextFormatTestUtil; using ::testing::AllOf; using ::testing::HasSubstr; @@ -94,10 +93,16 @@ constexpr absl::string_view kEscapeTestStringEscaped = constexpr absl::string_view value_replacement = "\\[REDACTED\\]"; +constexpr absl::string_view kTextMarkerRegex = "goo\\.gle/.+ +"; + class TextFormatTestBase : public testing::Test { public: void SetUp() override { - single_line_debug_format_prefix_ = ""; + // DebugString APIs insert a per-process randomized + // prefix. Here we obtain the prefixes by calling DebugString APIs on an + // empty proto. Note that Message::ShortDebugString() trims the last empty + // space so we have to add it back. + single_line_debug_format_prefix_ = proto_.ShortDebugString() + " "; multi_line_debug_format_prefix_ = proto_.DebugString(); } @@ -210,6 +215,7 @@ TEST_F(TextFormatTest, ShortFormat) { std::string value_replacement = "\\[REDACTED\\]"; EXPECT_THAT(google::protobuf::ShortFormat(proto), testing::MatchesRegex(absl::Substitute( + "$1" "optional_redacted_string: $0 " "optional_unredacted_string: \"bar\" " "repeated_redacted_string: $0 " @@ -226,7 +232,7 @@ TEST_F(TextFormatTest, ShortFormat) { "\\{ optional_unredacted_nested_string: \"8\" \\} " "map_redacted_string: $0 " "map_unredacted_string \\{ key: \"ghi\" value: \"jkl\" \\}", - value_replacement))); + value_replacement, kTextMarkerRegex))); } TEST_F(TextFormatTest, Utf8Format) { @@ -262,6 +268,7 @@ TEST_F(TextFormatTest, Utf8Format) { EXPECT_THAT(google::protobuf::Utf8Format(proto), testing::MatchesRegex(absl::Substitute( + "$1\n" "optional_redacted_string: $0\n" "optional_unredacted_string: \"bar\"\n" "repeated_redacted_string: $0\n" @@ -280,7 +287,7 @@ TEST_F(TextFormatTest, Utf8Format) { "map_redacted_string: $0\n" "map_unredacted_string \\{\n " "key: \"ghi\"\n value: \"jkl\"\n\\}\n", - value_replacement))); + value_replacement, kTextMarkerRegex))); } TEST_F(TextFormatTest, ShortPrimitiveRepeateds) { @@ -433,6 +440,7 @@ TEST_F(TextFormatTest, PrintUnknownFields) { message_text); EXPECT_THAT(absl::StrCat(message), testing::MatchesRegex(absl::Substitute( + "$1\n" "5: UNKNOWN_VARINT $0\n" "5: UNKNOWN_FIXED32 $0\n" "5: UNKNOWN_FIXED64 $0\n" @@ -441,7 +449,7 @@ TEST_F(TextFormatTest, PrintUnknownFields) { "8: UNKNOWN_VARINT $0\n" "8: UNKNOWN_VARINT $0\n" "8: UNKNOWN_VARINT $0\n", - value_replacement))); + value_replacement, kTextMarkerRegex))); } TEST_F(TextFormatTest, PrintUnknownFieldsDeepestStackWorks) { @@ -2679,6 +2687,13 @@ TEST(TextFormatUnknownFieldTest, TestUnknownExtension) { EXPECT_FALSE(parser.ParseFromString("unknown_field: 1", &proto)); } +TEST(AbslStringifyTest, DebugStringIsTheSame) { + unittest::TestAllTypes proto; + proto.set_optional_int32(1); + proto.set_optional_string("foo"); + + EXPECT_THAT(proto.DebugString(), absl::StrCat(proto)); +} TEST(AbslStringifyTest, TextFormatIsUnchanged) { unittest::TestAllTypes proto; @@ -2698,9 +2713,11 @@ TEST(AbslStringifyTest, StringifyHasRedactionMarker) { proto.set_optional_int32(1); proto.set_optional_string("foo"); - EXPECT_THAT(absl::StrCat(proto), testing::MatchesRegex( + EXPECT_THAT(absl::StrCat(proto), testing::MatchesRegex(absl::Substitute( + "$0\n" "optional_int32: 1\n" - "optional_string: \"foo\"\n")); + "optional_string: \"foo\"\n", + kTextMarkerRegex))); } @@ -2708,16 +2725,18 @@ TEST(AbslStringifyTest, StringifyMetaAnnotatedIsRedacted) { unittest::TestRedactedMessage proto; proto.set_meta_annotated("foo"); EXPECT_THAT(absl::StrCat(proto), testing::MatchesRegex(absl::Substitute( - "meta_annotated: $0\n", - value_replacement))); + "$0\n" + "meta_annotated: $1\n", + kTextMarkerRegex, value_replacement))); } TEST(AbslStringifyTest, StringifyRepeatedMetaAnnotatedIsRedacted) { unittest::TestRedactedMessage proto; proto.set_repeated_meta_annotated("foo"); EXPECT_THAT(absl::StrCat(proto), testing::MatchesRegex(absl::Substitute( - "repeated_meta_annotated: $0\n", - value_replacement))); + "$0\n" + "repeated_meta_annotated: $1\n", + kTextMarkerRegex, value_replacement))); } TEST(AbslStringifyTest, StringifyRepeatedMetaAnnotatedIsNotRedacted) { @@ -2725,7 +2744,9 @@ TEST(AbslStringifyTest, StringifyRepeatedMetaAnnotatedIsNotRedacted) { proto.set_unredacted_repeated_annotations("foo"); EXPECT_THAT(absl::StrCat(proto), testing::MatchesRegex( - "unredacted_repeated_annotations: \"foo\"\n")); + absl::Substitute("$0\n" + "unredacted_repeated_annotations: \"foo\"\n", + kTextMarkerRegex))); } TEST(AbslStringifyTest, TextFormatMetaAnnotatedIsNotRedacted) { @@ -2739,23 +2760,26 @@ TEST(AbslStringifyTest, StringifyDirectMessageEnumIsRedacted) { unittest::TestRedactedMessage proto; proto.set_test_direct_message_enum("foo"); EXPECT_THAT(absl::StrCat(proto), testing::MatchesRegex(absl::Substitute( - "test_direct_message_enum: $0\n", - value_replacement))); + "$0\n" + "test_direct_message_enum: $1\n", + kTextMarkerRegex, value_replacement))); } TEST(AbslStringifyTest, StringifyNestedMessageEnumIsRedacted) { unittest::TestRedactedMessage proto; proto.set_test_nested_message_enum("foo"); EXPECT_THAT(absl::StrCat(proto), testing::MatchesRegex(absl::Substitute( - "test_nested_message_enum: $0\n", - value_replacement))); + "$0\n" + "test_nested_message_enum: $1\n", + kTextMarkerRegex, value_replacement))); } TEST(AbslStringifyTest, StringifyRedactedOptionDoesNotRedact) { unittest::TestRedactedMessage proto; proto.set_test_redacted_message_enum("foo"); - EXPECT_THAT(absl::StrCat(proto), - testing::MatchesRegex( - "test_redacted_message_enum: \"foo\"\n")); + EXPECT_THAT(absl::StrCat(proto), testing::MatchesRegex(absl::Substitute( + "$0\n" + "test_redacted_message_enum: \"foo\"\n", + kTextMarkerRegex))); }