Rename and stabilize security flags (part deux) --keytab is renamed to --keytab_file to match Impala's equivalent, and stabilized.
--kerberos_principal is renamed --principal to match Impala's equivalent. --principal remains unstable, since support is not fully implemented in the clients. --webserver_[certificate_file, private_key_file, private_key_password_cmd] are stabilized. Change-Id: I54ea640d4d0669a5a8580e5a38f0e03243ba0e72 Reviewed-on: http://gerrit.cloudera.org:8080/6113 Reviewed-by: Adar Dembo <[email protected]> Tested-by: Dan Burkert <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/ed44a20d Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/ed44a20d Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/ed44a20d Branch: refs/heads/master Commit: ed44a20dfe23d3c52cd2ae72d72a5886f563a6d8 Parents: 73aa301 Author: Dan Burkert <[email protected]> Authored: Wed Feb 22 12:22:02 2017 -0800 Committer: Dan Burkert <[email protected]> Committed: Wed Feb 22 22:35:29 2017 +0000 ---------------------------------------------------------------------- .../org/apache/kudu/client/MiniKuduCluster.java | 8 ++++---- .../integration-tests/external_mini_cluster.cc | 4 ++-- src/kudu/rpc/messenger.cc | 4 ++-- src/kudu/rpc/negotiation.cc | 4 ++-- src/kudu/rpc/sasl_common.cc | 4 ++-- src/kudu/security/init.cc | 21 +++++++++++--------- src/kudu/security/init.h | 2 +- src/kudu/security/test/mini_kdc-test.cc | 8 ++++---- src/kudu/server/webserver_options.cc | 5 +++-- 9 files changed, 32 insertions(+), 28 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/ed44a20d/java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java ---------------------------------------------------------------------- diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java b/java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java index c008b9a..546fa6c 100644 --- a/java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java +++ b/java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java @@ -180,8 +180,8 @@ public class MiniKuduCluster implements AutoCloseable { "--rpc_bind_addresses=" + bindHost + ":" + rpcPort); if (miniKdc != null) { - commandLine.add("--keytab=" + keytab); - commandLine.add("--kerberos_principal=kudu/" + bindHost); + commandLine.add("--keytab_file=" + keytab); + commandLine.add("--principal=kudu/" + bindHost); commandLine.add("--rpc_authentication=required"); } @@ -261,8 +261,8 @@ public class MiniKuduCluster implements AutoCloseable { } if (miniKdc != null) { - commandLine.add("--keytab=" + keytab); - commandLine.add("--kerberos_principal=kudu/" + bindHost); + commandLine.add("--keytab_file=" + keytab); + commandLine.add("--principal=kudu/" + bindHost); commandLine.add("--rpc_authentication=required"); } http://git-wip-us.apache.org/repos/asf/kudu/blob/ed44a20d/src/kudu/integration-tests/external_mini_cluster.cc ---------------------------------------------------------------------- diff --git a/src/kudu/integration-tests/external_mini_cluster.cc b/src/kudu/integration-tests/external_mini_cluster.cc index d6c3d97..dcf024e 100644 --- a/src/kudu/integration-tests/external_mini_cluster.cc +++ b/src/kudu/integration-tests/external_mini_cluster.cc @@ -604,8 +604,8 @@ Status ExternalDaemon::EnableKerberos(MiniKdc* kdc, const string& bind_host) { RETURN_NOT_OK_PREPEND(kdc->CreateServiceKeytab(spn, &ktpath), "could not create keytab"); extra_env_ = kdc->GetEnvVars(); - extra_flags_.push_back(Substitute("--keytab=$0", ktpath)); - extra_flags_.push_back(Substitute("--kerberos_principal=$0", spn)); + extra_flags_.push_back(Substitute("--keytab_file=$0", ktpath)); + extra_flags_.push_back(Substitute("--principal=$0", spn)); extra_flags_.push_back("--rpc_authentication=required"); return Status::OK(); } http://git-wip-us.apache.org/repos/asf/kudu/blob/ed44a20d/src/kudu/rpc/messenger.cc ---------------------------------------------------------------------- diff --git a/src/kudu/rpc/messenger.cc b/src/kudu/rpc/messenger.cc index 5ba0141..59ad79e 100644 --- a/src/kudu/rpc/messenger.cc +++ b/src/kudu/rpc/messenger.cc @@ -103,7 +103,7 @@ DEFINE_int32(rpc_default_keepalive_time_ms, 65000, "will disconnect the client."); TAG_FLAG(rpc_default_keepalive_time_ms, advanced); -DECLARE_string(keytab); +DECLARE_string(keytab_file); namespace kudu { namespace rpc { @@ -268,7 +268,7 @@ Status Messenger::AddAcceptorPool(const Sockaddr &accept_addr, // Before listening, if we expect to require Kerberos, we want to verify // that everything is set up correctly. This way we'll generate errors on // startup rather than later on when we first receive a client connection. - if (!FLAGS_keytab.empty()) { + if (!FLAGS_keytab_file.empty()) { RETURN_NOT_OK_PREPEND(ServerNegotiation::PreflightCheckGSSAPI(), "GSSAPI/Kerberos not properly configured"); } http://git-wip-us.apache.org/repos/asf/kudu/blob/ed44a20d/src/kudu/rpc/negotiation.cc ---------------------------------------------------------------------- diff --git a/src/kudu/rpc/negotiation.cc b/src/kudu/rpc/negotiation.cc index e044c98..817be72 100644 --- a/src/kudu/rpc/negotiation.cc +++ b/src/kudu/rpc/negotiation.cc @@ -53,7 +53,7 @@ DEFINE_int32(rpc_negotiation_inject_delay_ms, 0, "the RPC negotiation process on the server side."); TAG_FLAG(rpc_negotiation_inject_delay_ms, unsafe); -DECLARE_string(keytab); +DECLARE_string(keytab_file); DEFINE_bool(rpc_encrypt_loopback_connections, false, "Whether to encrypt data transfer on RPC connections that stay within " @@ -210,7 +210,7 @@ static Status DoServerNegotiation(Connection* conn, const MonoTime& deadline) { // TODO(PKI): this should be enabling PLAIN if authn < required, and GSSAPI if // there is a keytab and authn > disabled. Same with client version. - if (FLAGS_keytab.empty()) { + if (FLAGS_keytab_file.empty()) { RETURN_NOT_OK(server_negotiation.EnablePlain()); } else { RETURN_NOT_OK(server_negotiation.EnableGSSAPI()); http://git-wip-us.apache.org/repos/asf/kudu/blob/ed44a20d/src/kudu/rpc/sasl_common.cc ---------------------------------------------------------------------- diff --git a/src/kudu/rpc/sasl_common.cc b/src/kudu/rpc/sasl_common.cc index c852675..8a1dc63 100644 --- a/src/kudu/rpc/sasl_common.cc +++ b/src/kudu/rpc/sasl_common.cc @@ -42,7 +42,7 @@ using std::set; -DECLARE_string(keytab); +DECLARE_string(keytab_file); namespace kudu { namespace rpc { @@ -318,7 +318,7 @@ Status WrapSaslCall(sasl_conn_t* conn, const std::function<int()>& call) { g_auth_failure_capture = &err; // Take the 'kerberos_reinit_lock' here to avoid a possible race with ticket renewal. - bool kerberos_supported = !FLAGS_keytab.empty(); + bool kerberos_supported = !FLAGS_keytab_file.empty(); if (kerberos_supported) kudu::security::KerberosReinitLock()->ReadLock(); int rc = call(); if (kerberos_supported) kudu::security::KerberosReinitLock()->ReadUnlock(); http://git-wip-us.apache.org/repos/asf/kudu/blob/ed44a20d/src/kudu/security/init.cc ---------------------------------------------------------------------- diff --git a/src/kudu/security/init.cc b/src/kudu/security/init.cc index 7e21d89..4ccbc05 100644 --- a/src/kudu/security/init.cc +++ b/src/kudu/security/init.cc @@ -30,18 +30,21 @@ #include "kudu/util/scoped_cleanup.h" #include "kudu/util/thread.h" -DEFINE_string(keytab, "", "Path to the Kerberos Keytab for this server"); -TAG_FLAG(keytab, experimental); +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_string(kerberos_principal, "kudu/_HOST", +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(kerberos_principal, experimental); +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(kerberos_principal, unsafe); +TAG_FLAG(principal, unsafe); using std::mt19937; using std::random_device; @@ -307,7 +310,7 @@ Status KinitContext::Kinit(const string& keytab_path, const string& principal) { } Status GetLoginPrincipal(string* principal) { - string p = FLAGS_kerberos_principal; + string p = FLAGS_principal; string hostname; // Try to fill in either the FQDN or hostname. if (!GetFQDN(&hostname).ok()) { @@ -325,19 +328,19 @@ RWMutex* KerberosReinitLock() { } Status InitKerberosForServer() { - if (FLAGS_keytab.empty()) return Status::OK(); + if (FLAGS_keytab_file.empty()) return Status::OK(); // Have the daemons use an in-memory ticket cache, so they don't accidentally // pick up credentials from test cases or any other daemon. // TODO(todd): extract these krb5 env vars into some constants since they're // typo-prone. setenv("KRB5CCNAME", "MEMORY:kudu", 1); - setenv("KRB5_KTNAME", FLAGS_keytab.c_str(), 1); + setenv("KRB5_KTNAME", FLAGS_keytab_file.c_str(), 1); g_kinit_ctx = new KinitContext(); string principal; RETURN_NOT_OK(GetLoginPrincipal(&principal)); - RETURN_NOT_OK_PREPEND(g_kinit_ctx->Kinit(FLAGS_keytab, principal), "unable to kinit"); + RETURN_NOT_OK_PREPEND(g_kinit_ctx->Kinit(FLAGS_keytab_file, principal), "unable to kinit"); g_kerberos_reinit_lock = new RWMutex(RWMutex::Priority::PREFER_WRITING); scoped_refptr<Thread> renew_thread; http://git-wip-us.apache.org/repos/asf/kudu/blob/ed44a20d/src/kudu/security/init.h ---------------------------------------------------------------------- diff --git a/src/kudu/security/init.h b/src/kudu/security/init.h index a86e119..51a1460 100644 --- a/src/kudu/security/init.h +++ b/src/kudu/security/init.h @@ -25,7 +25,7 @@ class RWMutex; namespace security { // Initializes Kerberos for a server. In particular, this processes -// the '--keytab' command line flag. +// the '--keytab_file' command line flag. Status InitKerberosForServer(); // Returns the process lock 'kerberos_reinit_lock' http://git-wip-us.apache.org/repos/asf/kudu/blob/ed44a20d/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 62bda5c..8e90930 100644 --- a/src/kudu/security/test/mini_kdc-test.cc +++ b/src/kudu/security/test/mini_kdc-test.cc @@ -27,8 +27,8 @@ using std::string; -DECLARE_string(keytab); -DECLARE_string(kerberos_principal); +DECLARE_string(keytab_file); +DECLARE_string(principal); namespace kudu { @@ -74,8 +74,8 @@ TEST_F(MiniKdcTest, TestBasicOperation) { // Test programmatic keytab login. kdc.SetKrb5Environment(); - FLAGS_keytab = kt_path; - FLAGS_kerberos_principal = kSPN; + FLAGS_keytab_file = kt_path; + FLAGS_principal = kSPN; ASSERT_OK(security::InitKerberosForServer()); } http://git-wip-us.apache.org/repos/asf/kudu/blob/ed44a20d/src/kudu/server/webserver_options.cc ---------------------------------------------------------------------- diff --git a/src/kudu/server/webserver_options.cc b/src/kudu/server/webserver_options.cc index 020dc9d..37c7bad 100644 --- a/src/kudu/server/webserver_options.cc +++ b/src/kudu/server/webserver_options.cc @@ -64,7 +64,9 @@ DEFINE_string(webserver_private_key_password_cmd, "", "A Unix command whose outp "password-protected, this command will not be invoked. The output of the command " "will be truncated to 1024 bytes, and then all trailing whitespace will be trimmed " "before it is used to decrypt the private key"); - +TAG_FLAG(webserver_certificate_file, stable); +TAG_FLAG(webserver_private_key_file, stable); +TAG_FLAG(webserver_private_key_password_cmd, stable); DEFINE_string(webserver_authentication_domain, "", "Domain used for debug webserver authentication"); @@ -72,7 +74,6 @@ DEFINE_string(webserver_password_file, "", "(Optional) Location of .htpasswd file containing user names and hashed passwords for" " debug webserver authentication"); - DEFINE_int32(webserver_num_worker_threads, 50, "Maximum number of threads to start for handling web server requests"); TAG_FLAG(webserver_num_worker_threads, advanced);
