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 <[email protected]> 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 <[email protected]> Authored: Fri Jan 27 14:15:12 2017 -0800 Committer: Dan Burkert <[email protected]> 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);
