Back out "Give ownership of new session callbacks to SSLContext"

Summary:
Original commit changeset: f1dc8bd17150

This accidentally enabled the openssl internal session cache which can use
excessive memory.

Reviewed By: jalopezsilva, mingtaoy

Differential Revision: D21433514

fbshipit-source-id: 91358f6fd2decc0d18eff7ca8194b677f3c8629c

folly/io/async/SSLContext.cpp

@@ -35,6 +35,10 @@ int getExDataIndex() {
   return index;
 }
 
+void setExData(folly::SSLContext* context, SSL_CTX* ctx) {
+  SSL_CTX_set_ex_data(ctx, getExDataIndex(), context);
+}
+
 } // namespace
 
 namespace folly {
@@ -88,7 +92,7 @@ SSLContext::SSLContext(SSLVersion version) {
 
   sslAcceptRunner_ = std::make_unique<SSLAcceptRunner>();
 
-  setupCtx(ctx_);
+  setExData(this, ctx_);
 
 #if FOLLY_OPENSSL_HAS_SNI
   SSL_CTX_set_tlsext_servername_callback(ctx_, baseServerNameOpenSSLCallback);
@@ -155,7 +159,7 @@ void SSLContext::setServerECCurve(const std::string& curveName) {
 }
 
 SSLContext::SSLContext(SSL_CTX* ctx) : ctx_(ctx) {
-  setupCtx(ctx);
+  setExData(this, ctx);
   if (SSL_CTX_up_ref(ctx) == 0) {
     throw std::runtime_error("Failed to increment SSL_CTX refcount");
   }
@@ -668,43 +672,10 @@ void SSLContext::enableTLS13() {
 #endif
 }
 
-void SSLContext::setupCtx(SSL_CTX* ctx) {
-  // Client caching required for receiving sessions in TLS 1.3
-  // Default value from OpenSSL is SSL_SESS_CACHE_SERVER
-  SSL_CTX_set_session_cache_mode(ctx, SSL_SESS_CACHE_BOTH);
-
-  SSL_CTX_set_ex_data(ctx, getExDataIndex(), this);
-  SSL_CTX_sess_set_new_cb(ctx, SSLContext::newSessionCallback);
-}
-
 SSLContext* SSLContext::getFromSSLCtx(const SSL_CTX* ctx) {
   return static_cast<SSLContext*>(SSL_CTX_get_ex_data(ctx, getExDataIndex()));
 }
 
-int SSLContext::newSessionCallback(SSL* ssl, SSL_SESSION* session) {
-  SSL_CTX* ctx = SSL_get_SSL_CTX(ssl);
-  SSLContext* context = getFromSSLCtx(ctx);
-
-  auto& cb = context->sessionLifecycleCallbacks_;
-  if (cb != nullptr && cb) {
-    SSL_SESSION_up_ref(session);
-    auto sessionPtr = folly::ssl::SSLSessionUniquePtr(session);
-    cb->onNewSession(ssl, std::move(sessionPtr));
-  }
-
-  SSL_SESSION_free(session);
-  return 1;
-}
-
-void SSLContext::attachSessionLifecycleCallbacks(
-    std::unique_ptr<SessionLifecycleCallbacks> cb) {
-  sessionLifecycleCallbacks_ = std::move(cb);
-}
-
-void SSLContext::removeSessionLifecycleCallbacks() {
-  sessionLifecycleCallbacks_ = nullptr;
-}
-
 std::ostream& operator<<(std::ostream& os, const PasswordCollector& collector) {
   os << collector.describe();
   return os;

folly/io/async/SSLContext.h

@@ -84,11 +84,6 @@ class SSLAcceptRunner {
  */
 class SSLContext {
  public:
-  struct SessionLifecycleCallbacks {
-    virtual void onNewSession(SSL*, folly::ssl::SSLSessionUniquePtr) = 0;
-    virtual ~SessionLifecycleCallbacks() = default;
-  };
-
   enum SSLVersion {
     SSLv2,
     SSLv3,
@@ -569,11 +564,6 @@ class SSLContext {
    */
   static SSLContext* getFromSSLCtx(const SSL_CTX* ctx);
 
-  void attachSessionLifecycleCallbacks(
-      std::unique_ptr<SessionLifecycleCallbacks> cb);
-
-  void removeSessionLifecycleCallbacks();
-
   [[deprecated("Use folly::ssl::init")]] static void initializeOpenSSL();
 
  protected:
@@ -648,13 +638,6 @@ class SSLContext {
 #endif
 
   std::string providedCiphersString_;
-
-  void setupCtx(SSL_CTX* ctx);
-
-  std::unique_ptr<SessionLifecycleCallbacks> sessionLifecycleCallbacks_{
-      nullptr};
-
-  static int newSessionCallback(SSL* ssl, SSL_SESSION* session);
 };
 
 typedef std::shared_ptr<SSLContext> SSLContextPtr;

folly/io/async/test/SSLSessionTest.cpp

@@ -30,18 +30,18 @@ using folly::ssl::detail::OpenSSLSession;
 
 namespace folly {
 
-class SimpleCallbackManager
-    : public folly::SSLContext::SessionLifecycleCallbacks {
+class SimpleCallbackManager {
  public:
-  void onNewSession(SSL* ssl, folly::ssl::SSLSessionUniquePtr session)
-      override {
+  static int sessionCallback(SSL* ssl, SSL_SESSION* session) {
+    auto sessionPtr = folly::ssl::SSLSessionUniquePtr(session);
     auto socket = folly::AsyncSSLSocket::getFromSSL(ssl);
     auto sslSession =
         std::dynamic_pointer_cast<folly::ssl::detail::OpenSSLSession>(
             socket->getSSLSessionV2());
     if (sslSession) {
-      sslSession->setActiveSession(std::move(session));
+      sslSession->setActiveSession(std::move(sessionPtr));
     }
+    return 1;
   }
 };
 
@@ -62,8 +62,13 @@ void getctx(
     std::shared_ptr<folly::SSLContext> serverCtx) {
   clientCtx->ciphers("ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH");
   clientCtx->loadTrustedCertificates(kTestCA);
-  clientCtx->attachSessionLifecycleCallbacks(
-      std::make_unique<SimpleCallbackManager>());
+  auto clientCtx_ = clientCtx->getSSLCtx();
+
+  // The following two actions (setting session cache mode and
+  // setting the session callback) will eventually be done in
+  // SSLContext instead
+  SSL_CTX_set_session_cache_mode(clientCtx_, SSL_SESS_CACHE_CLIENT);
+  SSL_CTX_sess_set_new_cb(clientCtx_, SimpleCallbackManager::sessionCallback);
 
   serverCtx->ciphers("ALL:!ADH:!LOW:!EXP:!MD5:@STRENGTH");
   serverCtx->loadCertificate(kTestCert);
@@ -171,6 +176,7 @@ TEST_F(SSLSessionTest, BasicRegressionTest) {
 
     sslSession = clientPtr->getSSLSession();
     ASSERT_NE(sslSession, nullptr);
+    SSL_SESSION_free(sslSession);
   }
 
   // Session resumption
@@ -192,7 +198,6 @@ TEST_F(SSLSessionTest, BasicRegressionTest) {
     eventBase.loop();
     ASSERT_TRUE(client.handshakeSuccess_);
     ASSERT_TRUE(clientPtr->getSSLSessionReused());
-    SSL_SESSION_free(sslSession);
   }
 }