You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Dan Burkert (Code Review)" <ge...@cloudera.org> on 2017/03/07 23:50:47 UTC

[kudu-CR](branch-1.3.x) [rpc] Do not send error reply for invalid connection context

Dan Burkert has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/6304

Change subject: [rpc] Do not send error reply for invalid connection context
......................................................................

[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>
(cherry picked from commit f24307db4deda6e9da037514afc5573f1231cdfb)
---
M src/kudu/rpc/server_negotiation.cc
1 file changed, 3 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/6304/1
-- 
To view, visit http://gerrit.cloudera.org:8080/6304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifc7d2726c37ad59fbe409375dc6605a73a4035b3
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Dan Burkert <da...@apache.org>

[kudu-CR](branch-1.3.x) [rpc] Do not send error reply for invalid connection context

Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has submitted this change and it was merged.

Change subject: [rpc] Do not send error reply for invalid connection context
......................................................................


[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>
(cherry picked from commit f24307db4deda6e9da037514afc5573f1231cdfb)
Reviewed-on: http://gerrit.cloudera.org:8080/6304
Reviewed-by: Jean-Daniel Cryans <jd...@apache.org>
Tested-by: Jean-Daniel Cryans <jd...@apache.org>
---
M src/kudu/rpc/server_negotiation.cc
1 file changed, 3 insertions(+), 9 deletions(-)

Approvals:
  Jean-Daniel Cryans: Looks good to me, approved; Verified



-- 
To view, visit http://gerrit.cloudera.org:8080/6304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ifc7d2726c37ad59fbe409375dc6605a73a4035b3
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>

[kudu-CR](branch-1.3.x) [rpc] Do not send error reply for invalid connection context

Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.

Change subject: [rpc] Do not send error reply for invalid connection context
......................................................................


Patch Set 1: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/6304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc7d2726c37ad59fbe409375dc6605a73a4035b3
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR](branch-1.3.x) [rpc] Do not send error reply for invalid connection context

Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.

Change subject: [rpc] Do not send error reply for invalid connection context
......................................................................


Patch Set 1: Verified+1

Build likely failed due to KUDU-1894 which we really need to fix.

-- 
To view, visit http://gerrit.cloudera.org:8080/6304
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc7d2726c37ad59fbe409375dc6605a73a4035b3
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-HasComments: No