Repository: kudu Updated Branches: refs/heads/master 889a04ad9 -> 28e9349fb
[security] disable PLAIN SASL mech when authentication is required Change-Id: Id22c43414ed96e2d9d25bf4d13b9f26b08359323 Reviewed-on: http://gerrit.cloudera.org:8080/6235 Reviewed-by: Todd Lipcon <[email protected]> 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/28e9349f Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/28e9349f Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/28e9349f Branch: refs/heads/master Commit: 28e9349fb775b0215cd2f7ae9a7531390f79d267 Parents: 889a04a Author: Dan Burkert <[email protected]> Authored: Thu Mar 2 17:04:29 2017 -0800 Committer: Dan Burkert <[email protected]> Committed: Fri Mar 3 02:09:01 2017 +0000 ---------------------------------------------------------------------- src/kudu/rpc/messenger.h | 2 + src/kudu/rpc/negotiation.cc | 78 +++++++++++++++++++++++++------------ src/kudu/rpc/negotiation.h | 6 ++- src/kudu/rpc/reactor.cc | 3 +- src/kudu/server/server_base.cc | 1 - 5 files changed, 61 insertions(+), 29 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/28e9349f/src/kudu/rpc/messenger.h ---------------------------------------------------------------------- diff --git a/src/kudu/rpc/messenger.h b/src/kudu/rpc/messenger.h index f3614a0..2ada341 100644 --- a/src/kudu/rpc/messenger.h +++ b/src/kudu/rpc/messenger.h @@ -224,6 +224,8 @@ class Messenger { authn_token_ = token; } + RpcAuthentication authentication() const { return authentication_; } + ThreadPool* negotiation_pool() const { return negotiation_pool_.get(); } RpczStore* rpcz_store() { return rpcz_store_.get(); } http://git-wip-us.apache.org/repos/asf/kudu/blob/28e9349f/src/kudu/rpc/negotiation.cc ---------------------------------------------------------------------- diff --git a/src/kudu/rpc/negotiation.cc b/src/kudu/rpc/negotiation.cc index 8b5b3d4..e912939 100644 --- a/src/kudu/rpc/negotiation.cc +++ b/src/kudu/rpc/negotiation.cc @@ -36,6 +36,7 @@ #include "kudu/rpc/rpc_header.pb.h" #include "kudu/rpc/sasl_common.h" #include "kudu/rpc/server_negotiation.h" +#include "kudu/security/tls_context.h" #include "kudu/util/errno.h" #include "kudu/util/flag_tags.h" #include "kudu/util/logging.h" @@ -54,6 +55,7 @@ DEFINE_int32(rpc_negotiation_inject_delay_ms, 0, TAG_FLAG(rpc_negotiation_inject_delay_ms, unsafe); DECLARE_string(keytab_file); +DECLARE_string(rpc_certificate_file); DEFINE_bool(rpc_encrypt_loopback_connections, false, "Whether to encrypt data transfer on RPC connections that stay within " @@ -151,7 +153,9 @@ static Status DisableSocketTimeouts(Socket* socket) { } // Perform client negotiation. We don't LOG() anything, we leave that to our caller. -static Status DoClientNegotiation(Connection* conn, MonoTime deadline) { +static Status DoClientNegotiation(Connection* conn, + RpcAuthentication authentication, + MonoTime deadline) { const auto* messenger = conn->reactor_thread()->reactor()->messenger(); ClientNegotiation client_negotiation(conn->release_socket(), &messenger->tls_context(), @@ -163,23 +167,36 @@ static Status DoClientNegotiation(Connection* conn, MonoTime deadline) { // canonical hostname to determine the expected server principal. client_negotiation.set_server_fqdn(conn->remote().host()); - Status s = client_negotiation.EnableGSSAPI(); - if (!s.ok()) { - // If we can't enable GSSAPI, it's likely the client is just missing the - // appropriate SASL plugin. We don't want to require it to be installed - // if the user doesn't care about connecting to servers using Kerberos - // authentication. So, we'll just VLOG this here. If we try to connect - // to a server which requires Kerberos, we'll get a negotiation error - // at that point. - if (VLOG_IS_ON(1)) { - KLOG_FIRST_N(INFO, 1) << "Couldn't enable GSSAPI (Kerberos) SASL plugin: " - << s.message().ToString() - << ". This process will be unable to connect to " - << "servers requiring Kerberos authentication."; + if (authentication != RpcAuthentication::DISABLED) { + Status s = client_negotiation.EnableGSSAPI(); + if (!s.ok()) { + // If we can't enable GSSAPI, it's likely the client is just missing the + // appropriate SASL plugin. We don't want to require it to be installed + // if the user doesn't care about connecting to servers using Kerberos + // authentication. So, we'll just VLOG this here. If we try to connect + // to a server which requires Kerberos, we'll get a negotiation error + // at that point. + if (VLOG_IS_ON(1)) { + KLOG_FIRST_N(INFO, 1) << "Couldn't enable GSSAPI (Kerberos) SASL plugin: " + << s.message().ToString() + << ". This process will be unable to connect to " + << "servers requiring Kerberos authentication."; + } + + if (authentication == RpcAuthentication::REQUIRED && + !messenger->authn_token() && + !messenger->tls_context().has_signed_cert()) { + return Status::InvalidArgument( + "Kerberos, token, or PKI certificate credentials must be provided in order to " + "require authentication for a client"); + } } } - RETURN_NOT_OK(client_negotiation.EnablePlain(conn->local_user_credentials().real_user(), "")); + if (authentication != RpcAuthentication::REQUIRED) { + RETURN_NOT_OK(client_negotiation.EnablePlain(conn->local_user_credentials().real_user(), "")); + } + client_negotiation.set_deadline(deadline); RETURN_NOT_OK(WaitForClientConnect(client_negotiation.socket(), deadline)); @@ -195,7 +212,17 @@ static Status DoClientNegotiation(Connection* conn, MonoTime deadline) { } // Perform server negotiation. We don't LOG() anything, we leave that to our caller. -static Status DoServerNegotiation(Connection* conn, const MonoTime& deadline) { +static Status DoServerNegotiation(Connection* conn, + RpcAuthentication authentication, + const MonoTime& deadline) { + if (authentication == RpcAuthentication::REQUIRED && + FLAGS_keytab_file.empty() && + FLAGS_rpc_certificate_file.empty()) { + return Status::InvalidArgument("RPC authentication (--rpc_authentication) may not be " + "required unless Kerberos (--keytab_file) or external PKI " + "(--rpc_certificate_file et al) are configured"); + } + if (FLAGS_rpc_negotiation_inject_delay_ms > 0) { LOG(WARNING) << "Injecting " << FLAGS_rpc_negotiation_inject_delay_ms << "ms delay in negotiation"; @@ -208,18 +235,17 @@ static Status DoServerNegotiation(Connection* conn, const MonoTime& deadline) { &messenger->tls_context(), &messenger->token_verifier()); - // 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_file.empty()) { - RETURN_NOT_OK(server_negotiation.EnablePlain()); - } else { + if (authentication != RpcAuthentication::DISABLED && !FLAGS_keytab_file.empty()) { RETURN_NOT_OK(server_negotiation.EnableGSSAPI()); } + if (authentication != RpcAuthentication::REQUIRED) { + RETURN_NOT_OK(server_negotiation.EnablePlain()); + } + server_negotiation.set_deadline(deadline); RETURN_NOT_OK(server_negotiation.socket()->SetNonBlocking(false)); - RETURN_NOT_OK(server_negotiation.Negotiate()); RETURN_NOT_OK(DisableSocketTimeouts(server_negotiation.socket())); @@ -231,12 +257,14 @@ static Status DoServerNegotiation(Connection* conn, const MonoTime& deadline) { return Status::OK(); } -void Negotiation::RunNegotiation(const scoped_refptr<Connection>& conn, MonoTime deadline) { +void Negotiation::RunNegotiation(const scoped_refptr<Connection>& conn, + RpcAuthentication authentication, + MonoTime deadline) { Status s; if (conn->direction() == Connection::SERVER) { - s = DoServerNegotiation(conn.get(), deadline); + s = DoServerNegotiation(conn.get(), authentication, deadline); } else { - s = DoClientNegotiation(conn.get(), deadline); + s = DoClientNegotiation(conn.get(), authentication, deadline); } if (PREDICT_FALSE(!s.ok())) { http://git-wip-us.apache.org/repos/asf/kudu/blob/28e9349f/src/kudu/rpc/negotiation.h ---------------------------------------------------------------------- diff --git a/src/kudu/rpc/negotiation.h b/src/kudu/rpc/negotiation.h index 83539c0..715fca2 100644 --- a/src/kudu/rpc/negotiation.h +++ b/src/kudu/rpc/negotiation.h @@ -23,10 +23,10 @@ #include "kudu/util/monotime.h" namespace kudu { - namespace rpc { class Connection; +enum class RpcAuthentication; enum class AuthenticationType { INVALID, @@ -42,7 +42,9 @@ class Negotiation { public: // Perform negotiation for a connection (either server or client) - static void RunNegotiation(const scoped_refptr<Connection>& conn, MonoTime deadline); + static void RunNegotiation(const scoped_refptr<Connection>& conn, + RpcAuthentication authentication, + MonoTime deadline); private: DISALLOW_IMPLICIT_CONSTRUCTORS(Negotiation); }; http://git-wip-us.apache.org/repos/asf/kudu/blob/28e9349f/src/kudu/rpc/reactor.cc ---------------------------------------------------------------------- diff --git a/src/kudu/rpc/reactor.cc b/src/kudu/rpc/reactor.cc index 85f2776..754bca4 100644 --- a/src/kudu/rpc/reactor.cc +++ b/src/kudu/rpc/reactor.cc @@ -371,8 +371,9 @@ Status ReactorThread::StartConnectionNegotiation(const scoped_refptr<Connection> scoped_refptr<Trace> trace(new Trace()); ADOPT_TRACE(trace.get()); TRACE("Submitting negotiation task for $0", conn->ToString()); + auto authentication = reactor()->messenger()->authentication(); RETURN_NOT_OK(reactor()->messenger()->negotiation_pool()->SubmitClosure( - Bind(&Negotiation::RunNegotiation, conn, deadline))); + Bind(&Negotiation::RunNegotiation, conn, authentication, deadline))); return Status::OK(); } http://git-wip-us.apache.org/repos/asf/kudu/blob/28e9349f/src/kudu/server/server_base.cc ---------------------------------------------------------------------- diff --git a/src/kudu/server/server_base.cc b/src/kudu/server/server_base.cc index 772207d..d15ab8d 100644 --- a/src/kudu/server/server_base.cc +++ b/src/kudu/server/server_base.cc @@ -228,7 +228,6 @@ Status ServerBase::Init() { .set_min_negotiation_threads(FLAGS_min_negotiation_threads) .set_max_negotiation_threads(FLAGS_max_negotiation_threads) .set_metric_entity(metric_entity()) - // TODO(PKI): make built-in PKI enabled/disabled based on a flag. .enable_inbound_tls(); RETURN_NOT_OK(builder.Build(&messenger_));
