Repository: kudu Updated Branches: refs/heads/master 4a5f1392b -> 73dcce642
TLS-negotiation [1/n]: deprecate unused SaslAuth fields This commit deprecates two fields in the SaslMessagePB.SaslAuth type: method: this field has been unused since 0.6. In 0.6 it was a required field, so this change technically breaks compat with 0.6 clients or servers, but I don't think we have or guarantee compat with pre 1.0. challenge: this field existed to allow the server to piggy back a challenge token along with the NEGOTIATE response, but we never used it. The Java client currently doesn't handle it, so we could not start using it in the future in a backwards compatible way, thus it's best to simplify things by removing it. Change-Id: I5e40e6453892a78b293f3dd63980eab839b671c1 Reviewed-on: http://gerrit.cloudera.org:8080/5755 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/13014cac Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/13014cac Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/13014cac Branch: refs/heads/master Commit: 13014cac649b4da47a64acfac318bb3fdf6f4347 Parents: 4a5f139 Author: Dan Burkert <[email protected]> Authored: Wed Dec 14 13:48:22 2016 -0800 Committer: Dan Burkert <[email protected]> Committed: Mon Jan 23 18:17:30 2017 +0000 ---------------------------------------------------------------------- src/kudu/rpc/rpc_header.proto | 8 +++----- src/kudu/rpc/sasl_client.cc | 8 -------- src/kudu/rpc/sasl_server.cc | 8 +------- 3 files changed, 4 insertions(+), 20 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/13014cac/src/kudu/rpc/rpc_header.proto ---------------------------------------------------------------------- diff --git a/src/kudu/rpc/rpc_header.proto b/src/kudu/rpc/rpc_header.proto index 895a267..c7adbcb 100644 --- a/src/kudu/rpc/rpc_header.proto +++ b/src/kudu/rpc/rpc_header.proto @@ -85,13 +85,11 @@ message SaslMessagePB { } message SaslAuth { - optional string method = 1; // Deprecated, but was 'required' in Kudu 0.5.0 and 0.6.0. required string mechanism = 2; // Standard SASL mechanism, i.e. ANONYMOUS, PLAIN, GSSAPI. - // SASL challenge token from server, if the client chooses to use this method. - // Only used when the server is piggy-backing a challenge on a NEGOTIATE response. - // Otherwise, SaslMessagePB::token is used as the challenge token. - optional bytes challenge = 5 [(REDACT) = true]; + // Deprecated: no longer used. + optional string DEPRECATED_method = 1; + optional bytes DEPRECATED_challenge = 5 [(REDACT) = true]; } // When the client sends its first NEGOTIATE message, it sends its set of supported http://git-wip-us.apache.org/repos/asf/kudu/blob/13014cac/src/kudu/rpc/sasl_client.cc ---------------------------------------------------------------------- diff --git a/src/kudu/rpc/sasl_client.cc b/src/kudu/rpc/sasl_client.cc index b8ea2be..83b1f97 100644 --- a/src/kudu/rpc/sasl_client.cc +++ b/src/kudu/rpc/sasl_client.cc @@ -396,14 +396,6 @@ Status SaslClient::HandleNegotiateResponse(const SaslMessagePB& response) { } negotiated_mech_ = SaslMechanism::value_of(negotiated_mech); - // Handle the case where the server sent a challenge with the NEGOTIATE response. - if (auth->has_challenge()) { - if (PREDICT_FALSE(nego_ok_)) { - LOG(DFATAL) << "Server sent challenge after sasl_client_start() returned SASL_OK"; - } - RETURN_NOT_OK(DoSaslStep(auth->challenge(), &init_msg, &init_msg_len)); - } - RETURN_NOT_OK(SendInitiateMessage(*auth, init_msg, init_msg_len)); return Status::OK(); } http://git-wip-us.apache.org/repos/asf/kudu/blob/13014cac/src/kudu/rpc/sasl_server.cc ---------------------------------------------------------------------- diff --git a/src/kudu/rpc/sasl_server.cc b/src/kudu/rpc/sasl_server.cc index 6fa38c8..6b3560d 100644 --- a/src/kudu/rpc/sasl_server.cc +++ b/src/kudu/rpc/sasl_server.cc @@ -323,13 +323,7 @@ Status SaslServer::SendNegotiateResponse(const set<string>& server_mechs) { response.set_state(SaslMessagePB::NEGOTIATE); for (const string& mech : server_mechs) { - SaslMessagePB::SaslAuth* auth = response.add_auths(); - - // The 'method' field is deprecated, but older versions of Kudu marked it 'required'. - // So, we have to set it to something to keep compatibility. At some point, we can - // consider removing it and breaking compatibility with Kudu <=0.6. - auth->set_method(""); - auth->set_mechanism(mech); + response.add_auths()->set_mechanism(mech); } // Tell the client which features we support.
