You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2019/11/16 00:48:30 UTC

[kudu-CR](branch-1.10.x) [java] fixed bug in the connection negotiation code

Hello Kudu Jenkins, Adar Dembo, Todd Lipcon,

I'd like you to do a code review. Please visit

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

to review the following change.


Change subject: [java] fixed bug in the connection negotiation code
......................................................................

[java] fixed bug in the connection negotiation code

This patch fixes a typo in the connection negotiation code in the Java
client.  Prior to this fix, channel binding information was not verified
during connection negotiation because the peer certificate was not set.

In addition, I modified the error handing code in Negotiator.java to
abort connection negotiation upon receiving SSLPeerUnverifiedException
due to the absence of the channel binding information or the presence
of the invalid one.

I also added a test to verify that Kudu Java client doesn't connect
to a Kudu server which doesn't provide valid channel binding information
during the connection negotiation phase.

Kudos to Andy Singer for pointing to the bug.

Change-Id: I7bfd428128e224f03901a6cd7b33283495a28d54
Reviewed-on: http://gerrit.cloudera.org:8080/14713
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Reviewed-by: Todd Lipcon <to...@apache.org>
(cherry picked from commit a0e896475c139d308e3b6e32110e97168b9562c6)
---
M java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
M src/kudu/rpc/server_negotiation.cc
4 files changed, 84 insertions(+), 3 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.10.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7bfd428128e224f03901a6cd7b33283495a28d54
Gerrit-Change-Number: 14724
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR](branch-1.10.x) [java] fixed bug in the connection negotiation code

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14724 )

Change subject: [java] fixed bug in the connection negotiation code
......................................................................

[java] fixed bug in the connection negotiation code

This patch fixes a typo in the connection negotiation code in the Java
client.  Prior to this fix, channel binding information was not verified
during connection negotiation because the peer certificate was not set.

In addition, I modified the error handing code in Negotiator.java to
abort connection negotiation upon receiving SSLPeerUnverifiedException
due to the absence of the channel binding information or the presence
of the invalid one.

I also added a test to verify that Kudu Java client doesn't connect
to a Kudu server which doesn't provide valid channel binding information
during the connection negotiation phase.

Kudos to Andy Singer for pointing to the bug.

Change-Id: I7bfd428128e224f03901a6cd7b33283495a28d54
Reviewed-on: http://gerrit.cloudera.org:8080/14713
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Reviewed-by: Todd Lipcon <to...@apache.org>
(cherry picked from commit a0e896475c139d308e3b6e32110e97168b9562c6)
Reviewed-on: http://gerrit.cloudera.org:8080/14724
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestSecurity.java
M src/kudu/rpc/server_negotiation.cc
4 files changed, 84 insertions(+), 3 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Alexey Serbin: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.10.x
Gerrit-MessageType: merged
Gerrit-Change-Id: I7bfd428128e224f03901a6cd7b33283495a28d54
Gerrit-Change-Number: 14724
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR](branch-1.10.x) [java] fixed bug in the connection negotiation code

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14724 )

Change subject: [java] fixed bug in the connection negotiation code
......................................................................


Patch Set 1: Code-Review+2

Carrying over +2 from http://gerrit.cloudera.org:8080/14713

I think it's worth adding the fix into 1.10.1 release as well.


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

Gerrit-Project: kudu
Gerrit-Branch: branch-1.10.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I7bfd428128e224f03901a6cd7b33283495a28d54
Gerrit-Change-Number: 14724
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Sat, 16 Nov 2019 01:40:25 +0000
Gerrit-HasComments: No