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/15 07:40:47 UTC

[kudu-CR] [java] fixed typo in the connection negotiation code

Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14713


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

[java] fixed typo in the connection negotiation code

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

In addition, I modified the error handing code in Negotiator.java to
about connection negotiation upon receiving SSLPeerUnverifiedException
due to the absence or the presence of invalid bindings.

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

Kudos to Andy Singer for pointing to the typo!

Change-Id: I7bfd428128e224f03901a6cd7b33283495a28d54
---
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, 92 insertions(+), 11 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7bfd428128e224f03901a6cd7b33283495a28d54
Gerrit-Change-Number: 14713
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>

[kudu-CR] [java] fixed typo in the connection negotiation code

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Attila Bukor, Kudu Jenkins, Adar Dembo, Todd Lipcon, 

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#4).

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

[java] fixed typo 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 typo!

Change-Id: I7bfd428128e224f03901a6cd7b33283495a28d54
---
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/13/14713/4
-- 
To view, visit http://gerrit.cloudera.org:8080/14713
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7bfd428128e224f03901a6cd7b33283495a28d54
Gerrit-Change-Number: 14713
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] [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/14713 )

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>
---
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
  Adar Dembo: Looks good to me, approved
  Todd Lipcon: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7bfd428128e224f03901a6cd7b33283495a28d54
Gerrit-Change-Number: 14713
Gerrit-PatchSet: 6
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] [java] fixed bug in the connection negotiation code

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

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


Patch Set 5: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7bfd428128e224f03901a6cd7b33283495a28d54
Gerrit-Change-Number: 14713
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 15 Nov 2019 22:59:29 +0000
Gerrit-HasComments: No

[kudu-CR] [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/14713 )

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


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/14713/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/14713/3//COMMIT_MSG@7
PS3, Line 7: typo
> typo? shouldn't it be a bug or error instead?
Done


http://gerrit.cloudera.org:8080/#/c/14713/3/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java:

http://gerrit.cloudera.org:8080/#/c/14713/3/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java@447
PS3, Line 447:       // There's a race in Netty where, when we call Channel.close(), it tries
> nit: indentation
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7bfd428128e224f03901a6cd7b33283495a28d54
Gerrit-Change-Number: 14713
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 15 Nov 2019 20:51:13 +0000
Gerrit-HasComments: Yes

[kudu-CR] [java] fixed typo in the connection negotiation code

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

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


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/14713/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/14713/3//COMMIT_MSG@7
PS3, Line 7: typo
typo? shouldn't it be a bug or error instead?


http://gerrit.cloudera.org:8080/#/c/14713/3/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java:

http://gerrit.cloudera.org:8080/#/c/14713/3/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java@447
PS3, Line 447:         // There's a race in Netty where, when we call Channel.close(), it tries
nit: indentation



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7bfd428128e224f03901a6cd7b33283495a28d54
Gerrit-Change-Number: 14713
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 15 Nov 2019 11:09:18 +0000
Gerrit-HasComments: Yes

[kudu-CR] [java] fixed bug in the connection negotiation code

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Attila Bukor, Kudu Jenkins, Adar Dembo, Todd Lipcon, 

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#5).

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
---
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/13/14713/5
-- 
To view, visit http://gerrit.cloudera.org:8080/14713
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7bfd428128e224f03901a6cd7b33283495a28d54
Gerrit-Change-Number: 14713
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] [java] fixed typo in the connection negotiation code

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo, Todd Lipcon, 

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#3).

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

[java] fixed typo 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 typo!

Change-Id: I7bfd428128e224f03901a6cd7b33283495a28d54
---
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, 92 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/13/14713/3
-- 
To view, visit http://gerrit.cloudera.org:8080/14713
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7bfd428128e224f03901a6cd7b33283495a28d54
Gerrit-Change-Number: 14713
Gerrit-PatchSet: 3
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] [java] fixed typo in the connection negotiation code

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo, Todd Lipcon, 

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#2).

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

[java] fixed typo 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 typo!

Change-Id: I7bfd428128e224f03901a6cd7b33283495a28d54
---
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, 92 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/13/14713/2
-- 
To view, visit http://gerrit.cloudera.org:8080/14713
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7bfd428128e224f03901a6cd7b33283495a28d54
Gerrit-Change-Number: 14713
Gerrit-PatchSet: 2
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] [java] fixed bug in the connection negotiation code

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

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


Patch Set 5: Code-Review+2

Todd do you want to take a look before merging?


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7bfd428128e224f03901a6cd7b33283495a28d54
Gerrit-Change-Number: 14713
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 15 Nov 2019 22:56:46 +0000
Gerrit-HasComments: No