Require renegotiation info from a whitelist of servers.

The renegotiation extension was introduced into TLS in [1], to address
the attack described in [2]. In order to fully protect ourselves,
clients need to require support from servers. However, these things
take a long time to roll out.

As an initial measure, we start to require the extension from a
whitelist of servers that we known to support the extension. We are
then protected from prefix attacks against these servers. Over time,
if the list grows we might want to share it with Mozilla, but we'll
start simple.

[1] http://tools.ietf.org/html/rfc5746
[2] http://www.educatedguesswork.org/2009/11/understanding_the_tls_renegoti.html

http://codereview.chromium.org/2045006/show

TEST=Setup a dummy TLS server without renegotiation extension support (Hardy) will do and force mail.google.com to it using /etc/hosts.  Navigate to https://mail.google.com and check for ERR_SSL_UNSAFE_NEGOTIATION.

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@51258 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 59 insertions, 15 deletions

diff --git a/net/base/net_error_list.h b/net/base/net_error_list.h
index 1b21084..b471930 100644
--- a/net/base/net_error_list.h
+++ b/net/base/net_error_list.h
@@ -159,6 +159,9 @@ NET_ERROR(SSL_BAD_RECORD_MAC_ALERT, -126)
 // The HTTP auth handler requires a DNS lookup to find the canonical name.
 NET_ERROR(AUTH_NEEDS_CANONICAL_NAME, -127)
 
+// A known TLS strict server didn't offer the renegotiation extension.
+NET_ERROR(SSL_UNSAFE_NEGOTIATION, -128)
+
 // Certificate error codes
 //
 // The values of certificate error codes must be consecutive.
diff --git a/net/base/ssl_config_service.cc b/net/base/ssl_config_service.cc
index 67d1349..6c2992f 100644
--- a/net/base/ssl_config_service.cc
+++ b/net/base/ssl_config_service.cc
@@ -25,4 +25,28 @@ SSLConfigService* SSLConfigService::CreateSystemSSLConfigService() {
 #endif
 }
 
+// static
+bool SSLConfigService::IsKnownStrictTLSServer(const std::string& hostname) {
+  // If you wish to add an entry to this list, please contact agl AT chromium
+  // DOT org.
+  //
+  // If this list starts growing, it'll need to be something more efficient
+  // than a linear list.
+  static const char kStrictServers[][20] = {
+      "www.google.com",
+      "mail.google.com",
+      "www.gmail.com",
+      "gmail.com",
+      "docs.google.com",
+      "clients1.google.com",
+  };
+
+  for (size_t i = 0; i < arraysize(kStrictServers); i++) {
+    if (strcmp(hostname.c_str(), kStrictServers[i]) == 0)
+      return true;
+  }
+
+  return false;
+}
+
 }  // namespace net
diff --git a/net/base/ssl_config_service.h b/net/base/ssl_config_service.h
index 0213f1f..d195039 100644
--- a/net/base/ssl_config_service.h
+++ b/net/base/ssl_config_service.h
@@ -84,6 +84,14 @@ class SSLConfigService : public base::RefCountedThreadSafe<SSLConfigService> {
   // May not be thread-safe, should only be called on the IO thread.
   virtual void GetSSLConfig(SSLConfig* config) = 0;
 
+  // Returns true if the given hostname is known to be 'strict'. This means
+  // that we will require the renegotiation extension and will always use TLS
+  // (no SSLv3 fallback).
+  //
+  // If you wish to add an element to this list, file a bug at
+  // http://crbug.com and email the link to agl AT chromium DOT org.
+  static bool IsKnownStrictTLSServer(const std::string& hostname);
+
  protected:
   friend class base::RefCountedThreadSafe<SSLConfigService>;
 
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index c228dab..ff9a04f 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -1,5 +1,5 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.  Use
-// of this source code is governed by a BSD-style license that can be
+// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/http/http_network_transaction.h"
@@ -1312,7 +1312,8 @@ int HttpNetworkTransaction::DoReadHeadersComplete(int result) {
         return result;
     } else if ((result == ERR_SSL_DECOMPRESSION_FAILURE_ALERT ||
                 result == ERR_SSL_BAD_RECORD_MAC_ALERT) &&
-               ssl_config_.tls1_enabled) {
+               ssl_config_.tls1_enabled &&
+               !SSLConfigService::IsKnownStrictTLSServer(request_->url.host())){
       // Some buggy servers select DEFLATE compression when offered and then
       // fail to ever decompress anything. They will send a fatal alert telling
       // us this. Normally we would pick this up during the handshake because
@@ -1842,7 +1843,8 @@ int HttpNetworkTransaction::HandleSSLHandshakeError(int error) {
     case ERR_SSL_VERSION_OR_CIPHER_MISMATCH:
     case ERR_SSL_DECOMPRESSION_FAILURE_ALERT:
     case ERR_SSL_BAD_RECORD_MAC_ALERT:
-      if (ssl_config_.tls1_enabled) {
+      if (ssl_config_.tls1_enabled &&
+          !SSLConfigService::IsKnownStrictTLSServer(request_->url.host())) {
         // This could be a TLS-intolerant server, an SSL 3.0 server that
         // chose a TLS-only cipher suite or a server with buggy DEFLATE
         // support. Turn off TLS 1.0, DEFLATE support and retry.
diff --git a/net/socket/ssl_client_socket_nss.cc b/net/socket/ssl_client_socket_nss.cc
index cf74ef1..285499e 100644
--- a/net/socket/ssl_client_socket_nss.cc
+++ b/net/socket/ssl_client_socket_nss.cc
@@ -193,6 +193,8 @@ int MapNSPRError(PRErrorCode err) {
       return ERR_SSL_DECOMPRESSION_FAILURE_ALERT;
     case SSL_ERROR_BAD_MAC_ALERT:
       return ERR_SSL_BAD_RECORD_MAC_ALERT;
+    case SSL_ERROR_UNSAFE_NEGOTIATION:
+      return ERR_SSL_UNSAFE_NEGOTIATION;
 
     default: {
       if (IS_SSL_ERROR(err)) {
@@ -460,19 +462,24 @@ int SSLClientSocketNSS::InitializeSSLOptions() {
 #endif
 
 #ifdef SSL_ENABLE_RENEGOTIATION
-  // We allow servers to request renegotiation. Since we're a client,
-  // prohibiting this is rather a waste of time. Only servers are in a position
-  // to prevent renegotiation attacks.
-  // http://extendedsubset.com/?p=8
-  //
-  // This should be changed when NSS 3.12.6 comes out with support for the
-  // renegotiation info extension.
-  // http://code.google.com/p/chromium/issues/detail?id=31647
-  rv = SSL_OptionSet(nss_fd_, SSL_ENABLE_RENEGOTIATION,
-                     SSL_RENEGOTIATE_UNRESTRICTED);
+  if (SSLConfigService::IsKnownStrictTLSServer(hostname_)) {
+    rv = SSL_OptionSet(nss_fd_, SSL_REQUIRE_SAFE_NEGOTIATION, PR_TRUE);
+    if (rv != SECSuccess)
+       LOG(INFO) << "SSL_REQUIRE_SAFE_NEGOTIATION failed.";
+    rv = SSL_OptionSet(nss_fd_, SSL_ENABLE_RENEGOTIATION,
+                       SSL_RENEGOTIATE_REQUIRES_XTN);
+  } else {
+    // We allow servers to request renegotiation. Since we're a client,
+    // prohibiting this is rather a waste of time. Only servers are in a
+    // position to prevent renegotiation attacks.
+    // http://extendedsubset.com/?p=8
+
+    rv = SSL_OptionSet(nss_fd_, SSL_ENABLE_RENEGOTIATION,
+                       SSL_RENEGOTIATE_UNRESTRICTED);
+  }
   if (rv != SECSuccess)
      LOG(INFO) << "SSL_ENABLE_RENEGOTIATION failed.";
-#endif
+#endif  // SSL_ENABLE_RENEGOTIATION
 
 #ifdef SSL_NEXT_PROTO_NEGOTIATED
   if (!ssl_config_.next_protos.empty()) {