Use cipher suite config options in SSLServerSocketOpenSSL.

Previously SSLServerSocketOpenSSL was ignoring disabled_cipher_suites
list and require_forward_secrecy flag from SSLConfig. Fixed
SSLServerSocketOpenSSL to trim the list of cipher suites used in BoringSSL.

BUG=481163

Committed: https://crrev.com/d0eae58087e6f45088d6ef349d9ebaa2da450ea1
Cr-Commit-Position: refs/heads/master@{#329528}

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

Cr-Commit-Position: refs/heads/master@{#329707}

net/socket/ssl_server_socket_nss.cc

@@ -349,7 +349,7 @@ int SSLServerSocketNSS::InitializeSSLOptions() {
     return ERR_NO_SSL_VERSIONS_ENABLED;
   }
 
-  if (ssl_config_.require_forward_secrecy) {
+  if (ssl_config_.require_ecdhe) {
     const PRUint16* const ssl_ciphers = SSL_GetImplementedCiphers();
     const PRUint16 num_ciphers = SSL_GetNumImplementedCiphers();
 

net/socket/ssl_server_socket_openssl.cc

@@ -9,6 +9,7 @@
 
 #include "base/callback_helpers.h"
 #include "base/logging.h"
+#include "base/strings/string_util.h"
 #include "crypto/openssl_util.h"
 #include "crypto/rsa_private_key.h"
 #include "crypto/scoped_openssl_types.h"
@@ -685,6 +686,48 @@ int SSLServerSocketOpenSSL::Init() {
   SSL_set_mode(ssl_, mode.set_mask);
   SSL_clear_mode(ssl_, mode.clear_mask);
 
+  // Removing ciphers by ID from OpenSSL is a bit involved as we must use the
+  // textual name with SSL_set_cipher_list because there is no public API to
+  // directly remove a cipher by ID.
+  STACK_OF(SSL_CIPHER)* ciphers = SSL_get_ciphers(ssl_);
+  DCHECK(ciphers);
+  // See SSLConfig::disabled_cipher_suites for description of the suites
+  // disabled by default. Note that !SHA256 and !SHA384 only remove HMAC-SHA256
+  // and HMAC-SHA384 cipher suites, not GCM cipher suites with SHA256 or SHA384
+  // as the handshake hash.
+  std::string command("DEFAULT:!SHA256:!SHA384:!AESGCM+AES256:!aPSK");
+  // Walk through all the installed ciphers, seeing if any need to be
+  // appended to the cipher removal |command|.
+  for (size_t i = 0; i < sk_SSL_CIPHER_num(ciphers); ++i) {
+    const SSL_CIPHER* cipher = sk_SSL_CIPHER_value(ciphers, i);
+    const uint16_t id = static_cast<uint16_t>(SSL_CIPHER_get_id(cipher));
+
+    bool disable = false;
+    if (ssl_config_.require_ecdhe) {
+      base::StringPiece kx_name(SSL_CIPHER_get_kx_name(cipher));
+      disable = kx_name != "ECDHE_RSA" && kx_name != "ECDHE_ECDSA";
+    }
+    if (!disable) {
+      disable = std::find(ssl_config_.disabled_cipher_suites.begin(),
+                          ssl_config_.disabled_cipher_suites.end(),
+                          id) != ssl_config_.disabled_cipher_suites.end();
+    }
+    if (disable) {
+      const char* name = SSL_CIPHER_get_name(cipher);
+      DVLOG(3) << "Found cipher to remove: '" << name << "', ID: " << id
+               << " strength: " << SSL_CIPHER_get_bits(cipher, NULL);
+      command.append(":!");
+      command.append(name);
+    }
+  }
+
+  int rv = SSL_set_cipher_list(ssl_, command.c_str());
+  // If this fails (rv = 0) it means there are no ciphers enabled on this SSL.
+  // This will almost certainly result in the socket failing to complete the
+  // handshake at which point the appropriate error is bubbled up to the client.
+  LOG_IF(WARNING, rv != 1) << "SSL_set_cipher_list('" << command
+                           << "') returned " << rv;
+
   return OK;
 }
 

net/socket/ssl_server_socket_unittest.cc

@@ -326,30 +326,30 @@ class SSLServerSocketTest : public PlatformTest {
     scoped_ptr<crypto::RSAPrivateKey> private_key(
         crypto::RSAPrivateKey::CreateFromPrivateKeyInfo(key_vector));
 
-    SSLConfig ssl_config;
-    ssl_config.false_start_enabled = false;
-    ssl_config.channel_id_enabled = false;
+    client_ssl_config_.false_start_enabled = false;
+    client_ssl_config_.channel_id_enabled = false;
 
     // Certificate provided by the host doesn't need authority.
     SSLConfig::CertAndStatus cert_and_status;
     cert_and_status.cert_status = CERT_STATUS_AUTHORITY_INVALID;
     cert_and_status.der_cert = cert_der;
-    ssl_config.allowed_bad_certs.push_back(cert_and_status);
+    client_ssl_config_.allowed_bad_certs.push_back(cert_and_status);
 
     HostPortPair host_and_pair("unittest", 0);
     SSLClientSocketContext context;
     context.cert_verifier = cert_verifier_.get();
     context.transport_security_state = transport_security_state_.get();
-    client_socket_ =
-        socket_factory_->CreateSSLClientSocket(
-            client_connection.Pass(), host_and_pair, ssl_config, context);
-    server_socket_ = CreateSSLServerSocket(
-        server_socket.Pass(),
-        cert.get(), private_key.get(), SSLConfig());
+    client_socket_ = socket_factory_->CreateSSLClientSocket(
+        client_connection.Pass(), host_and_pair, client_ssl_config_, context);
+    server_socket_ =
+        CreateSSLServerSocket(server_socket.Pass(), cert.get(),
+                              private_key.get(), server_ssl_config_);
   }
 
   FakeDataChannel channel_1_;
   FakeDataChannel channel_2_;
+  SSLConfig client_ssl_config_;
+  SSLConfig server_ssl_config_;
   scoped_ptr<SSLClientSocket> client_socket_;
   scoped_ptr<SSLServerSocket> server_socket_;
   ClientSocketFactory* socket_factory_;
@@ -591,4 +591,40 @@ TEST_F(SSLServerSocketTest, ExportKeyingMaterial) {
   EXPECT_NE(0, memcmp(server_out, client_bad, sizeof(server_out)));
 }
 
+// Verifies that SSLConfig::require_ecdhe flags works properly.
+TEST_F(SSLServerSocketTest, RequireEcdheFlag) {
+  // Disable all ECDHE suites on the client side.
+  uint16_t kEcdheCiphers[] = {
+      0xc007,  // ECDHE_ECDSA_WITH_RC4_128_SHA
+      0xc009,  // ECDHE_ECDSA_WITH_AES_128_CBC_SHA
+      0xc00a,  // ECDHE_ECDSA_WITH_AES_256_CBC_SHA
+      0xc011,  // ECDHE_RSA_WITH_RC4_128_SHA
+      0xc013,  // ECDHE_RSA_WITH_AES_128_CBC_SHA
+      0xc014,  // ECDHE_RSA_WITH_AES_256_CBC_SHA
+      0xc02b,  // ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
+      0xc02f,  // ECDHE_RSA_WITH_AES_128_GCM_SHA256
+      0xcc13,  // ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256
+      0xcc14,  // ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256
+  };
+  client_ssl_config_.disabled_cipher_suites.assign(
+      kEcdheCiphers, kEcdheCiphers + arraysize(kEcdheCiphers));
+
+  // Require ECDHE on the server.
+  server_ssl_config_.require_ecdhe = true;
+
+  Initialize();
+
+  TestCompletionCallback connect_callback;
+  TestCompletionCallback handshake_callback;
+
+  int client_ret = client_socket_->Connect(connect_callback.callback());
+  int server_ret = server_socket_->Handshake(handshake_callback.callback());
+
+  client_ret = connect_callback.GetResult(client_ret);
+  server_ret = handshake_callback.GetResult(server_ret);
+
+  ASSERT_EQ(ERR_SSL_VERSION_OR_CIPHER_MISMATCH, client_ret);
+  ASSERT_EQ(ERR_SSL_VERSION_OR_CIPHER_MISMATCH, server_ret);
+}
+
 }  // namespace net

net/ssl/ssl_config.cc

@@ -26,7 +26,7 @@ SSLConfig::SSLConfig()
       channel_id_enabled(true),
       false_start_enabled(true),
       signed_cert_timestamps_enabled(true),
-      require_forward_secrecy(false),
+      require_ecdhe(false),
       send_client_cert(false),
       verify_ev_cert(false),
       version_fallback(false),

net/ssl/ssl_config.h

@@ -115,10 +115,10 @@ struct NET_EXPORT SSLConfig {
   // TLS extension is enabled.
   bool signed_cert_timestamps_enabled;
 
-  // require_forward_secrecy, if true, causes only (EC)DHE cipher suites to be
-  // enabled. NOTE: this only applies to server sockets currently, although
-  // that could be extended if needed.
-  bool require_forward_secrecy;
+  // If true, causes only ECDHE cipher suites to be enabled. NOTE: This only
+  // applies to server sockets currently, although that could be extended if
+  // needed.
+  bool require_ecdhe;
 
   // TODO(wtc): move the following members to a new SSLParams structure.  They
   // are not SSL configuration settings.

net/ssl/ssl_config_service.cc

@@ -96,8 +96,7 @@ void SSLConfigService::ProcessConfigUpdate(const SSLConfig& orig_config,
        new_config.disabled_cipher_suites) ||
       (orig_config.channel_id_enabled != new_config.channel_id_enabled) ||
       (orig_config.false_start_enabled != new_config.false_start_enabled) ||
-      (orig_config.require_forward_secrecy !=
-       new_config.require_forward_secrecy);
+      (orig_config.require_ecdhe != new_config.require_ecdhe);
 
   if (config_changed)
     NotifySSLConfigChange();

remoting/protocol/ssl_hmac_channel_authenticator.cc

@@ -109,7 +109,7 @@ void SslHmacChannelAuthenticator::SecureAndAuthenticate(
     }
 
     net::SSLConfig ssl_config;
-    ssl_config.require_forward_secrecy = true;
+    ssl_config.require_ecdhe = true;
 
     scoped_ptr<net::SSLServerSocket> server_socket =
         net::CreateSSLServerSocket(socket.Pass(),