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: TLS-negotiation [1/n]: deprecate unused SaslAuth fields
Date Mon, 23 Jan 2017 18:17:57 GMT
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 <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/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 <danburkert@apache.org>
Authored: Wed Dec 14 13:48:22 2016 -0800
Committer: Dan Burkert <danburkert@apache.org>
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.


Mime
View raw message