kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From danburk...@apache.org
Subject kudu git commit: [security] add channel binding to krpc
Date Fri, 03 Feb 2017 19:13:07 GMT
Repository: kudu
Updated Branches:
  refs/heads/master 848180654 -> 6fec9dd52


[security] add channel binding to krpc

Channel binding prevents a MITM attack when using unauthenticated TLS
with Kerberos. The channel binding codepath is exercised by the existing
TLS + GSSAPI negotiation test, but I'm punting on testing that it
protects against a MITM for now.

Change-Id: Id73fceebfcb47c881c30f6904cfd6fc6d80f50b8
Reviewed-on: http://gerrit.cloudera.org:8080/5884
Reviewed-by: Todd Lipcon <todd@apache.org>
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/6fec9dd5
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/6fec9dd5
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/6fec9dd5

Branch: refs/heads/master
Commit: 6fec9dd52ecfd0248a46c5c5d6987e2ac2e0a4cf
Parents: 8481806
Author: Dan Burkert <danburkert@apache.org>
Authored: Fri Jan 27 14:15:12 2017 -0800
Committer: Dan Burkert <danburkert@apache.org>
Committed: Fri Feb 3 19:12:54 2017 +0000

----------------------------------------------------------------------
 docs/design-docs/rpc.md                 | 53 ++++++++++++++++++
 src/kudu/rpc/client_negotiation.cc      | 71 ++++++++++++++++++++++++
 src/kudu/rpc/client_negotiation.h       |  7 +++
 src/kudu/rpc/rpc_header.proto           | 20 ++++---
 src/kudu/rpc/sasl_common.cc             | 20 ++++++-
 src/kudu/rpc/sasl_common.h              | 11 ++--
 src/kudu/rpc/server_negotiation.cc      | 59 +++++++++++++++++---
 src/kudu/rpc/server_negotiation.h       |  8 ++-
 src/kudu/security/ca/cert_management.cc |  9 ----
 src/kudu/security/cert.cc               | 80 ++++++++++++++++++++++------
 src/kudu/security/cert.h                | 12 +++--
 src/kudu/security/openssl_util.cc       |  2 -
 src/kudu/security/openssl_util.h        | 29 ++++++++--
 src/kudu/security/tls_handshake.cc      |  5 --
 src/kudu/security/tls_socket.cc         | 26 +++++++++
 src/kudu/security/tls_socket.h          | 20 +++++--
 src/kudu/util/status.cc                 |  2 +-
 17 files changed, 369 insertions(+), 65 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/6fec9dd5/docs/design-docs/rpc.md
----------------------------------------------------------------------
diff --git a/docs/design-docs/rpc.md b/docs/design-docs/rpc.md
index 8b4fdeb..6c2c598 100644
--- a/docs/design-docs/rpc.md
+++ b/docs/design-docs/rpc.md
@@ -487,6 +487,59 @@ Client                                                              
     Server
    |                                                +---------------------+ |
 ```
 
+### Authentication
+
+When security is enabled, the negotiation process is responsible for mutually
+authenticating the client and server to each other. There are three distinct
+methods of mutual authentication, one of which will be chosen during
+negotiation. Which method is chosen for a particular connection depends on the
+configuration of the client and server. All authentication methods require the
+connection to be protected with TLS encryption.
+
+| server authentication of client | client authentication of server | notes |
+|---|---|---|
+| Kerberos | Kerberos | Kerberos provides strong mutual authentication, channel binding ties
the Kerberos authentication to the TLS channel |
+| Certificate | Certificate | client and server authenticate via certs signed by mutually
trusted CA |
+| Token | Certificate | client authenticates by passing a token signed by a key which is
trusted by the server |
+
+#### Kerberos Authentication
+
+Kerberos authentication requires the client and server to be configured with
+Kerberos principal credentials. Typically the client must have an active TGT
+acquired from the `kinit` command, and the server must be configured with a
+keytab. Kerberos authentication is handled through the SASL `GSSAPI` mechanism.
+When using Kerberos authentication TLS certificates are not verified.
+
+When the SASL `GSSAPI` negotiation is complete, the server returns a special
+channel binding token to the client as part of the `SASL_SUCCESS` message. The
+channel binding token contains a hash of the server's certificate, wrapped in a
+SASL integrity protected envelope. The client must check the channel binding
+token against the certificate presented by the server during the TLS handshake
+before continuing to use the connection. See RFC 5056 for more information on
+channel binding and why it is necessary, and RFC 5929 for a description of the
+specific 'tls-server-end-point' channel binding type used.
+
+#### Certificate Authentication
+
+When the client and server are configured with certificates signed by a mutually
+trusted certificate authority (CA), certificate authentication can be used
+to authenticate the client and server.
+
+TODO(dan): explain how the two sides decide on certificate authentication.
+           Probably via a special SASL mechanism (`KUDU_CERTIFICATE`) which
+           short-circuits any actual SASL messages.
+
+#### Token Authentication
+
+Token authentication requires the server to be configured with a certificate,
+and the client to be configured with a token. The server's certificate must be
+signed by a CA trusted by the client, and the client's token must be signed by a
+token-signing-key which is trusted by the server.
+
+TODO(dan): explain how the two sides decide on token authentication.
+           Probably via a special SASL mechanism (`KUDU_TOKEN`) which sends a
+           single round of SASL messages containing the token and a reply ack.
+
 ## Connection Context
 
 Once the SASL negotiation is complete, before the first request, the client

http://git-wip-us.apache.org/repos/asf/kudu/blob/6fec9dd5/src/kudu/rpc/client_negotiation.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/client_negotiation.cc b/src/kudu/rpc/client_negotiation.cc
index 6796d60..3d32e14 100644
--- a/src/kudu/rpc/client_negotiation.cc
+++ b/src/kudu/rpc/client_negotiation.cc
@@ -19,6 +19,7 @@
 
 #include <string.h>
 
+#include <algorithm>
 #include <map>
 #include <memory>
 #include <set>
@@ -27,6 +28,7 @@
 #include <glog/logging.h>
 #include <sasl/sasl.h>
 
+#include "kudu/gutil/casts.h"
 #include "kudu/gutil/endian.h"
 #include "kudu/gutil/map-util.h"
 #include "kudu/gutil/stl_util.h"
@@ -39,8 +41,10 @@
 #include "kudu/rpc/sasl_common.h"
 #include "kudu/rpc/sasl_helper.h"
 #include "kudu/rpc/serialization.h"
+#include "kudu/security/cert.h"
 #include "kudu/security/tls_context.h"
 #include "kudu/security/tls_handshake.h"
+#include "kudu/security/tls_socket.h"
 #include "kudu/util/faststring.h"
 #include "kudu/util/net/sockaddr.h"
 #include "kudu/util/net/socket.h"
@@ -100,6 +104,7 @@ ClientNegotiation::ClientNegotiation(unique_ptr<Socket> socket)
     : socket_(std::move(socket)),
       helper_(SaslHelper::CLIENT),
       tls_context_(nullptr),
+      tls_negotiated_(false),
       negotiated_mech_(SaslMechanism::INVALID),
       deadline_(MonoTime::Max()) {
   callbacks_.push_back(SaslBuildCallback(SASL_CB_GETOPT,
@@ -191,6 +196,7 @@ Status ClientNegotiation::Negotiate() {
       s = HandleTlsHandshake(response);
     }
     RETURN_NOT_OK(s);
+    tls_negotiated_ = true;
   }
 
   // Step 4: SASL negotiation.
@@ -207,6 +213,7 @@ Status ClientNegotiation::Negotiate() {
         break;
       // SASL_SUCCESS: Server has accepted our authentication request. Negotiation successful.
       case NegotiatePB::SASL_SUCCESS:
+        RETURN_NOT_OK(HandleSaslSuccess(response));
         cont = false;
         break;
       default:
@@ -451,6 +458,13 @@ Status ClientNegotiation::SendSaslInitiate() {
   // Check that the SASL library is using the mechanism that we picked.
   DCHECK_EQ(SaslMechanism::value_of(negotiated_mech), negotiated_mech_);
 
+  // If we are speaking TLS and the negotiated mechanism is GSSAPI (Kerberos),
+  // configure SASL to use integrity protection so that the channel bindings
+  // can be verified.
+  if (tls_negotiated_ && negotiated_mech_ == SaslMechanism::GSSAPI) {
+    RETURN_NOT_OK(EnableIntegrityProtection(sasl_conn_.get()));
+  }
+
   NegotiatePB msg;
   msg.set_step(NegotiatePB::SASL_INITIATE);
   msg.mutable_token()->assign(init_msg, init_msg_len);
@@ -481,6 +495,42 @@ Status ClientNegotiation::HandleSaslChallenge(const NegotiatePB&
response) {
   return SendSaslResponse(out, out_len);
 }
 
+Status ClientNegotiation::HandleSaslSuccess(const NegotiatePB& response) {
+  TRACE("Received SASL_SUCCESS response from server");
+
+  if (tls_negotiated_ && negotiated_mech_ == SaslMechanism::Type::GSSAPI) {
+    // Check the channel bindings provided by the server against the expected
+    // channel bindings.
+    security::TlsSocket* tls_socket = down_cast<security::TlsSocket*>(socket_.get());
+    security::Cert cert;
+    RETURN_NOT_OK(tls_socket->GetRemoteCert(&cert));
+
+    string expected_channel_bindings;
+    RETURN_NOT_OK_PREPEND(cert.GetServerEndPointChannelBindings(&expected_channel_bindings),
+                          "failed to generate expected channel bindings");
+
+    if (!response.has_channel_bindings()) {
+      return Status::NotAuthorized("no channel bindings provided by server");
+    }
+
+    string recieved_channel_bindings;
+    RETURN_NOT_OK_PREPEND(SaslDecode(response.channel_bindings(), &recieved_channel_bindings),
+                          "failed to decode channel bindings");
+
+    if (expected_channel_bindings != recieved_channel_bindings) {
+      Sockaddr addr;
+      ignore_result(socket_->GetPeerAddress(&addr));
+
+      LOG(WARNING) << "Recieved unexpected channel bindings from server "
+                   << addr.ToString()
+                   << ", this could indicate an active network man-in-the-middle";
+      return Status::NotAuthorized("channel bindings do not match");
+    }
+  }
+
+  return Status::OK();
+}
+
 Status ClientNegotiation::DoSaslStep(const string& in, const char** out, unsigned* out_len)
{
   TRACE("Calling sasl_client_step()");
 
@@ -489,6 +539,27 @@ Status ClientNegotiation::DoSaslStep(const string& in, const char**
out, unsigne
   });
 }
 
+Status ClientNegotiation::SaslDecode(const string& encoded, string* plaintext) {
+  size_t offset = 0;
+  const char* out;
+  unsigned out_len;
+
+  // The SASL library can only decode up to a maximum amount at a time, so we
+  // have to call decode multiple times if our input is larger than this max.
+  while (offset < encoded.size()) {
+    size_t len = std::min(kSaslMaxOutBufLen, encoded.size() - offset);
+
+    RETURN_NOT_OK(WrapSaslCall(sasl_conn_.get(), [&]() {
+        return sasl_decode(sasl_conn_.get(), encoded.data() + offset, len, &out, &out_len);
+    }));
+
+    plaintext->append(out, out_len);
+    offset += len;
+  }
+
+  return Status::OK();
+}
+
 Status ClientNegotiation::SendConnectionContext() {
   TRACE("Sending connection context");
   RequestHeader header;

http://git-wip-us.apache.org/repos/asf/kudu/blob/6fec9dd5/src/kudu/rpc/client_negotiation.h
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/client_negotiation.h b/src/kudu/rpc/client_negotiation.h
index 5563ff6..0e14ee9 100644
--- a/src/kudu/rpc/client_negotiation.h
+++ b/src/kudu/rpc/client_negotiation.h
@@ -164,6 +164,9 @@ class ClientNegotiation {
   // Handle case when server sends SASL_CHALLENGE response.
   Status HandleSaslChallenge(const NegotiatePB& response) WARN_UNUSED_RESULT;
 
+  // Handle case when server sends SASL_SUCCESS response.
+  Status HandleSaslSuccess(const NegotiatePB& response) WARN_UNUSED_RESULT;
+
   // Perform a client-side step of the SASL negotiation.
   // Input is what came from the server. Output is what we will send back to the server.
   // Returns:
@@ -172,6 +175,9 @@ class ClientNegotiation {
   // otherwise returns an appropriate error status.
   Status DoSaslStep(const std::string& in, const char** out, unsigned* out_len) WARN_UNUSED_RESULT;
 
+  // Decode the provided SASL-encoded data and append it to 'plaintext'.
+  Status SaslDecode(const std::string& encoded, std::string* plaintext) WARN_UNUSED_RESULT;
+
   Status SendConnectionContext() WARN_UNUSED_RESULT;
 
   // The socket to the remote server.
@@ -185,6 +191,7 @@ class ClientNegotiation {
   // TLS state.
   const security::TlsContext* tls_context_;
   security::TlsHandshake tls_handshake_;
+  bool tls_negotiated_;
 
   // Authentication state.
   std::string plain_auth_user_;

http://git-wip-us.apache.org/repos/asf/kudu/blob/6fec9dd5/src/kudu/rpc/rpc_header.proto
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/rpc_header.proto b/src/kudu/rpc/rpc_header.proto
index 3683a23..5bc628d 100644
--- a/src/kudu/rpc/rpc_header.proto
+++ b/src/kudu/rpc/rpc_header.proto
@@ -37,13 +37,11 @@ message UserInformationPB {
   required string real_user = 2;
 }
 
-/**
- * The connection context is sent as part of the connection establishment.
- * It establishes the context for ALL RPC calls within the connection.
- * This is sent on connection setup after the connection preamble is sent
- * and SASL has been negotiated.
- * No response is sent from the server to the client.
- */
+// The connection context is sent as part of the connection establishment.
+// It establishes the context for ALL RPC calls within the connection.
+// This is sent on connection setup after the connection preamble is sent
+// and SASL has been negotiated.
+// No response is sent from the server to the client.
 message ConnectionContextPB {
   // UserInfo beyond what is determined as part of security handshake
   // at connection time (kerberos, tokens etc).
@@ -119,6 +117,14 @@ message NegotiatePB {
   // During the TLS_HANDSHAKE step, contains the TLS handshake message.
   optional bytes tls_handshake = 5 [(REDACT) = true];
 
+  // The tls-server-end-point channel bindings as specified in RFC 5929.  Sent
+  // from the server to the client during the SASL_SUCCESS step when the
+  // Kerberos (GSSAPI) SASL mechanism is used with TLS, in order to bind the
+  // Kerberos authenticated channel to the TLS channel. The value is integrity
+  // protected through SASL. The client is responsible for validating that the
+  // value matches the expected value.
+  optional bytes channel_bindings = 6 [(REDACT) = true];
+
   // During the NEGOTIATE step, contains the supported SASL mechanisms.
   // During the SASL_INITIATE step, contains the single chosen SASL mechanism.
   repeated SaslAuth auths      = 4;

http://git-wip-us.apache.org/repos/asf/kudu/blob/6fec9dd5/src/kudu/rpc/sasl_common.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/sasl_common.cc b/src/kudu/rpc/sasl_common.cc
index 8027721..6b60839 100644
--- a/src/kudu/rpc/sasl_common.cc
+++ b/src/kudu/rpc/sasl_common.cc
@@ -17,10 +17,13 @@
 
 #include "kudu/rpc/sasl_common.h"
 
-#include <boost/algorithm/string/predicate.hpp>
+#include <string.h>
+
+#include <limits>
 #include <mutex>
 #include <string>
 
+#include <boost/algorithm/string/predicate.hpp>
 #include <gflags/gflags.h>
 #include <glog/logging.h>
 #include <regex.h>
@@ -42,6 +45,7 @@ namespace rpc {
 
 const char* const kSaslMechPlain = "PLAIN";
 const char* const kSaslMechGSSAPI = "GSSAPI";
+extern const size_t kSaslMaxOutBufLen = 1024;
 
 // See WrapSaslCall().
 static __thread string* g_auth_failure_capture = nullptr;
@@ -318,6 +322,7 @@ Status WrapSaslCall(sasl_conn_t* conn, const std::function<int()>&
call) {
       return Status::Incomplete("");
     case SASL_FAIL:      // Generic failure (encompasses missing krb5 credentials).
     case SASL_BADAUTH:   // Authentication failure.
+    case SASL_BADMAC:    // Decode failure.
     case SASL_NOAUTHZ:   // Authorization failure.
     case SASL_NOUSER:    // User not found.
     case SASL_WRONGMECH: // Server doesn't support requested mechanism.
@@ -366,6 +371,19 @@ sasl_callback_t SaslBuildCallback(int id, int (*proc)(void), void* context)
{
   return callback;
 }
 
+Status EnableIntegrityProtection(sasl_conn_t* sasl_conn) {
+  sasl_security_properties_t sec_props;
+  memset(&sec_props, 0, sizeof(sec_props));
+  sec_props.min_ssf = 1;
+  sec_props.max_ssf = std::numeric_limits<sasl_ssf_t>::max();
+  sec_props.maxbufsize = kSaslMaxOutBufLen;
+
+  RETURN_NOT_OK_PREPEND(WrapSaslCall(sasl_conn, [&] () {
+    return sasl_setprop(sasl_conn, SASL_SEC_PROPS, &sec_props);
+  }), "failed to set SASL security properties");
+  return Status::OK();
+}
+
 SaslMechanism::Type SaslMechanism::value_of(const string& mech) {
   if (boost::iequals(mech, "PLAIN")) {
     return PLAIN;

http://git-wip-us.apache.org/repos/asf/kudu/blob/6fec9dd5/src/kudu/rpc/sasl_common.h
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/sasl_common.h b/src/kudu/rpc/sasl_common.h
index d56f7ec..3d7e07f 100644
--- a/src/kudu/rpc/sasl_common.h
+++ b/src/kudu/rpc/sasl_common.h
@@ -38,6 +38,7 @@ using std::string;
 // Constants
 extern const char* const kSaslMechPlain;
 extern const char* const kSaslMechGSSAPI;
+extern const size_t kSaslMaxOutBufLen;
 
 struct SaslMechanism {
   enum Type {
@@ -63,10 +64,10 @@ struct SaslMechanism {
 //
 // This function is thread safe and uses a static lock.
 // This function should NOT be called during static initialization.
-Status SaslInit();
+Status SaslInit() WARN_UNUSED_RESULT;
 
 // Disable Kudu's initialization of SASL. See equivalent method in client.h.
-Status DisableSaslInitialization();
+Status DisableSaslInitialization() WARN_UNUSED_RESULT;
 
 // Wrap a call into the SASL library. 'call' should be a lambda which
 // returns a SASL error code.
@@ -79,7 +80,7 @@ Status DisableSaslInitialization();
 //
 // The Status message is beautified to be more user-friendly compared
 // to the underlying sasl_errdetails() call.
-Status WrapSaslCall(sasl_conn_t* conn, const std::function<int()>& call);
+Status WrapSaslCall(sasl_conn_t* conn, const std::function<int()>& call) WARN_UNUSED_RESULT;
 
 // Return <ip>;<port> string formatted for SASL library use.
 string SaslIpPortString(const Sockaddr& addr);
@@ -93,6 +94,10 @@ std::set<SaslMechanism::Type> SaslListAvailableMechs();
 // context: An object to pass to the callback as the context pointer, or NULL.
 sasl_callback_t SaslBuildCallback(int id, int (*proc)(void), void* context);
 
+// Require integrity protection on the SASL connection. Should be called before
+// initiating the SASL negotiation.
+Status EnableIntegrityProtection(sasl_conn_t* sasl_conn) WARN_UNUSED_RESULT;
+
 // Deleter for sasl_conn_t instances, for use with gscoped_ptr after calling sasl_*_new()
 struct SaslDeleter {
   inline void operator()(sasl_conn_t* conn) {

http://git-wip-us.apache.org/repos/asf/kudu/blob/6fec9dd5/src/kudu/rpc/server_negotiation.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/server_negotiation.cc b/src/kudu/rpc/server_negotiation.cc
index b1f2893..6f5c118 100644
--- a/src/kudu/rpc/server_negotiation.cc
+++ b/src/kudu/rpc/server_negotiation.cc
@@ -17,6 +17,7 @@
 
 #include "kudu/rpc/server_negotiation.h"
 
+#include <algorithm>
 #include <limits>
 #include <memory>
 #include <set>
@@ -26,14 +27,17 @@
 #include <google/protobuf/message_lite.h>
 #include <sasl/sasl.h>
 
+#include "kudu/gutil/casts.h"
 #include "kudu/gutil/endian.h"
 #include "kudu/gutil/map-util.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/rpc/blocking_ops.h"
 #include "kudu/rpc/constants.h"
 #include "kudu/rpc/serialization.h"
+#include "kudu/security/cert.h"
 #include "kudu/security/tls_context.h"
 #include "kudu/security/tls_handshake.h"
+#include "kudu/security/tls_socket.h"
 #include "kudu/util/net/sockaddr.h"
 #include "kudu/util/net/socket.h"
 #include "kudu/util/scoped_cleanup.h"
@@ -67,6 +71,7 @@ ServerNegotiation::ServerNegotiation(unique_ptr<Socket> socket)
     : socket_(std::move(socket)),
       helper_(SaslHelper::SERVER),
       tls_context_(nullptr),
+      tls_negotiated_(false),
       negotiated_mech_(SaslMechanism::INVALID),
       deadline_(MonoTime::Max()) {
   callbacks_.push_back(SaslBuildCallback(SASL_CB_GETOPT,
@@ -148,6 +153,7 @@ Status ServerNegotiation::Negotiate() {
       if (s.ok()) break;
       if (!s.IsIncomplete()) return s;
     }
+    tls_negotiated_ = true;
   }
 
   // Step 4: SASL negotiation.
@@ -426,6 +432,15 @@ Status ServerNegotiation::HandleSaslInitiate(const NegotiatePB& request)
{
   const NegotiatePB::SaslAuth& auth = request.auths(0);
   TRACE("Client requested to use mechanism: $0", auth.mechanism());
 
+  negotiated_mech_ = SaslMechanism::value_of(auth.mechanism());
+
+  // If we are speaking TLS and the negotiated mechanism is GSSAPI (Kerberos),
+  // configure SASL to use integrity protection so that the channel bindings
+  // can be verified.
+  if (tls_negotiated_ && negotiated_mech_ == SaslMechanism::GSSAPI) {
+    RETURN_NOT_OK(EnableIntegrityProtection(sasl_conn_.get()));
+  }
+
   const char* server_out = nullptr;
   uint32_t server_out_len = 0;
   TRACE("Calling sasl_server_start()");
@@ -445,17 +460,37 @@ Status ServerNegotiation::HandleSaslInitiate(const NegotiatePB&
request) {
     return s;
   }
 
-  negotiated_mech_ = SaslMechanism::value_of(auth.mechanism());
-
   // We have a valid mechanism match
   if (s.ok()) {
-    RETURN_NOT_OK(SendSaslSuccess(server_out, server_out_len));
+    DCHECK(server_out_len == 0);
+    RETURN_NOT_OK(SendSaslSuccess());
   } else { // s.IsIncomplete() (equivalent to SASL_CONTINUE)
     RETURN_NOT_OK(SendSaslChallenge(server_out, server_out_len));
   }
   return s;
 }
 
+Status ServerNegotiation::SaslEncode(const std::string& plaintext, std::string* encoded)
{
+  size_t offset = 0;
+  const char* out;
+  unsigned out_len;
+
+  // The SASL library can only encode up to a maximum amount at a time, so we
+  // have to call encode multiple times if our input is larger than this max.
+  while (offset < plaintext.size()) {
+    size_t len = std::min(kSaslMaxOutBufLen, plaintext.size() - offset);
+
+    RETURN_NOT_OK(WrapSaslCall(sasl_conn_.get(), [&]() {
+        return sasl_encode(sasl_conn_.get(), plaintext.data() + offset, len, &out, &out_len);
+    }));
+
+    encoded->append(out, out_len);
+    offset += len;
+  }
+
+  return Status::OK();
+}
+
 Status ServerNegotiation::HandleSaslResponse(const NegotiatePB& request) {
   if (PREDICT_FALSE(request.step() != NegotiatePB::SASL_RESPONSE)) {
     Status s =  Status::NotAuthorized("expected SASL_RESPONSE step",
@@ -484,7 +519,8 @@ Status ServerNegotiation::HandleSaslResponse(const NegotiatePB& request)
{
     });
 
   if (s.ok()) {
-    return SendSaslSuccess(server_out, server_out_len);
+    DCHECK(server_out_len == 0);
+    return SendSaslSuccess();
   }
   if (s.IsIncomplete()) {
     return SendSaslChallenge(server_out, server_out_len);
@@ -501,12 +537,21 @@ Status ServerNegotiation::SendSaslChallenge(const char* challenge, unsigned
clen
   return Status::Incomplete("");
 }
 
-Status ServerNegotiation::SendSaslSuccess(const char* token, unsigned tlen) {
+Status ServerNegotiation::SendSaslSuccess() {
   NegotiatePB response;
   response.set_step(NegotiatePB::SASL_SUCCESS);
-  if (PREDICT_FALSE(tlen > 0)) {
-    response.mutable_token()->assign(token, tlen);
+
+  if (tls_negotiated_ && negotiated_mech_ == SaslMechanism::Type::GSSAPI) {
+    // Send the channel bindings to the client.
+    security::TlsSocket* tls_socket = down_cast<security::TlsSocket*>(socket_.get());
+    security::Cert cert;
+    RETURN_NOT_OK(tls_socket->GetLocalCert(&cert));
+
+    string plaintext_channel_bindings;
+    RETURN_NOT_OK(cert.GetServerEndPointChannelBindings(&plaintext_channel_bindings));
+    RETURN_NOT_OK(SaslEncode(plaintext_channel_bindings, response.mutable_channel_bindings()));
   }
+
   RETURN_NOT_OK(SendNegotiatePB(response));
   return Status::OK();
 }

http://git-wip-us.apache.org/repos/asf/kudu/blob/6fec9dd5/src/kudu/rpc/server_negotiation.h
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/server_negotiation.h b/src/kudu/rpc/server_negotiation.h
index dca3fa0..88c24d9 100644
--- a/src/kudu/rpc/server_negotiation.h
+++ b/src/kudu/rpc/server_negotiation.h
@@ -172,8 +172,11 @@ class ServerNegotiation {
   // Send a SASL_CHALLENGE response to the client with a challenge token.
   Status SendSaslChallenge(const char* challenge, unsigned clen) WARN_UNUSED_RESULT;
 
-  // Send a SASL_SUCCESS response to the client with an token (typically empty).
-  Status SendSaslSuccess(const char* token, unsigned tlen) WARN_UNUSED_RESULT;
+  // Send a SASL_SUCCESS response to the client.
+  Status SendSaslSuccess() WARN_UNUSED_RESULT;
+
+  // Encode the provided data and append it to 'encoded'.
+  Status SaslEncode(const std::string& plaintext, std::string* encoded) WARN_UNUSED_RESULT;
 
   // Receive and validate the ConnectionContextPB.
   Status RecvConnectionContext(faststring* recv_buf) WARN_UNUSED_RESULT;
@@ -189,6 +192,7 @@ class ServerNegotiation {
   // TLS state.
   const security::TlsContext* tls_context_;
   security::TlsHandshake tls_handshake_;
+  bool tls_negotiated_;
 
   // The set of features supported by the client. Filled in during negotiation.
   std::set<RpcFeatureFlag> client_features_;

http://git-wip-us.apache.org/repos/asf/kudu/blob/6fec9dd5/src/kudu/security/ca/cert_management.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/ca/cert_management.cc b/src/kudu/security/ca/cert_management.cc
index a871b75..9f0dde2 100644
--- a/src/kudu/security/ca/cert_management.cc
+++ b/src/kudu/security/ca/cert_management.cc
@@ -56,15 +56,6 @@ template<> struct SslTypeTraits<ASN1_INTEGER> {
 template<> struct SslTypeTraits<BIGNUM> {
   static constexpr auto free = &BN_free;
 };
-template<> struct SslTypeTraits<X509> {
-  static constexpr auto free = &X509_free;
-};
-template<> struct SslTypeTraits<X509_REQ> {
-  static constexpr auto free = &X509_REQ_free;
-};
-template<> struct SslTypeTraits<X509_EXTENSION> {
-  static constexpr auto free = &X509_EXTENSION_free;
-};
 template<> struct SslTypeTraits<EVP_PKEY> {
   static constexpr auto free = &EVP_PKEY_free;
 };

http://git-wip-us.apache.org/repos/asf/kudu/blob/6fec9dd5/src/kudu/security/cert.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/cert.cc b/src/kudu/security/cert.cc
index f04577f..fa373da 100644
--- a/src/kudu/security/cert.cc
+++ b/src/kudu/security/cert.cc
@@ -19,6 +19,7 @@
 
 #include <string>
 
+#include <openssl/evp.h>
 #include <openssl/pem.h>
 #include <openssl/x509.h>
 
@@ -31,21 +32,6 @@ using std::string;
 namespace kudu {
 namespace security {
 
-template<> struct SslTypeTraits<X509> {
-  static constexpr auto free = &X509_free;
-  static constexpr auto read_pem = &PEM_read_bio_X509;
-  static constexpr auto read_der = &d2i_X509_bio;
-  static constexpr auto write_pem = &PEM_write_bio_X509;
-  static constexpr auto write_der = &i2d_X509_bio;
-};
-template<> struct SslTypeTraits<X509_REQ> {
-  static constexpr auto free = &X509_REQ_free;
-  static constexpr auto read_pem = &PEM_read_bio_X509_REQ;
-  static constexpr auto read_der = &d2i_X509_REQ_bio;
-  static constexpr auto write_pem = &PEM_write_bio_X509_REQ;
-  static constexpr auto write_der = &i2d_X509_REQ_bio;
-};
-
 string X509NameToString(X509_NAME* name) {
   CHECK(name);
   auto bio = ssl_make_unique(BIO_new(BIO_s_mem()));
@@ -76,6 +62,70 @@ string Cert::IssuerName() const {
   return X509NameToString(X509_get_issuer_name(data_.get()));
 }
 
+Status Cert::GetServerEndPointChannelBindings(string* channel_bindings) const {
+  // Find the signature type of the certificate. This corresponds to the digest
+  // (hash) algorithm, and the public key type which signed the cert.
+
+#if OPENSSL_VERSION_NUMBER >= 0x10002000L
+  int signature_nid = X509_get_signature_nid(data_.get());
+#else
+  // Older version of OpenSSL appear not to have a public way to get the
+  // signature digest method from a certificate. Instead, we reach into the
+  // 'private' internals.
+  int signature_nid = OBJ_obj2nid(data_->sig_alg->algorithm);
+#endif
+
+  // Retrieve the digest algorithm type.
+  int digest_nid;
+  OBJ_find_sigid_algs(signature_nid, &digest_nid, nullptr /* public_key_nid */);
+
+  // RFC 5929: if the certificate's signatureAlgorithm uses no hash functions or
+  // uses multiple hash functions, then this channel binding type's channel
+  // bindings are undefined at this time (updates to is channel binding type may
+  // occur to address this issue if it ever arises).
+  //
+  // TODO(dan): can the multiple hash function scenario actually happen? What
+  // does OBJ_find_sigid_algs do in that scenario?
+  if (digest_nid == NID_undef) {
+    return Status::NotSupported("server certificate has no signature digest (hash) algorithm");
+  }
+
+  // RFC 5929: if the certificate's signatureAlgorithm uses a single hash
+  // function, and that hash function is either MD5 [RFC1321] or SHA-1
+  // [RFC3174], then use SHA-256 [FIPS-180-3];
+  if (digest_nid == NID_md5 || digest_nid == NID_sha1) {
+    digest_nid = NID_sha256;
+  }
+
+  const EVP_MD* md = EVP_get_digestbynid(digest_nid);
+  OPENSSL_RET_IF_NULL(md, "digest for nid not found");
+
+  // Create a digest BIO. All data written to the BIO will be sent through the
+  // digest (hash) function. The digest BIO requires a null BIO to writethrough to.
+  auto null_bio = ssl_make_unique(BIO_new(BIO_s_null()));
+  auto md_bio = ssl_make_unique(BIO_new(BIO_f_md()));
+  OPENSSL_RET_NOT_OK(BIO_set_md(md_bio.get(), md), "failed to set digest for BIO");
+  BIO_push(md_bio.get(), null_bio.get());
+
+  // Write the cert to the digest BIO.
+  RETURN_NOT_OK(ToBIO(md_bio.get(), DataFormat::DER, data_.get()));
+
+  // Read the digest from the BIO and append it to 'channel_bindings'.
+  char buf[EVP_MAX_MD_SIZE];
+  int digest_len = BIO_gets(md_bio.get(), buf, sizeof(buf));
+  OPENSSL_RET_NOT_OK(digest_len, "failed to get cert digest from BIO");
+  channel_bindings->assign(buf, digest_len);
+  return Status::OK();
+}
+
+void Cert::AdoptAndAddRefRawData(X509* data) {
+#if OPENSSL_VERSION_NUMBER < 0x10100000L
+  CHECK_GT(CRYPTO_add(&data->references, 1, CRYPTO_LOCK_X509), 1) << "X509 use-after-free
detected";
+#else
+  OPENSSL_CHECK_OK(X509_up_ref(data)) << "X509 use-after-free detected: " <<
GetOpenSSLErrors();
+#endif
+  AdoptRawData(data);
+}
 
 Status CertSignRequest::FromString(const std::string& data, DataFormat format) {
   return ::kudu::security::FromString(data, format, &data_);

http://git-wip-us.apache.org/repos/asf/kudu/blob/6fec9dd5/src/kudu/security/cert.h
----------------------------------------------------------------------
diff --git a/src/kudu/security/cert.h b/src/kudu/security/cert.h
index c21ae0b..e43127f 100644
--- a/src/kudu/security/cert.h
+++ b/src/kudu/security/cert.h
@@ -21,11 +21,6 @@
 
 #include "kudu/security/openssl_util.h"
 
-// Forward declarations for the OpenSSL typedefs.
-typedef struct x509_st X509;
-typedef struct X509_req_st X509_REQ;
-typedef struct X509_name_st X509_NAME;
-
 namespace kudu {
 
 class Status;
@@ -43,6 +38,13 @@ class Cert : public RawDataWrapper<X509> {
 
   std::string SubjectName() const;
   std::string IssuerName() const;
+
+  // Returns the 'tls-server-end-point' channel bindings for the certificate as
+  // specified in RFC 5929.
+  Status GetServerEndPointChannelBindings(std::string* channel_bindings) const;
+
+  // Adopts the provided X509 certificate, and increments the reference count.
+  void AdoptAndAddRefRawData(X509* data);
 };
 
 class CertSignRequest : public RawDataWrapper<X509_REQ> {

http://git-wip-us.apache.org/repos/asf/kudu/blob/6fec9dd5/src/kudu/security/openssl_util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/openssl_util.cc b/src/kudu/security/openssl_util.cc
index 9f552f8..ea4a259 100644
--- a/src/kudu/security/openssl_util.cc
+++ b/src/kudu/security/openssl_util.cc
@@ -28,7 +28,6 @@
 #include <openssl/rand.h>
 #include <openssl/ssl.h>
 
-#include "kudu/gutil/strings/substitute.h"
 #include "kudu/util/debug/leakcheck_disabler.h"
 #include "kudu/util/errno.h"
 #include "kudu/util/mutex.h"
@@ -37,7 +36,6 @@
 
 using std::ostringstream;
 using std::string;
-using strings::Substitute;
 
 namespace kudu {
 namespace security {

http://git-wip-us.apache.org/repos/asf/kudu/blob/6fec9dd5/src/kudu/security/openssl_util.h
----------------------------------------------------------------------
diff --git a/src/kudu/security/openssl_util.h b/src/kudu/security/openssl_util.h
index 1691727..35ff8d6 100644
--- a/src/kudu/security/openssl_util.h
+++ b/src/kudu/security/openssl_util.h
@@ -21,28 +21,31 @@
 #include <memory>
 #include <string>
 
+#include <openssl/pem.h>
+#include <openssl/x509.h>
+
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/util/status.h"
 
 // Forward declarations for the OpenSSL typedefs.
+typedef struct X509_req_st X509_REQ;
 typedef struct bio_st BIO;
 typedef struct evp_pkey_st EVP_PKEY;
 typedef struct ssl_ctx_st SSL_CTX;
 typedef struct ssl_st SSL;
+typedef struct x509_st X509;
 
 #define OPENSSL_CHECK_OK(call) \
   CHECK_GT((call), 0)
 
 #define OPENSSL_RET_NOT_OK(call, msg) \
   if ((call) <= 0) { \
-    return Status::RuntimeError(::strings::Substitute("$0: $1", \
-        (msg), GetOpenSSLErrors())); \
+    return Status::RuntimeError((msg), GetOpenSSLErrors()); \
   }
 
 #define OPENSSL_RET_IF_NULL(call, msg) \
   if ((call) == nullptr) { \
-    return Status::RuntimeError(::strings::Substitute("$0: $1", \
-        (msg), GetOpenSSLErrors())); \
+    return Status::RuntimeError((msg), GetOpenSSLErrors()); \
   }
 
 namespace kudu {
@@ -80,6 +83,24 @@ using c_unique_ptr = std::unique_ptr<T, std::function<void(T*)>>;
 template<typename SSL_TYPE>
 struct SslTypeTraits {};
 
+template<> struct SslTypeTraits<X509> {
+  static constexpr auto free = &X509_free;
+  static constexpr auto read_pem = &PEM_read_bio_X509;
+  static constexpr auto read_der = &d2i_X509_bio;
+  static constexpr auto write_pem = &PEM_write_bio_X509;
+  static constexpr auto write_der = &i2d_X509_bio;
+};
+template<> struct SslTypeTraits<X509_EXTENSION> {
+  static constexpr auto free = &X509_EXTENSION_free;
+};
+template<> struct SslTypeTraits<X509_REQ> {
+  static constexpr auto free = &X509_REQ_free;
+  static constexpr auto read_pem = &PEM_read_bio_X509_REQ;
+  static constexpr auto read_der = &d2i_X509_REQ_bio;
+  static constexpr auto write_pem = &PEM_write_bio_X509_REQ;
+  static constexpr auto write_der = &i2d_X509_REQ_bio;
+};
+
 template<typename SSL_TYPE, typename Traits = SslTypeTraits<SSL_TYPE>>
 c_unique_ptr<SSL_TYPE> ssl_make_unique(SSL_TYPE* d) {
   return {d, Traits::free};

http://git-wip-us.apache.org/repos/asf/kudu/blob/6fec9dd5/src/kudu/security/tls_handshake.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/tls_handshake.cc b/src/kudu/security/tls_handshake.cc
index 49f0252..522fee6 100644
--- a/src/kudu/security/tls_handshake.cc
+++ b/src/kudu/security/tls_handshake.cc
@@ -40,11 +40,6 @@ using std::unique_ptr;
 namespace kudu {
 namespace security {
 
-template<> struct SslTypeTraits<X509> {
-  static constexpr auto free = &X509_free;
-};
-
-
 void TlsHandshake::SetSSLVerify() {
   CHECK(ssl_);
   CHECK(!has_started_);

http://git-wip-us.apache.org/repos/asf/kudu/blob/6fec9dd5/src/kudu/security/tls_socket.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/tls_socket.cc b/src/kudu/security/tls_socket.cc
index 1eec3dd..27a6af6 100644
--- a/src/kudu/security/tls_socket.cc
+++ b/src/kudu/security/tls_socket.cc
@@ -19,8 +19,10 @@
 
 #include <openssl/err.h>
 #include <openssl/ssl.h>
+#include <openssl/x509.h>
 
 #include "kudu/gutil/basictypes.h"
+#include "kudu/security/cert.h"
 #include "kudu/security/openssl_util.h"
 
 namespace kudu {
@@ -35,6 +37,30 @@ TlsSocket::~TlsSocket() {
   ignore_result(Close());
 }
 
+Status TlsSocket::GetLocalCert(Cert* cert) const {
+  X509* raw_cert = SSL_get_certificate(ssl_.get());
+
+  // This can happen if the cert has not been set (e.g. this is a client->server
+  // socket with no cert), or a non-PKI cipher is being used.
+  OPENSSL_RET_IF_NULL(raw_cert, "TLS socket has no local certificate");
+
+  // For whatever reason, SSL_get_certificate (unlike SSL_get_peer_certificate)
+  // does not increment the X509's reference count.
+  cert->AdoptAndAddRefRawData(raw_cert);
+  return Status::OK();
+}
+
+Status TlsSocket::GetRemoteCert(Cert* cert) const {
+  X509* raw_cert = SSL_get_peer_certificate(ssl_.get());
+
+  // This can happen if the cert has not been set (e.g. this is a server->client
+  // socket with no verification), or a non-PKI cipher is being used.
+  OPENSSL_RET_IF_NULL(raw_cert, "TLS socket has no remote certificate");
+
+  cert->AdoptRawData(raw_cert);
+  return Status::OK();
+}
+
 Status TlsSocket::Write(const uint8_t *buf, int32_t amt, int32_t *nwritten) {
   CHECK(ssl_);
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/6fec9dd5/src/kudu/security/tls_socket.h
----------------------------------------------------------------------
diff --git a/src/kudu/security/tls_socket.h b/src/kudu/security/tls_socket.h
index 70da1e2..1d0b26f 100644
--- a/src/kudu/security/tls_socket.h
+++ b/src/kudu/security/tls_socket.h
@@ -27,18 +27,30 @@ typedef ssl_st SSL;
 namespace kudu {
 namespace security {
 
+class Cert;
+
 class TlsSocket : public Socket {
  public:
 
   ~TlsSocket() override;
 
-  Status Write(const uint8_t *buf, int32_t amt, int32_t *nwritten) override;
+  // Retrieve the local certificate. This will return an error status if there
+  // is no local certificate.
+  Status GetLocalCert(Cert* cert) const WARN_UNUSED_RESULT;
+
+  // Retrieve the remote peer's certificate. This will return an error status if
+  // there is no remote certificate.
+  Status GetRemoteCert(Cert* cert) const WARN_UNUSED_RESULT;
+
+  Status Write(const uint8_t *buf, int32_t amt, int32_t *nwritten) override WARN_UNUSED_RESULT;
 
-  Status Writev(const struct ::iovec *iov, int iov_len, int32_t *nwritten) override;
+  Status Writev(const struct ::iovec *iov,
+                int iov_len,
+                int32_t *nwritten) override WARN_UNUSED_RESULT;
 
-  Status Recv(uint8_t *buf, int32_t amt, int32_t *nread) override;
+  Status Recv(uint8_t *buf, int32_t amt, int32_t *nread) override WARN_UNUSED_RESULT;
 
-  Status Close() override;
+  Status Close() override WARN_UNUSED_RESULT;
 
  private:
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/6fec9dd5/src/kudu/util/status.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/status.cc b/src/kudu/util/status.cc
index d13eeea..9f88da1 100644
--- a/src/kudu/util/status.cc
+++ b/src/kudu/util/status.cc
@@ -22,7 +22,7 @@ const char* Status::CopyState(const char* state) {
 
 Status::Status(Code code, const Slice& msg, const Slice& msg2,
                int16_t posix_code) {
-  assert(code != kOk);
+  DCHECK(code != kOk);
   const uint32_t len1 = msg.size();
   const uint32_t len2 = msg2.size();
   const uint32_t size = len1 + (len2 ? (2 + len2) : 0);


Mime
View raw message