kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From danburk...@apache.org
Subject [1/2] kudu git commit: [security] adjust TLS certificate verification
Date Fri, 03 Feb 2017 00:28:30 GMT
Repository: kudu
Updated Branches:
  refs/heads/master e4a649c20 -> 85fd90996


[security] adjust TLS certificate verification

This commit adjusts when TLS certificate verification is performed in
the client and server, and adds a test of negotiating GSSAPI with TLS.
In the client, the server's cert is not verified when using the GSSAPI
SASL mech, since Kerberos provides strong authn. In the server, client
cert verification is disabled entirely. We'll have to turn it on in the
future selectively when we add support for client certs.

As part of this change, we no longer allow the SASL library to choose
the mechanism to use.  Instead, we determine the mechanism manually
using a simple heuristic: prefer GSSAPI to PLAIN. When we add support
for more mechanisms (e.g. KUDU_TOKEN), we'll update the heuristic
accordingly.

Change-Id: Id3b1698ccd8434b8d40d567e9d0fa506e4cdc0ca
Reviewed-on: http://gerrit.cloudera.org:8080/5865
Reviewed-by: Alexey Serbin <aserbin@cloudera.com>
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/4ae35c29
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/4ae35c29
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/4ae35c29

Branch: refs/heads/master
Commit: 4ae35c295c02737663ec80f36580c7caa38b1479
Parents: e4a649c
Author: Dan Burkert <danburkert@apache.org>
Authored: Wed Feb 1 19:32:50 2017 -0800
Committer: Todd Lipcon <todd@apache.org>
Committed: Thu Feb 2 23:25:33 2017 +0000

----------------------------------------------------------------------
 src/kudu/rpc/client_negotiation.cc | 91 ++++++++++++++++++++++++---------
 src/kudu/rpc/client_negotiation.h  |  4 --
 src/kudu/rpc/negotiation-test.cc   | 75 +++++++++++++++++++++++++--
 src/kudu/rpc/sasl_common.cc        | 15 +++---
 src/kudu/rpc/sasl_common.h         | 30 ++++-------
 src/kudu/rpc/sasl_helper.cc        | 10 ++--
 src/kudu/rpc/sasl_helper.h         | 11 ++--
 src/kudu/rpc/server_negotiation.cc | 14 +++--
 src/kudu/rpc/server_negotiation.h  |  2 +-
 src/kudu/security/tls_handshake.h  |  2 +-
 10 files changed, 181 insertions(+), 73 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/4ae35c29/src/kudu/rpc/client_negotiation.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/client_negotiation.cc b/src/kudu/rpc/client_negotiation.cc
index 9210864..6796d60 100644
--- a/src/kudu/rpc/client_negotiation.cc
+++ b/src/kudu/rpc/client_negotiation.cc
@@ -52,6 +52,8 @@ using std::set;
 using std::string;
 using std::unique_ptr;
 
+using strings::Substitute;
+
 namespace kudu {
 namespace rpc {
 
@@ -168,6 +170,14 @@ Status ClientNegotiation::Negotiate() {
     RETURN_NOT_OK(tls_context_->InitiateHandshake(security::TlsHandshakeType::CLIENT,
                                                   &tls_handshake_));
 
+    if (negotiated_mech_ == SaslMechanism::GSSAPI) {
+      // When using GSSAPI, we don't verify the server's certificate. Instead,
+      // we rely on Kerberos authentication, and use channel binding to tie the
+      // SASL authentication to the TLS channel.
+      // TODO(PKI): implement channel binding when TLS and GSSAPI are used.
+      tls_handshake_.set_verification_mode(security::TlsVerificationMode::VERIFY_NONE);
+    }
+
     // To initiate the TLS handshake, we pretend as if the server sent us an
     // empty TLS_HANDSHAKE token.
     NegotiatePB initial;
@@ -310,26 +320,53 @@ Status ClientNegotiation::HandleNegotiate(const NegotiatePB& response)
{
   }
 
   // Build a map of the SASL mechanisms offered by the server.
-  const set<string>& enabled_mechs = helper_.EnabledMechs();
-  set<string> server_mechs;
-  map<string, NegotiatePB::SaslAuth> server_mech_map;
+  const set<SaslMechanism::Type>& client_mechs = helper_.EnabledMechs();
+  set<SaslMechanism::Type> server_mechs;
+  map<SaslMechanism::Type, NegotiatePB::SaslAuth> server_mech_map;
   for (const NegotiatePB::SaslAuth& auth : response.auths()) {
-    const auto& mech = auth.mechanism();
+    auto mech = SaslMechanism::value_of(auth.mechanism());
+    if (mech == SaslMechanism::INVALID) {
+      continue;
+    }
     server_mech_map[mech] = auth;
     server_mechs.insert(mech);
   }
 
-  // Determine which server mechs are also enabled by the client.
-  // Cyrus SASL 2.1.25 and later supports doing this set intersection via
-  // the 'client_mech_list' option, but that version is not available on
-  // RHEL 6, so we have to do it manually.
-  common_mechs_ = STLSetIntersection(enabled_mechs, server_mechs);
-
-  if (common_mechs_.empty() &&
-      ContainsKey(server_mechs, kSaslMechGSSAPI) &&
-      !ContainsKey(enabled_mechs, kSaslMechGSSAPI)) {
-    return Status::NotAuthorized("server requires GSSAPI (Kerberos) authentication and "
-                                 "client was missing the required SASL module");
+  // Determine which SASL mechanism to use for authenticating the connection.
+  // We pick the most preferred mechanism which is supported by both parties.
+  // The preference list in order of most to least preferred:
+  //  * GSSAPI
+  //  * PLAIN
+  set<SaslMechanism::Type> common_mechs = STLSetIntersection(client_mechs, server_mechs);
+
+  if (common_mechs.empty()) {
+    if (ContainsKey(server_mechs, SaslMechanism::GSSAPI) &&
+        !ContainsKey(client_mechs, SaslMechanism::GSSAPI)) {
+      return Status::NotAuthorized("server requires authentication, "
+                                   "but client does not have Kerberos enabled");
+    }
+    if (!ContainsKey(server_mechs, SaslMechanism::GSSAPI) &&
+               ContainsKey(client_mechs, SaslMechanism::GSSAPI)) {
+      return Status::NotAuthorized("client requires authentication, "
+                                   "but server does not have Kerberos enabled");
+    }
+    string msg = Substitute("client/server supported SASL mechanism mismatch; "
+                            "client mechanisms: [$0], server mechanisms: [$1]",
+                            JoinMapped(client_mechs, SaslMechanism::name_of, ", "),
+                            JoinMapped(server_mechs, SaslMechanism::name_of, ", "));
+
+    // For now, there should never be a SASL mechanism mismatch that isn't due
+    // to one of the sides requiring Kerberos and the other not having it, so
+    // lets sanity check that.
+    DLOG(FATAL) << msg;
+    return Status::NotAuthorized(msg);
+  }
+
+  if (ContainsKey(common_mechs, SaslMechanism::GSSAPI)) {
+    negotiated_mech_ = SaslMechanism::GSSAPI;
+  } else {
+    DCHECK(ContainsKey(common_mechs, SaslMechanism::PLAIN));
+    negotiated_mech_ = SaslMechanism::PLAIN;
   }
 
   return Status::OK();
@@ -371,8 +408,12 @@ Status ClientNegotiation::HandleTlsHandshake(const NegotiatePB& response)
{
 }
 
 Status ClientNegotiation::SendSaslInitiate() {
-  string matching_mechs_str = JoinElements(common_mechs_, " ");
-  TRACE("Matching mech list: $0", matching_mechs_str);
+  TRACE("Initiating SASL $0 handshake", negotiated_mech_);
+
+  // At this point we've already chosen the SASL mechanism to use
+  // (negotiated_mech_), but we need to let the SASL library know. SASL likes to
+  // choose the mechanism from among a list of possible options, so we simply
+  // provide it one option, and then check that it picks that option.
 
   const char* init_msg = nullptr;
   unsigned init_msg_len = 0;
@@ -395,20 +436,20 @@ Status ClientNegotiation::SendSaslInitiate() {
   TRACE("Calling sasl_client_start()");
   Status s = WrapSaslCall(sasl_conn_.get(), [&]() {
       return sasl_client_start(
-          sasl_conn_.get(),           // The SASL connection context created by init()
-          matching_mechs_str.c_str(), // The list of mechanisms to negotiate.
-          nullptr,                    // Disables INTERACT return if NULL.
-          &init_msg,                  // Filled in on success.
-          &init_msg_len,              // Filled in on success.
-          &negotiated_mech);          // Filled in on success.
+          sasl_conn_.get(),                         // The SASL connection context created
by init()
+          SaslMechanism::name_of(negotiated_mech_), // The list of mechanisms to negotiate.
+          nullptr,                                  // Disables INTERACT return if NULL.
+          &init_msg,                                // Filled in on success.
+          &init_msg_len,                            // Filled in on success.
+          &negotiated_mech);                        // Filled in on success.
   });
 
   if (PREDICT_FALSE(!s.IsIncomplete() && !s.ok())) {
     return s;
   }
 
-  // The server matched one of our mechanisms.
-  negotiated_mech_ = SaslMechanism::value_of(negotiated_mech);
+  // Check that the SASL library is using the mechanism that we picked.
+  DCHECK_EQ(SaslMechanism::value_of(negotiated_mech), negotiated_mech_);
 
   NegotiatePB msg;
   msg.set_step(NegotiatePB::SASL_INITIATE);

http://git-wip-us.apache.org/repos/asf/kudu/blob/4ae35c29/src/kudu/rpc/client_negotiation.h
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/client_negotiation.h b/src/kudu/rpc/client_negotiation.h
index 5ed4467..5563ff6 100644
--- a/src/kudu/rpc/client_negotiation.h
+++ b/src/kudu/rpc/client_negotiation.h
@@ -194,10 +194,6 @@ class ClientNegotiation {
   // The set of features supported by the server. Filled in during negotiation.
   std::set<RpcFeatureFlag> server_features_;
 
-  // The set of SASL mechanisms supported by the server and the client. Filled
-  // in during negotiation.
-  std::set<std::string> common_mechs_;
-
   // The SASL mechanism used by the connection. Filled in during negotiation.
   SaslMechanism::Type negotiated_mech_;
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/4ae35c29/src/kudu/rpc/negotiation-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/negotiation-test.cc b/src/kudu/rpc/negotiation-test.cc
index 984dff1..c05386f 100644
--- a/src/kudu/rpc/negotiation-test.cc
+++ b/src/kudu/rpc/negotiation-test.cc
@@ -36,6 +36,7 @@
 #include "kudu/rpc/sasl_common.h"
 #include "kudu/rpc/server_negotiation.h"
 #include "kudu/security/test/mini_kdc.h"
+#include "kudu/security/tls_context.h"
 #include "kudu/util/monotime.h"
 #include "kudu/util/net/sockaddr.h"
 #include "kudu/util/net/socket.h"
@@ -65,7 +66,7 @@ namespace rpc {
 
 class TestNegotiation : public RpcTestBase {
  public:
-  virtual void SetUp() OVERRIDE {
+  void SetUp() override {
     RpcTestBase::SetUp();
     ASSERT_OK(SaslInit());
   }
@@ -84,7 +85,7 @@ static void RunAcceptingDelegator(Socket* acceptor,
   server_runner(std::move(conn));
 }
 
-// Set up a socket and run a SASL negotiation.
+// Set up a socket and run a negotiation sequence.
 static void RunNegotiationTest(const SocketCallable& server_runner,
                                const SocketCallable& client_runner) {
   Socket server_sock;
@@ -220,7 +221,7 @@ TEST_F(TestNegotiation, TestNoMatchingMechanisms) {
         // The client enables both PLAIN and GSSAPI.
         CHECK_OK(client_negotiation.EnablePlain("foo", "bar"));
         Status s = client_negotiation.Negotiate();
-        ASSERT_STR_CONTAINS(s.ToString(), "client was missing the required SASL module");
+        ASSERT_STR_CONTAINS(s.ToString(), "client does not have Kerberos enabled");
       });
 }
 
@@ -537,5 +538,73 @@ TEST_F(TestDisableInit, TestMultipleSaslInit_NoMutexImpl) {
 }
 #endif
 
+////////////////////////////////////////////////////////////////////////////////
+
+// Server which has TLS and SASL GSSAPI enabled.
+static void RunTlsGssapiNegotiationServer(unique_ptr<Socket> socket) {
+  ServerNegotiation server_negotiation(std::move(socket));
+
+  // TODO(PKI): switch this to use an in-memory cert and key.
+  std::string server_cert_path = JoinPathSegments(GetTestDataDirectory(), "server-cert.pem");
+  std::string private_key_path = JoinPathSegments(GetTestDataDirectory(), "server-key.pem");
+  ASSERT_OK(security::CreateSSLServerCert(server_cert_path));
+  ASSERT_OK(security::CreateSSLPrivateKey(private_key_path));
+
+  security::TlsContext tls_context;
+  ASSERT_OK(tls_context.Init());
+  ASSERT_OK(tls_context.LoadCertificate(server_cert_path));
+  ASSERT_OK(tls_context.LoadPrivateKey(private_key_path));
+  server_negotiation.EnableTls(&tls_context);
+
+  server_negotiation.set_server_fqdn("127.0.0.1");
+  ASSERT_OK(server_negotiation.EnableGSSAPI());
+
+  ASSERT_OK(server_negotiation.Negotiate());
+  ASSERT_TRUE(ContainsKey(server_negotiation.client_features(), APPLICATION_FEATURE_FLAGS));
+  ASSERT_TRUE(ContainsKey(server_negotiation.client_features(), TLS));
+
+  ASSERT_EQ(SaslMechanism::GSSAPI, server_negotiation.negotiated_mechanism());
+  ASSERT_EQ("testuser", server_negotiation.authenticated_user());
+
+  // TODO(PKI): assert that TLS is actually negotiated.
+}
+
+static void RunTlsGssapiNegotiationClient(unique_ptr<Socket> socket) {
+  ClientNegotiation client_negotiation(std::move(socket));
+
+  security::TlsContext tls_context;
+  ASSERT_OK(tls_context.Init());
+  client_negotiation.EnableTls(&tls_context);
+
+  client_negotiation.set_server_fqdn("127.0.0.1");
+  CHECK_OK(client_negotiation.EnableGSSAPI());
+  CHECK_OK(client_negotiation.Negotiate());
+  CHECK(ContainsKey(client_negotiation.server_features(), APPLICATION_FEATURE_FLAGS));
+  CHECK(ContainsKey(client_negotiation.server_features(), TLS));
+
+  // TODO(PKI): assert that TLS is actually negotiated.
+}
+
+// Test TLS and GSSAPI (kerberos) SASL negotiation.
+TEST_F(TestNegotiation, TestTlsWithGssapiNegotiation) {
+  MiniKdc kdc;
+  ASSERT_OK(kdc.Start());
+
+  // Create the server principal and keytab.
+  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*/));
+
+  // Create and kinit as a client user.
+  ASSERT_OK(kdc.CreateUserPrincipal("testuser"));
+  ASSERT_OK(kdc.Kinit("testuser"));
+  ASSERT_OK(kdc.SetKrb5Environment());
+
+  // Authentication should succeed on both sides.
+  RunNegotiationTest(RunTlsGssapiNegotiationServer, RunTlsGssapiNegotiationClient);
+}
+
+////////////////////////////////////////////////////////////////////////////////
+
 } // namespace rpc
 } // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/4ae35c29/src/kudu/rpc/sasl_common.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/sasl_common.cc b/src/kudu/rpc/sasl_common.cc
index 5e4aae6..8027721 100644
--- a/src/kudu/rpc/sasl_common.cc
+++ b/src/kudu/rpc/sasl_common.cc
@@ -343,14 +343,17 @@ string SaslIpPortString(const Sockaddr& addr) {
   return addr_str;
 }
 
-set<string> SaslListAvailableMechs() {
-  set<string> mechs;
+set<SaslMechanism::Type> SaslListAvailableMechs() {
+  set<SaslMechanism::Type> mechs;
 
   // Array of NULL-terminated strings. Array terminated with NULL.
-  const char** mech_strings = sasl_global_listmech();
-  while (mech_strings != nullptr && *mech_strings != nullptr) {
-    mechs.insert(*mech_strings);
-    mech_strings++;
+  for (const char** mech_strings = sasl_global_listmech();
+       mech_strings != nullptr && *mech_strings != nullptr;
+       mech_strings++) {
+    auto mech = SaslMechanism::value_of(*mech_strings);
+    if (mech != SaslMechanism::INVALID) {
+      mechs.insert(mech);
+    }
   }
   return mechs;
 }

http://git-wip-us.apache.org/repos/asf/kudu/blob/4ae35c29/src/kudu/rpc/sasl_common.h
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/sasl_common.h b/src/kudu/rpc/sasl_common.h
index 53b713e..d56f7ec 100644
--- a/src/kudu/rpc/sasl_common.h
+++ b/src/kudu/rpc/sasl_common.h
@@ -39,6 +39,16 @@ using std::string;
 extern const char* const kSaslMechPlain;
 extern const char* const kSaslMechGSSAPI;
 
+struct SaslMechanism {
+  enum Type {
+    INVALID,
+    PLAIN,
+    GSSAPI
+  };
+  static Type value_of(const std::string& mech);
+  static const char* name_of(Type val);
+};
+
 // Initialize the SASL library.
 // appname: Name of the application for logging messages & sasl plugin configuration.
 //          Note that this string must remain allocated for the lifetime of the program.
@@ -75,7 +85,7 @@ Status WrapSaslCall(sasl_conn_t* conn, const std::function<int()>&
call);
 string SaslIpPortString(const Sockaddr& addr);
 
 // Return available plugin mechanisms for the given connection.
-std::set<string> SaslListAvailableMechs();
+std::set<SaslMechanism::Type> SaslListAvailableMechs();
 
 // Initialize and return a libsasl2 callback data structure based on the passed args.
 // id: A SASL callback identifier (e.g., SASL_CB_GETOPT).
@@ -90,24 +100,6 @@ struct SaslDeleter {
   }
 };
 
-struct SaslNegotiationState {
-  enum Type {
-    NEW,
-    INITIALIZED,
-    NEGOTIATED
-  };
-};
-
-struct SaslMechanism {
-  enum Type {
-    INVALID,
-    PLAIN,
-    GSSAPI
-  };
-  static Type value_of(const std::string& mech);
-  static const char* name_of(Type val);
-};
-
 // Internals exposed in the header for test purposes.
 namespace internal {
 void SaslSetMutex();

http://git-wip-us.apache.org/repos/asf/kudu/blob/4ae35c29/src/kudu/rpc/sasl_helper.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/sasl_helper.cc b/src/kudu/rpc/sasl_helper.cc
index 0614f3e..e6e96bd 100644
--- a/src/kudu/rpc/sasl_helper.cc
+++ b/src/kudu/rpc/sasl_helper.cc
@@ -70,7 +70,7 @@ const char* SaslHelper::server_fqdn() const {
 }
 
 const char* SaslHelper::EnabledMechsString() const {
-  JoinStrings(enabled_mechs_, " ", &enabled_mechs_string_);
+  enabled_mechs_string_ = JoinMapped(enabled_mechs_, SaslMechanism::name_of, " ");
   return enabled_mechs_string_.c_str();
 }
 
@@ -102,20 +102,20 @@ int SaslHelper::GetOptionCb(const char* plugin_name, const char* option,
 }
 
 Status SaslHelper::EnablePlain() {
-  RETURN_NOT_OK(EnableMechanism(kSaslMechPlain));
+  RETURN_NOT_OK(EnableMechanism(SaslMechanism::PLAIN));
   plain_enabled_ = true;
   return Status::OK();
 }
 
 Status SaslHelper::EnableGSSAPI() {
-  RETURN_NOT_OK(EnableMechanism(kSaslMechGSSAPI));
+  RETURN_NOT_OK(EnableMechanism(SaslMechanism::GSSAPI));
   gssapi_enabled_ = true;
   return Status::OK();
 }
 
-Status SaslHelper::EnableMechanism(const string& mech) {
+Status SaslHelper::EnableMechanism(SaslMechanism::Type mech) {
   if (PREDICT_FALSE(!ContainsKey(global_mechs_, mech))) {
-    return Status::InvalidArgument("unable to find SASL plugin", mech);
+    return Status::InvalidArgument("unable to find SASL plugin", SaslMechanism::name_of(mech));
   }
   enabled_mechs_.insert(mech);
   return Status::OK();

http://git-wip-us.apache.org/repos/asf/kudu/blob/4ae35c29/src/kudu/rpc/sasl_helper.h
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/sasl_helper.h b/src/kudu/rpc/sasl_helper.h
index 05d6904..ed38c05 100644
--- a/src/kudu/rpc/sasl_helper.h
+++ b/src/kudu/rpc/sasl_helper.h
@@ -23,6 +23,7 @@
 
 #include <sasl/sasl.h>
 
+#include "kudu/rpc/sasl_common.h"
 #include "kudu/util/status.h"
 
 namespace kudu {
@@ -59,12 +60,12 @@ class SaslHelper {
   const char* server_fqdn() const;
 
   // Globally-registered available SASL plugins.
-  const std::set<std::string>& GlobalMechs() const {
+  const std::set<SaslMechanism::Type>& GlobalMechs() const {
     return global_mechs_;
   }
 
   // Helper functions for managing the list of active SASL mechanisms.
-  const std::set<std::string>& EnabledMechs() const {
+  const std::set<SaslMechanism::Type>& EnabledMechs() const {
     return enabled_mechs_;
   }
 
@@ -88,7 +89,7 @@ class SaslHelper {
   Status ParseNegotiatePB(const Slice& param_buf, NegotiatePB* msg);
 
  private:
-  Status EnableMechanism(const std::string& mech);
+  Status EnableMechanism(SaslMechanism::Type mech);
 
   // Returns space-delimited local mechanism list string suitable for passing
   // to libsasl2, such as via "mech_list" callbacks.
@@ -102,8 +103,8 @@ class SaslHelper {
   // Authentication types and data.
   const PeerType peer_type_;
   std::string tag_;
-  std::set<std::string> global_mechs_;       // Cache of global mechanisms.
-  std::set<std::string> enabled_mechs_;      // Active mechanisms.
+  std::set<SaslMechanism::Type> global_mechs_;       // Cache of global mechanisms.
+  std::set<SaslMechanism::Type> enabled_mechs_;      // Active mechanisms.
   mutable std::string enabled_mechs_string_; // Mechanism list string returned by callbacks.
 
   bool plain_enabled_;

http://git-wip-us.apache.org/repos/asf/kudu/blob/4ae35c29/src/kudu/rpc/server_negotiation.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/server_negotiation.cc b/src/kudu/rpc/server_negotiation.cc
index dad6194..b1f2893 100644
--- a/src/kudu/rpc/server_negotiation.cc
+++ b/src/kudu/rpc/server_negotiation.cc
@@ -135,6 +135,12 @@ Status ServerNegotiation::Negotiate() {
   if (tls_context_ && ContainsKey(client_features_, TLS)) {
     RETURN_NOT_OK(tls_context_->InitiateHandshake(security::TlsHandshakeType::SERVER,
                                                   &tls_handshake_));
+
+    // The server does not verify the client's certificate.
+    // TODO(PKI): Add client-certificate authn support, which will verify the
+    // client cert.
+    tls_handshake_.set_verification_mode(security::TlsVerificationMode::VERIFY_NONE);
+
     while (true) {
       NegotiatePB request;
       RETURN_NOT_OK(RecvNegotiatePB(&request, &recv_buf));
@@ -326,7 +332,7 @@ Status ServerNegotiation::HandleNegotiate(const NegotiatePB& request)
{
     }
   }
 
-  set<string> server_mechs = helper_.EnabledMechs();
+  set<SaslMechanism::Type> server_mechs = helper_.EnabledMechs();
   if (PREDICT_FALSE(server_mechs.empty())) {
     // This will happen if no mechanisms are enabled before calling Init()
     Status s = Status::NotAuthorized("SASL server mechanism list is empty!");
@@ -340,12 +346,12 @@ Status ServerNegotiation::HandleNegotiate(const NegotiatePB& request)
{
   return Status::OK();
 }
 
-Status ServerNegotiation::SendNegotiate(const set<string>& server_mechs) {
+Status ServerNegotiation::SendNegotiate(const set<SaslMechanism::Type>& server_mechs)
{
   NegotiatePB response;
   response.set_step(NegotiatePB::NEGOTIATE);
 
-  for (const string& mech : server_mechs) {
-    response.add_auths()->set_mechanism(mech);
+  for (auto mech : server_mechs) {
+    response.add_auths()->set_mechanism(SaslMechanism::name_of(mech));
   }
 
   // Tell the client which features we support.

http://git-wip-us.apache.org/repos/asf/kudu/blob/4ae35c29/src/kudu/rpc/server_negotiation.h
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/server_negotiation.h b/src/kudu/rpc/server_negotiation.h
index fa6adeb..dca3fa0 100644
--- a/src/kudu/rpc/server_negotiation.h
+++ b/src/kudu/rpc/server_negotiation.h
@@ -153,7 +153,7 @@ class ServerNegotiation {
   Status HandleNegotiate(const NegotiatePB& request) WARN_UNUSED_RESULT;
 
   // Send a NEGOTIATE response to the client with the list of available mechanisms.
-  Status SendNegotiate(const std::set<std::string>& server_mechs) WARN_UNUSED_RESULT;
+  Status SendNegotiate(const std::set<SaslMechanism::Type>& server_mechs) WARN_UNUSED_RESULT;
 
   // Handle a TLS_HANDSHAKE request message from the server.
   Status HandleTlsHandshake(const NegotiatePB& request) WARN_UNUSED_RESULT;

http://git-wip-us.apache.org/repos/asf/kudu/blob/4ae35c29/src/kudu/security/tls_handshake.h
----------------------------------------------------------------------
diff --git a/src/kudu/security/tls_handshake.h b/src/kudu/security/tls_handshake.h
index 2eb3781..4937578 100644
--- a/src/kudu/security/tls_handshake.h
+++ b/src/kudu/security/tls_handshake.h
@@ -72,7 +72,7 @@ class TlsHandshake {
   // Set the verification mode for this handshake. The default verification mode
   // is VERIFY_REMOTE_CERT_AND_HOST.
   //
-  // This must be not be called after the first call to Continue()
+  // This must be called before the first call to Continue().
   void set_verification_mode(TlsVerificationMode mode) {
     DCHECK(!has_started_);
     verification_mode_ = mode;


Mime
View raw message