kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From danburk...@apache.org
Subject kudu git commit: [security] Make the kerberos principal configurable for Kudu servers
Date Mon, 04 Dec 2017 23:29:34 GMT
Repository: kudu
Updated Branches:
  refs/heads/master 2c77f95b5 -> d42c29164


[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 <danburkert@apache.org>
Reviewed-by: Alexey Serbin <aserbin@cloudera.com>
Tested-by: Kudu Jenkins


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/d42c2916
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/d42c2916
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/d42c2916

Branch: refs/heads/master
Commit: d42c2916467b83347f064ddea59f7a65202f7247
Parents: 2c77f95
Author: Sailesh Mukil <sailesh@apache.org>
Authored: Thu Nov 30 13:33:52 2017 -0800
Committer: Dan Burkert <danburkert@apache.org>
Committed: Mon Dec 4 23:17:10 2017 +0000

----------------------------------------------------------------------
 src/kudu/security/init.cc               | 33 ++++++++++++----------------
 src/kudu/security/init.h                |  5 ++++-
 src/kudu/security/test/mini_kdc-test.cc |  3 +--
 src/kudu/server/server_base.cc          | 12 +++++++++-
 4 files changed, 30 insertions(+), 23 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/d42c2916/src/kudu/security/init.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/init.cc b/src/kudu/security/init.cc
index 6ce2135..468f0d0 100644
--- a/src/kudu/security/init.cc
+++ b/src/kudu/security/init.cc
@@ -56,16 +56,6 @@ DEFINE_string(keytab_file, "",
               "to be used to authenticate RPC connections.");
 TAG_FLAG(keytab_file, stable);
 
-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_bool(allow_world_readable_credentials, false,
             "Enable the use of keytab files and TLS private keys with "
             "world-readable permissions.");
@@ -385,10 +375,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()) {
@@ -396,9 +390,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
@@ -472,7 +465,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);
@@ -487,9 +481,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/kudu/blob/d42c2916/src/kudu/security/init.h
----------------------------------------------------------------------
diff --git a/src/kudu/security/init.h b/src/kudu/security/init.h
index 0832256..dccbcc7 100644
--- a/src/kudu/security/init.h
+++ b/src/kudu/security/init.h
@@ -37,10 +37,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/kudu/blob/d42c2916/src/kudu/security/test/mini_kdc-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/test/mini_kdc-test.cc b/src/kudu/security/test/mini_kdc-test.cc
index 08f493c..d8efa81 100644
--- a/src/kudu/security/test/mini_kdc-test.cc
+++ b/src/kudu/security/test/mini_kdc-test.cc
@@ -77,8 +77,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.

http://git-wip-us.apache.org/repos/asf/kudu/blob/d42c2916/src/kudu/server/server_base.cc
----------------------------------------------------------------------
diff --git a/src/kudu/server/server_base.cc b/src/kudu/server/server_base.cc
index 20a3a97..668eeb5 100644
--- a/src/kudu/server/server_base.cc
+++ b/src/kudu/server/server_base.cc
@@ -109,6 +109,16 @@ 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);
+
 DECLARE_bool(use_hybrid_clock);
 
 using std::ostringstream;
@@ -221,7 +231,7 @@ Status ServerBase::Init() {
   // if we're having clock problems.
   RETURN_NOT_OK_PREPEND(clock_->Init(), "Cannot initialize clock");
 
-  RETURN_NOT_OK(security::InitKerberosForServer());
+  RETURN_NOT_OK(security::InitKerberosForServer(FLAGS_principal));
 
   fs::FsReport report;
   Status s = fs_manager_->Open(&report);


Mime
View raw message