From 45e87488fe7fbd8860b040b70c41e4557e06edc4 Mon Sep 17 00:00:00 2001
From: davidben <davidben@chromium.org>
Date: Thu, 10 Dec 2015 16:09:38 -0800
Subject: Only enable RC4 when manually enabled and in a fallback handshake

While the option to manually enable RC4 exists (via SSLConfig's
|rc4_enabled|, which can be controlled via admin policy or field trial), it
should only be enabled in fallback handshakes, where no other
ciphersuite could be negotiated.

BUG=568694

Review URL: https://codereview.chromium.org/1512753007

Cr-Commit-Position: refs/heads/master@{#364534}
---
 net/socket/ssl_client_socket_nss.cc      |  5 ++++-
 net/socket/ssl_client_socket_openssl.cc  |  4 +++-
 net/socket/ssl_client_socket_unittest.cc | 16 ++++++++++++++--
 3 files changed, 21 insertions(+), 4 deletions(-)

(limited to 'net/socket')

diff --git a/net/socket/ssl_client_socket_nss.cc b/net/socket/ssl_client_socket_nss.cc
index 032840a..273d665 100644
--- a/net/socket/ssl_client_socket_nss.cc
+++ b/net/socket/ssl_client_socket_nss.cc
@@ -2788,8 +2788,11 @@ int SSLClientSocketNSS::InitializeSSLOptions() {
         SECSuccess) {
       continue;
     }
-    if (info.symCipher == ssl_calg_rc4 && !ssl_config_.rc4_enabled)
+    if (info.symCipher == ssl_calg_rc4 &&
+        !(ssl_config_.rc4_enabled &&
+          ssl_config_.deprecated_cipher_suites_enabled)) {
       SSL_CipherPrefSet(nss_fd_, ssl_ciphers[i], PR_FALSE);
+    }
     if (info.keaType == ssl_kea_dh &&
         !ssl_config_.deprecated_cipher_suites_enabled) {
       // Only offer DHE on the second handshake. https://crbug.com/538690
diff --git a/net/socket/ssl_client_socket_openssl.cc b/net/socket/ssl_client_socket_openssl.cc
index 090f2bd..871e0e3 100644
--- a/net/socket/ssl_client_socket_openssl.cc
+++ b/net/socket/ssl_client_socket_openssl.cc
@@ -938,8 +938,10 @@ int SSLClientSocketOpenSSL::Init() {
   if (ssl_config_.require_ecdhe)
     command.append(":!kRSA:!kDHE");
 
-  if (!ssl_config_.rc4_enabled)
+  if (!(ssl_config_.rc4_enabled &&
+        ssl_config_.deprecated_cipher_suites_enabled)) {
     command.append(":!RC4");
+  }
 
   if (ssl_config_.deprecated_cipher_suites_enabled) {
     // Add TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384 under a fallback. This is
diff --git a/net/socket/ssl_client_socket_unittest.cc b/net/socket/ssl_client_socket_unittest.cc
index dce9f0a..6ab66c0 100644
--- a/net/socket/ssl_client_socket_unittest.cc
+++ b/net/socket/ssl_client_socket_unittest.cc
@@ -2506,7 +2506,8 @@ TEST_F(SSLClientSocketTest, FallbackShardSessionCache) {
             SSLConnectionStatusToVersion(ssl_info.connection_status));
 }
 
-// Test that RC4 is only enabled if rc4_enabled is set.
+// Test that RC4 is only enabled if rc4_enabled and
+// deprecated_cipher_suites_enabled are both set.
 TEST_F(SSLClientSocketTest, RC4Enabled) {
   SpawnedTestServer::SSLOptions ssl_options;
   ssl_options.bulk_ciphers = SpawnedTestServer::SSLOptions::BULK_CIPHER_RC4;
@@ -2518,9 +2519,20 @@ TEST_F(SSLClientSocketTest, RC4Enabled) {
   ASSERT_TRUE(CreateAndConnectSSLClientSocket(ssl_config, &rv));
   EXPECT_EQ(ERR_SSL_VERSION_OR_CIPHER_MISMATCH, rv);
 
-  // Enabling RC4 works fine.
+  // RC4 is also not enabled in the fallback handshake.
+  ssl_config.deprecated_cipher_suites_enabled = true;
+  ASSERT_TRUE(CreateAndConnectSSLClientSocket(ssl_config, &rv));
+  EXPECT_EQ(ERR_SSL_VERSION_OR_CIPHER_MISMATCH, rv);
+
+  // Even if RC4 is enabled, it is not sent in the initial handshake.
+  ssl_config.deprecated_cipher_suites_enabled = false;
   ssl_config.rc4_enabled = true;
   ASSERT_TRUE(CreateAndConnectSSLClientSocket(ssl_config, &rv));
+  EXPECT_EQ(ERR_SSL_VERSION_OR_CIPHER_MISMATCH, rv);
+
+  // If enabled, RC4 works in the fallback handshake.
+  ssl_config.deprecated_cipher_suites_enabled = true;
+  ASSERT_TRUE(CreateAndConnectSSLClientSocket(ssl_config, &rv));
   EXPECT_EQ(OK, rv);
 }
 
-- 
cgit v1.1