KUDU-1896 (part 1). Add redaction to JSON protobuf output This enables the redaction attribute when outputting a protobuf as JSON. This is important in the context of the tracing endpoint as well as /rpcz, to avoid such traces containing user data.
Change-Id: I506b3710d2b50ec7a8947c49ec208eb4b53b63ea Reviewed-on: http://gerrit.cloudera.org:8080/6158 Reviewed-by: Hao Hao <[email protected]> Tested-by: Todd Lipcon <[email protected]> Reviewed-by: Todd Lipcon <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/92fe87f5 Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/92fe87f5 Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/92fe87f5 Branch: refs/heads/master Commit: 92fe87f5e2db57f520ba96fc4b63f1d13a48b185 Parents: c11a315 Author: Todd Lipcon <[email protected]> Authored: Sun Feb 26 16:12:24 2017 -0800 Committer: Todd Lipcon <[email protected]> Committed: Tue Feb 28 21:13:04 2017 +0000 ---------------------------------------------------------------------- src/kudu/util/CMakeLists.txt | 1 + src/kudu/util/jsonwriter-test.cc | 35 +++++++++++++++++++++++++++++++- src/kudu/util/jsonwriter.cc | 10 +++++++-- src/kudu/util/jsonwriter.h | 2 ++ src/kudu/util/jsonwriter_test.proto | 8 +++++++- src/kudu/util/logging.h | 4 ++++ src/kudu/util/pb_util.cc | 2 +- 7 files changed, 57 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/92fe87f5/src/kudu/util/CMakeLists.txt ---------------------------------------------------------------------- diff --git a/src/kudu/util/CMakeLists.txt b/src/kudu/util/CMakeLists.txt index 27f5d70..685f01f 100644 --- a/src/kudu/util/CMakeLists.txt +++ b/src/kudu/util/CMakeLists.txt @@ -405,6 +405,7 @@ PROTOBUF_GENERATE_CPP( PROTO_FILES jsonwriter_test.proto) add_library(jsonwriter_test_proto ${JSONWRITER_TEST_PROTO_SRCS} ${JSONWRITER_TEST_PROTO_HDRS}) target_link_libraries(jsonwriter_test_proto + pb_util_proto protobuf) ####################################### http://git-wip-us.apache.org/repos/asf/kudu/blob/92fe87f5/src/kudu/util/jsonwriter-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/util/jsonwriter-test.cc b/src/kudu/util/jsonwriter-test.cc index 81e7d85..7f458fa 100644 --- a/src/kudu/util/jsonwriter-test.cc +++ b/src/kudu/util/jsonwriter-test.cc @@ -17,8 +17,10 @@ #include <gtest/gtest.h> +#include "kudu/gutil/strings/substitute.h" #include "kudu/util/jsonwriter.h" #include "kudu/util/jsonwriter_test.pb.h" +#include "kudu/util/logging.h" #include "kudu/util/test_util.h" using jsonwriter_test::TestAllTypes; @@ -33,6 +35,7 @@ TEST_F(TestJsonWriter, TestPBEmpty) { } TEST_F(TestJsonWriter, TestPBAllFieldTypes) { + FLAGS_log_redact_user_data = true; TestAllTypes pb; pb.set_optional_int32(1); pb.set_optional_int64(2); @@ -48,6 +51,7 @@ TEST_F(TestJsonWriter, TestPBAllFieldTypes) { pb.set_optional_double(12); pb.set_optional_bool(true); pb.set_optional_string("hello world"); + pb.set_optional_redacted_string("secret!"); pb.set_optional_nested_enum(TestAllTypes::FOO); ASSERT_EQ("{\n" " \"optional_int32\": 1,\n" @@ -64,6 +68,7 @@ TEST_F(TestJsonWriter, TestPBAllFieldTypes) { " \"optional_double\": 12,\n" " \"optional_bool\": true,\n" " \"optional_string\": \"hello world\",\n" + " \"optional_redacted_string\": \"<redacted>\",\n" " \"optional_nested_enum\": \"FOO\"\n" "}", JsonWriter::ToJson(pb, JsonWriter::PRETTY)); ASSERT_EQ("{" @@ -81,15 +86,20 @@ TEST_F(TestJsonWriter, TestPBAllFieldTypes) { "\"optional_double\":12," "\"optional_bool\":true," "\"optional_string\":\"hello world\"," + "\"optional_redacted_string\":\"<redacted>\"," "\"optional_nested_enum\":\"FOO\"" "}", JsonWriter::ToJson(pb, JsonWriter::COMPACT)); } TEST_F(TestJsonWriter, TestPBRepeatedPrimitives) { + FLAGS_log_redact_user_data = true; TestAllTypes pb; for (int i = 0; i <= 3; i++) { pb.add_repeated_int32(i); + pb.add_repeated_string(strings::Substitute("hi $0", i)); + pb.add_repeated_redacted_string("secret!"); + pb.add_repeated_redacted_bytes("secret!"); } ASSERT_EQ("{\n" " \"repeated_int32\": [\n" @@ -97,9 +107,32 @@ TEST_F(TestJsonWriter, TestPBRepeatedPrimitives) { " 1,\n" " 2,\n" " 3\n" + " ],\n" + " \"repeated_string\": [\n" + " \"hi 0\",\n" + " \"hi 1\",\n" + " \"hi 2\",\n" + " \"hi 3\"\n" + " ],\n" + " \"repeated_redacted_string\": [\n" + " \"<redacted>\",\n" + " \"<redacted>\",\n" + " \"<redacted>\",\n" + " \"<redacted>\"\n" + " ],\n" + " \"repeated_redacted_bytes\": [\n" + " \"<redacted>\",\n" + " \"<redacted>\",\n" + " \"<redacted>\",\n" + " \"<redacted>\"\n" " ]\n" "}", JsonWriter::ToJson(pb, JsonWriter::PRETTY)); - ASSERT_EQ("{\"repeated_int32\":[0,1,2,3]}", + ASSERT_EQ("{\"repeated_int32\":[0,1,2,3]," + "\"repeated_string\":[\"hi 0\",\"hi 1\",\"hi 2\",\"hi 3\"]," + "\"repeated_redacted_string\":[\"<redacted>\",\"<redacted>\"," + "\"<redacted>\",\"<redacted>\"]," + "\"repeated_redacted_bytes\":[\"<redacted>\",\"<redacted>\"," + "\"<redacted>\",\"<redacted>\"]}", JsonWriter::ToJson(pb, JsonWriter::COMPACT)); } http://git-wip-us.apache.org/repos/asf/kudu/blob/92fe87f5/src/kudu/util/jsonwriter.cc ---------------------------------------------------------------------- diff --git a/src/kudu/util/jsonwriter.cc b/src/kudu/util/jsonwriter.cc index 64f8706..ef9d5ba 100644 --- a/src/kudu/util/jsonwriter.cc +++ b/src/kudu/util/jsonwriter.cc @@ -22,10 +22,14 @@ #include <glog/logging.h> #include <google/protobuf/descriptor.h> +#include <google/protobuf/descriptor.pb.h> #include <google/protobuf/message.h> #include <rapidjson/prettywriter.h> #include <rapidjson/rapidjson.h> +#include "kudu/util/logging.h" +#include "kudu/util/pb_util.pb.h" + using google::protobuf::FieldDescriptor; using google::protobuf::Message; using google::protobuf::Reflection; @@ -213,7 +217,8 @@ void JsonWriter::ProtobufField(const Message& pb, const FieldDescriptor* field) String(reflection->GetEnum(pb, field)->name()); break; case FieldDescriptor::CPPTYPE_STRING: - String(reflection->GetString(pb, field)); + String(KUDU_MAYBE_REDACT_IF(field->options().GetExtension(REDACT), + reflection->GetString(pb, field))); break; case FieldDescriptor::CPPTYPE_MESSAGE: Protobuf(reflection->GetMessage(pb, field)); @@ -251,7 +256,8 @@ void JsonWriter::ProtobufRepeatedField(const Message& pb, const FieldDescriptor* String(reflection->GetRepeatedEnum(pb, field, index)->name()); break; case FieldDescriptor::CPPTYPE_STRING: - String(reflection->GetRepeatedString(pb, field, index)); + String(KUDU_MAYBE_REDACT_IF(field->options().GetExtension(REDACT), + reflection->GetRepeatedString(pb, field, index))); break; case FieldDescriptor::CPPTYPE_MESSAGE: Protobuf(reflection->GetRepeatedMessage(pb, field, index)); http://git-wip-us.apache.org/repos/asf/kudu/blob/92fe87f5/src/kudu/util/jsonwriter.h ---------------------------------------------------------------------- diff --git a/src/kudu/util/jsonwriter.h b/src/kudu/util/jsonwriter.h index 1909bc0..d3fb604 100644 --- a/src/kudu/util/jsonwriter.h +++ b/src/kudu/util/jsonwriter.h @@ -66,6 +66,8 @@ class JsonWriter { void String(const char* str); void String(const std::string& str); + // Convert the given protobuf message to JSON. + // The output respects redaction for 'string' and 'bytes' fields. void Protobuf(const google::protobuf::Message& message); template<typename T> http://git-wip-us.apache.org/repos/asf/kudu/blob/92fe87f5/src/kudu/util/jsonwriter_test.proto ---------------------------------------------------------------------- diff --git a/src/kudu/util/jsonwriter_test.proto b/src/kudu/util/jsonwriter_test.proto index d16cc11..cab7201 100644 --- a/src/kudu/util/jsonwriter_test.proto +++ b/src/kudu/util/jsonwriter_test.proto @@ -16,6 +16,8 @@ // under the License. package jsonwriter_test; +import "kudu/util/pb_util.proto"; + // This proto includes every type of field in both singular and repeated // forms. This is mostly copied from 'unittest.proto' in the protobuf source // (hence the odd field numbers which skip some). @@ -45,7 +47,9 @@ message TestAllTypes { optional double optional_double = 12; optional bool optional_bool = 13; optional string optional_string = 14; - optional bytes optional_bytes = 15; + optional string optional_redacted_string = 15 [ (kudu.REDACT) = true ]; + optional bytes optional_bytes = 16; + optional bytes optional_redacted_bytes = 17 [ (kudu.REDACT) = true ]; optional NestedMessage optional_nested_message = 18; optional NestedEnum optional_nested_enum = 21; @@ -66,6 +70,8 @@ message TestAllTypes { repeated bool repeated_bool = 43; repeated string repeated_string = 44; repeated bytes repeated_bytes = 45; + repeated string repeated_redacted_string = 46 [ (kudu.REDACT) = true ]; + repeated string repeated_redacted_bytes = 47 [ (kudu.REDACT) = true ]; repeated NestedMessage repeated_nested_message = 48; repeated NestedEnum repeated_nested_enum = 51; http://git-wip-us.apache.org/repos/asf/kudu/blob/92fe87f5/src/kudu/util/logging.h ---------------------------------------------------------------------- diff --git a/src/kudu/util/logging.h b/src/kudu/util/logging.h index 5cbf820..e0b5dff 100644 --- a/src/kudu/util/logging.h +++ b/src/kudu/util/logging.h @@ -65,6 +65,10 @@ #define KUDU_REDACT(expr) \ (KUDU_SHOULD_REDACT() ? kudu::kRedactionMessage : (expr)) +// Like the above, but with the additional condition that redaction will only +// be performed if 'cond' must be true. +#define KUDU_MAYBE_REDACT_IF(cond, expr) \ + ((KUDU_SHOULD_REDACT() && (cond)) ? kudu::kRedactionMessage : (expr)) //////////////////////////////////////// // Redaction implementation details follow. http://git-wip-us.apache.org/repos/asf/kudu/blob/92fe87f5/src/kudu/util/pb_util.cc ---------------------------------------------------------------------- diff --git a/src/kudu/util/pb_util.cc b/src/kudu/util/pb_util.cc index 4c0ff5c..6deb726 100644 --- a/src/kudu/util/pb_util.cc +++ b/src/kudu/util/pb_util.cc @@ -567,7 +567,7 @@ class SecureFieldPrinter : public TextFormat::FieldValuePrinter { string PrintBytes(const string& val) const override { if (hide_next_string_) { hide_next_string_ = false; - return KUDU_REDACT(super::PrintString(val)); + return KUDU_REDACT(super::PrintBytes(val)); } return super::PrintBytes(val); }
