Workaround a leak in OpenSSL 1.0.0
This fixes another leak seen occasionally as a flaky test on RHEL 6:
Direct leak of 56 byte(s) in 1 object(s) allocated from:
#2 0x7f21e5fe037c in CRYPTO_malloc src/openssl/crypto/mem.c:306
#3 0x7f21e607f45c in EVP_PKEY_new src/openssl/crypto/evp/p_lib.c:186
#4 0x7f21e608ff33 in X509_PUBKEY_get src/openssl/crypto/asn1/x_pubkey.c:147
#5 0x7f21e60b23ad in X509_get_pubkey src/openssl/crypto/x509/x509_cmp.c:292
#6 0x7f21e60b623d in internal_verify src/openssl/crypto/x509/x509_vfy.c:1624
#7 0x7f21e60b3b8b in X509_verify_cert src/openssl/crypto/x509/x509_vfy.c:372
#8 0x7f21e5d5c096 in ssl_verify_cert_chain src/openssl/ssl/ssl_cert.c:535
#9 0x7f21e5d34157 in ssl3_get_server_certificate
src/openssl/ssl/s3_clnt.c:1047
#10 0x7f21e5d3289f in ssl3_connect src/openssl/ssl/s3_clnt.c:310
#11 0x7f21e5d5705b in SSL_connect src/openssl/ssl/ssl_lib.c:933
#12 0x7f21e5d43340 in ssl23_get_server_hello src/openssl/ssl/s23_clnt.c:693
#13 0x7f21e5d422b0 in ssl23_connect src/openssl/ssl/s23_clnt.c:222
#14 0x7f21e5d59f46 in SSL_do_handshake src/openssl/ssl/ssl_lib.c:2380
#15 0x7f21e6a028a7 in kudu::security::TlsHandshake::Continue(std::string
const&, std::string*) src/kudu/security/tls_handshake.cc:92:12
The issue turned out to be an OpenSSL 1.0.0 bug that was since fixed. But,
since we build/test against OpenSSL 1.0.0 on el6, we hit the leak.
This patch has a workaround - see the included comment for details.
Before the patch, all_types-itest failed about 10% of the time in dist-test,
and now passes reliably.
Change-Id: I65fe981c523f6fe73b3668bfb2f30a95ebf3e759
Reviewed-on: http://gerrit.cloudera.org:8080/6197
Reviewed-by: Dan Burkert <[email protected]>
Tested-by: Kudu Jenkins
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/0609277c
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/0609277c
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/0609277c
Branch: refs/heads/master
Commit: 0609277c4209c02fd94d7c6fd6c23abd8b1d3b67
Parents: f726f6a
Author: Todd Lipcon <[email protected]>
Authored: Tue Feb 28 18:53:35 2017 -0800
Committer: Todd Lipcon <[email protected]>
Committed: Wed Mar 1 18:57:13 2017 +0000
----------------------------------------------------------------------
src/kudu/security/tls_context.cc | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/0609277c/src/kudu/security/tls_context.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/tls_context.cc b/src/kudu/security/tls_context.cc
index 28c8e0e..17b7f69 100644
--- a/src/kudu/security/tls_context.cc
+++ b/src/kudu/security/tls_context.cc
@@ -223,6 +223,22 @@ Status TlsContext::UseCertificateAndKey(const Cert& cert,
const PrivateKey& key)
Status TlsContext::AddTrustedCertificate(const Cert& cert) {
VLOG(2) << "Trusting certificate " << cert.SubjectName();
+ {
+ // Workaround for a leak in OpenSSL <1.0.1:
+ //
+ // If we start trusting a cert, and its internal public-key field hasn't
+ // yet been populated, then the first time it's used for verification will
+ // populate it. In the case that two threads try to populate it at the
same time,
+ // one of the thread's copies will be leaked.
+ //
+ // To avoid triggering the race, we populate the internal public key cache
+ // field up front before adding it to the trust store.
+ //
+ // See OpenSSL commit 33a688e80674aaecfac6d9484ec199daa0ee5b61.
+ PublicKey k;
+ CHECK_OK(cert.GetPublicKey(&k));
+ }
+
unique_lock<RWMutex> lock(lock_);
ERR_clear_error();
auto* cert_store = SSL_CTX_get_cert_store(ctx_.get());
