Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10436 )

Change subject: WIP: KUDU-1889: support openssl 1.1
......................................................................


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/10436/3/src/kudu/security/ca/cert_management.cc
File src/kudu/security/ca/cert_management.cc:

http://gerrit.cloudera.org:8080/#/c/10436/3/src/kudu/security/ca/cert_management.cc@345
PS3, Line 345:   if (!req->req_info ||
> So is this safe to skip because sometime between 1.0.0 and 1.1.0, OpenSSL i
Whether or not it's safe is somewhat orthogonal: in OpenSSL 1.1 we can't access 
these fields anymore, so even if we should check them, we can't.

Anyway, these checks are still live for pre-1.1.0 OpenSSL. Or maybe I'm 
misunderstanding your question?


http://gerrit.cloudera.org:8080/#/c/10436/3/src/kudu/security/openssl_util.cc
File src/kudu/security/openssl_util.cc:

http://gerrit.cloudera.org:8080/#/c/10436/3/src/kudu/security/openssl_util.cc@114
PS3, Line 114:   OPENSSL_init_ssl(0, nullptr);
> What happens if the user application is buggy as per the check below, has a
I'm just going off the manpages, but yeah, the call is expected to be 
idempotent. Notably, SSL_CTX_new() always returns not-null in 1.1 (because it 
implicitly initializes the library), so we can't use that as a test to see if 
someone else has initialized the library.

I didn't check the return value because I assumed that 
SCOPED_OPENSSL_NO_PENDING_ERRORS would surface any error, as it does for the 
various 1.0 initialization calls. I guess I could CHECK on it though.


http://gerrit.cloudera.org:8080/#/c/10436/3/src/kudu/security/openssl_util.cc@117
PS3, Line 117:   // In case the user's thread has left some error around, clear 
it.
             :   ERR_clear_error();
             :   SCOPED_OPENSSL_NO_PENDING_ERRORS;
> I think it might be crucial to keep in on top, doing the clean-up even if g
Okay. But we can't do that for 1.1; the library must be initialized before any 
openssl error-related calls, otherwise we may leak memory.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If1e1c57b5563d1a4cd926b4c4a9a3c271460be04
Gerrit-Change-Number: 10436
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <danburk...@apache.org>
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-Comment-Date: Thu, 17 May 2018 21:48:37 +0000
Gerrit-HasComments: Yes

Reply via email to