Repository: kudu Updated Branches: refs/heads/master 1b89ec813 -> d0270172e
Work around another OpenSSL thread safety bug In the course of debugging some CHECK failures and TSAN errors, I found that older versions of OpenSSL have non-threadsafe OBJ_create and even ERR_peek_error methods. This commit fixes an instance where we were calling OBJ_create concurrently by wrapping it in a std::call_once. I don't have a fix for ERR_peek_err unsafety, since that's used pervasively in most methods touching OpenSSL. Side note: for debugging issues like this, I find it helpful to run ASAN with the following options: ASAN_OPTIONS="fast_unwind_on_malloc=0" That option typically makes races more reproducible, and produces better stack traces as well. Change-Id: I9a9fe1a32f77bf24a5c7e692a55b8ad96488d409 Reviewed-on: http://gerrit.cloudera.org:8080/6997 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/d0270172 Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/d0270172 Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/d0270172 Branch: refs/heads/master Commit: d0270172e91bf6bcb045cb63a731609472fa6be4 Parents: 1b89ec8 Author: Dan Burkert <[email protected]> Authored: Thu May 25 16:24:17 2017 -0700 Committer: Alexey Serbin <[email protected]> Committed: Sat May 27 00:45:13 2017 +0000 ---------------------------------------------------------------------- src/kudu/security/cert-test.cc | 24 ++++++++++++++++++++++++ src/kudu/security/cert.cc | 13 +++++++------ 2 files changed, 31 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/d0270172/src/kudu/security/cert-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/security/cert-test.cc b/src/kudu/security/cert-test.cc index 275a60b..acd0f74 100644 --- a/src/kudu/security/cert-test.cc +++ b/src/kudu/security/cert-test.cc @@ -15,7 +15,9 @@ // specific language governing permissions and limitations // under the License. +#include <thread> #include <utility> +#include <vector> #include <boost/optional.hpp> #include <boost/optional/optional_io.hpp> @@ -25,11 +27,14 @@ #include "kudu/security/crypto.h" #include "kudu/security/openssl_util.h" #include "kudu/security/test/test_certs.h" +#include "kudu/util/barrier.h" #include "kudu/util/status.h" #include "kudu/util/test_macros.h" #include "kudu/util/test_util.h" using std::pair; +using std::thread; +using std::vector; namespace kudu { namespace security { @@ -60,6 +65,25 @@ class CertTest : public KuduTest { PrivateKey ca_exp_private_key_; }; +// Regression test to make sure that GetKuduKerberosPrincipalOidNid is thread +// safe. OpenSSL 1.0.0's OBJ_create method is not thread safe. +TEST_F(CertTest, GetKuduKerberosPrincipalOidNidConcurrent) { + int kConcurrency = 16; + Barrier barrier(kConcurrency); + + vector<thread> threads; + for (int i = 0; i < kConcurrency; i++) { + threads.emplace_back([&] () { + barrier.Wait(); + CHECK_NE(NID_undef, GetKuduKerberosPrincipalOidNid()); + }); + } + + for (auto& thread : threads) { + thread.join(); + } +} + // Check input/output of the X509 certificates in PEM format. TEST_F(CertTest, CertInputOutputPEM) { const Cert& cert = ca_cert_; http://git-wip-us.apache.org/repos/asf/kudu/blob/d0270172/src/kudu/security/cert.cc ---------------------------------------------------------------------- diff --git a/src/kudu/security/cert.cc b/src/kudu/security/cert.cc index cde4e47..fa4c753 100644 --- a/src/kudu/security/cert.cc +++ b/src/kudu/security/cert.cc @@ -17,6 +17,7 @@ #include "kudu/security/cert.h" +#include <mutex> #include <string> #include <openssl/evp.h> @@ -57,12 +58,12 @@ string X509NameToString(X509_NAME* name) { int GetKuduKerberosPrincipalOidNid() { InitializeOpenSSL(); - SCOPED_OPENSSL_NO_PENDING_ERRORS; - - int nid = OBJ_txt2nid(kKuduKerberosPrincipalOidStr); - if (nid != NID_undef) return nid; - nid = OBJ_create(kKuduKerberosPrincipalOidStr, "kuduPrinc", "kuduKerberosPrincipal"); - CHECK_NE(nid, NID_undef) << "could not create kuduPrinc oid"; + static std::once_flag flag; + static int nid; + std::call_once(flag, [&] () { + nid = OBJ_create(kKuduKerberosPrincipalOidStr, "kuduPrinc", "kuduKerberosPrincipal"); + CHECK_NE(nid, NID_undef) << "failed to create kuduPrinc oid: " << GetOpenSSLErrors(); + }); return nid; }
