Skip to content

Commit dc3a5a1

Browse files
mkruskal-googlecopybara-github
authored andcommitted
Prevent accidental stripping of debug_redact options via import option.
Because we use reflection to check this metadata on custom options during debug string redaction, there's nothing enforcing that these options will be linked into the binary. In order to avoid stripping this and leaking PII, we just ban optional dependencies on protos used to annotate sensitive data. PiperOrigin-RevId: 814426143
1 parent cdb39db commit dc3a5a1

File tree

2 files changed

+184
-0
lines changed

2 files changed

+184
-0
lines changed

src/google/protobuf/compiler/command_line_interface.cc

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include <fstream>
2525
#include <iostream>
2626
#include <memory>
27+
#include <optional>
2728
#include <ostream>
2829
#include <string>
2930
#include <utility>
@@ -1039,6 +1040,90 @@ bool HasReservedFieldNumber(const FieldDescriptor* field) {
10391040
return false;
10401041
}
10411042

1043+
bool HasDebugRedact(const EnumDescriptor& enm) {
1044+
for (int i = 0; i < enm.value_count(); ++i) {
1045+
const EnumValueDescriptor* value = enm.value(i);
1046+
if (value->options().debug_redact()) {
1047+
return true;
1048+
}
1049+
}
1050+
return false;
1051+
}
1052+
1053+
bool HasDebugRedact(const Descriptor& desc,
1054+
absl::flat_hash_map<const Descriptor*, bool>& visited);
1055+
1056+
bool HasDebugRedact(const FieldDescriptor& field,
1057+
absl::flat_hash_map<const Descriptor*, bool>& visited) {
1058+
if (field.enum_type() != nullptr && HasDebugRedact(*field.enum_type())) {
1059+
return true;
1060+
}
1061+
if (field.message_type() != nullptr &&
1062+
HasDebugRedact(*field.message_type(), visited)) {
1063+
return true;
1064+
}
1065+
return false;
1066+
}
1067+
1068+
bool HasDebugRedact(const Descriptor& desc,
1069+
absl::flat_hash_map<const Descriptor*, bool>& visited) {
1070+
auto result = visited.emplace(&desc, false);
1071+
if (!result.second) {
1072+
return result.first->second;
1073+
}
1074+
for (int i = 0; i < desc.field_count(); ++i) {
1075+
if (HasDebugRedact(*desc.field(i), visited)) {
1076+
visited[&desc] = true;
1077+
return true;
1078+
}
1079+
}
1080+
return false;
1081+
}
1082+
1083+
// Look for any enums with values marked debug_redact within this message
1084+
// schema. These can be used to mark fields that need to be redacted in debug
1085+
// string APIs, but are only discoverable via reflection that doesn't force
1086+
// linkage.
1087+
std::optional<std::string> FindDebugRedactMarker(const FileDescriptor& desc) {
1088+
std::optional<std::string> debug_redact_value;
1089+
absl::flat_hash_map<const Descriptor*, bool> visited;
1090+
google::protobuf::internal::VisitDescriptors(
1091+
desc, [&debug_redact_value, &visited](const FieldDescriptor& field) {
1092+
if (field.is_extension() && HasDebugRedact(field, visited)) {
1093+
debug_redact_value = field.full_name();
1094+
}
1095+
});
1096+
return debug_redact_value;
1097+
}
1098+
1099+
bool ValidateOptionImports(const FileDescriptor& file,
1100+
const DescriptorPool& pool,
1101+
DescriptorPool::ErrorCollector* printer) {
1102+
for (int i = 0; i < file.option_dependency_count(); ++i) {
1103+
const FileDescriptor* dep =
1104+
pool.FindFileByName(file.option_dependency_name(i));
1105+
if (dep == nullptr) {
1106+
// If we don't have the dependency we can't validate it, assume it's ok.
1107+
continue;
1108+
}
1109+
std::optional<std::string> debug_redact_value = FindDebugRedactMarker(*dep);
1110+
if (debug_redact_value.has_value()) {
1111+
printer->RecordError(
1112+
file.name(), "", nullptr, DescriptorPool::ErrorCollector::OPTION_NAME,
1113+
absl::StrCat(
1114+
"Optional dependency ", dep->name(), " contains a custom option ",
1115+
*debug_redact_value,
1116+
" marked debug_redact, which is used to mark fields that need to "
1117+
"be redacted in debug string APIs. Switch to a regular import or "
1118+
"remove the debug_redact annotation to avoid potentially leaking "
1119+
"sensitive data."));
1120+
return false;
1121+
}
1122+
}
1123+
1124+
return true;
1125+
}
1126+
10421127
} // namespace
10431128

10441129
namespace {

src/google/protobuf/compiler/command_line_interface_unittest.cc

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3151,6 +3151,105 @@ TEST_F(CommandLineInterfaceTest, WriteTransitiveOptionImportDescriptorSet) {
31513151
EXPECT_EQ("bar.proto", descriptor_set.file(3).name());
31523152
}
31533153

3154+
TEST_F(CommandLineInterfaceTest, OptionImportWithDebugRedactFieldIsNotError) {
3155+
CreateTempFile("google/protobuf/descriptor.proto",
3156+
google::protobuf::DescriptorProto::descriptor()->file()->DebugString());
3157+
CreateTempFile("custom_option.proto",
3158+
R"schema(
3159+
syntax = "proto2";
3160+
import "google/protobuf/descriptor.proto";
3161+
extend .google.protobuf.FileOptions {
3162+
optional int32 file_opt = 5000 [debug_redact = true];
3163+
}
3164+
)schema");
3165+
CreateTempFile("bar.proto",
3166+
R"schema(
3167+
edition = "2024";
3168+
import option "custom_option.proto";
3169+
option (file_opt) = 1;
3170+
message Bar {
3171+
int32 foo = 1;
3172+
}
3173+
)schema");
3174+
3175+
Run("protocol_compiler --descriptor_set_out=$tmpdir/descriptor_set "
3176+
"--include_imports --proto_path=$tmpdir bar.proto "
3177+
"--experimental_editions");
3178+
3179+
ExpectNoErrors();
3180+
}
3181+
3182+
TEST_F(CommandLineInterfaceTest, OptionImportWithDebugRedactIsError) {
3183+
CreateTempFile("google/protobuf/descriptor.proto",
3184+
google::protobuf::DescriptorProto::descriptor()->file()->DebugString());
3185+
CreateTempFile("custom_option.proto",
3186+
R"schema(
3187+
syntax = "proto2";
3188+
import "google/protobuf/descriptor.proto";
3189+
enum MyEnum {
3190+
MY_ENUM_VALUE = 0 [debug_redact = true];
3191+
}
3192+
extend .google.protobuf.FieldOptions {
3193+
optional MyEnum file_opt = 5000 [debug_redact = true];
3194+
}
3195+
)schema");
3196+
CreateTempFile("bar.proto",
3197+
R"schema(
3198+
edition = "2024";
3199+
import option "custom_option.proto";
3200+
message Bar {
3201+
int32 foo = 1 [(file_opt) = MY_ENUM_VALUE];
3202+
}
3203+
)schema");
3204+
3205+
Run("protocol_compiler --descriptor_set_out=$tmpdir/descriptor_set "
3206+
"--include_imports --proto_path=$tmpdir bar.proto "
3207+
"--experimental_editions");
3208+
3209+
ExpectErrorSubstring(
3210+
"bar.proto: Optional dependency custom_option.proto contains a custom "
3211+
"option file_opt marked debug_redact");
3212+
}
3213+
3214+
TEST_F(CommandLineInterfaceTest, OptionImportWithDebugRedactDeeplyNested) {
3215+
CreateTempFile("google/protobuf/descriptor.proto",
3216+
google::protobuf::DescriptorProto::descriptor()->file()->DebugString());
3217+
CreateTempFile("custom_option.proto",
3218+
R"schema(
3219+
syntax = "proto2";
3220+
import "google/protobuf/descriptor.proto";
3221+
enum MyEnum {
3222+
MY_ENUM_VALUE = 0 [debug_redact = true];
3223+
}
3224+
message Container {
3225+
optional MyEnum enm = 1;
3226+
}
3227+
message MyMessage {
3228+
optional MyMessage msg = 1;
3229+
optional Container ctr = 2;
3230+
}
3231+
extend .google.protobuf.FieldOptions {
3232+
optional MyMessage file_opt = 5000 [debug_redact = true];
3233+
}
3234+
)schema");
3235+
CreateTempFile("bar.proto",
3236+
R"schema(
3237+
edition = "2024";
3238+
import option "custom_option.proto";
3239+
message Bar {
3240+
int32 foo = 1 [(file_opt).msg = {}];
3241+
}
3242+
)schema");
3243+
3244+
Run("protocol_compiler --descriptor_set_out=$tmpdir/descriptor_set "
3245+
"--include_imports --proto_path=$tmpdir bar.proto "
3246+
"--experimental_editions");
3247+
3248+
ExpectErrorSubstring(
3249+
"bar.proto: Optional dependency custom_option.proto contains a custom "
3250+
"option file_opt marked debug_redact");
3251+
}
3252+
31543253
TEST_F(CommandLineInterfaceTest, DisallowMissingOptionImportsDescriptorSetIn) {
31553254
FileDescriptorSet file_descriptor_set;
31563255

0 commit comments

Comments
 (0)