[kudu] branch master updated: KUDU-3210 Add thread ID callback to OpenSSL init

[granthenke]

This is an automated email from the ASF dual-hosted git repository.

granthenke pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/master by this push:
     new d1285eb  KUDU-3210 Add thread ID callback to OpenSSL init
d1285eb is described below

commit d1285eb3855ca1125dab4aac318e8acaf3edf58a
Author: Attila Bukor <abu...@apache.org>
AuthorDate: Mon Nov 16 21:22:28 2020 +0100

    KUDU-3210 Add thread ID callback to OpenSSL init
    
    It seems the race condition bug we worked around in
    f9f3189a6dbe0636d578d80b1d8e60cf7b2e6686 was caused by using the default
    thread ID callback.
    
    It seems it's not a bug in SafeLogic after all, but this is likely
    reproducible in upstream OpenSSL as well. We didn't find this before as
    we always tested in older OpenSSL versions, while the commit[1]
    responsible for this behavior was included only in OpenSSL 1.0.2i[2].
    
    The threads(3) man page claims that "If the application does not
    register such a callback using CRYPTO_THREADID_set_callback(), then a
    default implementation is used - on Windows and BeOS this uses the
    system's default thread identifying APIs, and on all other platforms it
    uses the address of errno. The latter is satisfactory for thread-safety
    if and only if the platform has a thread-local error number facility."
    
    This seems to be no longer true in 1.0.2i and later.
    
    Redefining the thread ID callback seems to fix the problem without any
    additional locking and f9f3189a6dbe0636d578d80b1d8e60cf7b2e6686 can be
    reverted safely. I tested these changes on the host I discovered the
    race condition.
    
    [1] 
https://github.com/openssl/openssl/commit/a43cfd7bb1fc681d563e5efa75cc926d7e8e5c36
    [2] 
https://mta.openssl.org/pipermail/openssl-commits/2016-September/010743.html
    
    Change-Id: Icec6da3a9380206fe6ba4a31ea8fb4dcbc34dd00
    Reviewed-on: http://gerrit.cloudera.org:8080/16730
    Reviewed-by: Grant Henke <granthe...@apache.org>
    Reviewed-by: Alexey Serbin <aser...@cloudera.com>
    Tested-by: Kudu Jenkins
---
 src/kudu/security/openssl_util.cc | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/kudu/security/openssl_util.cc 
b/src/kudu/security/openssl_util.cc
index 3604964..f846048 100644
--- a/src/kudu/security/openssl_util.cc
+++ b/src/kudu/security/openssl_util.cc
@@ -50,6 +50,9 @@
 #include "kudu/util/scoped_cleanup.h"
 #include "kudu/util/status.h"
 #include "kudu/util/subprocess.h"
+#if OPENSSL_VERSION_NUMBER < 0x10100000L
+#include "kudu/util/thread.h"
+#endif
 
 DEFINE_bool(openssl_defensive_locking,
             false,
@@ -100,6 +103,10 @@ void LockingCB(int mode, int type, const char* /*file*/, 
int /*line*/) {
     m->unlock();
   }
 }
+
+void ThreadIdCB(CRYPTO_THREADID* tid) {
+  CRYPTO_THREADID_set_numeric(tid, Thread::UniqueThreadId());
+}
 #endif
 
 void CheckFIPSMode() {
@@ -212,6 +219,8 @@ void DoInitializeOpenSSL() {
 
     // Callbacks used by OpenSSL required in a multi-threaded setting.
     CRYPTO_set_locking_callback(LockingCB);
+
+    CRYPTO_THREADID_set_callback(ThreadIdCB);
   }
 #endif
   CheckFIPSMode();