Skip to content

Commit

Permalink
proto: Handle maps correctly in redact. (envoyproxy#18254)
Browse files Browse the repository at this point in the history
proto: handle maps correctly in `redact`.
Additional Description: Currently, when a map is marked as sensitive, `redact` will turn every key into the string "[redacted]", thereby invalidating the map. This change causes the map to only have values converted to "[redacted]".
Risk Level: low
Testing: unit tested.
Docs Changes:
Release Notes:
Platform Specific Features:

Signed-off-by: Paul Gallagher <pgal@google.com>
  • Loading branch information
paul-r-gall authored and soulxu committed Oct 15, 2021
1 parent d0b5900 commit 47eb7fd
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 1 deletion.
20 changes: 19 additions & 1 deletion source/common/protobuf/utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,25 @@ void redact(Protobuf::Message* message, bool ancestor_is_sensitive) {

if (field_descriptor->type() == Protobuf::FieldDescriptor::TYPE_MESSAGE) {
// Recursive case: traverse message fields.
if (field_descriptor->is_repeated()) {
if (field_descriptor->is_map()) {
// Redact values of maps only. Redacting both leaves the map with multiple "[redacted]"
// keys.
const int field_size = reflection->FieldSize(*message, field_descriptor);
for (int i = 0; i < field_size; ++i) {
Protobuf::Message* map_pair =
reflection->MutableRepeatedMessage(message, field_descriptor, i);
auto* value_field_desc = map_pair->GetDescriptor()->FindFieldByName("value");
if (sensitive && (value_field_desc->type() == Protobuf::FieldDescriptor::TYPE_STRING ||
value_field_desc->type() == Protobuf::FieldDescriptor::TYPE_BYTES)) {
map_pair->GetReflection()->SetString(map_pair, value_field_desc, "[redacted]");
} else if (value_field_desc->type() == Protobuf::FieldDescriptor::TYPE_MESSAGE) {
redact(map_pair->GetReflection()->MutableMessage(map_pair, value_field_desc),
sensitive);
} else if (sensitive) {
map_pair->GetReflection()->ClearField(map_pair, value_field_desc);
}
}
} else if (field_descriptor->is_repeated()) {
const int field_size = reflection->FieldSize(*message, field_descriptor);
for (int i = 0; i < field_size; ++i) {
redact(reflection->MutableRepeatedMessage(message, field_descriptor, i), sensitive);
Expand Down
31 changes: 31 additions & 0 deletions test/common/protobuf/utility_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,37 @@ insensitive_string: This field should not be redacted.
EXPECT_TRUE(TestUtility::protoEqual(expected, actual));
}

// Fields that are values in a sensitive map should be redacted.
TEST_F(ProtobufUtilityTest, RedactMap) {
envoy::test::Sensitive actual, expected;
TestUtility::loadFromYaml(R"EOF(
sensitive_string_map:
"a": "b"
sensitive_int_map:
"x": 12345
insensitive_string_map:
"c": "d"
insensitive_int_map:
"y": 123456
)EOF",
actual);

TestUtility::loadFromYaml(R"EOF(
sensitive_string_map:
"a": "[redacted]"
sensitive_int_map:
"x":
insensitive_string_map:
"c": "d"
insensitive_int_map:
"y": 123456
)EOF",
expected);

MessageUtil::redact(actual);
EXPECT_TRUE(TestUtility::protoEqual(expected, actual));
}

// Bytes fields annotated as sensitive should be converted to the ASCII / UTF-8 encoding of the
// string "[redacted]". Bytes fields that are neither annotated as sensitive nor contained in a
// sensitive message should be left alone.
Expand Down
4 changes: 4 additions & 0 deletions test/proto/sensitive.proto
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ message Sensitive {
udpa.type.v1.TypedStruct sensitive_typed_struct = 11 [(udpa.annotations.sensitive) = true];
repeated udpa.type.v1.TypedStruct sensitive_repeated_typed_struct = 12
[(udpa.annotations.sensitive) = true];
map<string, string> sensitive_string_map = 13 [(udpa.annotations.sensitive) = true];
map<string, int64> sensitive_int_map = 14 [(udpa.annotations.sensitive) = true];

string insensitive_string = 101;
repeated string insensitive_repeated_string = 102;
Expand All @@ -39,4 +41,6 @@ message Sensitive {
repeated google.protobuf.Any insensitive_repeated_any = 110;
udpa.type.v1.TypedStruct insensitive_typed_struct = 111;
repeated udpa.type.v1.TypedStruct insensitive_repeated_typed_struct = 112;
map<string, string> insensitive_string_map = 113;
map<string, int64> insensitive_int_map = 114;
}

0 comments on commit 47eb7fd

Please sign in to comment.