This is an automated email from the ASF dual-hosted git repository. todd pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 802623741e163333e20ad57e059f4ce20701973e Author: Hao Hao <[email protected]> AuthorDate: Wed Mar 11 16:57:23 2020 -0700 [ranger] pass 'principal' and 'keytab' to the subprocess This patch adds the C++ side change to pass the Kudu principal and keytab to the Java Ranger subprocess. Change-Id: Ie30b835b6d44ddb51d95c587f1329bfefebeb37c Reviewed-on: http://gerrit.cloudera.org:8080/15416 Reviewed-by: Adar Dembo <[email protected]> Reviewed-by: Andrew Wong <[email protected]> Tested-by: Hao Hao <[email protected]> --- src/kudu/ranger/CMakeLists.txt | 3 +- src/kudu/ranger/ranger_client.cc | 90 ++++++++++++++++++++++++++++------------ src/kudu/ranger/ranger_client.h | 18 ++++---- src/kudu/security/init.cc | 54 ++++++++++++++---------- src/kudu/security/init.h | 6 +++ src/kudu/server/server_base.cc | 18 +------- 6 files changed, 113 insertions(+), 76 deletions(-) diff --git a/src/kudu/ranger/CMakeLists.txt b/src/kudu/ranger/CMakeLists.txt index d735f33..f486a19 100644 --- a/src/kudu/ranger/CMakeLists.txt +++ b/src/kudu/ranger/CMakeLists.txt @@ -41,7 +41,8 @@ set(RANGER_SRCS set(RANGER_DEPS gflags kudu_subprocess - ranger_proto) + ranger_proto + security) add_library(kudu_ranger ${RANGER_SRCS}) target_link_libraries(kudu_ranger ${RANGER_DEPS}) diff --git a/src/kudu/ranger/ranger_client.cc b/src/kudu/ranger/ranger_client.cc index 7899b6e..73e17d2 100644 --- a/src/kudu/ranger/ranger_client.cc +++ b/src/kudu/ranger/ranger_client.cc @@ -31,6 +31,7 @@ #include "kudu/gutil/strings/join.h" #include "kudu/gutil/strings/substitute.h" #include "kudu/ranger/ranger.pb.h" +#include "kudu/security/init.h" #include "kudu/util/env.h" #include "kudu/util/flag_tags.h" #include "kudu/util/flag_validators.h" @@ -117,6 +118,9 @@ METRIC_DEFINE_histogram(server, ranger_server_outbound_queue_time_ms, kudu::MetricLevel::kInfo, 60000LU, 1); +DECLARE_string(keytab_file); +DECLARE_string(principal); + namespace kudu { namespace ranger { @@ -127,13 +131,50 @@ using std::unordered_set; using std::vector; using strings::Substitute; +static const char* kUnauthorizedAction = "Unauthorized action"; +static const char* kDenyNonRangerTableTemplate = "Denying action on table with invalid name $0. " + "Use 'kudu table rename_table' to rename it to " + "a Ranger-compatible name."; +static const char* kMainClass = "org.apache.kudu.subprocess.ranger.RangerSubprocessMain"; + // Returns the path to the JAR file containing the Ranger subprocess. static string GetRangerJarPath() { - string exe; - CHECK_OK(Env::Default()->GetExecutablePath(&exe)); - const string bin_dir = DirName(exe); - return FLAGS_ranger_jar_path.empty() ? JoinPathSegments(bin_dir, "kudu-subprocess.jar") : - FLAGS_ranger_jar_path; + if (FLAGS_ranger_jar_path.empty()) { + string exe; + CHECK_OK(Env::Default()->GetExecutablePath(&exe)); + const string bin_dir = DirName(exe); + return JoinPathSegments(bin_dir, "kudu-subprocess.jar"); + } + return FLAGS_ranger_jar_path; +} + +// Returns the classpath to be used for the Ranger subprocess. +static string GetJavaClasspath() { + return Substitute("$0:$1", GetRangerJarPath(), FLAGS_ranger_config_path); +} + +// Builds the arguments for the Ranger subprocess. Specifically pass +// the principal and keytab file that the Ranger subprocess will log in with +// if Kerberos is enabled. 'args' has the final arguments. +// Returns 'OK' if arguments successfully created, error otherwise. +static Status BuildArgv(vector<string>* argv) { + DCHECK(argv); + // Pass the required arguments to run the Ranger subprocess. + vector<string> ret = { FLAGS_ranger_java_path, "-cp", GetJavaClasspath(), kMainClass }; + // When Kerberos is enabled in Kudu, pass both Kudu principal and keytab file + // to the Ranger subprocess. + if (!FLAGS_keytab_file.empty()) { + string configured_principal; + RETURN_NOT_OK_PREPEND(security::GetConfiguredPrincipal(FLAGS_principal, &configured_principal), + "unable to get the configured principal from for the Ranger subprocess"); + ret.emplace_back("-i"); + ret.emplace_back(std::move(configured_principal)); + ret.emplace_back("-k"); + ret.emplace_back(FLAGS_keytab_file); + } + + *argv = std::move(ret); + return Status::OK(); } static bool ValidateRangerConfiguration() { @@ -145,14 +186,14 @@ static bool ValidateRangerConfiguration() { string p; Status s = Subprocess::Call({ "which", FLAGS_ranger_java_path }, "", &p); if (!s.ok()) { - LOG(ERROR) << Substitute("FLAGS_ranger_java_path has invalid java binary path: $0", + LOG(ERROR) << Substitute("--ranger_java_path has invalid java binary path: $0", FLAGS_ranger_java_path); return false; } } string ranger_jar_path = GetRangerJarPath(); if (!Env::Default()->FileExists(ranger_jar_path)) { - LOG(ERROR) << Substitute("FLAGS_ranger_jar_path has invalid JAR file path: $0", + LOG(ERROR) << Substitute("--ranger_jar_path has invalid JAR file path: $0", ranger_jar_path); return false; } @@ -161,12 +202,6 @@ static bool ValidateRangerConfiguration() { } GROUP_FLAG_VALIDATOR(ranger_config_flags, ValidateRangerConfiguration); -static const char* kUnauthorizedAction = "Unauthorized action"; -static const char* kDenyNonRangerTableTemplate = "Denying action on table with invalid name $0. " - "Use 'kudu table rename_table' to rename it to " - "a Ranger-compatible name."; -const char* kMainClass = "org.apache.kudu.subprocess.ranger.RangerSubprocessMain"; - #define HISTINIT(member, x) member = METRIC_##x.Instantiate(entity) RangerSubprocessMetrics::RangerSubprocessMetrics(const scoped_refptr<MetricEntity>& entity) { HISTINIT(sp_inbound_queue_length, ranger_subprocess_inbound_queue_length); @@ -181,19 +216,24 @@ RangerSubprocessMetrics::RangerSubprocessMetrics(const scoped_refptr<MetricEntit } #undef HISTINIT -RangerClient::RangerClient(const scoped_refptr<MetricEntity>& metric_entity) : - subprocess_({ FLAGS_ranger_java_path, "-cp", GetJavaClasspath(), kMainClass }, - metric_entity) {} +RangerClient::RangerClient(const scoped_refptr<MetricEntity>& metric_entity) + : metric_entity_(metric_entity) { + DCHECK(metric_entity); +} Status RangerClient::Start() { VLOG(1) << "Initializing Ranger subprocess server"; - return subprocess_.Start(); + vector<string> argv; + RETURN_NOT_OK(BuildArgv(&argv)); + subprocess_.reset(new RangerSubprocess(std::move(argv), metric_entity_)); + return subprocess_->Start(); } // TODO(abukor): refactor to avoid code duplication Status RangerClient::AuthorizeAction(const string& user_name, const ActionPB& action, const string& table_name) { + DCHECK(subprocess_); string db; Slice tbl; @@ -213,7 +253,7 @@ Status RangerClient::AuthorizeAction(const string& user_name, req->set_database(db); req->set_table(tbl.ToString()); - RETURN_NOT_OK(subprocess_.Execute(req_list, &resp_list)); + RETURN_NOT_OK(subprocess_->Execute(req_list, &resp_list)); CHECK_EQ(1, resp_list.responses_size()); if (resp_list.responses().begin()->allowed()) { @@ -229,6 +269,7 @@ Status RangerClient::AuthorizeActionMultipleColumns(const string& user_name, const ActionPB& action, const string& table_name, unordered_set<string>* column_names) { + DCHECK(subprocess_); DCHECK(!column_names->empty()); string db; @@ -252,7 +293,7 @@ Status RangerClient::AuthorizeActionMultipleColumns(const string& user_name, req->set_column(col); } - RETURN_NOT_OK(subprocess_.Execute(req_list, &resp_list)); + RETURN_NOT_OK(subprocess_->Execute(req_list, &resp_list)); DCHECK_EQ(column_names->size(), resp_list.responses_size()); @@ -277,6 +318,7 @@ Status RangerClient::AuthorizeActionMultipleColumns(const string& user_name, Status RangerClient::AuthorizeActionMultipleTables(const string& user_name, const ActionPB& action, unordered_set<string>* table_names) { + DCHECK(subprocess_); if (table_names->empty()) { return Status::InvalidArgument("Empty set of tables"); } @@ -304,7 +346,7 @@ Status RangerClient::AuthorizeActionMultipleTables(const string& user_name, } } - RETURN_NOT_OK(subprocess_.Execute(req_list, &resp_list)); + RETURN_NOT_OK(subprocess_->Execute(req_list, &resp_list)); DCHECK_EQ(orig_table_names.size(), resp_list.responses_size()); @@ -329,6 +371,7 @@ Status RangerClient::AuthorizeActionMultipleTables(const string& user_name, Status RangerClient::AuthorizeActions(const string& user_name, const string& table_name, unordered_set<ActionPB, ActionHash>* actions) { + DCHECK(subprocess_); DCHECK(!actions->empty()); string db; @@ -351,7 +394,7 @@ Status RangerClient::AuthorizeActions(const string& user_name, req->set_table(tbl.ToString()); } - RETURN_NOT_OK(subprocess_.Execute(req_list, &resp_list)); + RETURN_NOT_OK(subprocess_->Execute(req_list, &resp_list)); DCHECK_EQ(actions->size(), resp_list.responses_size()); @@ -372,10 +415,5 @@ Status RangerClient::AuthorizeActions(const string& user_name, return Status::OK(); } - -string RangerClient::GetJavaClasspath() { - return Substitute("$0:$1", GetRangerJarPath(), FLAGS_ranger_config_path); -} - } // namespace ranger } // namespace kudu diff --git a/src/kudu/ranger/ranger_client.h b/src/kudu/ranger/ranger_client.h index c1f091e..d6e920b 100644 --- a/src/kudu/ranger/ranger_client.h +++ b/src/kudu/ranger/ranger_client.h @@ -27,12 +27,11 @@ #include "kudu/ranger/ranger.pb.h" #include "kudu/subprocess/server.h" #include "kudu/subprocess/subprocess_proxy.h" +#include "kudu/util/metrics.h" #include "kudu/util/status.h" namespace kudu { -class MetricEntity; - namespace ranger { struct ActionHash { @@ -91,17 +90,16 @@ class RangerClient { // Replaces the subprocess server in the subprocess proxy. void ReplaceServerForTests(std::unique_ptr<subprocess::SubprocessServer> server) { - subprocess_.ReplaceServerForTests(std::move(server)); + // Creates a dummy RangerSubprocess if it is not initialized. + if (!subprocess_) { + subprocess_.reset(new RangerSubprocess({""}, metric_entity_)); + } + subprocess_->ReplaceServerForTests(std::move(server)); } private: - // Sends request to the subprocess and parses the response. - Status SendRequest(RangerRequestListPB* req, RangerResponseListPB* resp) WARN_UNUSED_RESULT; - - // Returns classpath to be used for the Ranger subprocess. - static std::string GetJavaClasspath(); - - RangerSubprocess subprocess_; + std::unique_ptr<RangerSubprocess> subprocess_; + scoped_refptr<MetricEntity> metric_entity_; }; } // namespace ranger diff --git a/src/kudu/security/init.cc b/src/kudu/security/init.cc index e740fcb..1268386 100644 --- a/src/kudu/security/init.cc +++ b/src/kudu/security/init.cc @@ -73,6 +73,21 @@ DEFINE_bool(use_system_auth_to_local, kDefaultSystemAuthToLocal, "'kudu/foo.example.com@EXAMPLE' will map to 'kudu'."); TAG_FLAG(use_system_auth_to_local, advanced); +DEFINE_string(principal, "kudu/_HOST", + "Kerberos principal that this daemon will log in as. The special token " + "_HOST will be replaced with the FQDN of the local host."); +TAG_FLAG(principal, experimental); +// This is currently tagged as unsafe because there is no way for users to configure +// clients to expect a non-default principal. As such, configuring a server to login +// as a different one would end up with a cluster that can't be connected to. +// See KUDU-1884. +TAG_FLAG(principal, unsafe); + +DEFINE_string(keytab_file, "", + "Path to the Kerberos Keytab file for this server. Specifying a " + "keytab file will cause the server to kinit, and enable Kerberos " + "to be used to authenticate RPC connections."); +TAG_FLAG(keytab_file, stable); using std::mt19937; using std::random_device; @@ -363,29 +378,6 @@ Status KinitContext::KinitInternal() { return Status::OK(); } -namespace { -// 'in_principal' is the user specified principal to use with Kerberos. It may have a token -// in the string of the form '_HOST', which if present, needs to be replaced with the FQDN of the -// current host. -// 'out_principal' has the final principal with which one may Kinit. -Status GetConfiguredPrincipal(const std::string& in_principal, string* out_principal) { - *out_principal = in_principal; - const auto& kHostToken = "_HOST"; - if (in_principal.find(kHostToken) != string::npos) { - string hostname; - // Try to fill in either the FQDN or hostname. - if (!GetFQDN(&hostname).ok()) { - RETURN_NOT_OK(GetHostname(&hostname)); - } - // Hosts in principal names are canonicalized to lower-case. - std::transform(hostname.begin(), hostname.end(), hostname.begin(), tolower); - GlobalReplaceSubstring(kHostToken, hostname, out_principal); - } - return Status::OK(); -} -} // anonymous namespace - - RWMutex* KerberosReinitLock() { return g_kerberos_reinit_lock; } @@ -444,6 +436,22 @@ Status MapPrincipalToLocalName(const std::string& principal, std::string* local_ return Status::OK(); } +Status GetConfiguredPrincipal(const string& in_principal, string* out_principal) { + *out_principal = in_principal; + static const auto& kHostToken = "_HOST"; + if (in_principal.find(kHostToken) != string::npos) { + string hostname; + // Try to fill in either the FQDN or hostname. + if (!GetFQDN(&hostname).ok()) { + RETURN_NOT_OK(GetHostname(&hostname)); + } + // Hosts in principal names are canonicalized to lower-case. + std::transform(hostname.begin(), hostname.end(), hostname.begin(), tolower); + GlobalReplaceSubstring(kHostToken, hostname, out_principal); + } + return Status::OK(); +} + boost::optional<string> GetLoggedInPrincipalFromKeytab() { if (!g_kinit_ctx) return boost::none; return g_kinit_ctx->principal_str(); diff --git a/src/kudu/security/init.h b/src/kudu/security/init.h index 80074b3..31dba47 100644 --- a/src/kudu/security/init.h +++ b/src/kudu/security/init.h @@ -86,5 +86,11 @@ Status CanonicalizeKrb5Principal(std::string* principal); // exist yet, and trying to avoid rebase pain). Status MapPrincipalToLocalName(const std::string& principal, std::string* local_name); +// Get the configured principal. 'in_principal' is the user specified principal to use with +// Kerberos. It may have a token in the string of the form '_HOST', which if present, needs +// to be replaced with the FQDN of the current host. 'out_principal' has the final principal +// with which one may Kinit. +Status GetConfiguredPrincipal(const std::string& in_principal, std::string* out_principal); + } // namespace security } // namespace kudu diff --git a/src/kudu/server/server_base.cc b/src/kudu/server/server_base.cc index 828b3b2..754c5a5 100644 --- a/src/kudu/server/server_base.cc +++ b/src/kudu/server/server_base.cc @@ -126,22 +126,6 @@ DEFINE_string(user_acl, "*", TAG_FLAG(user_acl, stable); TAG_FLAG(user_acl, sensitive); -DEFINE_string(principal, "kudu/_HOST", - "Kerberos principal that this daemon will log in as. The special token " - "_HOST will be replaced with the FQDN of the local host."); -TAG_FLAG(principal, experimental); -// This is currently tagged as unsafe because there is no way for users to configure -// clients to expect a non-default principal. As such, configuring a server to login -// as a different one would end up with a cluster that can't be connected to. -// See KUDU-1884. -TAG_FLAG(principal, unsafe); - -DEFINE_string(keytab_file, "", - "Path to the Kerberos Keytab file for this server. Specifying a " - "keytab file will cause the server to kinit, and enable Kerberos " - "to be used to authenticate RPC connections."); -TAG_FLAG(keytab_file, stable); - DEFINE_bool(allow_world_readable_credentials, false, "Enable the use of keytab files and TLS private keys with " "world-readable permissions."); @@ -226,6 +210,8 @@ DECLARE_int32(dns_resolver_max_threads_num); DECLARE_uint32(dns_resolver_cache_capacity_mb); DECLARE_uint32(dns_resolver_cache_ttl_sec); DECLARE_string(log_filename); +DECLARE_string(keytab_file); +DECLARE_string(principal); METRIC_DECLARE_gauge_size(merged_entities_count_of_server);
