You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by da...@apache.org on 2016/12/23 01:21:52 UTC
[1/2] kudu git commit: KUDU-1812. Redaction support for protobufs
Repository: kudu
Updated Branches:
refs/heads/branch-1.2.x 73aba1fb9 -> 5a6ef37bd
KUDU-1812. Redaction support for protobufs
This adds a new local protobuf option 'kudu.REDACT' which can be used to
mark a field as containing user data and thus needing redaction during
stringification.
New 'DebugString' and 'ShortDebugString' variants are provided which
respect the redaction option and use the KUDU_REDACT macro to implement
it on fields tagged with this option.
This patch includes a test protobuf using this feature and also tags the
relevant fields in our various protobufs. However, it does not yet
change call sites to use the SecureDebugString variants -- that is done
in a follow-on commit.
This commit also does not implement redaction for the JSONWriter-based
stringification. As far as I am aware, we do not use JSON output in any
log messages, and we are not yet aiming to redact /tracing.html output,
etc.
Change-Id: Ie07bfdcbc33d38d55315faae2c4906899a5450fb
Reviewed-on: http://gerrit.cloudera.org:8080/5553
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
(cherry picked from commit cef7b10239a1cf860bfcb526d503b07503442a49)
Reviewed-on: http://gerrit.cloudera.org:8080/5570
Reviewed-by: Dan Burkert <da...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/8a1a38ea
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/8a1a38ea
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/8a1a38ea
Branch: refs/heads/branch-1.2.x
Commit: 8a1a38eac2fe036f05ffbc073457e21150d15e67
Parents: 73aba1f
Author: Todd Lipcon <to...@apache.org>
Authored: Mon Dec 19 14:06:30 2016 +0700
Committer: Dan Burkert <da...@apache.org>
Committed: Fri Dec 23 00:55:42 2016 +0000
----------------------------------------------------------------------
src/kudu/cfile/CMakeLists.txt | 1 +
src/kudu/cfile/cfile.proto | 7 +++-
src/kudu/client/client.proto | 9 ++---
src/kudu/common/CMakeLists.txt | 2 +-
src/kudu/common/common.proto | 10 +++---
src/kudu/common/wire_protocol.proto | 5 +--
src/kudu/master/master.proto | 5 +--
src/kudu/rpc/CMakeLists.txt | 2 +-
src/kudu/rpc/rpc_header.proto | 5 +--
src/kudu/tserver/tablet_copy.proto | 3 +-
src/kudu/tserver/tserver.proto | 13 +++----
src/kudu/util/CMakeLists.txt | 12 ++++---
src/kudu/util/pb_util-test.cc | 31 +++++++++++++++-
src/kudu/util/pb_util.cc | 61 ++++++++++++++++++++++++++++++++
src/kudu/util/pb_util.h | 12 +++++++
src/kudu/util/pb_util.proto | 4 +++
src/kudu/util/pb_util_test.proto | 28 +++++++++++++++
17 files changed, 180 insertions(+), 30 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/8a1a38ea/src/kudu/cfile/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/src/kudu/cfile/CMakeLists.txt b/src/kudu/cfile/CMakeLists.txt
index dbaa2a3..eee6b00 100644
--- a/src/kudu/cfile/CMakeLists.txt
+++ b/src/kudu/cfile/CMakeLists.txt
@@ -22,6 +22,7 @@ PROTOBUF_GENERATE_CPP(
PROTO_FILES cfile.proto)
set(CFILE_PROTO_LIBS
kudu_common_proto
+ pb_util_proto
protobuf)
ADD_EXPORTABLE_LIBRARY(cfile_proto
SRCS ${CFILE_PROTO_SRCS}
http://git-wip-us.apache.org/repos/asf/kudu/blob/8a1a38ea/src/kudu/cfile/cfile.proto
----------------------------------------------------------------------
diff --git a/src/kudu/cfile/cfile.proto b/src/kudu/cfile/cfile.proto
index 34794b0..f59a030 100644
--- a/src/kudu/cfile/cfile.proto
+++ b/src/kudu/cfile/cfile.proto
@@ -19,10 +19,15 @@ package kudu.cfile;
option java_package = "org.apache.kudu.cfile";
import "kudu/common/common.proto";
+import "kudu/util/pb_util.proto";
message FileMetadataPairPB {
required string key = 1;
- required bytes value = 2;
+
+ // We redact the metadata values here because, despite being "metadata"
+ // from the perspective of the CFile, they can in many cases store actual
+ // row data (e.g min/max key values).
+ required bytes value = 2 [ (REDACT) = true ];
}
message CFileHeaderPB {
http://git-wip-us.apache.org/repos/asf/kudu/blob/8a1a38ea/src/kudu/client/client.proto
----------------------------------------------------------------------
diff --git a/src/kudu/client/client.proto b/src/kudu/client/client.proto
index fcff805..a26e447 100644
--- a/src/kudu/client/client.proto
+++ b/src/kudu/client/client.proto
@@ -22,6 +22,7 @@ package kudu.client;
option java_package = "org.apache.kudu.client";
import "kudu/common/common.proto";
+import "kudu/util/pb_util.proto";
// Serialization format for client scan tokens. Scan tokens are serializable
// scan descriptors that are used by query engines to plan a set of parallizable
@@ -55,16 +56,16 @@ message ScanTokenPB {
repeated ColumnPredicatePB column_predicates = 4;
// Encoded primary key to begin scanning at (inclusive).
- optional bytes lower_bound_primary_key = 5;
+ optional bytes lower_bound_primary_key = 5 [(kudu.REDACT) = true];
// Encoded primary key to stop scanning at (exclusive).
- optional bytes upper_bound_primary_key = 6;
+ optional bytes upper_bound_primary_key = 6 [(kudu.REDACT) = true];
// Encoded partition key to begin scanning at (inclusive).
- optional bytes lower_bound_partition_key = 7;
+ optional bytes lower_bound_partition_key = 7 [(kudu.REDACT) = true];
// Encoded partition key to stop scanning at (exclusive).
- optional bytes upper_bound_partition_key = 8;
+ optional bytes upper_bound_partition_key = 8 [(kudu.REDACT) = true];
// The maximum number of rows to scan.
// The scanner will automatically stop yielding results and close
http://git-wip-us.apache.org/repos/asf/kudu/blob/8a1a38ea/src/kudu/common/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/src/kudu/common/CMakeLists.txt b/src/kudu/common/CMakeLists.txt
index 7280db8..e8c4071 100644
--- a/src/kudu/common/CMakeLists.txt
+++ b/src/kudu/common/CMakeLists.txt
@@ -22,7 +22,7 @@ PROTOBUF_GENERATE_CPP(
PROTO_FILES common.proto)
ADD_EXPORTABLE_LIBRARY(kudu_common_proto
SRCS ${COMMON_PROTO_SRCS}
- DEPS protobuf
+ DEPS pb_util_proto protobuf
NONLINK_DEPS ${COMMON_PROTO_TGTS})
PROTOBUF_GENERATE_CPP(
http://git-wip-us.apache.org/repos/asf/kudu/blob/8a1a38ea/src/kudu/common/common.proto
----------------------------------------------------------------------
diff --git a/src/kudu/common/common.proto b/src/kudu/common/common.proto
index c85e65b..19ac6b0 100644
--- a/src/kudu/common/common.proto
+++ b/src/kudu/common/common.proto
@@ -27,6 +27,8 @@ package kudu;
option java_package = "org.apache.kudu";
+import "kudu/util/pb_util.proto";
+
// If you add a new type keep in mind to add it to the end
// or update AddMapping() functions like the one in key_encoder.cc
// that have a vector that maps the protobuf tag with the index.
@@ -300,22 +302,22 @@ message ColumnPredicatePB {
// predicate type for null-ness.
// The inclusive lower bound.
- optional bytes lower = 1;
+ optional bytes lower = 1 [(kudu.REDACT) = true];
// The exclusive upper bound.
- optional bytes upper = 2;
+ optional bytes upper = 2 [(kudu.REDACT) = true];
}
message Equality {
// The inclusive lower bound. See comment in Range for notes on the
// encoding.
- optional bytes value = 1;
+ optional bytes value = 1 [(kudu.REDACT) = true];
}
message InList {
// A list of values for the field. See comment in Range for notes on
// the encoding.
- repeated bytes values = 1;
+ repeated bytes values = 1 [(kudu.REDACT) = true];
}
message IsNotNull {}
http://git-wip-us.apache.org/repos/asf/kudu/blob/8a1a38ea/src/kudu/common/wire_protocol.proto
----------------------------------------------------------------------
diff --git a/src/kudu/common/wire_protocol.proto b/src/kudu/common/wire_protocol.proto
index 27365eb..93186d0 100644
--- a/src/kudu/common/wire_protocol.proto
+++ b/src/kudu/common/wire_protocol.proto
@@ -26,6 +26,7 @@ option java_package = "org.apache.kudu";
import "kudu/common/common.proto";
import "kudu/consensus/metadata.proto";
+import "kudu/util/pb_util.proto";
// Error status returned by any RPC method.
// Every RPC method which could generate an application-level error
@@ -189,6 +190,6 @@ message RowOperationsPB {
// For string data, the pointers are relative to 'indirect_data'.
//
// The rows are concatenated end-to-end with no padding/alignment.
- optional bytes rows = 2;
- optional bytes indirect_data = 3;
+ optional bytes rows = 2 [(kudu.REDACT) = true];
+ optional bytes indirect_data = 3 [(kudu.REDACT) = true];
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/8a1a38ea/src/kudu/master/master.proto
----------------------------------------------------------------------
diff --git a/src/kudu/master/master.proto b/src/kudu/master/master.proto
index fb02dc1..25672ef 100644
--- a/src/kudu/master/master.proto
+++ b/src/kudu/master/master.proto
@@ -22,6 +22,7 @@ import "kudu/common/common.proto";
import "kudu/common/wire_protocol.proto";
import "kudu/consensus/metadata.proto";
import "kudu/tablet/metadata.proto";
+import "kudu/util/pb_util.proto";
////////////////////////////////////////////////////////////
// Common data structures
@@ -390,8 +391,8 @@ message GetTableLocationsRequestPB {
required TableIdentifierPB table = 1;
// Partition-key range.
- optional bytes partition_key_start = 3;
- optional bytes partition_key_end = 4;
+ optional bytes partition_key_start = 3 [(kudu.REDACT) = true];
+ optional bytes partition_key_end = 4 [(kudu.REDACT) = true];
optional uint32 max_returned_locations = 5 [ default = 10 ];
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/8a1a38ea/src/kudu/rpc/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/CMakeLists.txt b/src/kudu/rpc/CMakeLists.txt
index 6d10586..f35965c 100644
--- a/src/kudu/rpc/CMakeLists.txt
+++ b/src/kudu/rpc/CMakeLists.txt
@@ -23,7 +23,7 @@ PROTOBUF_GENERATE_CPP(
PROTO_FILES rpc_header.proto)
ADD_EXPORTABLE_LIBRARY(rpc_header_proto
SRCS ${RPC_HEADER_PROTO_SRCS}
- DEPS protobuf
+ DEPS protobuf pb_util_proto
NONLINK_DEPS ${RPC_HEADER_PROTO_TGTS})
PROTOBUF_GENERATE_CPP(
http://git-wip-us.apache.org/repos/asf/kudu/blob/8a1a38ea/src/kudu/rpc/rpc_header.proto
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/rpc_header.proto b/src/kudu/rpc/rpc_header.proto
index aa5abf1..895a267 100644
--- a/src/kudu/rpc/rpc_header.proto
+++ b/src/kudu/rpc/rpc_header.proto
@@ -22,6 +22,7 @@ package kudu.rpc;
option java_package = "org.apache.kudu.rpc";
import "google/protobuf/descriptor.proto";
+import "kudu/util/pb_util.proto";
// The Kudu RPC protocol is similar to the RPC protocol of Hadoop and HBase.
// See the following for reference on those other protocols:
@@ -90,7 +91,7 @@ message SaslMessagePB {
// SASL challenge token from server, if the client chooses to use this method.
// Only used when the server is piggy-backing a challenge on a NEGOTIATE response.
// Otherwise, SaslMessagePB::token is used as the challenge token.
- optional bytes challenge = 5;
+ optional bytes challenge = 5 [(REDACT) = true];
}
// When the client sends its first NEGOTIATE message, it sends its set of supported
@@ -103,7 +104,7 @@ message SaslMessagePB {
repeated RpcFeatureFlag supported_features = 1;
required SaslState state = 2; // RPC system SASL state.
- optional bytes token = 3;
+ optional bytes token = 3 [(REDACT) = true];
repeated SaslAuth auths = 4;
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/8a1a38ea/src/kudu/tserver/tablet_copy.proto
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/tablet_copy.proto b/src/kudu/tserver/tablet_copy.proto
index 1e89919..5959ef4 100644
--- a/src/kudu/tserver/tablet_copy.proto
+++ b/src/kudu/tserver/tablet_copy.proto
@@ -23,6 +23,7 @@ import "kudu/consensus/metadata.proto";
import "kudu/fs/fs.proto";
import "kudu/rpc/rpc_header.proto";
import "kudu/tablet/metadata.proto";
+import "kudu/util/pb_util.proto";
// RaftConfig tablet copy RPC calls.
service TabletCopyService {
@@ -172,7 +173,7 @@ message DataChunkPB {
required uint64 offset = 1;
// Actual bytes of data from the data block, starting at 'offset'.
- required bytes data = 2;
+ required bytes data = 2 [(kudu.REDACT) = true];
// CRC32C of the bytes contained in 'data'.
required fixed32 crc32 = 3;
http://git-wip-us.apache.org/repos/asf/kudu/blob/8a1a38ea/src/kudu/tserver/tserver.proto
----------------------------------------------------------------------
diff --git a/src/kudu/tserver/tserver.proto b/src/kudu/tserver/tserver.proto
index 95f4fce..2fc8184 100644
--- a/src/kudu/tserver/tserver.proto
+++ b/src/kudu/tserver/tserver.proto
@@ -21,6 +21,7 @@ option java_package = "org.apache.kudu.tserver";
import "kudu/common/common.proto";
import "kudu/common/wire_protocol.proto";
import "kudu/tablet/tablet.proto";
+import "kudu/util/pb_util.proto";
// Tablet-server specific errors use this protobuf.
message TabletServerErrorPB {
@@ -196,8 +197,8 @@ message ColumnRangePredicatePB {
// predicate type for null-ness.
//
// Both bounds are inclusive.
- optional bytes lower_bound = 2;
- optional bytes inclusive_upper_bound = 3;
+ optional bytes lower_bound = 2 [(kudu.REDACT) = true];
+ optional bytes inclusive_upper_bound = 3 [(kudu.REDACT) = true];
}
// List of predicates used by the Java client. Will rapidly evolve into something more reusable
@@ -224,9 +225,9 @@ message NewScanRequestPB {
repeated ColumnPredicatePB column_predicates = 13;
// Encoded primary key to begin scanning at (inclusive).
- optional bytes start_primary_key = 8;
+ optional bytes start_primary_key = 8 [(kudu.REDACT) = true];
// Encoded primary key to stop scanning at (exclusive).
- optional bytes stop_primary_key = 9;
+ optional bytes stop_primary_key = 9 [(kudu.REDACT) = true];
// Which columns to select.
// if this is an empty list, no data will be returned, but the num_rows
@@ -259,7 +260,7 @@ message NewScanRequestPB {
// If retrying a scan, the final primary key retrieved in the previous scan
// attempt. If set, this will take precedence over the `start_primary_key`
// field, and functions as an exclusive start primary key.
- optional bytes last_primary_key = 12;
+ optional bytes last_primary_key = 12 [(kudu.REDACT) = true];
}
// A scan request. Initially, it should specify a scan. Later on, you
@@ -343,7 +344,7 @@ message ScanResponsePB {
// If this is a fault-tolerant scanner, this is set to the encoded primary
// key of the last row returned in the response.
- optional bytes last_primary_key = 7;
+ optional bytes last_primary_key = 7 [(kudu.REDACT) = true];
// The resource usage of this RPC.
optional ResourceMetricsPB resource_metrics = 8;
http://git-wip-us.apache.org/repos/asf/kudu/blob/8a1a38ea/src/kudu/util/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/src/kudu/util/CMakeLists.txt b/src/kudu/util/CMakeLists.txt
index 25bac24..d6fd37d 100644
--- a/src/kudu/util/CMakeLists.txt
+++ b/src/kudu/util/CMakeLists.txt
@@ -385,7 +385,7 @@ if(NOT "${NO_TESTS}")
endif()
#######################################
-# proto_container_test_proto
+# pb_util_test_proto
#######################################
PROTOBUF_GENERATE_CPP(
@@ -395,11 +395,13 @@ PROTOBUF_GENERATE_CPP(
PROTO_FILES
proto_container_test.proto
proto_container_test2.proto
- proto_container_test3.proto)
-add_library(proto_container_test_proto
+ proto_container_test3.proto
+ pb_util_test.proto)
+add_library(pb_util_test_proto
${PROTO_CONTAINER_TEST_PROTO_SRCS}
${PROTO_CONTAINER_TEST_PROTO_HDRS})
-target_link_libraries(proto_container_test_proto
+target_link_libraries(pb_util_test_proto
+ pb_util_proto
protobuf)
#######################################
@@ -409,5 +411,5 @@ target_link_libraries(proto_container_test_proto
ADD_KUDU_TEST(pb_util-test)
if(NOT "${NO_TESTS}")
target_link_libraries(pb_util-test
- proto_container_test_proto)
+ pb_util_test_proto)
endif()
http://git-wip-us.apache.org/repos/asf/kudu/blob/8a1a38ea/src/kudu/util/pb_util-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/pb_util-test.cc b/src/kudu/util/pb_util-test.cc
index 3ec57da..94acd21 100644
--- a/src/kudu/util/pb_util-test.cc
+++ b/src/kudu/util/pb_util-test.cc
@@ -22,18 +22,22 @@
#include <string>
#include <vector>
+#include <gflags/gflags_declare.h>
#include <google/protobuf/descriptor.pb.h>
#include <gtest/gtest.h>
#include "kudu/util/env_util.h"
-#include "kudu/util/pb_util.h"
#include "kudu/util/pb_util-internal.h"
+#include "kudu/util/pb_util.h"
+#include "kudu/util/pb_util_test.pb.h"
#include "kudu/util/proto_container_test.pb.h"
#include "kudu/util/proto_container_test2.pb.h"
#include "kudu/util/proto_container_test3.pb.h"
#include "kudu/util/status.h"
#include "kudu/util/test_util.h"
+DECLARE_bool(log_redact_user_data);
+
namespace kudu {
namespace pb_util {
@@ -580,5 +584,30 @@ TEST_F(TestPBUtil, TestOverwriteExistingPB) {
ASSERT_OK(CreateKnownGoodContainerFile(OVERWRITE));
}
+TEST_F(TestPBUtil, TestRedaction) {
+ FLAGS_log_redact_user_data = true;
+ TestSecurePrintingPB pb;
+
+ pb.set_insecure1("public 1");
+ pb.set_insecure2("public 2");
+ pb.set_secure1("private 1");
+ pb.set_secure2("private 2");
+ pb.add_repeated_secure("private 3");
+ pb.add_repeated_secure("private 4");
+ pb.set_insecure3("public 3");
+
+ for (auto s : {SecureDebugString(pb), SecureShortDebugString(pb)}) {
+ ASSERT_EQ(string::npos, s.find("private"));
+ ASSERT_STR_CONTAINS(s, "<redacted>");
+ ASSERT_STR_CONTAINS(s, "public 1");
+ ASSERT_STR_CONTAINS(s, "public 2");
+ ASSERT_STR_CONTAINS(s, "public 3");
+ }
+
+ // If we disable redaction, we should see the private fields.
+ FLAGS_log_redact_user_data = false;
+ ASSERT_STR_CONTAINS(SecureDebugString(pb), "private");
+}
+
} // namespace pb_util
} // namespace kudu
http://git-wip-us.apache.org/repos/asf/kudu/blob/8a1a38ea/src/kudu/util/pb_util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/pb_util.cc b/src/kudu/util/pb_util.cc
index 44c7534..09a642b 100644
--- a/src/kudu/util/pb_util.cc
+++ b/src/kudu/util/pb_util.cc
@@ -43,6 +43,7 @@
#include <google/protobuf/io/zero_copy_stream_impl_lite.h>
#include <google/protobuf/message.h>
#include <google/protobuf/message_lite.h>
+#include <google/protobuf/text_format.h>
#include "kudu/gutil/bind.h"
#include "kudu/gutil/callback.h"
@@ -58,6 +59,7 @@
#include "kudu/util/env.h"
#include "kudu/util/env_util.h"
#include "kudu/util/jsonwriter.h"
+#include "kudu/util/logging.h"
#include "kudu/util/mutex.h"
#include "kudu/util/path_util.h"
#include "kudu/util/pb_util-internal.h"
@@ -77,6 +79,7 @@ using google::protobuf::Message;
using google::protobuf::MessageLite;
using google::protobuf::Reflection;
using google::protobuf::SimpleDescriptorDatabase;
+using google::protobuf::TextFormat;
using kudu::crc::Crc;
using kudu::pb_util::internal::SequentialFileFileInputStream;
using kudu::pb_util::internal::WritableFileOutputStream;
@@ -541,6 +544,64 @@ void TruncateFields(Message* message, int max_len) {
}
}
+namespace {
+class SecureFieldPrinter : public TextFormat::FieldValuePrinter {
+ public:
+ using super = TextFormat::FieldValuePrinter;
+
+ string PrintFieldName(const Message& message,
+ const Reflection* reflection,
+ const FieldDescriptor* field) const override {
+ hide_next_string_ = field->cpp_type() == FieldDescriptor::CPPTYPE_STRING &&
+ field->options().GetExtension(REDACT);
+ return super::PrintFieldName(message, reflection, field);
+ }
+
+ string PrintString(const string& val) const override {
+ if (hide_next_string_) {
+ hide_next_string_ = false;
+ return KUDU_REDACT(super::PrintString(val));
+ }
+ return super::PrintString(val);
+ }
+ string PrintBytes(const string& val) const override {
+ if (hide_next_string_) {
+ hide_next_string_ = false;
+ return KUDU_REDACT(super::PrintString(val));
+ }
+ return super::PrintBytes(val);
+ }
+
+ mutable bool hide_next_string_ = false;
+};
+} // anonymous namespace
+
+string SecureDebugString(const Message& msg) {
+ string debug_string;
+ TextFormat::Printer printer;
+ printer.SetDefaultFieldValuePrinter(new SecureFieldPrinter());
+ printer.PrintToString(msg, &debug_string);
+ return debug_string;
+}
+
+string SecureShortDebugString(const Message& msg) {
+ string debug_string;
+
+ TextFormat::Printer printer;
+ printer.SetSingleLineMode(true);
+ printer.SetDefaultFieldValuePrinter(new SecureFieldPrinter());
+
+ printer.PrintToString(msg, &debug_string);
+ // Single line mode currently might have an extra space at the end.
+ if (!debug_string.empty() &&
+ debug_string[debug_string.size() - 1] == ' ') {
+ debug_string.resize(debug_string.size() - 1);
+ }
+
+ return debug_string;
+}
+
+
WritablePBContainerFile::WritablePBContainerFile(shared_ptr<RWFile> writer)
: state_(FileState::NOT_INITIALIZED),
offset_(0),
http://git-wip-us.apache.org/repos/asf/kudu/blob/8a1a38ea/src/kudu/util/pb_util.h
----------------------------------------------------------------------
diff --git a/src/kudu/util/pb_util.h b/src/kudu/util/pb_util.h
index eef5d27..513f033 100644
--- a/src/kudu/util/pb_util.h
+++ b/src/kudu/util/pb_util.h
@@ -99,6 +99,18 @@ Status WritePBToPath(Env* env, const std::string& path, const MessageLite& msg,
// The text "<truncated>" is appended to any such truncated fields.
void TruncateFields(google::protobuf::Message* message, int max_len);
+// Redaction-sensitive variant of Message::DebugString.
+//
+// For most protobufs, this has identical output to Message::DebugString. However,
+// a field with string or binary type may be tagged with the 'kudu.REDACT' option,
+// available by importing 'pb_util.proto'. When such a field is encountered by this
+// method, its contents will be redacted using the 'KUDU_REDACT' macro as documented
+// in kudu/util/logging.h.
+std::string SecureDebugString(const google::protobuf::Message& msg);
+
+// Same as SecureDebugString() above, but equivalent to Message::ShortDebugString.
+std::string SecureShortDebugString(const google::protobuf::Message& msg);
+
// A protobuf "container" has the following format (all integers in
// little-endian byte order).
//
http://git-wip-us.apache.org/repos/asf/kudu/blob/8a1a38ea/src/kudu/util/pb_util.proto
----------------------------------------------------------------------
diff --git a/src/kudu/util/pb_util.proto b/src/kudu/util/pb_util.proto
index 71e8bdb..9688f8f 100644
--- a/src/kudu/util/pb_util.proto
+++ b/src/kudu/util/pb_util.proto
@@ -38,3 +38,7 @@ message ContainerSupHeaderPB {
// be fully qualified (i.e. kudu.tablet.TabletSuperBlockPB).
required string pb_type = 2;
}
+
+extend google.protobuf.FieldOptions {
+ optional bool REDACT = 50001 [default=false];
+}
\ No newline at end of file
http://git-wip-us.apache.org/repos/asf/kudu/blob/8a1a38ea/src/kudu/util/pb_util_test.proto
----------------------------------------------------------------------
diff --git a/src/kudu/util/pb_util_test.proto b/src/kudu/util/pb_util_test.proto
new file mode 100644
index 0000000..0b7be32
--- /dev/null
+++ b/src/kudu/util/pb_util_test.proto
@@ -0,0 +1,28 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you 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.
+package kudu;
+
+import "kudu/util/pb_util.proto";
+
+message TestSecurePrintingPB {
+ optional string insecure1 = 1;
+ optional string secure1 = 2 [(kudu.REDACT) = true];
+ optional string insecure2 = 3;
+ optional string secure2 = 4 [(kudu.REDACT) = true];
+ repeated string repeated_secure = 5 [(kudu.REDACT) = true];
+ optional string insecure3 = 6;
+}
[2/2] kudu git commit: KUDU-1812. address comments on 1179bbdb
Posted by da...@apache.org.
KUDU-1812. address comments on 1179bbdb
Change-Id: Ia6d4d1b76c0c24b3468c27f68354c44cb4223bc4
Reviewed-on: http://gerrit.cloudera.org:8080/5566
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
(cherry picked from commit f3447a104fd5b6ca71122914838ea5f7d73c8efe)
Reviewed-on: http://gerrit.cloudera.org:8080/5572
Reviewed-by: Dan Burkert <da...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/5a6ef37b
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/5a6ef37b
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/5a6ef37b
Branch: refs/heads/branch-1.2.x
Commit: 5a6ef37bd1c93ef0bb6e2f7da82eb214efe25058
Parents: 8a1a38e
Author: Dan Burkert <da...@apache.org>
Authored: Thu Dec 22 11:14:09 2016 -0800
Committer: Dan Burkert <da...@apache.org>
Committed: Fri Dec 23 01:19:35 2016 +0000
----------------------------------------------------------------------
src/kudu/client/client.h | 3 ++-
src/kudu/client/write_op.h | 3 ++-
src/kudu/common/column_predicate-test.cc | 4 +++-
src/kudu/common/partial_row.h | 4 +++-
src/kudu/common/partition-test.cc | 2 +-
src/kudu/common/types-test.cc | 17 ++++++++++++++++-
src/kudu/tools/tool_main.cc | 10 +++++++---
src/kudu/util/logging.h | 2 +-
8 files changed, 35 insertions(+), 10 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/5a6ef37b/src/kudu/client/client.h
----------------------------------------------------------------------
diff --git a/src/kudu/client/client.h b/src/kudu/client/client.h
index 2cf5dbc..3c1f9b1 100644
--- a/src/kudu/client/client.h
+++ b/src/kudu/client/client.h
@@ -1924,7 +1924,8 @@ class KUDU_EXPORT KuduScanner {
/// @return String representation of this scan.
///
- /// @internal This method must not be used in log messages because it contains
+ /// @internal
+ /// @note This method must not be used in log messages because it contains
/// sensitive predicate values. Use Scanner::Data::DebugString instead.
std::string ToString() const;
http://git-wip-us.apache.org/repos/asf/kudu/blob/5a6ef37b/src/kudu/client/write_op.h
----------------------------------------------------------------------
diff --git a/src/kudu/client/write_op.h b/src/kudu/client/write_op.h
index b4433e5..734ec31 100644
--- a/src/kudu/client/write_op.h
+++ b/src/kudu/client/write_op.h
@@ -76,7 +76,8 @@ class KUDU_EXPORT KuduWriteOperation {
/// @return String representation of the operation.
///
- /// @internal this method does *NOT* redact row values. The
+ /// @internal
+ /// @note this method does note redact row values. The
/// caller must handle redaction themselves, if necessary.
virtual std::string ToString() const = 0;
protected:
http://git-wip-us.apache.org/repos/asf/kudu/blob/5a6ef37b/src/kudu/common/column_predicate-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/common/column_predicate-test.cc b/src/kudu/common/column_predicate-test.cc
index 5c13cfa..9f56186 100644
--- a/src/kudu/common/column_predicate-test.cc
+++ b/src/kudu/common/column_predicate-test.cc
@@ -17,9 +17,11 @@
#include "kudu/common/column_predicate.h"
+#include <vector>
+
+#include <gflags/gflags_declare.h>
#include <glog/logging.h>
#include <gtest/gtest.h>
-#include <vector>
#include "kudu/common/schema.h"
#include "kudu/common/types.h"
http://git-wip-us.apache.org/repos/asf/kudu/blob/5a6ef37b/src/kudu/common/partial_row.h
----------------------------------------------------------------------
diff --git a/src/kudu/common/partial_row.h b/src/kudu/common/partial_row.h
index 96f4d4b..adbed4f 100644
--- a/src/kudu/common/partial_row.h
+++ b/src/kudu/common/partial_row.h
@@ -459,7 +459,9 @@ class KUDU_EXPORT KuduPartialRow {
/// @return String representation for the partial row.
///
- /// @internal NOTE: this is not redacted.
+ /// @internal
+ /// @note this method does note redact row values. The
+ /// caller must handle redaction themselves, if necessary.
std::string ToString() const;
/// @return The schema object for the partial row.
http://git-wip-us.apache.org/repos/asf/kudu/blob/5a6ef37b/src/kudu/common/partition-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/common/partition-test.cc b/src/kudu/common/partition-test.cc
index 5b2464c..d338a82 100644
--- a/src/kudu/common/partition-test.cc
+++ b/src/kudu/common/partition-test.cc
@@ -23,7 +23,7 @@
#include <vector>
#include <boost/optional.hpp>
-#include <gflags/gflags.h>
+#include <gflags/gflags_declare.h>
#include "kudu/common/common.pb.h"
#include "kudu/common/partial_row.h"
http://git-wip-us.apache.org/repos/asf/kudu/blob/5a6ef37b/src/kudu/common/types-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/common/types-test.cc b/src/kudu/common/types-test.cc
index 5e9f835..c297444 100644
--- a/src/kudu/common/types-test.cc
+++ b/src/kudu/common/types-test.cc
@@ -16,11 +16,13 @@
// under the License.
#include <cmath>
-#include <gtest/gtest.h>
#include <string>
#include <tuple>
#include <vector>
+#include <gflags/gflags.h>
+#include <gtest/gtest.h>
+
#include "kudu/common/types.h"
#include "kudu/gutil/strings/substitute.h"
#include "kudu/util/test_util.h"
@@ -32,6 +34,8 @@ using std::string;
using std::tuple;
using std::vector;
+DECLARE_bool(log_redact_user_data);
+
namespace kudu {
class TestTypes : public KuduTest {};
@@ -71,6 +75,17 @@ TEST_F(TestTypes, TestTimestampPrinting) {
time = MathLimits<int64>::kMax;
info->AppendDebugStringForValue(&time, &result);
ASSERT_EQ("294247-01-10T04:00:54.775807Z", result);
+ result = "";
+
+ {
+ // Check that row values are redacted when the log_redact_user_data flag is set.
+ google::FlagSaver flag_saver;
+ FLAGS_log_redact_user_data = true;
+ time = 0;
+ info->AppendDebugStringForValue(&time, &result);
+ ASSERT_EQ("<redacted>", result);
+ result = "";
+ }
}
namespace {
http://git-wip-us.apache.org/repos/asf/kudu/blob/5a6ef37b/src/kudu/tools/tool_main.cc
----------------------------------------------------------------------
diff --git a/src/kudu/tools/tool_main.cc b/src/kudu/tools/tool_main.cc
index 2d0533b..d48d95e 100644
--- a/src/kudu/tools/tool_main.cc
+++ b/src/kudu/tools/tool_main.cc
@@ -222,10 +222,14 @@ static bool ParseCommandLineFlags(int* argc, char*** argv) {
}
int main(int argc, char** argv) {
- bool show_help = ParseCommandLineFlags(&argc, &argv);
FLAGS_logtostderr = true;
- // Disable redaction so that user data printed to the console will be shown in full.
- FLAGS_log_redact_user_data = false;
+ bool show_help = ParseCommandLineFlags(&argc, &argv);
+
+ if (google::GetCommandLineFlagInfoOrDie("log_redact_user_data").is_default) {
+ // Disable redaction so that user data printed to the console will be shown in full.
+ FLAGS_log_redact_user_data = false;
+ }
+
kudu::InitGoogleLoggingSafe(argv[0]);
return kudu::tools::RunTool(argc, argv, show_help);
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/5a6ef37b/src/kudu/util/logging.h
----------------------------------------------------------------------
diff --git a/src/kudu/util/logging.h b/src/kudu/util/logging.h
index 9bdc370..18a8a3c 100644
--- a/src/kudu/util/logging.h
+++ b/src/kudu/util/logging.h
@@ -60,7 +60,7 @@
// to short-circuit expensive logic.
#define KUDU_SHOULD_REDACT() (FLAGS_log_redact_user_data && kudu::tls_redact_user_data)
-// Either evaluate and return 'expr', or return the string "redacted", depending on whether
+// Either evaluate and return 'expr', or return the string "<redacted>", depending on whether
// redaction is enabled in the current scope.
#define KUDU_REDACT(expr) \
(KUDU_SHOULD_REDACT() ? kudu::kRedactionMessage : (expr))