You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by da...@apache.org on 2017/07/11 00:06:13 UTC
kudu git commit: Work around another OpenSSL thread safety bug
Repository: kudu
Updated Branches:
refs/heads/branch-1.3.x 65c5ac592 -> 98b7f62bf
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 <as...@cloudera.com>
(cherry picked from commit d0270172e91bf6bcb045cb63a731609472fa6be4)
Reviewed-on: http://gerrit.cloudera.org:8080/7386
Reviewed-by: Todd Lipcon <to...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/98b7f62b
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/98b7f62b
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/98b7f62b
Branch: refs/heads/branch-1.3.x
Commit: 98b7f62bf7bbb832f4f3bc2c4d3ab01d3e97eab6
Parents: 65c5ac5
Author: Dan Burkert <da...@apache.org>
Authored: Thu May 25 16:24:17 2017 -0700
Committer: Todd Lipcon <to...@apache.org>
Committed: Mon Jul 10 17:46:28 2017 +0000
----------------------------------------------------------------------
src/kudu/security/cert-test.cc | 24 ++++++++++++++++++++++++
src/kudu/security/cert.cc | 12 +++++++-----
2 files changed, 31 insertions(+), 5 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/98b7f62b/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/98b7f62b/src/kudu/security/cert.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/cert.cc b/src/kudu/security/cert.cc
index a47c7b9..6cc1e96 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>
@@ -56,11 +57,12 @@ string X509NameToString(X509_NAME* name) {
int GetKuduKerberosPrincipalOidNid() {
InitializeOpenSSL();
-
- 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;
}