You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2017/05/06 00:37:54 UTC
[kudu-CR] cert: add check for pending SSL errors in cert-related code
Todd Lipcon has uploaded a new change for review.
http://gerrit.cloudera.org:8080/6814
Change subject: cert: add check for pending SSL errors in cert-related code
......................................................................
cert: add check for pending SSL errors in cert-related code
We missed these functions which use SSL libraries before. Adar saw a
test failure with a pending error from the OBJ library in an unrelated
test, so my best guess is it came from here. The scoped checker should
help us find if this is the case.
Change-Id: I94bb6b71926051d2f8a7eabaa1eceb048ea9ec30
---
M src/kudu/security/cert.cc
1 file changed, 6 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6814/1
--
To view, visit http://gerrit.cloudera.org:8080/6814
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I94bb6b71926051d2f8a7eabaa1eceb048ea9ec30
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
[kudu-CR] cert: add check for pending SSL errors in cert-related code
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged.
Change subject: cert: add check for pending SSL errors in cert-related code
......................................................................
cert: add check for pending SSL errors in cert-related code
We missed these functions which use SSL libraries before. Adar saw a
test failure with a pending error from the OBJ library in an unrelated
test, so my best guess is it came from here. The scoped checker should
help us find if this is the case.
Change-Id: I94bb6b71926051d2f8a7eabaa1eceb048ea9ec30
Reviewed-on: http://gerrit.cloudera.org:8080/6814
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M src/kudu/security/cert.cc
1 file changed, 9 insertions(+), 0 deletions(-)
Approvals:
Alexey Serbin: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/6814
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I94bb6b71926051d2f8a7eabaa1eceb048ea9ec30
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] cert: add check for pending SSL errors in cert-related code
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: cert: add check for pending SSL errors in cert-related code
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/6814/1/src/kudu/security/cert.cc
File src/kudu/security/cert.cc:
Line 139: OPENSSL_RET_NOT_OK(X509_check_private_key(data_.get(), key.GetRawData()),
Would it make sense to add SCOPED_OPENSSL_NO_PENDING errors here as well?
Line 212: EVP_PKEY* raw_key = X509_get_pubkey(data_.get());
Ditto?
Line 240: EVP_PKEY* raw_key = X509_REQ_get_pubkey(data_.get());
ditto?
--
To view, visit http://gerrit.cloudera.org:8080/6814
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I94bb6b71926051d2f8a7eabaa1eceb048ea9ec30
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] cert: add check for pending SSL errors in cert-related code
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.
Change subject: cert: add check for pending SSL errors in cert-related code
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/6814
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I94bb6b71926051d2f8a7eabaa1eceb048ea9ec30
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] cert: add check for pending SSL errors in cert-related code
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: cert: add check for pending SSL errors in cert-related code
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/6814/1/src/kudu/security/cert.cc
File src/kudu/security/cert.cc:
Line 139: OPENSSL_RET_NOT_OK(X509_check_private_key(data_.get(), key.GetRawData()),
> Would it make sense to add SCOPED_OPENSSL_NO_PENDING errors here as well?
shouldn't be necessary because OPENSSL_RET_NOT_OK clears errors (in GetOpenSSLErrors()). But I guess there isn't any harm for the extra check in debug mode.
Line 212: EVP_PKEY* raw_key = X509_get_pubkey(data_.get());
> Ditto?
Done
Line 240: EVP_PKEY* raw_key = X509_REQ_get_pubkey(data_.get());
> ditto?
Done
--
To view, visit http://gerrit.cloudera.org:8080/6814
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I94bb6b71926051d2f8a7eabaa1eceb048ea9ec30
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] cert: add check for pending SSL errors in cert-related code
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6814
to look at the new patch set (#2).
Change subject: cert: add check for pending SSL errors in cert-related code
......................................................................
cert: add check for pending SSL errors in cert-related code
We missed these functions which use SSL libraries before. Adar saw a
test failure with a pending error from the OBJ library in an unrelated
test, so my best guess is it came from here. The scoped checker should
help us find if this is the case.
Change-Id: I94bb6b71926051d2f8a7eabaa1eceb048ea9ec30
---
M src/kudu/security/cert.cc
1 file changed, 9 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/6814/2
--
To view, visit http://gerrit.cloudera.org:8080/6814
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I94bb6b71926051d2f8a7eabaa1eceb048ea9ec30
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins