[Impala-ASF-CR] KUDU-2220: GetEndOfChainX509 does not return end-user cert

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8622 )

Change subject: KUDU-2220: GetEndOfChainX509 does not return end-user cert
..


Patch Set 1: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e3f913259ec4c855ff211726fa6ecea94d328e7
Gerrit-Change-Number: 8622
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Wed, 22 Nov 2017 00:34:55 +
Gerrit-HasComments: No

[Impala-ASF-CR] KUDU-2220: GetEndOfChainX509 does not return end-user cert

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8622 )

Change subject: KUDU-2220: GetEndOfChainX509 does not return end-user cert
..

KUDU-2220: GetEndOfChainX509 does not return end-user cert

KUDU-2091 introduced a function GetEndOfChainX509() which was supposed
to return the "end-user" certificate. However, the end-user certificate
is not at the end of the chain, but rather at the beginning of the chain
as specificed by the RFC:
https://tools.ietf.org/html/rfc5246#section-7.4.2

  | This is a sequence (chain) of certificates. The sender's certificate MUST
  | come first in the list. Each following certificate MUST directly certify
  | the one preceding it.

This patch fixes this by changing the GetEndOfChainX509() to
GetTopOfChainX509(). An existing test is modified to test this patch. It does
not pass without this change.

Change-Id: I0e3f913259ec4c855ff211726fa6ecea94d328e7
Reviewed-on: http://gerrit.cloudera.org:8080/8595
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin 
Reviewed-by: Todd Lipcon 
Reviewed-on: http://gerrit.cloudera.org:8080/8622
Reviewed-by: Michael Brown 
Reviewed-by: Michael Ho 
Tested-by: Impala Public Jenkins
---
M be/src/kudu/rpc/rpc-test.cc
M be/src/kudu/security/ca/cert_management.cc
M be/src/kudu/security/cert.cc
M be/src/kudu/security/cert.h
M be/src/kudu/security/test/test_certs.cc
M be/src/kudu/security/tls_context.cc
M be/src/kudu/security/tls_handshake.cc
7 files changed, 65 insertions(+), 22 deletions(-)

Approvals:
  Michael Brown: Looks good to me, but someone else must approve
  Michael Ho: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I0e3f913259ec4c855ff211726fa6ecea94d328e7
Gerrit-Change-Number: 8622
Gerrit-PatchSet: 2
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 

[Impala-ASF-CR] KUDU-2220: GetEndOfChainX509 does not return end-user cert

[Impala Public Jenkins (Code Review)]

Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8622 )

Change subject: KUDU-2220: GetEndOfChainX509 does not return end-user cert
..


Patch Set 1:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1508/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e3f913259ec4c855ff211726fa6ecea94d328e7
Gerrit-Change-Number: 8622
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 21 Nov 2017 21:01:41 +
Gerrit-HasComments: No

[Impala-ASF-CR] KUDU-2220: GetEndOfChainX509 does not return end-user cert

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8622 )

Change subject: KUDU-2220: GetEndOfChainX509 does not return end-user cert
..


Patch Set 1: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8622/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8622/1//COMMIT_MSG@7
PS1, Line 7: KUDU-2220: GetEndOfChainX509 does not return end-user cert
> Yes, I think we can just make IMPALA-6172 as a dup of KUDU-2220 instead of
SGTM.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e3f913259ec4c855ff211726fa6ecea94d328e7
Gerrit-Change-Number: 8622
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 21 Nov 2017 19:49:06 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] KUDU-2220: GetEndOfChainX509 does not return end-user cert

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8622 )

Change subject: KUDU-2220: GetEndOfChainX509 does not return end-user cert
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8622/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8622/1//COMMIT_MSG@7
PS1, Line 7: KUDU-2220: GetEndOfChainX509 does not return end-user cert
> What about mentioning IMPALA-6172 in the commit message header?
Yes, I think we can just make IMPALA-6172 as a dup of KUDU-2220 instead of 
modifying the commit message.

We haven't modified the commit messages unless we for some reason or the other 
had to modify the cherry-pick'd patch itself.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e3f913259ec4c855ff211726fa6ecea94d328e7
Gerrit-Change-Number: 8622
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 21 Nov 2017 19:33:12 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] KUDU-2220: GetEndOfChainX509 does not return end-user cert

[Michael Brown (Code Review)]

Michael Brown has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8622 )

Change subject: KUDU-2220: GetEndOfChainX509 does not return end-user cert
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8622/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8622/1//COMMIT_MSG@7
PS1, Line 7: KUDU-2220: GetEndOfChainX509 does not return end-user cert
What about mentioning IMPALA-6172 in the commit message header?

If the convention is that Kudu patches are meant to be as unaltered as 
possible, then I guess it makes sense to leave the commit message header as-is. 
In that case, should the IMPALA-6172 Jira just be duped to KUDU-2220?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e3f913259ec4c855ff211726fa6ecea94d328e7
Gerrit-Change-Number: 8622
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Michael Brown 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 21 Nov 2017 19:26:47 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] KUDU-2220: GetEndOfChainX509 does not return end-user cert

[Sailesh Mukil (Code Review)]

Sailesh Mukil has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8622 )

Change subject: KUDU-2220: GetEndOfChainX509 does not return end-user cert
..


Patch Set 1:

This patch fixes IMPALA-6172.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0e3f913259ec4c855ff211726fa6ecea94d328e7
Gerrit-Change-Number: 8622
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Sailesh Mukil 
Gerrit-Comment-Date: Tue, 21 Nov 2017 19:19:34 +
Gerrit-HasComments: No

[Impala-ASF-CR] KUDU-2220: GetEndOfChainX509 does not return end-user cert

[Sailesh Mukil (Code Review)]

Sailesh Mukil has removed Todd Lipcon from this change.  ( 
http://gerrit.cloudera.org:8080/8622 )

Change subject: KUDU-2220: GetEndOfChainX509 does not return end-user cert
..


Removed reviewer Todd Lipcon.
--
To view, visit http://gerrit.cloudera.org:8080/8622
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I0e3f913259ec4c855ff211726fa6ecea94d328e7
Gerrit-Change-Number: 8622
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 

[Impala-ASF-CR] KUDU-2220: GetEndOfChainX509 does not return end-user cert

[Sailesh Mukil (Code Review)]

Sailesh Mukil has removed Alexey Serbin from this change.  ( 
http://gerrit.cloudera.org:8080/8622 )

Change subject: KUDU-2220: GetEndOfChainX509 does not return end-user cert
..


Removed reviewer Alexey Serbin.
--
To view, visit http://gerrit.cloudera.org:8080/8622
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I0e3f913259ec4c855ff211726fa6ecea94d328e7
Gerrit-Change-Number: 8622
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[Impala-ASF-CR] KUDU-2220: GetEndOfChainX509 does not return end-user cert

[Sailesh Mukil (Code Review)]

Sailesh Mukil has removed Kudu Jenkins from this change.  ( 
http://gerrit.cloudera.org:8080/8622 )

Change subject: KUDU-2220: GetEndOfChainX509 does not return end-user cert
..


Removed reviewer Kudu Jenkins.
--
To view, visit http://gerrit.cloudera.org:8080/8622
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I0e3f913259ec4c855ff211726fa6ecea94d328e7
Gerrit-Change-Number: 8622
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Todd Lipcon 

[Impala-ASF-CR] KUDU-2220: GetEndOfChainX509 does not return end-user cert

[Sailesh Mukil (Code Review)]

Hello Alexey Serbin, Kudu Jenkins, Todd Lipcon,

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

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

to review the following change.


Change subject: KUDU-2220: GetEndOfChainX509 does not return end-user cert
..

KUDU-2220: GetEndOfChainX509 does not return end-user cert

KUDU-2091 introduced a function GetEndOfChainX509() which was supposed
to return the "end-user" certificate. However, the end-user certificate
is not at the end of the chain, but rather at the beginning of the chain
as specificed by the RFC:
https://tools.ietf.org/html/rfc5246#section-7.4.2

  | This is a sequence (chain) of certificates. The sender's certificate MUST
  | come first in the list. Each following certificate MUST directly certify
  | the one preceding it.

This patch fixes this by changing the GetEndOfChainX509() to
GetTopOfChainX509(). An existing test is modified to test this patch. It does
not pass without this change.

Change-Id: I0e3f913259ec4c855ff211726fa6ecea94d328e7
Reviewed-on: http://gerrit.cloudera.org:8080/8595
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin 
Reviewed-by: Todd Lipcon 
---
M be/src/kudu/rpc/rpc-test.cc
M be/src/kudu/security/ca/cert_management.cc
M be/src/kudu/security/cert.cc
M be/src/kudu/security/cert.h
M be/src/kudu/security/test/test_certs.cc
M be/src/kudu/security/tls_context.cc
M be/src/kudu/security/tls_handshake.cc
7 files changed, 65 insertions(+), 22 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/22/8622/1
--
To view, visit http://gerrit.cloudera.org:8080/8622
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0e3f913259ec4c855ff211726fa6ecea94d328e7
Gerrit-Change-Number: 8622
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon