[2/2] kudu git commit: [rpc] Do not send error reply for invalid connection context

[da...@apache.org]

[rpc] Do not send error reply for invalid connection context

We recently introduced a nonce into the KRPC protocol when using
Kerberos for authentication. The client sends the encoded nonce to the
server in the connection context message, to which the client does not
expect a reply. This commit changes the server to not reply when the
nonce is mismatched, because it causes the client to issue a warning:

    I0307 14:37:44.546371 166811 connection.cc:524] client connection to 10.17.246.26:7051: Got a response for call id -33 which was not pending!  Ignoring.

This message could be a red-herring since it indicates that the protocol
implementation is broken. Instead, we just have the server drop the
connection on nonce mismatch, and rely on server-side logs to indicate
the failure.

This issue was found when a client version with Kerberos support but
without nonce support connected to a server with nonce support, which
will never happen 'in the real world' with released Kudu versions.

Change-Id: Ifc7d2726c37ad59fbe409375dc6605a73a4035b3
Reviewed-on: http://gerrit.cloudera.org:8080/6302
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/f24307db
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/f24307db
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/f24307db

Branch: refs/heads/master
Commit: f24307db4deda6e9da037514afc5573f1231cdfb
Parents: b6cf6b6
Author: Dan Burkert <da...@apache.org>
Authored: Tue Mar 7 15:03:28 2017 -0800
Committer: Dan Burkert <da...@apache.org>
Committed: Tue Mar 7 23:50:37 2017 +0000

----------------------------------------------------------------------
 src/kudu/rpc/server_negotiation.cc | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/f24307db/src/kudu/rpc/server_negotiation.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/server_negotiation.cc b/src/kudu/rpc/server_negotiation.cc
index aab311e..ab31c0b 100644
--- a/src/kudu/rpc/server_negotiation.cc
+++ b/src/kudu/rpc/server_negotiation.cc
@@ -777,17 +777,13 @@ Status ServerNegotiation::RecvConnectionContext(faststring* recv_buf) {
     Status s;
     // Validate that the client returned the correct SASL protected nonce.
     if (!conn_context.has_encoded_nonce()) {
-      s = Status::NotAuthorized("ConnectionContextPB wrapped nonce missing");
-      RETURN_NOT_OK(SendError(ErrorStatusPB::FATAL_UNAUTHORIZED, s));
-      return s;
+      return Status::NotAuthorized("ConnectionContextPB wrapped nonce missing");
     }
 
     string decoded_nonce;
     s = SaslDecode(sasl_conn_.get(), conn_context.encoded_nonce(), &decoded_nonce);
     if (!s.ok()) {
-      s = Status::NotAuthorized("failed to decode nonce", s.message());
-      RETURN_NOT_OK(SendError(ErrorStatusPB::FATAL_UNAUTHORIZED, s));
-      return s;
+      return Status::NotAuthorized("failed to decode nonce", s.message());
     }
 
     if (*nonce_ != decoded_nonce) {
@@ -796,9 +792,7 @@ Status ServerNegotiation::RecvConnectionContext(faststring* recv_buf) {
       LOG(WARNING) << "Received an invalid connection nonce from client "
                    << addr.ToString()
                    << ", this could indicate a replay attack";
-      s = Status::NotAuthorized("nonce mismatch");
-      RETURN_NOT_OK(SendError(ErrorStatusPB::FATAL_UNAUTHORIZED, s));
-      return s;
+      return Status::NotAuthorized("nonce mismatch");
     }
   }