You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2017/12/06 01:58:33 UTC
impala git commit: [security] Make the kerberos principal
configurable for Kudu servers
Repository: impala
Updated Branches:
refs/heads/master c505a8159 -> 9303b0aed
[security] Make the kerberos principal configurable for Kudu servers
The Kudu security library currently sources the kerberos principal
directly from FLAGS_principal. Since this is a library, we'd rather
move this to be an option that is passed through InitKerberosForServer().
Users of the security library may pass any principal that they want
to.
Testing: All current tests pass. Since this is a minor refactor, no new
tests are needed.
Change-Id: Idd16b3360d8d2df5a609eb897bb9810e662fc695
Reviewed-on: http://gerrit.cloudera.org:8080/8700
Reviewed-by: Dan Burkert <da...@apache.org>
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Kudu Jenkins
Reviewed-on: http://gerrit.cloudera.org:8080/8760
Reviewed-by: Dan Hecht <dh...@cloudera.com>
Reviewed-by: Michael Ho <kw...@cloudera.com>
Tested-by: Tim Armstrong <ta...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/9303b0ae
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/9303b0ae
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/9303b0ae
Branch: refs/heads/master
Commit: 9303b0aed808c7b323d7ec9a7583a15193dda5df
Parents: c505a81
Author: Sailesh Mukil <sa...@apache.org>
Authored: Thu Nov 30 13:33:52 2017 -0800
Committer: Tim Armstrong <ta...@cloudera.com>
Committed: Wed Dec 6 01:56:29 2017 +0000
----------------------------------------------------------------------
be/src/kudu/security/init.cc | 31 +++++++++++--------------
be/src/kudu/security/init.h | 5 +++-
be/src/kudu/security/test/mini_kdc-test.cc | 3 +--
3 files changed, 19 insertions(+), 20 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/9303b0ae/be/src/kudu/security/init.cc
----------------------------------------------------------------------
diff --git a/be/src/kudu/security/init.cc b/be/src/kudu/security/init.cc
index 9c4bdda..cbba8c8 100644
--- a/be/src/kudu/security/init.cc
+++ b/be/src/kudu/security/init.cc
@@ -43,14 +43,6 @@
DECLARE_string(keytab_file);
TAG_FLAG(keytab_file, stable);
-DECLARE_string(principal);
-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);
-
using std::mt19937;
using std::random_device;
using std::string;
@@ -360,10 +352,14 @@ Status KinitContext::Kinit(const string& keytab_path, const string& principal) {
return Status::OK();
}
-Status GetConfiguredPrincipal(string* principal) {
- string p = FLAGS_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, string* out_principal) {
+ *out_principal = in_principal;
const auto& kHostToken = "_HOST";
- if (p.find(kHostToken) != string::npos) {
+ if (in_principal.find(kHostToken) != string::npos) {
string hostname;
// Try to fill in either the FQDN or hostname.
if (!GetFQDN(&hostname).ok()) {
@@ -371,9 +367,8 @@ Status GetConfiguredPrincipal(string* principal) {
}
// Hosts in principal names are canonicalized to lower-case.
std::transform(hostname.begin(), hostname.end(), hostname.begin(), tolower);
- GlobalReplaceSubstring(kHostToken, hostname, &p);
+ GlobalReplaceSubstring(kHostToken, hostname, out_principal);
}
- *principal = p;
return Status::OK();
}
} // anonymous namespace
@@ -450,7 +445,8 @@ boost::optional<string> GetLoggedInUsernameFromKeytab() {
return g_kinit_ctx->username_str();
}
-Status InitKerberosForServer(const std::string& krb5ccname, bool disable_krb5_replay_cache) {
+Status InitKerberosForServer(const std::string& raw_principal, const std::string& krb5ccname,
+ bool disable_krb5_replay_cache) {
if (FLAGS_keytab_file.empty()) return Status::OK();
setenv("KRB5CCNAME", krb5ccname.c_str(), 1);
@@ -465,9 +461,10 @@ Status InitKerberosForServer(const std::string& krb5ccname, bool disable_krb5_re
}
g_kinit_ctx = new KinitContext();
- string principal;
- RETURN_NOT_OK(GetConfiguredPrincipal(&principal));
- RETURN_NOT_OK_PREPEND(g_kinit_ctx->Kinit(FLAGS_keytab_file, principal), "unable to kinit");
+ string configured_principal;
+ RETURN_NOT_OK(GetConfiguredPrincipal(raw_principal, &configured_principal));
+ RETURN_NOT_OK_PREPEND(g_kinit_ctx->Kinit(
+ FLAGS_keytab_file, configured_principal), "unable to kinit");
g_kerberos_reinit_lock = new RWMutex(RWMutex::Priority::PREFER_WRITING);
scoped_refptr<Thread> reacquire_thread;
http://git-wip-us.apache.org/repos/asf/impala/blob/9303b0ae/be/src/kudu/security/init.h
----------------------------------------------------------------------
diff --git a/be/src/kudu/security/init.h b/be/src/kudu/security/init.h
index c6ee264..656efb7 100644
--- a/be/src/kudu/security/init.h
+++ b/be/src/kudu/security/init.h
@@ -34,10 +34,13 @@ static const std::string kKrb5CCName = "MEMORY:kudu";
// Initializes Kerberos for a server. In particular, this processes
// the '--keytab_file' command line flag.
+// 'raw_principal' is the principal to Kinit with after calling GetConfiguredPrincipal()
+// on it.
// 'krb5ccname' is passed into the KRB5CCNAME env var.
// 'disable_krb5_replay_cache' if set to true, disables the kerberos replay cache by setting
// the KRB5RCACHETYPE env var to "none".
-Status InitKerberosForServer(const std::string& krb5ccname = kKrb5CCName,
+Status InitKerberosForServer(const std::string& raw_principal,
+ const std::string& krb5ccname = kKrb5CCName,
bool disable_krb5_replay_cache = true);
// Returns the process lock 'kerberos_reinit_lock'
http://git-wip-us.apache.org/repos/asf/impala/blob/9303b0ae/be/src/kudu/security/test/mini_kdc-test.cc
----------------------------------------------------------------------
diff --git a/be/src/kudu/security/test/mini_kdc-test.cc b/be/src/kudu/security/test/mini_kdc-test.cc
index 01ed84d..e93c10d 100644
--- a/be/src/kudu/security/test/mini_kdc-test.cc
+++ b/be/src/kudu/security/test/mini_kdc-test.cc
@@ -76,8 +76,7 @@ TEST_F(MiniKdcTest, TestBasicOperation) {
// Test programmatic keytab login.
kdc.SetKrb5Environment();
FLAGS_keytab_file = kt_path;
- FLAGS_principal = kSPN;
- ASSERT_OK(security::InitKerberosForServer());
+ ASSERT_OK(security::InitKerberosForServer(kSPN));
ASSERT_EQ("kudu/foo.example.com@KRBTEST.COM", *security::GetLoggedInPrincipalFromKeytab());
// Test principal canonicalization.