From 4fbcb4b8f2fa332bbe2d491340b4995ac553e025 Mon Sep 17 00:00:00 2001
From: Xiangyu Bu <xbu@fb.com>
Date: Wed, 26 Jul 2017 11:32:54 -0700
Subject: [PATCH] Kill folly::SSLContext::switchCiphersIfTLS11.

Summary: It has been years since BEAST attack surfaced. The vulnerabilities have been patched and the mitigation using RC4 cipher is no longer needed. This diff removes the code relevant to mitigating BEAST years ago.

Reviewed By: anirudhvr

Differential Revision: D5409859

fbshipit-source-id: 58178e68a447f372b19491832a7be590af9402e9
---
 folly/io/async/SSLContext.cpp | 49 -----------------------------------
 folly/io/async/SSLContext.h   | 14 ----------
 2 files changed, 63 deletions(-)

diff --git a/folly/io/async/SSLContext.cpp b/folly/io/async/SSLContext.cpp
index 95ae99a1..501245ba 100644
--- a/folly/io/async/SSLContext.cpp
+++ b/folly/io/async/SSLContext.cpp
@@ -387,55 +387,6 @@ int SSLContext::baseServerNameOpenSSLCallback(SSL* ssl, int* al, void* data) {
 
   return SSL_TLSEXT_ERR_NOACK;
 }
-
-void SSLContext::switchCiphersIfTLS11(
-    SSL* ssl,
-    const std::string& tls11CipherString,
-    const std::vector<std::pair<std::string, int>>& tls11AltCipherlist) {
-  CHECK(!(tls11CipherString.empty() && tls11AltCipherlist.empty()))
-      << "Shouldn't call if empty ciphers / alt ciphers";
-
-  if (TLS1_get_client_version(ssl) <= TLS1_VERSION) {
-    // We only do this for TLS v 1.1 and later
-    return;
-  }
-
-  const std::string* ciphers = &tls11CipherString;
-  if (!tls11AltCipherlist.empty()) {
-    if (!cipherListPicker_) {
-      std::vector<int> weights;
-      std::for_each(
-          tls11AltCipherlist.begin(),
-          tls11AltCipherlist.end(),
-          [&](const std::pair<std::string, int>& e) {
-            weights.push_back(e.second);
-          });
-      cipherListPicker_.reset(
-          new std::discrete_distribution<int>(weights.begin(), weights.end()));
-    }
-    auto rng = ThreadLocalPRNG();
-    auto index = (*cipherListPicker_)(rng);
-    if ((size_t)index >= tls11AltCipherlist.size()) {
-      LOG(ERROR) << "Trying to pick alt TLS11 cipher index " << index
-                 << ", but tls11AltCipherlist is of length "
-                 << tls11AltCipherlist.size();
-    } else {
-      ciphers = &tls11AltCipherlist[size_t(index)].first;
-    }
-  }
-
-  // Prefer AES for TLS versions 1.1 and later since these are not
-  // vulnerable to BEAST attacks on AES.  Note that we're setting the
-  // cipher list on the SSL object, not the SSL_CTX object, so it will
-  // only last for this request.
-  int rc = SSL_set_cipher_list(ssl, ciphers->c_str());
-  if ((rc == 0) || ERR_peek_error() != 0) {
-    // This shouldn't happen since we checked for this when proxygen
-    // started up.
-    LOG(WARNING) << "ssl_cipher: No specified ciphers supported for switch";
-    SSL_set_cipher_list(ssl, providedCiphersString_.c_str());
-  }
-}
 #endif // FOLLY_OPENSSL_HAS_SNI
 
 #if FOLLY_OPENSSL_HAS_ALPN
diff --git a/folly/io/async/SSLContext.h b/folly/io/async/SSLContext.h
index bf377ec8..73cfedf2 100644
--- a/folly/io/async/SSLContext.h
+++ b/folly/io/async/SSLContext.h
@@ -454,17 +454,6 @@ class SSLContext {
    */
   static std::string getErrors(int errnoCopy);
 
-  /**
-   * We want to vary which cipher we'll use based on the client's TLS version.
-   *
-   * XXX: The refernces to tls11CipherString and tls11AltCipherlist are reused
-   * for * each >= TLS 1.1 handshake, so we expect these fields to not change.
-   */
-  void switchCiphersIfTLS11(
-      SSL* ssl,
-      const std::string& tls11CipherString,
-      const std::vector<std::pair<std::string, int>>& tls11AltCipherlist);
-
   bool checkPeerName() { return checkPeerName_; }
   std::string peerFixedName() { return peerFixedName_; }
 
@@ -503,9 +492,6 @@ class SSLContext {
 
   static bool initialized_;
 
-  // To provide control over choice of server ciphersuites
-  std::unique_ptr<std::discrete_distribution<int>> cipherListPicker_;
-
 #ifdef OPENSSL_NPN_NEGOTIATED
 
   struct AdvertisedNextProtocolsItem {
-- 
2.34.1