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

diff --git a/folly/io/async/SSLContext.cpp b/folly/io/async/SSLContext.cpp
index 95ae99a12edff49cdaa921f2cf243f769b0503b8..501245ba4c05c5a5858d1142f3fc4ec6023a67b8 100644 (file)
--- 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 bf377ec8110f6db72344ef45d740b207b493477d..73cfedf2e80a18eae5fd514f52184b92150723c4 100644 (file)
--- 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 {