kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From t...@apache.org
Subject [4/6] kudu git commit: rpc: add a pre-flight check for server GSSAPI setup
Date Wed, 02 Nov 2016 21:39:46 GMT
rpc: add a pre-flight check for server GSSAPI setup

This adds a pre-flight check that runs when adding an acceptor pool to a
messenger (which indicates that the messenger will act as a server). The
pre-flight check runs through a partial SASL GSSAPI authentication flow
in order to verify that the plugin is set up and a keytab is available.

Tested this manually as well as with a new unit test.

Change-Id: Ia26482003144504dee77ffb2e9de7ebe3c1e2e3d
Reviewed-on: http://gerrit.cloudera.org:8080/4909
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <todd@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/d5d9afbf
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/d5d9afbf
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/d5d9afbf

Branch: refs/heads/master
Commit: d5d9afbfc7a7aef375494f2f2971d30312b82a20
Parents: e6cb2ef
Author: Todd Lipcon <todd@apache.org>
Authored: Tue Nov 1 17:04:35 2016 -0700
Committer: Todd Lipcon <todd@apache.org>
Committed: Wed Nov 2 20:31:20 2016 +0000

----------------------------------------------------------------------
 src/kudu/rpc/connection.cc    | 11 +----------
 src/kudu/rpc/messenger.cc     | 16 ++++++++++++++++
 src/kudu/rpc/messenger.h      |  4 ++++
 src/kudu/rpc/sasl_rpc-test.cc | 31 +++++++++++++++++++++++++++++++
 src/kudu/rpc/sasl_server.cc   | 38 ++++++++++++++++++++++++++++++++++++++
 src/kudu/rpc/sasl_server.h    |  4 ++++
 6 files changed, 94 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/d5d9afbf/src/kudu/rpc/connection.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/connection.cc b/src/kudu/rpc/connection.cc
index f134d7c..e074ec0 100644
--- a/src/kudu/rpc/connection.cc
+++ b/src/kudu/rpc/connection.cc
@@ -53,13 +53,7 @@ using std::shared_ptr;
 using std::vector;
 using strings::Substitute;
 
-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_bool(server_require_kerberos);
 
 namespace kudu {
 namespace rpc {
@@ -659,9 +653,6 @@ Status Connection::InitSaslClient() {
 
 Status Connection::InitSaslServer() {
   if (FLAGS_server_require_kerberos) {
-    // TODO(todd): should we use krb5 APIs directly to verify that we have a valid
-    // keytab when we start up, rather than starting up and then rejecting
-    // connections?
     RETURN_NOT_OK(sasl_server().EnableGSSAPI());
   } else {
     RETURN_NOT_OK(sasl_server().EnablePlain());

http://git-wip-us.apache.org/repos/asf/kudu/blob/d5d9afbf/src/kudu/rpc/messenger.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/messenger.cc b/src/kudu/rpc/messenger.cc
index 501191d..4f1f0e3 100644
--- a/src/kudu/rpc/messenger.cc
+++ b/src/kudu/rpc/messenger.cc
@@ -60,6 +60,14 @@ DEFINE_int32(rpc_default_keepalive_time_ms, 65000,
              "will disconnect the client.");
 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);
+
 namespace kudu {
 namespace rpc {
 
@@ -158,6 +166,14 @@ void Messenger::Shutdown() {
 
 Status Messenger::AddAcceptorPool(const Sockaddr &accept_addr,
                                   shared_ptr<AcceptorPool>* pool) {
+  // 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) {
+    RETURN_NOT_OK_PREPEND(SaslServer::PreflightCheckGSSAPI(kSaslAppName),
+                          "GSSAPI/Kerberos not properly configured");
+  }
+
   Socket sock;
   RETURN_NOT_OK(sock.Init(0));
   RETURN_NOT_OK(sock.SetReuseAddr(true));

http://git-wip-us.apache.org/repos/asf/kudu/blob/d5d9afbf/src/kudu/rpc/messenger.h
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/messenger.h b/src/kudu/rpc/messenger.h
index 1424913..a7ac1ae 100644
--- a/src/kudu/rpc/messenger.h
+++ b/src/kudu/rpc/messenger.h
@@ -139,6 +139,10 @@ class Messenger {
   //
   // NOTE: the returned pool is not initially started. You must call
   // pool->Start(...) to begin accepting connections.
+  //
+  // If Kerberos is enabled, this also runs a pre-flight check that makes
+  // sure the environment is appropriately configured to authenticate
+  // clients via Kerberos. If not, this returns a RuntimeError.
   Status AddAcceptorPool(const Sockaddr &accept_addr,
                          std::shared_ptr<AcceptorPool>* pool);
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/d5d9afbf/src/kudu/rpc/sasl_rpc-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/sasl_rpc-test.cc b/src/kudu/rpc/sasl_rpc-test.cc
index 5458345..2786d89 100644
--- a/src/kudu/rpc/sasl_rpc-test.cc
+++ b/src/kudu/rpc/sasl_rpc-test.cc
@@ -18,6 +18,7 @@
 #include "kudu/rpc/rpc-test-base.h"
 
 #include <stdlib.h>
+#include <sys/stat.h>
 
 #include <functional>
 #include <memory>
@@ -339,6 +340,36 @@ TEST_F(TestSaslRpc, TestGSSAPINegotiation) {
 
 }
 
+// Test that the pre-flight check for servers requiring Kerberos provides
+// nice error messages for missing or bad keytabs.
+TEST_F(TestSaslRpc, TestPreflight) {
+  // Try pre-flight with no keytab.
+  Status s = SaslServer::PreflightCheckGSSAPI(kSaslAppName);
+  ASSERT_STR_MATCHES(s.ToString(), "Key table file.*not found");
+
+  // Try with a valid krb5 environment and keytab.
+  MiniKdc kdc;
+  ASSERT_OK(kdc.Start());
+  ASSERT_OK(kdc.SetKrb5Environment());
+  string kt_path;
+  ASSERT_OK(kdc.CreateServiceKeytab("kudu/127.0.0.1", &kt_path));
+  CHECK_ERR(setenv("KRB5_KTNAME", kt_path.c_str(), 1 /*replace*/));
+
+  ASSERT_OK(SaslServer::PreflightCheckGSSAPI(kSaslAppName));
+
+  // Try with an inaccessible keytab.
+  CHECK_ERR(chmod(kt_path.c_str(), 0000));
+  s = SaslServer::PreflightCheckGSSAPI(kSaslAppName);
+  ASSERT_STR_MATCHES(s.ToString(), "error accessing keytab: Permission denied");
+  CHECK_ERR(unlink(kt_path.c_str()));
+
+  // Try with a keytab that has the wrong credentials.
+  ASSERT_OK(kdc.CreateServiceKeytab("wrong-service/127.0.0.1", &kt_path));
+  CHECK_ERR(setenv("KRB5_KTNAME", kt_path.c_str(), 1 /*replace*/));
+  s = SaslServer::PreflightCheckGSSAPI(kSaslAppName);
+  ASSERT_STR_MATCHES(s.ToString(), "No key table entry found matching kudu/.*");
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 
 static void RunTimeoutExpectingServer(Socket* conn) {

http://git-wip-us.apache.org/repos/asf/kudu/blob/d5d9afbf/src/kudu/rpc/sasl_server.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/sasl_server.cc b/src/kudu/rpc/sasl_server.cc
index c8e5fc0..9a6523e 100644
--- a/src/kudu/rpc/sasl_server.cc
+++ b/src/kudu/rpc/sasl_server.cc
@@ -470,5 +470,43 @@ int SaslServer::PlainAuthCb(sasl_conn_t * /*conn*/, const char * /*user*/,
const
   return SASL_OK;
 }
 
+Status SaslServer::PreflightCheckGSSAPI(const string& app_name) {
+  // Initialize a SaslServer with a bogus socket fd, and enable
+  // only GSSAPI.
+  //
+  // We aren't going to actually send/receive any messages, but
+  // this makes it easier to reuse the initialization code.
+  SaslServer server(app_name, -1);
+  Status s = server.EnableGSSAPI();
+  if (!s.ok()) {
+    return Status::RuntimeError(s.message());
+  }
+
+  RETURN_NOT_OK(server.Init(app_name));
+
+  // Start the SASL server as if we were accepting a connection.
+  const char* server_out = nullptr; // ignored
+  uint32_t server_out_len = 0;
+  s = WrapSaslCall(server.sasl_conn_.get(), [&]() {
+      return sasl_server_start(
+          server.sasl_conn_.get(),
+          kSaslMechGSSAPI,
+          "", 0,  // Pass a 0-length token.
+          &server_out, &server_out_len);
+    });
+
+  // We expect 'Incomplete' status to indicate that the first step of negotiation
+  // was correct.
+  if (s.IsIncomplete()) return Status::OK();
+
+  string err_msg = s.message().ToString();
+  if (err_msg == "Permission denied") {
+    // For bad keytab permissions, we get a rather vague message. So,
+    // we make it more specific for better usability.
+    err_msg = "error accessing keytab: " + err_msg;
+  }
+  return Status::RuntimeError(err_msg);
+}
+
 } // namespace rpc
 } // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/d5d9afbf/src/kudu/rpc/sasl_server.h
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/sasl_server.h b/src/kudu/rpc/sasl_server.h
index 0f5946f..a05725c 100644
--- a/src/kudu/rpc/sasl_server.h
+++ b/src/kudu/rpc/sasl_server.h
@@ -112,6 +112,10 @@ class SaslServer {
   int PlainAuthCb(sasl_conn_t* conn, const char* user, const char* pass,
                   unsigned passlen, struct propctx* propctx);
 
+  // Perform a "pre-flight check" that everything required to act as a Kerberos
+  // server is properly set up.
+  static Status PreflightCheckGSSAPI(const std::string& app_name);
+
  private:
   // Parse and validate connection header.
   Status ValidateConnectionHeader(faststring* recv_buf);


Mime
View raw message