You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by to...@apache.org on 2017/03/03 20:53:47 UTC
[4/5] kudu git commit: [security] disable PLAIN SASL mech when
authentication is required
[security] disable PLAIN SASL mech when authentication is required
Change-Id: Id22c43414ed96e2d9d25bf4d13b9f26b08359323
Reviewed-on: http://gerrit.cloudera.org:8080/6235
Reviewed-by: Todd Lipcon <to...@apache.org>
Tested-by: Kudu Jenkins
(cherry picked from commit 28e9349fb775b0215cd2f7ae9a7531390f79d267)
Reviewed-on: http://gerrit.cloudera.org:8080/6243
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/dabf958b
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/dabf958b
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/dabf958b
Branch: refs/heads/branch-1.3.x
Commit: dabf958b56e3dfe4989a4c30ad44df670e1d8e49
Parents: 1183e9d
Author: Dan Burkert <da...@apache.org>
Authored: Thu Mar 2 17:04:29 2017 -0800
Committer: Todd Lipcon <to...@apache.org>
Committed: Fri Mar 3 18:39:13 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/dabf958b/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/dabf958b/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/dabf958b/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/dabf958b/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/dabf958b/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_));