You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by to...@apache.org on 2020/03/20 04:51:52 UTC
[kudu] 01/03: [ranger] pass 'principal' and 'keytab' to the
subprocess
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 <ha...@cloudera.com>
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 <ad...@cloudera.com>
Reviewed-by: Andrew Wong <aw...@cloudera.com>
Tested-by: Hao Hao <ha...@cloudera.com>
---
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);