Guard SSLContext lock checks with init mutex and add setSSLLockTypesAndInitOpenSSL

Summary:
Improve concurrency guards for `setSSLLockTypes` and `isSSLLockDisabled` by using initMutex.  Also verify that openssl has been initialized w/ DCHECK in isSSLLockDisabled.

We also add a method to do the setting of locks and initialization in one shot.

Reviewed By: knekritz

Differential Revision: D4937242

fbshipit-source-id: 308f516c17485281604d4322954c09beb58688e2

diff --git a/folly/io/async/SSLContext.cpp b/folly/io/async/SSLContext.cpp
index 3abb5d0b8a307439a2ec09d3a7c19ac32f014c06..353ae298e5dc464ef6304c070f4d3cf232ed738b 100644 (file)
--- a/folly/io/async/SSLContext.cpp
+++ b/folly/io/async/SSLContext.cpp
@@ -793,18 +793,32 @@ static void dyn_destroy(struct CRYPTO_dynlock_value* lock, const char*, int) {
   delete lock;
 }
 
+void SSLContext::setSSLLockTypesLocked(std::map<int, SSLLockType> inLockTypes) {
+  lockTypes() = inLockTypes;
+}
+
 void SSLContext::setSSLLockTypes(std::map<int, SSLLockType> inLockTypes) {
+  std::lock_guard<std::mutex> g(initMutex());
   if (initialized_) {
     // We set the locks on initialization, so if we are already initialized
     // this would have no affect.
     LOG(INFO) << "Ignoring setSSLLockTypes after initialization";
     return;
   }
+  setSSLLockTypesLocked(std::move(inLockTypes));
+}
 
-  lockTypes() = inLockTypes;
+void SSLContext::setSSLLockTypesAndInitOpenSSL(
+    std::map<int, SSLLockType> inLockTypes) {
+  std::lock_guard<std::mutex> g(initMutex());
+  CHECK(!initialized_) << "OpenSSL is already initialized";
+  setSSLLockTypesLocked(std::move(inLockTypes));
+  initializeOpenSSLLocked();
 }
 
 bool SSLContext::isSSLLockDisabled(int lockId) {
+  std::lock_guard<std::mutex> g(initMutex());
+  CHECK(initialized_) << "OpenSSL is not initialized yet";
   const auto& sslLocks = lockTypes();
   const auto it = sslLocks.find(lockId);
   return it != sslLocks.end() &&

diff --git a/folly/io/async/SSLContext.h b/folly/io/async/SSLContext.h
index 5e539e0c20f4eefa69b32f80650bafa9059e396b..a381eb37e578a3f5c5fd49f66f5409f234fcf98b 100644 (file)
--- a/folly/io/async/SSLContext.h
+++ b/folly/io/async/SSLContext.h
@@ -449,6 +449,13 @@ class SSLContext {
    */
   static void setSSLLockTypes(std::map<int, SSLLockType> lockTypes);
 
+  /**
+   * Set the lock types and initialize OpenSSL in an atomic fashion.  This
+   * aborts if the library has already been initialized.
+   */
+  static void setSSLLockTypesAndInitOpenSSL(
+      std::map<int, SSLLockType> lockTypes);
+
   /**
    * Determine if the SSL lock with the specified id (i.e.
    * CRYPTO_LOCK_SSL_SESSION) is disabled.  This should be called after
@@ -594,6 +601,7 @@ class SSLContext {
   // Functions are called when locked by the calling function.
   static void initializeOpenSSLLocked();
   static void cleanupOpenSSLLocked();
+  static void setSSLLockTypesLocked(std::map<int, SSLLockType> inLockTypes);
 };
 
 typedef std::shared_ptr<SSLContext> SSLContextPtr;

diff --git a/folly/io/async/test/AsyncSSLSocketTest2.cpp b/folly/io/async/test/AsyncSSLSocketTest2.cpp
index 54454c9fbbaf75a7eb376926608e7485c955fde7..c221d5d1b50fd5c688ac018cbfaf83b7bdde21aa 100644 (file)
--- a/folly/io/async/test/AsyncSSLSocketTest2.cpp
+++ b/folly/io/async/test/AsyncSSLSocketTest2.cpp
@@ -213,6 +213,19 @@ TEST(AsyncSSLSocketTest2, SSLContextLocksSetAfterInitIgnored) {
 #endif
 }
 
+TEST(AsyncSSLSocketTest2, SSLContextSetLocksAndInitialize) {
+  SSLContext::cleanupOpenSSL();
+  SSLContext::setSSLLockTypesAndInitOpenSSL({});
+  EXPECT_DEATH(
+      SSLContext::setSSLLockTypesAndInitOpenSSL({}),
+      "OpenSSL is already initialized");
+
+  SSLContext::cleanupOpenSSL();
+  SSLContext::initializeOpenSSL();
+  EXPECT_DEATH(
+      SSLContext::setSSLLockTypesAndInitOpenSSL({}),
+      "OpenSSL is already initialized");
+}
 }  // folly
 
 int main(int argc, char *argv[]) {