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 <danburk...@apache.org> Reviewed-by: Alexey Serbin <aser...@cloudera.com> Tested-by: Kudu Jenkins Reviewed-on: http://gerrit.cloudera.org:8080/8760 Reviewed-by: Dan Hecht <dhe...@cloudera.com> Reviewed-by: Michael Ho <k...@cloudera.com> Tested-by: Tim Armstrong <tarmstr...@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 <sail...@apache.org> Authored: Thu Nov 30 13:33:52 2017 -0800 Committer: Tim Armstrong <tarmstr...@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....@krbtest.com", *security::GetLoggedInPrincipalFromKeytab()); // Test principal canonicalization.