You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by ro...@apache.org on 2014/07/26 22:27:28 UTC
[1/3] git commit: Add unit test for OpenSSL manual initialization
Repository: thrift
Updated Branches:
refs/heads/master 8345772f2 -> bee7b7380
Add unit test for OpenSSL manual initialization
This test checks whether Thrift leaves OpenSSL functionality available
after the last TSSLSocketFactory is destroyed when manual
initialization is set. It uses the EVP_get_digestbyname function as
an example function that requires OpenSSL initialization to work
properly.
Signed-off-by: Alan Dunn <am...@gmail.com>
Signed-off-by: Roger Meier <ro...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/thrift/repo
Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/bee7b738
Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/bee7b738
Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/bee7b738
Branch: refs/heads/master
Commit: bee7b738025ea7f2fa861a9567570ca502468c46
Parents: c0ff556
Author: Alan Dunn <am...@gmail.com>
Authored: Sat Jul 26 13:48:43 2014 -0500
Committer: Roger Meier <ro...@apache.org>
Committed: Sat Jul 26 22:13:55 2014 +0200
----------------------------------------------------------------------
.gitignore | 1 +
lib/cpp/test/Makefile.am | 11 ++++-
lib/cpp/test/OpenSSLManualInitTest.cpp | 77 +++++++++++++++++++++++++++++
3 files changed, 88 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/thrift/blob/bee7b738/.gitignore
----------------------------------------------------------------------
diff --git a/.gitignore b/.gitignore
index 9f652a3..c8c421e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -89,6 +89,7 @@ test-driver
/lib/cpp/test/TransportTest
/lib/cpp/test/UnitTests
/lib/cpp/test/ZlibTest
+/lib/cpp/test/OpenSSLManualInitTest
/lib/cpp/test/concurrency_test
/lib/cpp/test/link_test
/lib/cpp/test/processor_test
http://git-wip-us.apache.org/repos/asf/thrift/blob/bee7b738/lib/cpp/test/Makefile.am
----------------------------------------------------------------------
diff --git a/lib/cpp/test/Makefile.am b/lib/cpp/test/Makefile.am
index 3c97452..6779ac6 100755
--- a/lib/cpp/test/Makefile.am
+++ b/lib/cpp/test/Makefile.am
@@ -64,7 +64,8 @@ check_PROGRAMS = \
ZlibTest \
TFileTransportTest \
UnitTests \
- link_test
+ link_test \
+ OpenSSLManualInitTest
# disable these test ... too strong
# processor_test
# concurrency_test
@@ -215,6 +216,14 @@ processor_test_LDADD = libprocessortest.la \
$(BOOST_LDFLAGS) \
-levent \
-l:libboost_unit_test_framework.a
+
+OpenSSLManualInitTest_SOURCES = \
+ OpenSSLManualInitTest.cpp
+
+OpenSSLManualInitTest_LDADD = \
+ $(top_builddir)/lib/cpp/libthrift.la \
+ -l:libboost_unit_test_framework.a
+
#
# Common thrift code generation rules
#
http://git-wip-us.apache.org/repos/asf/thrift/blob/bee7b738/lib/cpp/test/OpenSSLManualInitTest.cpp
----------------------------------------------------------------------
diff --git a/lib/cpp/test/OpenSSLManualInitTest.cpp b/lib/cpp/test/OpenSSLManualInitTest.cpp
new file mode 100644
index 0000000..c403b18
--- /dev/null
+++ b/lib/cpp/test/OpenSSLManualInitTest.cpp
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+// To show that this test actually tests something, you can change
+// MANUAL_OPENSSL_INIT to 0 to cause automatic OpenSSL init/cleanup,
+// which will cause the test to fail
+#define MANUAL_OPENSSL_INIT 1
+
+#include <boost/test/unit_test.hpp>
+
+#include <openssl/evp.h>
+
+#include <thrift/transport/TSSLSocket.h>
+
+using namespace std;
+using namespace apache::thrift::transport;
+
+void make_isolated_sslsocketfactory() {
+ // Here we create an isolated TSSLSocketFactory to ensure the
+ // constructor and destructor of TSSLSocketFactory get run. Thus
+ // without manual initialization normally OpenSSL would be
+ // uninitialized after this function.
+ TSSLSocketFactory factory;
+}
+
+void openssl_init() {
+#if MANUAL_OPENSSL_INIT
+ TSSLSocketFactory::setManualOpenSSLInitialization(true);
+ initializeOpenSSL();
+#endif
+}
+
+void openssl_cleanup() {
+#if MANUAL_OPENSSL_INIT
+ cleanupOpenSSL();
+#endif
+}
+
+void test_openssl_availability() {
+ // Check whether Thrift leaves OpenSSL functionality available after
+ // the last TSSLSocketFactory is destroyed when manual
+ // initialization is set
+ openssl_init();
+ make_isolated_sslsocketfactory();
+
+ // The following function is one that will fail if OpenSSL is
+ // uninitialized. It might also fail on very old versions of
+ // OpenSSL...
+ const EVP_MD* md = EVP_get_digestbyname("SHA256");
+ BOOST_CHECK(md != NULL);
+ openssl_cleanup();
+}
+
+boost::unit_test::test_suite* init_unit_test_suite(int argc, char* argv[]) {
+ boost::unit_test::test_suite* suite =
+ &boost::unit_test::framework::master_test_suite();
+ suite->p_name.value = "OpenSSLManualInit";
+
+ suite->add(BOOST_TEST_CASE(test_openssl_availability));
+
+ return NULL;
+}
[2/3] git commit: Add ability to take control of OpenSSL
initialization
Posted by ro...@apache.org.
Add ability to take control of OpenSSL initialization
Signed-off-by: Alan Dunn <am...@gmail.com>
Signed-off-by: Roger Meier <ro...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/thrift/repo
Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/c0ff5561
Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/c0ff5561
Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/c0ff5561
Branch: refs/heads/master
Commit: c0ff5561fefa4b690d6c72ac7d792f9a1e480bda
Parents: 8953e70
Author: Alan Dunn <am...@gmail.com>
Authored: Sat Jul 26 13:44:24 2014 -0500
Committer: Roger Meier <ro...@apache.org>
Committed: Sat Jul 26 22:13:55 2014 +0200
----------------------------------------------------------------------
lib/cpp/src/thrift/transport/TSSLSocket.cpp | 7 +++++--
lib/cpp/src/thrift/transport/TSSLSocket.h | 16 +++++++++++-----
2 files changed, 16 insertions(+), 7 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/thrift/blob/c0ff5561/lib/cpp/src/thrift/transport/TSSLSocket.cpp
----------------------------------------------------------------------
diff --git a/lib/cpp/src/thrift/transport/TSSLSocket.cpp b/lib/cpp/src/thrift/transport/TSSLSocket.cpp
index 4b36f8c..fd285db 100644
--- a/lib/cpp/src/thrift/transport/TSSLSocket.cpp
+++ b/lib/cpp/src/thrift/transport/TSSLSocket.cpp
@@ -460,11 +460,14 @@ void TSSLSocket::authorize() {
// TSSLSocketFactory implementation
uint64_t TSSLSocketFactory::count_ = 0;
Mutex TSSLSocketFactory::mutex_;
+bool TSSLSocketFactory::manualOpenSSLInitialization_ = false;
TSSLSocketFactory::TSSLSocketFactory(const SSLProtocol& protocol): server_(false) {
Guard guard(mutex_);
if (count_ == 0) {
- initializeOpenSSL();
+ if (!manualOpenSSLInitialization_) {
+ initializeOpenSSL();
+ }
randomize();
}
count_++;
@@ -475,7 +478,7 @@ TSSLSocketFactory::~TSSLSocketFactory() {
Guard guard(mutex_);
ctx_.reset();
count_--;
- if (count_ == 0) {
+ if (count_ == 0 && !manualOpenSSLInitialization_) {
cleanupOpenSSL();
}
}
http://git-wip-us.apache.org/repos/asf/thrift/blob/c0ff5561/lib/cpp/src/thrift/transport/TSSLSocket.h
----------------------------------------------------------------------
diff --git a/lib/cpp/src/thrift/transport/TSSLSocket.h b/lib/cpp/src/thrift/transport/TSSLSocket.h
index eca9591..a4b805b 100644
--- a/lib/cpp/src/thrift/transport/TSSLSocket.h
+++ b/lib/cpp/src/thrift/transport/TSSLSocket.h
@@ -44,15 +44,17 @@ enum SSLProtocol {
/**
* Initialize OpenSSL library. This function, or some other
* equivalent function to initialize OpenSSL, must be called before
- * TSSLSocket is used. Currently TSSLSocketFactory automatically
- * calls this function, so you should not.
+ * TSSLSocket is used. If you set TSSLSocketFactory to use manual
+ * OpenSSL initialization, you should call this function or otherwise
+ * ensure OpenSSL is initialized yourself.
*/
void initializeOpenSSL();
/**
* Cleanup OpenSSL library. This function should be called to clean
- * up OpenSSL after use of OpenSSL functionality is finished.
- * Currently TSSLSocketFactory automatically calls this function, so
- * you should not.
+ * up OpenSSL after use of OpenSSL functionality is finished. If you
+ * set TSSLSocketFactory to use manual OpenSSL initialization, you
+ * should call this function yourself or ensure that whatever
+ * initialized OpenSSL cleans it up too.
*/
void cleanupOpenSSL();
@@ -216,6 +218,9 @@ class TSSLSocketFactory {
virtual void access(boost::shared_ptr<AccessManager> manager) {
access_ = manager;
}
+ static void setManualOpenSSLInitialization(bool manualOpenSSLInitialization) {
+ manualOpenSSLInitialization_ = manualOpenSSLInitialization;
+ }
protected:
boost::shared_ptr<SSLContext> ctx_;
@@ -232,6 +237,7 @@ class TSSLSocketFactory {
boost::shared_ptr<AccessManager> access_;
static concurrency::Mutex mutex_;
static uint64_t count_;
+ static bool manualOpenSSLInitialization_;
void setup(boost::shared_ptr<TSSLSocket> ssl);
static int passwordCallback(char* password, int size, int, void* data);
};
[3/3] git commit: Expose OpenSSL initialization functions
Posted by ro...@apache.org.
Expose OpenSSL initialization functions
Otherwise, commit is a logical no-op; it keeps the same OpenSSL
initialization behavior as before. Move the SSL initialization
functionality to one place to make it easier to track.
Signed-off-by: Alan Dunn <am...@gmail.com>
Signed-off-by: Roger Meier <ro...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/thrift/repo
Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/8953e701
Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/8953e701
Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/8953e701
Branch: refs/heads/master
Commit: 8953e7016a42de9dc45af92799245e1033575318
Parents: 8345772
Author: Alan Dunn <am...@gmail.com>
Authored: Sat Jul 26 13:41:04 2014 -0500
Committer: Roger Meier <ro...@apache.org>
Committed: Sat Jul 26 22:13:55 2014 +0200
----------------------------------------------------------------------
lib/cpp/src/thrift/transport/TSSLSocket.cpp | 161 ++++++++++++-----------
lib/cpp/src/thrift/transport/TSSLSocket.h | 18 ++-
2 files changed, 96 insertions(+), 83 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/thrift/blob/8953e701/lib/cpp/src/thrift/transport/TSSLSocket.cpp
----------------------------------------------------------------------
diff --git a/lib/cpp/src/thrift/transport/TSSLSocket.cpp b/lib/cpp/src/thrift/transport/TSSLSocket.cpp
index 9a8c758..4b36f8c 100644
--- a/lib/cpp/src/thrift/transport/TSSLSocket.cpp
+++ b/lib/cpp/src/thrift/transport/TSSLSocket.cpp
@@ -49,6 +49,87 @@ struct CRYPTO_dynlock_value {
namespace apache { namespace thrift { namespace transport {
+// OpenSSL initialization/cleanup
+
+static bool openSSLInitialized = false;
+static boost::shared_array<Mutex> mutexes;
+
+static void callbackLocking(int mode, int n, const char*, int) {
+ if (mode & CRYPTO_LOCK) {
+ mutexes[n].lock();
+ } else {
+ mutexes[n].unlock();
+ }
+}
+
+#if (OPENSSL_VERSION_NUMBER < OPENSSL_VERSION_NO_THREAD_ID)
+static unsigned long callbackThreadID() {
+ return (unsigned long) pthread_self();
+}
+#endif
+
+static CRYPTO_dynlock_value* dyn_create(const char*, int) {
+ return new CRYPTO_dynlock_value;
+}
+
+static void dyn_lock(int mode,
+ struct CRYPTO_dynlock_value* lock,
+ const char*, int) {
+ if (lock != NULL) {
+ if (mode & CRYPTO_LOCK) {
+ lock->mutex.lock();
+ } else {
+ lock->mutex.unlock();
+ }
+ }
+}
+
+static void dyn_destroy(struct CRYPTO_dynlock_value* lock, const char*, int) {
+ delete lock;
+}
+
+void initializeOpenSSL() {
+ if (openSSLInitialized) {
+ return;
+ }
+ openSSLInitialized = true;
+ SSL_library_init();
+ SSL_load_error_strings();
+ // static locking
+ mutexes = boost::shared_array<Mutex>(new Mutex[::CRYPTO_num_locks()]);
+ if (mutexes == NULL) {
+ throw TTransportException(TTransportException::INTERNAL_ERROR,
+ "initializeOpenSSL() failed, "
+ "out of memory while creating mutex array");
+ }
+#if (OPENSSL_VERSION_NUMBER < OPENSSL_VERSION_NO_THREAD_ID)
+ CRYPTO_set_id_callback(callbackThreadID);
+#endif
+ CRYPTO_set_locking_callback(callbackLocking);
+ // dynamic locking
+ CRYPTO_set_dynlock_create_callback(dyn_create);
+ CRYPTO_set_dynlock_lock_callback(dyn_lock);
+ CRYPTO_set_dynlock_destroy_callback(dyn_destroy);
+}
+
+void cleanupOpenSSL() {
+ if (!openSSLInitialized) {
+ return;
+ }
+ openSSLInitialized = false;
+#if (OPENSSL_VERSION_NUMBER < OPENSSL_VERSION_NO_THREAD_ID)
+ CRYPTO_set_id_callback(NULL);
+#endif
+ CRYPTO_set_locking_callback(NULL);
+ CRYPTO_set_dynlock_create_callback(NULL);
+ CRYPTO_set_dynlock_lock_callback(NULL);
+ CRYPTO_set_dynlock_destroy_callback(NULL);
+ CRYPTO_cleanup_all_ex_data();
+ ERR_free_strings();
+ EVP_cleanup();
+ ERR_remove_state(0);
+ mutexes.reset();
+}
static void buildErrors(string& message, int error = 0);
static bool matchName(const char* host, const char* pattern, int size);
@@ -377,7 +458,6 @@ void TSSLSocket::authorize() {
}
// TSSLSocketFactory implementation
-bool TSSLSocketFactory::initialized = false;
uint64_t TSSLSocketFactory::count_ = 0;
Mutex TSSLSocketFactory::mutex_;
@@ -520,85 +600,6 @@ int TSSLSocketFactory::passwordCallback(char* password,
return length;
}
-static boost::shared_array<Mutex> mutexes;
-
-static void callbackLocking(int mode, int n, const char*, int) {
- if (mode & CRYPTO_LOCK) {
- mutexes[n].lock();
- } else {
- mutexes[n].unlock();
- }
-}
-
-#if (OPENSSL_VERSION_NUMBER < OPENSSL_VERSION_NO_THREAD_ID)
-static unsigned long callbackThreadID() {
- return (unsigned long) pthread_self();
-}
-#endif
-
-static CRYPTO_dynlock_value* dyn_create(const char*, int) {
- return new CRYPTO_dynlock_value;
-}
-
-static void dyn_lock(int mode,
- struct CRYPTO_dynlock_value* lock,
- const char*, int) {
- if (lock != NULL) {
- if (mode & CRYPTO_LOCK) {
- lock->mutex.lock();
- } else {
- lock->mutex.unlock();
- }
- }
-}
-
-static void dyn_destroy(struct CRYPTO_dynlock_value* lock, const char*, int) {
- delete lock;
-}
-
-void TSSLSocketFactory::initializeOpenSSL() {
- if (initialized) {
- return;
- }
- initialized = true;
- SSL_library_init();
- SSL_load_error_strings();
- // static locking
- mutexes = boost::shared_array<Mutex>(new Mutex[::CRYPTO_num_locks()]);
- if (mutexes == NULL) {
- throw TTransportException(TTransportException::INTERNAL_ERROR,
- "initializeOpenSSL() failed, "
- "out of memory while creating mutex array");
- }
-#if (OPENSSL_VERSION_NUMBER < OPENSSL_VERSION_NO_THREAD_ID)
- CRYPTO_set_id_callback(callbackThreadID);
-#endif
- CRYPTO_set_locking_callback(callbackLocking);
- // dynamic locking
- CRYPTO_set_dynlock_create_callback(dyn_create);
- CRYPTO_set_dynlock_lock_callback(dyn_lock);
- CRYPTO_set_dynlock_destroy_callback(dyn_destroy);
-}
-
-void TSSLSocketFactory::cleanupOpenSSL() {
- if (!initialized) {
- return;
- }
- initialized = false;
-#if (OPENSSL_VERSION_NUMBER < OPENSSL_VERSION_NO_THREAD_ID)
- CRYPTO_set_id_callback(NULL);
-#endif
- CRYPTO_set_locking_callback(NULL);
- CRYPTO_set_dynlock_create_callback(NULL);
- CRYPTO_set_dynlock_lock_callback(NULL);
- CRYPTO_set_dynlock_destroy_callback(NULL);
- CRYPTO_cleanup_all_ex_data();
- ERR_free_strings();
- EVP_cleanup();
- ERR_remove_state(0);
- mutexes.reset();
-}
-
// extract error messages from error queue
void buildErrors(string& errors, int errno_copy) {
unsigned long errorCode;
http://git-wip-us.apache.org/repos/asf/thrift/blob/8953e701/lib/cpp/src/thrift/transport/TSSLSocket.h
----------------------------------------------------------------------
diff --git a/lib/cpp/src/thrift/transport/TSSLSocket.h b/lib/cpp/src/thrift/transport/TSSLSocket.h
index 7c19206..eca9591 100644
--- a/lib/cpp/src/thrift/transport/TSSLSocket.h
+++ b/lib/cpp/src/thrift/transport/TSSLSocket.h
@@ -42,6 +42,21 @@ enum SSLProtocol {
/**
+ * Initialize OpenSSL library. This function, or some other
+ * equivalent function to initialize OpenSSL, must be called before
+ * TSSLSocket is used. Currently TSSLSocketFactory automatically
+ * calls this function, so you should not.
+ */
+void initializeOpenSSL();
+/**
+ * Cleanup OpenSSL library. This function should be called to clean
+ * up OpenSSL after use of OpenSSL functionality is finished.
+ * Currently TSSLSocketFactory automatically calls this function, so
+ * you should not.
+ */
+void cleanupOpenSSL();
+
+/**
* OpenSSL implementation for SSL socket interface.
*/
class TSSLSocket: public TSocket {
@@ -204,8 +219,6 @@ class TSSLSocketFactory {
protected:
boost::shared_ptr<SSLContext> ctx_;
- static void initializeOpenSSL();
- static void cleanupOpenSSL();
/**
* Override this method for custom password callback. It may be called
* multiple times at any time during a session as necessary.
@@ -217,7 +230,6 @@ class TSSLSocketFactory {
private:
bool server_;
boost::shared_ptr<AccessManager> access_;
- static bool initialized;
static concurrency::Mutex mutex_;
static uint64_t count_;
void setup(boost::shared_ptr<TSSLSocket> ssl);