You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by ad...@apache.org on 2019/02/13 09:07:29 UTC
[kudu] branch branch-1.9.x updated (320a799 -> c7a994d)
This is an automated email from the ASF dual-hosted git repository.
adar pushed a change to branch branch-1.9.x
in repository https://gitbox.apache.org/repos/asf/kudu.git.
from 320a799 [client] Fix GetUnscaledDecimal backward compatiblity
new caaaa5e KUDU-2411: fix dep pattern matching in relocation script
new 14e50e3 [security] KUDU-2695 fix CheckOpenSSLInitialized()
new c7a994d [CMakeLists] get rid of warnings in ASAN builds
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
CMakeLists.txt | 2 +-
.../relocate_binaries_for_mini_cluster.py | 2 +-
src/kudu/client/client-unittest.cc | 17 +++++++++--
src/kudu/security/openssl_util.cc | 34 ++++++++++++++++++----
4 files changed, 45 insertions(+), 10 deletions(-)
[kudu] 02/03: [security] KUDU-2695 fix CheckOpenSSLInitialized()
Posted by ad...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
adar pushed a commit to branch branch-1.9.x
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 14e50e38d80d50be5785a0576e6b86a90b0a77f5
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Mon Feb 11 12:15:12 2019 -0800
[security] KUDU-2695 fix CheckOpenSSLInitialized()
With OpenSSL 1.1.0, the CRYPTO_get_locking_callback() and the whole old
threading API has been removed. With the new threading API in OpenSSL,
there is no need to set locking callbacks. In other words, starting
with OpenSSL 1.1.0, the library is initialized to be multithread-safe,
and that's exactly what Kudu needs from the OpenSSL's initialization.
Prior to this patch, the sample C++ Kudu client application from
$KUDU_ROOT/examples/cpp
with added call of
KUDU_CHECK_OK(kudu::client::DisableOpenSSLInitialization());
would fail on Ubuntu 18.04 with error message like below:
Bad status: Runtime error: Locking callback not initialized
Prior to this patch, the sample Python Kudu client application from
$KUDU_HOME/examples/python/basic-python-example/basic_example.py
was failing exactly as reported in KUDU-2695.
With this patch, the same modified C++ Kudu client application
works fine at Ubuntu 18.04 (OpenSSL 1.1.0g, with packages
libssl-dev:amd64@1.1.0g-2ubuntu4.3, libssl1.1:amd64@1.1.0g-2ubuntu4.3).
The above mentioned Python example also works as intended with this fix.
I also verified that the kudu CLI utility works fine and uses TLS wire
encryption with this patch.
Change-Id: Ica7cf22ef81bbeffd25ef2826d925c41b97dc2d8
Reviewed-on: http://gerrit.cloudera.org:8080/12445
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
(cherry picked from commit 1b138a06a4222ad822a2b5cf05dcd0bf988371ef)
Reviewed-on: http://gerrit.cloudera.org:8080/12464
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
src/kudu/client/client-unittest.cc | 17 ++++++++++++++---
src/kudu/security/openssl_util.cc | 34 +++++++++++++++++++++++++++++-----
2 files changed, 43 insertions(+), 8 deletions(-)
diff --git a/src/kudu/client/client-unittest.cc b/src/kudu/client/client-unittest.cc
index bd8126a..3112ca8 100644
--- a/src/kudu/client/client-unittest.cc
+++ b/src/kudu/client/client-unittest.cc
@@ -26,6 +26,7 @@
#include <boost/bind.hpp>
#include <boost/function.hpp>
#include <gtest/gtest.h>
+#include <openssl/opensslv.h>
#include "kudu/client/client-internal.h"
#include "kudu/client/error_collector.h"
@@ -168,10 +169,20 @@ TEST(ClientUnitTest, TestSchemaBuilder_CompoundKey_BadColumnName) {
}
TEST(ClientUnitTest, TestDisableSslFailsIfNotInitialized) {
- // If we try to disable SSL initialization without setting up SSL properly,
- // it should return an error.
- Status s = DisableOpenSSLInitialization();
+ const auto s = DisableOpenSSLInitialization();
+#if OPENSSL_VERSION_NUMBER < 0x10100000L
+ // With the pre-1.1.0 OpenSSL library, if we try to disable SSL
+ // initialization without setting up SSL properly, it should return an error.
ASSERT_STR_MATCHES(s.ToString(), "Locking callback not initialized");
+#else
+ // Starting with OpenSSL 1.1.0, the library can be implicitly initialized
+ // upon calling the relevant methods of the API (e.g. SSL_CTX_new()) and
+ // overall there is no reliable non-intrusive way to determine that the
+ // library has already been initialized. So, the requirement to have
+ // the library initialized before calling DisableOpenSSLInitialization()
+ // is gone since OpenSSL 1.1.0.
+ ASSERT_TRUE(s.ok()) << s.ToString();
+#endif
}
namespace {
diff --git a/src/kudu/security/openssl_util.cc b/src/kudu/security/openssl_util.cc
index 1576ea8..e96fcb3 100644
--- a/src/kudu/security/openssl_util.cc
+++ b/src/kudu/security/openssl_util.cc
@@ -82,25 +82,49 @@ void LockingCB(int mode, int type, const char* /*file*/, int /*line*/) {
#endif
Status CheckOpenSSLInitialized() {
+#if OPENSSL_VERSION_NUMBER < 0x10100000L
+ // Starting with OpenSSL 1.1.0, the old thread API became obsolete
+ // (see changelist 2e52e7df5 in the OpenSSL upstream repo), and
+ // CRYPTO_get_locking_callback() always returns nullptr. Also, the library
+ // always initializes its internals for multi-threaded usage.
+ // Another point is that starting with version 1.1.0, SSL_CTX_new()
+ // initializes the OpenSSL library under the hood, so SSL_CTX_new() would
+ // not return nullptr unless there was an error during the initialization
+ // of the library. That makes this code in CheckOpenSSLInitialized() obsolete
+ // starting with OpenSSL version 1.1.0.
+ //
+ // Starting with OpenSSL 1.1.0, there isn't a straightforward way to
+ // determine whether the library has already been initialized if using just
+ // the API (well, there is CRYPTO_secure_malloc_initialized() but that's just
+ // for the crypto library and it's implementation-dependent). But from the
+ // other side, the whole idea that this method should check whether the
+ // library has already been initialized is not relevant anymore: even if it's
+ // not yet initialized, the first call to SSL_CTX_new() (from, say,
+ // TlsContext::Init()) will initialize the library under the hood, so the
+ // library will be ready for multi-thread usage by Kudu.
if (!CRYPTO_get_locking_callback()) {
return Status::RuntimeError("Locking callback not initialized");
}
auto ctx = ssl_make_unique(SSL_CTX_new(SSLv23_method()));
if (!ctx) {
ERR_clear_error();
- return Status::RuntimeError("SSL library appears uninitialized (cannot create SSL_CTX)");
+ return Status::RuntimeError(
+ "SSL library appears uninitialized (cannot create SSL_CTX)");
}
+#endif
return Status::OK();
}
void DoInitializeOpenSSL() {
-#if OPENSSL_VERSION_NUMBER > 0x10100000L
+#if OPENSSL_VERSION_NUMBER >= 0x10100000L
// The OPENSSL_init_ssl manpage [1] says "As of version 1.1.0 OpenSSL will
// automatically allocate all resources it needs so no explicit initialisation
// is required." However, eliding library initialization leads to a memory
- // leak in some versions of OpenSSL 1.1 when the first OpenSSL is
- // ERR_peek_error [2]. In Kudu this is often the
- // case due to prolific application of SCOPED_OPENSSL_NO_PENDING_ERRORS.
+ // leak in some versions of OpenSSL 1.1 when the first OpenSSL call is
+ // ERR_peek_error (see [2] for details; the issue was addressed in OpenSSL
+ // 1.1.0i (OPENSSL_VERSION_NUMBER 0x1010009f)). In Kudu this is often the
+ // case due to prolific application of the SCOPED_OPENSSL_NO_PENDING_ERRORS
+ // macro.
//
// Rather than determine whether this particular OpenSSL instance is
// leak-free, we'll initialize the library explicitly.
[kudu] 03/03: [CMakeLists] get rid of warnings in ASAN builds
Posted by ad...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
adar pushed a commit to branch branch-1.9.x
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit c7a994dab825868c757b7f3b9070bc92f6befe2c
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Tue Feb 12 11:38:44 2019 -0800
[CMakeLists] get rid of warnings in ASAN builds
This patch updates the flags for ASAN builds to get rid of the
warnings like below:
clang-6.0: warning: argument '-fno-sanitize-recover' is deprecated, \
use '-fno-sanitize-recover=undefined,integer' or \
'-fno-sanitize-recover=all' instead [-Wdeprecated]
This is a follow-up to 678dbac6fb05d0370e40e6645d4b1ec530fa0180.
Change-Id: I0a5877ed1743387881ab1d31f42519a361b64ea7
Reviewed-on: http://gerrit.cloudera.org:8080/12459
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
(cherry picked from commit 5cfd81f7ed89b46dfdacc2389159fadbb59f0d8b)
Reviewed-on: http://gerrit.cloudera.org:8080/12466
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
CMakeLists.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 7a1c16c..c9b0394 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -353,7 +353,7 @@ if (${KUDU_USE_UBSAN})
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-sanitize=alignment")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize-blacklist=${CMAKE_CURRENT_SOURCE_DIR}/build-support/ubsan-blacklist.txt")
# Stop execution after UB or overflow is detected.
- set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-sanitize-recover")
+ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-sanitize-recover=undefined,integer")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DUNDEFINED_SANITIZER")
endif ()
[kudu] 01/03: KUDU-2411: fix dep pattern matching in relocation
script
Posted by ad...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
adar pushed a commit to branch branch-1.9.x
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit caaaa5e148106aea33a9d3aa1f70c51620321e7e
Author: Adar Dembo <ad...@cloudera.com>
AuthorDate: Tue Feb 12 12:07:32 2019 -0800
KUDU-2411: fix dep pattern matching in relocation script
The trailing '|' caused all shared objects except for the sasl2 modules to
be excluded. The resulting archive wasn't usable.
Change-Id: If90f95bd7a1a8f8c2a3f6441d047096542f1f048
Reviewed-on: http://gerrit.cloudera.org:8080/12460
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
build-support/mini-cluster/relocate_binaries_for_mini_cluster.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/build-support/mini-cluster/relocate_binaries_for_mini_cluster.py b/build-support/mini-cluster/relocate_binaries_for_mini_cluster.py
index cb6dfef..30ecdba 100755
--- a/build-support/mini-cluster/relocate_binaries_for_mini_cluster.py
+++ b/build-support/mini-cluster/relocate_binaries_for_mini_cluster.py
@@ -66,7 +66,7 @@ PAT_LINUX_LIB_EXCLUDE = re.compile(r"""(libpthread|
libcom_err|
libdb-[\d.]+|
libselinux|
- libtinfo|
+ libtinfo
)\.so""", re.VERBOSE)
# We don't want to ship libSystem because it includes kernel and thread