Repository: kudu
Updated Branches:
refs/heads/master dc454dcd0 -> dc1e45a9a
[security] security-flags
This commit introduces, removes, and renames security flags in an effort
to make the flags more consistent, more understandable, and easier to
use from the command line. Affected flags:
--rpc_authentication
This is a new flag which will apply to Kudu servers and the kudu
command line tool which will allow operators to configure a policy
for authentication of RPC connections. The possible values are
'enabled', 'disabled', and 'required'. Three states are necessary
(as opposed to just 'disabled' and 'required') in order to provide a
graceful upgrade path for clusters from unsecured to secured.
'enabled' is the default. A follow up commit will hook this flag
into the RPC system to ensure that authentication is enforced as
necessary.
--rpc_encryption
This is a new flag which will apply to Kudu servers and the 'kudu'
command line tool which allows operators to configure a policy for
encryption on RPC connections. This is a tristate flag for the same
reasons as outlined in --rpc_authentication. A follow up commit will
hook this flag into the RPC system to ensure that encryption is
enforced as necessary.
--server_require_kerberos
This flag has been removed, and in it's place the --keytab and
--rpc_authentication=required flags are provided. --keytab is used
to enable Kerberos authentication on a server, and
--rpc_authentication=required is used to ensure that all RPCs use
authentication.
--rpc_certificate_file
--rpc_ssl_server_certificate
--rpc_certificate_file is replacing --rpc_ssl_server_certificate.
The latter has a few issues. 1) It's not strictly a server flag, it
also applies to the kudu CLI tool. 3) It's not consistent with the
similar --webserver_certificate_file flag.
--rpc_private_key_file
--rpc_ssl_private_key
--rpc_private_key_file is replacing --rpc_ssl_private_key. Same
reasons as --rpc_cert.
--rpc_ca_certificate_file
--rpc_ssl_certificate_authority
--rpc_ca_certificate_file is replacing
--rpc_ssl_certificate_authority. Same reasons as --rpc_cert.
Change-Id: Iaa53348b8969e83d9f794e1e0553bdec12252d9a
Reviewed-on: http://gerrit.cloudera.org:8080/6052
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <[email protected]>
Reviewed-by: Alexey Serbin <[email protected]>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/70ea7fb1
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/70ea7fb1
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/70ea7fb1
Branch: refs/heads/master
Commit: 70ea7fb19cbd4474a0c4bc09e84f2f6276a1e917
Parents: dc454dc
Author: Dan Burkert <[email protected]>
Authored: Thu Feb 16 16:39:35 2017 -0800
Committer: Todd Lipcon <[email protected]>
Committed: Wed Feb 22 17:09:28 2017 +0000
----------------------------------------------------------------------
.../org/apache/kudu/client/MiniKuduCluster.java | 4 +-
.../integration-tests/external_mini_cluster.cc | 2 +-
src/kudu/rpc/messenger.cc | 111 ++++++++++++++-----
src/kudu/rpc/messenger.h | 21 ++++
src/kudu/rpc/negotiation.cc | 10 +-
src/kudu/rpc/rpc-test-base.h | 16 +--
src/kudu/rpc/sasl_common.cc | 7 +-
7 files changed, 122 insertions(+), 49 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/70ea7fb1/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 fa4fea4..c008b9a 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
@@ -182,7 +182,7 @@ public class MiniKuduCluster implements AutoCloseable {
if (miniKdc != null) {
commandLine.add("--keytab=" + keytab);
commandLine.add("--kerberos_principal=kudu/" + bindHost);
- commandLine.add("--server_require_kerberos");
+ commandLine.add("--rpc_authentication=required");
}
commandLine.addAll(extraTserverFlags);
@@ -263,7 +263,7 @@ public class MiniKuduCluster implements AutoCloseable {
if (miniKdc != null) {
commandLine.add("--keytab=" + keytab);
commandLine.add("--kerberos_principal=kudu/" + bindHost);
- commandLine.add("--server_require_kerberos");
+ commandLine.add("--rpc_authentication=required");
}
commandLine.addAll(extraMasterFlags);
http://git-wip-us.apache.org/repos/asf/kudu/blob/70ea7fb1/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 4c6dfb1..d6c3d97 100644
--- a/src/kudu/integration-tests/external_mini_cluster.cc
+++ b/src/kudu/integration-tests/external_mini_cluster.cc
@@ -606,7 +606,7 @@ Status ExternalDaemon::EnableKerberos(MiniKdc* kdc, const
string& bind_host) {
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("--server_require_kerberos");
+ extra_flags_.push_back("--rpc_authentication=required");
return Status::OK();
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/70ea7fb1/src/kudu/rpc/messenger.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/messenger.cc b/src/kudu/rpc/messenger.cc
index 591e7d0..5ba0141 100644
--- a/src/kudu/rpc/messenger.cc
+++ b/src/kudu/rpc/messenger.cc
@@ -22,6 +22,7 @@
#include <sys/types.h>
#include <unistd.h>
+#include <boost/algorithm/string/predicate.hpp>
#include <gflags/gflags.h>
#include <glog/logging.h>
#include <list>
@@ -59,31 +60,50 @@ using std::string;
using std::shared_ptr;
using strings::Substitute;
-DEFINE_string(rpc_ssl_server_certificate, "", "Path to the SSL certificate to
be used for the RPC "
- "layer.");
-DEFINE_string(rpc_ssl_private_key, "",
- "Path to the private key to be used to complement the public key present
in "
- "--ssl_server_certificate");
-DEFINE_string(rpc_ssl_certificate_authority, "",
- "Path to the certificate authority to be used by the client side of the
connection to verify "
- "the validity of the certificate presented by the server.");
+DEFINE_string(rpc_authentication, "optional",
+ "Whether to require RPC connections to authenticate. Must be one
"
+ "of 'disabled', 'optional', or 'required'. If 'optional', "
+ "authentication will be used when the remote end supports it. If
"
+ "'required', connections which are not able to authenticate "
+ "(because the remote end lacks support) are rejected. Secure "
+ "clusters should use 'required'.");
+DEFINE_string(rpc_encryption, "optional",
+ "Whether to require RPC connections to be encrypted. Must be one
"
+ "of 'disabled', 'optional', or 'required'. If 'optional', "
+ "encryption will be used when the remote end supports it. If "
+ "'required', connections which are not able to use encryption "
+ "(because the remote end lacks support) are rejected. If
'disabled', "
+ "encryption will not be used, and RPC authentication "
+ "(--rpc_authentication) must also be disabled as well. "
+ "Secure clusters should use 'required'.");
+TAG_FLAG(rpc_authentication, stable);
+TAG_FLAG(rpc_encryption, stable);
+
+DEFINE_string(rpc_certificate_file, "",
+ "Path to a PEM encoded X509 certificate to use for securing RPC "
+ "connections with SSL/TLS. If set, '--rpc_private_key_file' and "
+ "'--rpc_ca_certificate_file' must be set as well.");
+DEFINE_string(rpc_private_key_file, "",
+ "Path to a PEM encoded private key paired with the certificate "
+ "from '--rpc_certificate_file'");
+DEFINE_string(rpc_ca_certificate_file, "",
+ "Path to the PEM encoded X509 certificate of the trusted
external "
+ "certificate authority. The provided certificate should be the
root "
+ "issuer of the certificate passed in '--rpc_certificate_file'.");
+
+// Setting TLS certs and keys via CLI flags is only necessary for external
+// PKI-based security, which is not yet production ready. Instead, see
+// internal PKI (ipki) and Kerberos-based authentication.
+TAG_FLAG(rpc_certificate_file, experimental);
+TAG_FLAG(rpc_private_key_file, experimental);
+TAG_FLAG(rpc_ca_certificate_file, experimental);
DEFINE_int32(rpc_default_keepalive_time_ms, 65000,
"If an RPC connection from a client is idle for this amount of
time, the server "
"will disconnect the client.");
-
-TAG_FLAG(rpc_ssl_server_certificate, experimental);
-TAG_FLAG(rpc_ssl_private_key, experimental);
-TAG_FLAG(rpc_ssl_certificate_authority, experimental);
TAG_FLAG(rpc_default_keepalive_time_ms, advanced);
-DEFINE_bool(server_require_kerberos, false,
- "Whether to force all inbound RPC connections to authenticate "
- "with Kerberos");
-// TODO(todd): this flag is too coarse-grained, since secure servers still
-// need to allow non-kerberized connections authenticated by tokens. But
-// it's a useful stop-gap.
-TAG_FLAG(server_require_kerberos, experimental);
+DECLARE_string(keytab);
namespace kudu {
namespace rpc {
@@ -145,17 +165,52 @@ Status MessengerBuilder::Build(shared_ptr<Messenger>
*msgr) {
new_msgr->AllExternalReferencesDropped();
});
+ if (boost::iequals(FLAGS_rpc_authentication, "required")) {
+ new_msgr->authentication_ = RpcAuthentication::REQUIRED;
+ } else if (boost::iequals(FLAGS_rpc_authentication, "optional")) {
+ new_msgr->authentication_ = RpcAuthentication::OPTIONAL;
+ } else if (boost::iequals(FLAGS_rpc_authentication, "disabled")) {
+ new_msgr->authentication_ = RpcAuthentication::DISABLED;
+ } else {
+ return Status::InvalidArgument(
+ "--rpc_authentication flag must be one of 'required', 'optional', or
'disabled'");
+ }
+
+ if (boost::iequals(FLAGS_rpc_encryption, "required")) {
+ new_msgr->encryption_ = RpcEncryption::REQUIRED;
+ } else if (boost::iequals(FLAGS_rpc_encryption, "optional")) {
+ new_msgr->encryption_ = RpcEncryption::OPTIONAL;
+ } else if (boost::iequals(FLAGS_rpc_encryption, "disabled")) {
+ new_msgr->encryption_ = RpcEncryption::DISABLED;
+ } else {
+ return Status::InvalidArgument(
+ "--rpc_encryption flag must be one of 'required', 'optional', or
'disabled'");
+ }
+
+ if (new_msgr->encryption_ == RpcEncryption::DISABLED &&
+ new_msgr->authentication_ != RpcAuthentication::DISABLED) {
+ return Status::InvalidArgument("RPC authentication (--rpc_authentication)
must be disabled "
+ "if RPC encryption (--rpc_encryption) is
disabled");
+ }
+
RETURN_NOT_OK(new_msgr->Init());
- if (enable_inbound_tls_server_uuid_) {
+ if (new_msgr->encryption_ != RpcEncryption::DISABLED &&
enable_inbound_tls_server_uuid_) {
auto* tls_context = new_msgr->mutable_tls_context();
- if (!FLAGS_rpc_ssl_server_certificate.empty() ||
- !FLAGS_rpc_ssl_private_key.empty() ||
- !FLAGS_rpc_ssl_certificate_authority.empty()) {
+
+ if (!FLAGS_rpc_certificate_file.empty() &&
+ !FLAGS_rpc_private_key_file.empty() &&
+ !FLAGS_rpc_ca_certificate_file.empty()) {
+
// TODO(PKI): should we try and enforce that the server UUID and/or
// hostname is in the subject or alt names of the cert?
-
RETURN_NOT_OK(tls_context->LoadCertificateAuthority(FLAGS_rpc_ssl_certificate_authority));
-
RETURN_NOT_OK(tls_context->LoadCertificateAndKey(FLAGS_rpc_ssl_server_certificate,
-
FLAGS_rpc_ssl_private_key));
+
RETURN_NOT_OK(tls_context->LoadCertificateAuthority(FLAGS_rpc_ca_certificate_file));
+
RETURN_NOT_OK(tls_context->LoadCertificateAndKey(FLAGS_rpc_certificate_file,
+
FLAGS_rpc_private_key_file));
+ } else if (!FLAGS_rpc_certificate_file.empty() ||
+ !FLAGS_rpc_private_key_file.empty() ||
+ !FLAGS_rpc_ca_certificate_file.empty()) {
+ return Status::InvalidArgument("--rpc_certificate_file,
--rpc_private_key_file, and "
+ "--rpc_ca_certificate_file flags must be
set as a group");
} else {
RETURN_NOT_OK(tls_context->GenerateSelfSignedCertAndKey(*enable_inbound_tls_server_uuid_));
}
@@ -213,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_server_require_kerberos) {
+ if (!FLAGS_keytab.empty()) {
RETURN_NOT_OK_PREPEND(ServerNegotiation::PreflightCheckGSSAPI(),
"GSSAPI/Kerberos not properly configured");
}
@@ -292,6 +347,8 @@ void Messenger::RegisterInboundSocket(Socket *new_socket,
const Sockaddr &remote
Messenger::Messenger(const MessengerBuilder &bld)
: name_(bld.name_),
closing_(false),
+ authentication_(RpcAuthentication::REQUIRED),
+ encryption_(RpcEncryption::REQUIRED),
tls_context_(new security::TlsContext()),
token_verifier_(new security::TokenVerifier()),
rpcz_store_(new RpczStore()),
http://git-wip-us.apache.org/repos/asf/kudu/blob/70ea7fb1/src/kudu/rpc/messenger.h
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/messenger.h b/src/kudu/rpc/messenger.h
index c232bf5..89dd933 100644
--- a/src/kudu/rpc/messenger.h
+++ b/src/kudu/rpc/messenger.h
@@ -74,6 +74,20 @@ struct AcceptorPoolInfo {
Sockaddr bind_address_;
};
+// Authentication configuration for RPC connections.
+enum class RpcAuthentication {
+ DISABLED,
+ OPTIONAL,
+ REQUIRED,
+};
+
+// Encryption configuration for RPC connections.
+enum class RpcEncryption {
+ DISABLED,
+ OPTIONAL,
+ REQUIRED,
+};
+
// Used to construct a Messenger.
class MessengerBuilder {
public:
@@ -252,6 +266,13 @@ class Messenger {
bool closing_;
+ // Whether to require authentication and encryption on the connections
managed
+ // by this messenger.
+ // TODO(PKI): scope these to individual proxies, so that messengers can be
+ // reused by different clients.
+ RpcAuthentication authentication_;
+ RpcEncryption encryption_;
+
// Pools which are listening on behalf of this messenger.
// Note that the user may have called Shutdown() on one of these
// pools, so even though we retain the reference, it may no longer
http://git-wip-us.apache.org/repos/asf/kudu/blob/70ea7fb1/src/kudu/rpc/negotiation.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/negotiation.cc b/src/kudu/rpc/negotiation.cc
index f303179..e044c98 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_bool(server_require_kerberos);
+DECLARE_string(keytab);
DEFINE_bool(rpc_encrypt_loopback_connections, false,
"Whether to encrypt data transfer on RPC connections that stay
within "
@@ -208,10 +208,12 @@ static Status DoServerNegotiation(Connection* conn, const
MonoTime& deadline) {
&messenger->tls_context(),
&messenger->token_verifier());
- if (FLAGS_server_require_kerberos) {
- RETURN_NOT_OK(server_negotiation.EnableGSSAPI());
- } else {
+ // 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()) {
RETURN_NOT_OK(server_negotiation.EnablePlain());
+ } else {
+ RETURN_NOT_OK(server_negotiation.EnableGSSAPI());
}
server_negotiation.set_deadline(deadline);
http://git-wip-us.apache.org/repos/asf/kudu/blob/70ea7fb1/src/kudu/rpc/rpc-test-base.h
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/rpc-test-base.h b/src/kudu/rpc/rpc-test-base.h
index 5bdacee..47f209f 100644
--- a/src/kudu/rpc/rpc-test-base.h
+++ b/src/kudu/rpc/rpc-test-base.h
@@ -50,10 +50,6 @@
#include "kudu/util/test_util.h"
#include "kudu/util/trace.h"
-DECLARE_string(rpc_ssl_server_certificate);
-DECLARE_string(rpc_ssl_private_key);
-DECLARE_string(rpc_ssl_certificate_authority);
-
namespace kudu { namespace rpc {
using kudu::rpc_test::AddRequestPB;
@@ -367,16 +363,12 @@ class RpcTestBase : public KuduTest {
std::shared_ptr<Messenger> CreateMessenger(const string &name,
int n_reactors = 1,
bool enable_ssl = false) {
+ MessengerBuilder bld(name);
+
if (enable_ssl) {
- std::string server_cert_path = GetTestPath("server-cert.pem");
- std::string private_key_path = GetTestPath("server-key.pem");
- CHECK_OK(security::CreateSSLServerCert(server_cert_path));
- CHECK_OK(security::CreateSSLPrivateKey(private_key_path));
- FLAGS_rpc_ssl_server_certificate = server_cert_path;
- FLAGS_rpc_ssl_private_key = private_key_path;
- FLAGS_rpc_ssl_certificate_authority = server_cert_path;
+ bld.enable_inbound_tls(name);
}
- MessengerBuilder bld(name);
+
bld.set_num_reactors(n_reactors);
bld.set_connection_keepalive_time(
MonoDelta::FromMilliseconds(keepalive_time_ms_));
http://git-wip-us.apache.org/repos/asf/kudu/blob/70ea7fb1/src/kudu/rpc/sasl_common.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/sasl_common.cc b/src/kudu/rpc/sasl_common.cc
index a0ae7e1..c852675 100644
--- a/src/kudu/rpc/sasl_common.cc
+++ b/src/kudu/rpc/sasl_common.cc
@@ -42,7 +42,7 @@
using std::set;
-DECLARE_bool(server_require_kerberos);
+DECLARE_string(keytab);
namespace kudu {
namespace rpc {
@@ -318,9 +318,10 @@ 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.
- if (FLAGS_server_require_kerberos)
kudu::security::KerberosReinitLock()->ReadLock();
+ bool kerberos_supported = !FLAGS_keytab.empty();
+ if (kerberos_supported) kudu::security::KerberosReinitLock()->ReadLock();
int rc = call();
- if (FLAGS_server_require_kerberos)
kudu::security::KerberosReinitLock()->ReadUnlock();
+ if (kerberos_supported) kudu::security::KerberosReinitLock()->ReadUnlock();
g_auth_failure_capture = nullptr;
switch (rc) {
