From: Neel Goyal <ngoyal@fb.com>
Date: Tue, 25 Apr 2017 14:31:39 +0000 (-0700)
Subject: Guard SSLContext lock checks with init mutex and add setSSLLockTypesAndInitOpenSSL
X-Git-Tag: v2017.05.01.00~13
X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=5699919fdcb518d9974a93c7f1791d3e8ac2a544;p=folly.git

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 3abb5d0b..353ae298 100644
--- 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 5e539e0c..a381eb37 100644
--- 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 54454c9f..c221d5d1 100644
--- 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[]) {