[kudu-CR] cert: add check for pending SSL errors in cert-related code

[Todd Lipcon (Code Review)]

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 
---
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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] cert: add check for pending SSL errors in cert-related code

[Alexey Serbin (Code Review)]

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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] cert: add check for pending SSL errors in cert-related code

[Todd Lipcon (Code Review)]

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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] cert: add check for pending SSL errors in cert-related code

[Todd Lipcon (Code Review)]

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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] cert: add check for pending SSL errors in cert-related code

[Alexey Serbin (Code Review)]

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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] cert: add check for pending SSL errors in cert-related code

[Todd Lipcon (Code Review)]

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