You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Michael Ho (Code Review)" <ge...@cloudera.org> on 2017/08/20 01:11:59 UTC

[Impala-ASF-CR] KUDU-2091: Certificates with intermediate CA's do not work with Kudu

Hello Todd Lipcon,

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

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

to review the following change.

Change subject: KUDU-2091: Certificates with intermediate CA's do not work with Kudu
......................................................................

KUDU-2091: Certificates with intermediate CA's do not work with Kudu

Kudu previously did not recognize chain certificates. This patch
enables support for chain certificates by changing the Cert class'
underlying data type to STACK_OF(X509) instead of just X509.

STACK_OF(X509) allows multiple certificates to be held by the same
pointer. When we are presented with a file or a string that contains
multiple X509 certificates, they will be stored inside this
STACK_OF(X509) object.

When we call AddTrustedCertificate(Cert&), we iterate through the
STACK_OF(X509) contained in the Cert and add each one individually to
the X509_STORE for later verification.

Currently, IPKI does not make use of this ability and still works
with single certificates. DCHECKS are added to make sure that multiple
X509 certificates are not accidentally added to a Cert object.
Although this patch provides a general framework to use chain
certificates, if we want to use IPKI with chain certificates,
additional functionality will need to be added with clearer APIs.

External PKI makes use of this ability to add a chain CA if necessary.

Testing: A new test is added to rpc-test that uses a chain CA. This
test does not work without this patch.

Change-Id: I7334a5b2f1643848152562bbb1dee27b5290e83f
Reviewed-on: http://gerrit.cloudera.org:8080/7662
Reviewed-by: Todd Lipcon <to...@apache.org>
Tested-by: Todd Lipcon <to...@apache.org>
---
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/openssl_util.cc
M be/src/kudu/security/openssl_util.h
M be/src/kudu/security/test/test_certs.cc
M be/src/kudu/security/test/test_certs.h
M be/src/kudu/security/tls_context.cc
M be/src/kudu/security/tls_handshake.cc
10 files changed, 431 insertions(+), 55 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7334a5b2f1643848152562bbb1dee27b5290e83f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[Impala-ASF-CR] KUDU-2091: Certificates with intermediate CA's do not work with Kudu

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: KUDU-2091: Certificates with intermediate CA's do not work with Kudu
......................................................................


Patch Set 1:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7334a5b2f1643848152562bbb1dee27b5290e83f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[Impala-ASF-CR] KUDU-2091: Certificates with intermediate CA's do not work with Kudu

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: KUDU-2091: Certificates with intermediate CA's do not work with Kudu
......................................................................


Patch Set 1: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7334a5b2f1643848152562bbb1dee27b5290e83f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[Impala-ASF-CR] KUDU-2091: Certificates with intermediate CA's do not work with Kudu

Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change.

Change subject: KUDU-2091: Certificates with intermediate CA's do not work with Kudu
......................................................................


Patch Set 1:

The merge of this patch is clean for all files in the be/src/kudu directory.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7334a5b2f1643848152562bbb1dee27b5290e83f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[Impala-ASF-CR] KUDU-2091: Certificates with intermediate CA's do not work with Kudu

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged.

Change subject: KUDU-2091: Certificates with intermediate CA's do not work with Kudu
......................................................................


KUDU-2091: Certificates with intermediate CA's do not work with Kudu

Kudu previously did not recognize chain certificates. This patch
enables support for chain certificates by changing the Cert class'
underlying data type to STACK_OF(X509) instead of just X509.

STACK_OF(X509) allows multiple certificates to be held by the same
pointer. When we are presented with a file or a string that contains
multiple X509 certificates, they will be stored inside this
STACK_OF(X509) object.

When we call AddTrustedCertificate(Cert&), we iterate through the
STACK_OF(X509) contained in the Cert and add each one individually to
the X509_STORE for later verification.

Currently, IPKI does not make use of this ability and still works
with single certificates. DCHECKS are added to make sure that multiple
X509 certificates are not accidentally added to a Cert object.
Although this patch provides a general framework to use chain
certificates, if we want to use IPKI with chain certificates,
additional functionality will need to be added with clearer APIs.

External PKI makes use of this ability to add a chain CA if necessary.

Testing: A new test is added to rpc-test that uses a chain CA. This
test does not work without this patch.

Change-Id: I7334a5b2f1643848152562bbb1dee27b5290e83f
Reviewed-on: http://gerrit.cloudera.org:8080/7662
Reviewed-by: Todd Lipcon <to...@apache.org>
Tested-by: Todd Lipcon <to...@apache.org>
Reviewed-on: http://gerrit.cloudera.org:8080/7746
Reviewed-by: Sailesh Mukil <sa...@cloudera.com>
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/openssl_util.cc
M be/src/kudu/security/openssl_util.h
M be/src/kudu/security/test/test_certs.cc
M be/src/kudu/security/test/test_certs.h
M be/src/kudu/security/tls_context.cc
M be/src/kudu/security/tls_handshake.cc
10 files changed, 431 insertions(+), 55 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Sailesh Mukil: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I7334a5b2f1643848152562bbb1dee27b5290e83f
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[Impala-ASF-CR] KUDU-2091: Certificates with intermediate CA's do not work with Kudu

Posted by "Sailesh Mukil (Code Review)" <ge...@cloudera.org>.
Sailesh Mukil has posted comments on this change.

Change subject: KUDU-2091: Certificates with intermediate CA's do not work with Kudu
......................................................................


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7334a5b2f1643848152562bbb1dee27b5290e83f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No