TLS-negotiation [4/n]: rename Negotiation steps This makes it more clear which steps are SASL-specific.
Change-Id: Id19c0e9904d39229c1007c6ea8aec76d7d26dddb Reviewed-on: http://gerrit.cloudera.org:8080/5758 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/9d0421e8 Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/9d0421e8 Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/9d0421e8 Branch: refs/heads/master Commit: 9d0421e807887a83f011e9cdc265901593b2fdf8 Parents: 2960e69 Author: Dan Burkert <[email protected]> Authored: Fri Dec 16 16:36:15 2016 -0800 Committer: Dan Burkert <[email protected]> Committed: Mon Jan 23 23:25:36 2017 +0000 ---------------------------------------------------------------------- .../org/apache/kudu/client/SecureRpcHelper.java | 9 +++-- src/kudu/rpc/rpc_header.proto | 37 ++++++++++++-------- src/kudu/rpc/sasl_client.cc | 26 +++++++------- src/kudu/rpc/sasl_server.cc | 16 ++++----- 4 files changed, 48 insertions(+), 40 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/9d0421e8/java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java ---------------------------------------------------------------------- diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java b/java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java index 90cd167..0ec6547 100644 --- a/java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java +++ b/java/kudu-client/src/main/java/org/apache/kudu/client/SecureRpcHelper.java @@ -26,7 +26,6 @@ package org.apache.kudu.client; -import java.security.PrivilegedExceptionAction; import java.util.Map; import java.util.Set; import javax.security.auth.Subject; @@ -133,10 +132,10 @@ public class SecureRpcHelper { case NEGOTIATE: handleNegotiateResponse(chan, response); break; - case CHALLENGE: + case SASL_CHALLENGE: handleChallengeResponse(chan, response); break; - case SUCCESS: + case SASL_SUCCESS: handleSuccessResponse(chan); break; default: @@ -227,7 +226,7 @@ public class SecureRpcHelper { if (saslToken != null) { builder.setToken(ZeroCopyLiteralByteString.wrap(saslToken)); } - builder.setStep(RpcHeader.NegotiatePB.NegotiateStep.INITIATE); + builder.setStep(RpcHeader.NegotiatePB.NegotiateStep.SASL_INITIATE); builder.addAuths(negotiatedAuth); sendSaslMessage(chan, builder.build()); } @@ -240,7 +239,7 @@ public class SecureRpcHelper { } RpcHeader.NegotiatePB.Builder builder = RpcHeader.NegotiatePB.newBuilder(); builder.setToken(ZeroCopyLiteralByteString.wrap(saslToken)); - builder.setStep(RpcHeader.NegotiatePB.NegotiateStep.RESPONSE); + builder.setStep(RpcHeader.NegotiatePB.NegotiateStep.SASL_RESPONSE); sendSaslMessage(chan, builder.build()); } http://git-wip-us.apache.org/repos/asf/kudu/blob/9d0421e8/src/kudu/rpc/rpc_header.proto ---------------------------------------------------------------------- diff --git a/src/kudu/rpc/rpc_header.proto b/src/kudu/rpc/rpc_header.proto index f2e4c8c..26e56ed 100644 --- a/src/kudu/rpc/rpc_header.proto +++ b/src/kudu/rpc/rpc_header.proto @@ -76,33 +76,42 @@ enum RpcFeatureFlag { // Message type passed back & forth for the SASL negotiation. message NegotiatePB { enum NegotiateStep { - UNKNOWN = 999; - SUCCESS = 0; - NEGOTIATE = 1; - INITIATE = 2; - CHALLENGE = 3; - RESPONSE = 4; + UNKNOWN = 999; + NEGOTIATE = 1; + SASL_SUCCESS = 0; + SASL_INITIATE = 2; + SASL_CHALLENGE = 3; + SASL_RESPONSE = 4; } message SaslAuth { - required string mechanism = 2; // Standard SASL mechanism, i.e. ANONYMOUS, PLAIN, GSSAPI. + // The SASL mechanism, i.e. 'PLAIN' or 'GSSAPI'. + required string mechanism = 2; // 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 - // RPC system features. In the response to this message, the server sends back its own. - // This allows the two peers to agree on whether newer extensions of the - // RPC system may be used on this connection. We use a list of features rather than - // a simple version number to make it easier for the Java and C++ clients to implement - // features in different orders while still maintaining compatibility, as well as - // to simplify backporting of features out-of-order. + // When the client sends its NEGOTIATE step message, it sends its set of + // supported RPC system features. In the response to this message, the server + // sends back its own. This allows the two peers to agree on whether newer + // extensions of the RPC system may be used on this connection. We use a list + // of features rather than a simple version number to make it easier for the + // Java and C++ clients to implement features in different orders while still + // maintaining compatibility, as well as to simplify backporting of features + // out-of-order. repeated RpcFeatureFlag supported_features = 1; + // The current negotiation step. required NegotiateStep step = 2; + + // The SASL token, containing either the challenge during the SASL_CHALLENGE + // step, or the response during the SASL_RESPONSE step. optional bytes token = 3 [(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/9d0421e8/src/kudu/rpc/sasl_client.cc ---------------------------------------------------------------------- diff --git a/src/kudu/rpc/sasl_client.cc b/src/kudu/rpc/sasl_client.cc index 6e0cbab..c88e3cc 100644 --- a/src/kudu/rpc/sasl_client.cc +++ b/src/kudu/rpc/sasl_client.cc @@ -217,13 +217,13 @@ Status SaslClient::Negotiate() { RETURN_NOT_OK(HandleNegotiateResponse(response)); break; - // CHALLENGE: Server sent us a follow-up to an INITIATE or RESPONSE request. - case NegotiatePB::CHALLENGE: + // SASL_CHALLENGE: Server sent us a follow-up to an SASL_INITIATE or SASL_RESPONSE request. + case NegotiatePB::SASL_CHALLENGE: RETURN_NOT_OK(HandleChallengeResponse(response)); break; - // SUCCESS: Server has accepted our authentication request. Negotiation successful. - case NegotiatePB::SUCCESS: + // SASL_SUCCESS: Server has accepted our authentication request. Negotiation successful. + case NegotiatePB::SASL_SUCCESS: RETURN_NOT_OK(HandleSuccessResponse(response)); break; @@ -282,10 +282,10 @@ Status SaslClient::SendNegotiateMessage() { Status SaslClient::SendInitiateMessage(const NegotiatePB_SaslAuth& auth, const char* init_msg, unsigned init_msg_len) { NegotiatePB msg; - msg.set_step(NegotiatePB::INITIATE); + msg.set_step(NegotiatePB::SASL_INITIATE); msg.mutable_token()->assign(init_msg, init_msg_len); msg.add_auths()->CopyFrom(auth); - TRACE("SASL Client: Sending INITIATE request to server."); + TRACE("SASL Client: Sending SASL_INITIATE request to server."); RETURN_NOT_OK(SendNegotiatePB(msg)); nego_response_expected_ = true; return Status::OK(); @@ -293,9 +293,9 @@ Status SaslClient::SendInitiateMessage(const NegotiatePB_SaslAuth& auth, Status SaslClient::SendResponseMessage(const char* resp_msg, unsigned resp_msg_len) { NegotiatePB reply; - reply.set_step(NegotiatePB::RESPONSE); + reply.set_step(NegotiatePB::SASL_RESPONSE); reply.mutable_token()->assign(resp_msg, resp_msg_len); - TRACE("SASL Client: Sending RESPONSE request to server."); + TRACE("SASL Client: Sending SASL_RESPONSE request to server."); RETURN_NOT_OK(SendNegotiatePB(reply)); nego_response_expected_ = true; return Status::OK(); @@ -396,13 +396,13 @@ Status SaslClient::HandleNegotiateResponse(const NegotiatePB& response) { } Status SaslClient::HandleChallengeResponse(const NegotiatePB& response) { - TRACE("SASL Client: Received CHALLENGE response from server"); + TRACE("SASL Client: Received SASL_CHALLENGE response from server"); if (PREDICT_FALSE(nego_ok_)) { - LOG(DFATAL) << "Server sent CHALLENGE response after client library returned SASL_OK"; + LOG(DFATAL) << "Server sent SASL_CHALLENGE response after client library returned SASL_OK"; } if (PREDICT_FALSE(!response.has_token())) { - return Status::InvalidArgument("No token in CHALLENGE response from server"); + return Status::InvalidArgument("No token in SASL_CHALLENGE response from server"); } const char* out = nullptr; @@ -416,7 +416,7 @@ Status SaslClient::HandleChallengeResponse(const NegotiatePB& response) { } Status SaslClient::HandleSuccessResponse(const NegotiatePB& response) { - TRACE("SASL Client: Received SUCCESS response from server"); + TRACE("SASL Client: Received SASL_SUCCESS response from server"); if (!nego_ok_) { const char* out = nullptr; unsigned out_len = 0; @@ -427,7 +427,7 @@ Status SaslClient::HandleSuccessResponse(const NegotiatePB& response) { } RETURN_NOT_OK(s); if (out_len > 0) { - return Status::IllegalState("SASL client library generated spurious token after SUCCESS", + return Status::IllegalState("SASL client library generated spurious token after SASL_SUCCESS", string(out, out_len)); } CHECK(nego_ok_); http://git-wip-us.apache.org/repos/asf/kudu/blob/9d0421e8/src/kudu/rpc/sasl_server.cc ---------------------------------------------------------------------- diff --git a/src/kudu/rpc/sasl_server.cc b/src/kudu/rpc/sasl_server.cc index f8d4040..d8c627d 100644 --- a/src/kudu/rpc/sasl_server.cc +++ b/src/kudu/rpc/sasl_server.cc @@ -188,12 +188,12 @@ Status SaslServer::Negotiate() { break; // INITIATE: They want to initiate negotiation based on their specified mechanism. - case NegotiatePB::INITIATE: + case NegotiatePB::SASL_INITIATE: RETURN_NOT_OK(HandleInitiateRequest(request)); break; - // RESPONSE: Client sent a new request as a follow-up to a CHALLENGE response. - case NegotiatePB::RESPONSE: + // RESPONSE: Client sent a new request as a follow-up to a SASL_CHALLENGE response. + case NegotiatePB::SASL_RESPONSE: RETURN_NOT_OK(HandleResponseRequest(request)); break; @@ -381,20 +381,20 @@ Status SaslServer::HandleInitiateRequest(const NegotiatePB& request) { Status SaslServer::SendChallengeResponse(const char* challenge, unsigned clen) { NegotiatePB response; - response.set_step(NegotiatePB::CHALLENGE); + response.set_step(NegotiatePB::SASL_CHALLENGE); response.mutable_token()->assign(challenge, clen); - TRACE("SASL Server: Sending CHALLENGE response to client"); + TRACE("SASL Server: Sending SASL_CHALLENGE response to client"); RETURN_NOT_OK(SendNegotiatePB(response)); return Status::OK(); } Status SaslServer::SendSuccessResponse(const char* token, unsigned tlen) { NegotiatePB response; - response.set_step(NegotiatePB::SUCCESS); + response.set_step(NegotiatePB::SASL_SUCCESS); if (PREDICT_FALSE(tlen > 0)) { response.mutable_token()->assign(token, tlen); } - TRACE("SASL Server: Sending SUCCESS response to client"); + TRACE("SASL Server: Sending SASL_SUCCESS response to client"); RETURN_NOT_OK(SendNegotiatePB(response)); return Status::OK(); } @@ -404,7 +404,7 @@ Status SaslServer::HandleResponseRequest(const NegotiatePB& request) { TRACE("SASL Server: Received RESPONSE request from client"); if (!request.has_token()) { - Status s = Status::InvalidArgument("No token in CHALLENGE RESPONSE from client"); + Status s = Status::InvalidArgument("No token in SASL_RESPONSE from client"); RETURN_NOT_OK(SendRpcError(ErrorStatusPB::FATAL_UNAUTHORIZED, s)); return s; }
