Repository: kudu Updated Branches: refs/heads/master 1db453f54 -> fec9b8879
sasl: don't use the 'client_mech_list' SASL option This option allows the client to specify which mechanisms it's willing to negotiate. It turns out the option was added in Cyrus SASL 2.1.25, which isn't available on el6. While that option is useful, it's not doing anything magic: it just filters the list of mechanisms that we pass from the server. This patch just implements this filtering on our side. This fixes the TestSaslRpc.TestNoMatchingMechanisms test case on el6. Change-Id: I570a91e176803fdd0e4324e6f4443b8297ad0395 Reviewed-on: http://gerrit.cloudera.org:8080/5039 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/ed01e91a Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/ed01e91a Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/ed01e91a Branch: refs/heads/master Commit: ed01e91aa3b3754984618f0e677a75fb640b6fa8 Parents: 1db453f Author: Todd Lipcon <[email protected]> Authored: Thu Nov 10 14:23:38 2016 -0800 Committer: Todd Lipcon <[email protected]> Committed: Fri Nov 11 02:06:14 2016 +0000 ---------------------------------------------------------------------- src/kudu/rpc/sasl_client.cc | 59 +++++++++++++++++++++------------------- src/kudu/rpc/sasl_helper.cc | 7 +---- 2 files changed, 32 insertions(+), 34 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/ed01e91a/src/kudu/rpc/sasl_client.cc ---------------------------------------------------------------------- diff --git a/src/kudu/rpc/sasl_client.cc b/src/kudu/rpc/sasl_client.cc index 99cb822..b8ea2be 100644 --- a/src/kudu/rpc/sasl_client.cc +++ b/src/kudu/rpc/sasl_client.cc @@ -28,7 +28,9 @@ #include "kudu/gutil/endian.h" #include "kudu/gutil/map-util.h" +#include "kudu/gutil/stl_util.h" #include "kudu/gutil/stringprintf.h" +#include "kudu/gutil/strings/join.h" #include "kudu/gutil/strings/substitute.h" #include "kudu/gutil/strings/util.h" #include "kudu/rpc/blocking_ops.h" @@ -317,8 +319,6 @@ Status SaslClient::DoSaslStep(const string& in, const char** out, unsigned* out_ Status SaslClient::HandleNegotiateResponse(const SaslMessagePB& response) { TRACE("SASL Client: Received NEGOTIATE response from server"); - map<string, SaslMessagePB::SaslAuth> mech_auth_map; - // Fill in the set of features supported by the server. for (int flag : response.supported_features()) { // We only add the features that our local build knows about. @@ -329,17 +329,30 @@ Status SaslClient::HandleNegotiateResponse(const SaslMessagePB& response) { } } - // Build the list of SASL mechanisms requested by the client, and a map - // back to to the SaslAuth PBs. - string mech_list; - mech_list.reserve(64); // Avoid resizing the buffer later. + // Build a map of the mechanisms offered by the server. + const set<string>& local_mechs = helper_.LocalMechs(); + set<string> server_mechs; + map<string, SaslMessagePB::SaslAuth> server_mech_map; for (const SaslMessagePB::SaslAuth& auth : response.auths()) { - if (mech_list.length() > 0) mech_list.append(" "); - string mech = auth.mechanism(); - mech_list.append(mech); - mech_auth_map[mech] = auth; + const auto& mech = auth.mechanism(); + 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. + set<string> matching_mechs = STLSetIntersection(local_mechs, server_mechs); + + if (matching_mechs.empty() && + ContainsKey(server_mechs, kSaslMechGSSAPI) && + !ContainsKey(local_mechs, kSaslMechGSSAPI)) { + return Status::NotAuthorized("server requires GSSAPI (Kerberos) authentication and " + "client was missing the required SASL module"); } - TRACE("SASL Client: Server mech list: $0", mech_list); + + string matching_mechs_str = JoinElements(matching_mechs, " "); + TRACE("SASL Client: Matching mech list: $0", matching_mechs_str); const char* init_msg = nullptr; unsigned init_msg_len = 0; @@ -362,32 +375,22 @@ Status SaslClient::HandleNegotiateResponse(const SaslMessagePB& response) { TRACE("SASL Client: Calling sasl_client_start()"); Status s = WrapSaslCall(sasl_conn_.get(), [&]() { return sasl_client_start( - sasl_conn_.get(), // The SASL connection context created by init() - mech_list.c_str(), // The list of mechanisms from the server. - 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() + 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. }); if (s.ok()) { nego_ok_ = true; } else if (!s.IsIncomplete()) { - // If we failed to negotiate because we didn't share any mechanisms, - // the most likely case is that the client is missing the GSSAPI SASL - // module, and the server is configured to only allow Kerberos connections. - // Return a more usable error message in this case. - if (MatchPattern(s.ToString(), "*No worthy mechs found") && - ContainsKey(mech_auth_map, kSaslMechGSSAPI) && - !ContainsKey(helper_.LocalMechs(), kSaslMechGSSAPI)) { - return Status::NotAuthorized("server requires GSSAPI (Kerberos) authentication and " - "client was missing the required SASL module"); - } return s; } // The server matched one of our mechanisms. - SaslMessagePB::SaslAuth* auth = FindOrNull(mech_auth_map, negotiated_mech); + SaslMessagePB::SaslAuth* auth = FindOrNull(server_mech_map, negotiated_mech); if (PREDICT_FALSE(auth == nullptr)) { return Status::IllegalState("Unable to find auth in map, unexpected error", negotiated_mech); } http://git-wip-us.apache.org/repos/asf/kudu/blob/ed01e91a/src/kudu/rpc/sasl_helper.cc ---------------------------------------------------------------------- diff --git a/src/kudu/rpc/sasl_helper.cc b/src/kudu/rpc/sasl_helper.cc index 334ae32..06fc82c 100644 --- a/src/kudu/rpc/sasl_helper.cc +++ b/src/kudu/rpc/sasl_helper.cc @@ -99,11 +99,6 @@ const char* SaslHelper::LocalMechListString() const { int SaslHelper::GetOptionCb(const char* plugin_name, const char* option, const char** result, unsigned* len) { - string cb_name("client_mech_list"); - if (peer_type_ == SERVER) { - cb_name = "mech_list"; - } - DVLOG(4) << tag_ << ": GetOption Callback called. "; DVLOG(4) << tag_ << ": GetOption Plugin name: " << (plugin_name == nullptr ? "NULL" : plugin_name); @@ -116,7 +111,7 @@ int SaslHelper::GetOptionCb(const char* plugin_name, const char* option, if (plugin_name == nullptr) { // SASL library option, not a plugin option - if (cb_name == option) { + if (strcmp(option, "mech_list") == 0) { *result = LocalMechListString(); if (len != nullptr) *len = strlen(*result); VLOG(4) << tag_ << ": Enabled mech list: " << *result;
