Matthew Jacobs has posted comments on this change. Change subject: KUDU-1738. Allow users of the client to disable OpenSSL initialization ......................................................................
Patch Set 4: (1 comment) I think this API should work, though Impala will have to make some changes as it looks like our OpenSSL configuration is a mess right now. We'll need to: 1) update squeasel to get your change to disable ssl init, and disable the squeasel init 2) change our process startup to force thrift to do its ssl init early 3) potentially add a follow up call to set the function ID callback, though I'm curious to see what you say to my comment in openssl_util.cc. 4) add a call to the kudu disable ssl fn. Does that smell right to you? Thanks 3) potentially http://gerrit.cloudera.org:8080/#/c/5992/4/src/kudu/security/openssl_util.cc File src/kudu/security/openssl_util.cc: PS4, Line 75: if (!CRYPTO_THREADID_get_callback()) { : return Status::RuntimeError("Thread-ID callback not initialized"); : } Does this need to be a requirement? Impala relies on thrift to initialize SSL currently, and that code [1] won't end up setting this fn. However, according to the OpenSSL doc, the address of errno is used if this isn't called and that should work fine as long as errno itself is threadsafe, which should be true [2]. 1: https://github.com/apache/thrift/blob/0.9.0/lib/cpp/src/thrift/transport/TSSLSocket.cpp#L527 2: http://stackoverflow.com/questions/1694164/is-errno-thread-safe It's fine if you say you don't feel comfortable relying on this, though it seems like it could be up to the consumer to determine if this is good enough for MT OpenSSL or not. -- To view, visit http://gerrit.cloudera.org:8080/5992 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I43eab5c848b30362356422d0380a336f16587562 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Jordan Birdsell <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
