net: boost AES-GCM ciphers if the machine has AES-NI.

net/socket/nss_ssl_util.cc

 // Copyright (c) 2012 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/socket/nss_ssl_util.h"
 
 #include <nss.h>
 #include <secerr.h>
 #include <ssl.h>
 #include <sslerr.h>
 #include <sslproto.h>
 
 #include <string>
 
 #include "base/bind.h"
+#include "base/cpu.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/memory/singleton.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/values.h"
 #include "build/build_config.h"
 #include "crypto/nss_util.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_log.h"
 
 #if defined(OS_WIN)
 #include "base/win/windows_version.h"
 #endif
 
+namespace {
+
+// CiphersRemove takes a zero-terminated array of cipher suite ids in
+// |to_remove| and sets every instance of them in |ciphers| to zero. It returns
+// true if it found and removed every element of |to_remove|. It assumes that
+// there are no duplicates in |ciphers| nor in |to_remove|.
+bool CiphersRemove(const uint16* to_remove, uint16* ciphers, size_t num) {
+  size_t num_remove, found = 0;
+
+  for (num_remove = 0; ; num_remove++) {
+    if (to_remove[num_remove] == 0)
+      break;
+  }
+
+  for (size_t i = 0; i < num; i++) {
+    for (size_t j = 0; j < num_remove; j++) {
+      if (to_remove[j] == ciphers[i]) {
+        ciphers[i] = 0;
+        found++;
+        break;
+      }
+    }
+  }

[wtc, 2013/11/22 01:14:16]

I think it is less work to reverse the nested for loops because the ciphers we
want to remove are likely to be in the top half of the |ciphers| array.

[agl, 2013/11/22 17:53:49]

Done.

+
+  return found == num_remove;
+}
+
+// CiphersCompact takes an array of cipher suite ids in |ciphers|, where some
+// entries are zero, and moves the entries so that all the non-zero elements
+// are compacted at the end of the array.
+size_t CiphersCompact(uint16* ciphers, size_t num) {
+  size_t j = num - 1;
+
+  for (size_t i = num - 1; i < num; i--) {

[wtc, 2013/11/22 01:14:16]

The i < num check looks wrong at first glance. You are relying on wrapping of an
unsigned integer type, right?

[agl, 2013/11/22 17:53:49]

Yes.

+    if (ciphers[i] == 0)
+      continue;
+    ciphers[j--] = ciphers[i];
+  }
+  return j+1;

[wtc, 2013/11/22 01:14:16]

Nit: the Style Guide recommends spaces around '+' and '-'.

Since the only caller of this function ignores the return value, this function
can return void.

[agl, 2013/11/22 17:53:49]

Done.

+}
+
+// CiphersCopy copies the zero-terminated array |in| to |out|.

[wtc, 2013/11/22 01:14:16]

Document the return value.

[agl, 2013/11/22 17:53:49]

Done.

+size_t CiphersCopy(const uint16* in, uint16* out) {
+  for (size_t i = 0; ; i++) {
+    if (in[i] == 0)
+      return i;
+    out[i] = in[i];
+  }
+}
+
+}  // anonymous namespace
+
+

[wtc, 2013/11/22 01:14:16]

Nit: delete one blank line

[agl, 2013/11/22 17:53:49]

Done.

 namespace net {
 
 class NSSSSLInitSingleton {
  public:
   NSSSSLInitSingleton() {

[wtc, 2013/11/22 01:14:16]

Initialize the new num_ciphers_ member to 0.

[agl, 2013/11/22 17:53:49]

Done.

     crypto::EnsureNSSInit();
 
     NSS_SetDomesticPolicy();
 
     const PRUint16* const ssl_ciphers = SSL_GetImplementedCiphers();
     const PRUint16 num_ciphers = SSL_GetNumImplementedCiphers();
 
     // Disable ECDSA cipher suites on platforms that do not support ECDSA
     // signed certificates, as servers may use the presence of such
     // ciphersuites as a hint to send an ECDSA certificate.
@@ skipping 34 matching lines @@
           // Enabled to allow servers with only a DSA certificate to function.
           enabled = true;
         }
         SSL_CipherPrefSetDefault(ssl_ciphers[i], enabled);
       }
     }
 
     // Enable SSL.
     SSL_OptionSetDefault(SSL_SECURITY, PR_TRUE);
 
+    // Calculate the order of ciphers that we'll use for NSS sockets. (Note
+    // that, even if a cipher is specified in the ordering, it won't be
+    // included if it is disabled.)

[wtc, 2013/11/22 01:14:16]

"it won't be included if it is disabled" sounds wrong. Did you mean "it won't be
used if it is disabled"?

[agl, 2013/11/22 17:53:49]

I really did mean that, but it's clearly confusing. I've changed it to "Note
that, even if a cipher is specified in the ordering, it must still be enabled in
order to be included in a ClientHello."

+    //
+    // Our top preference cipher suites are either forward-secure AES-GCM or
+    // forward secure ChaCha20-Poly1305. If the local machine has AES-NI then

[wtc, 2013/11/22 01:14:16]

Nit: forward secure => forward secret (two occurrences)

[agl, 2013/11/22 17:53:49]

Done.

+    // we prefer AES-GCM, otherwise ChaCha20. The remainder of the cipher suite
+    // preference is inheriented from NSS. */
+    static const uint16 chacha_ciphers[] = {
+      0xcc14  /* TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305 */,

[wtc, 2013/11/22 01:14:16]

You should be able to use TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305, etc. These
macros are defined in the NSS header sslproto.h, which we include.

[agl, 2013/11/22 17:53:49]

Done.

+      0xcc13  /* TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305 */,
+      0,
+    };
+    static const uint16 aes_gcm_ciphers[] = {
+      0xc02b  /* TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 */,
+      0xc02f  /* TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 */,
+      0x9e    /* TLS_DHE_RSA_WITH_AES_128_GCM_SHA256 */,
+      0,
+    };
+    const PRUint16 *all_ciphers = SSL_GetImplementedCiphers();

[wtc, 2013/11/22 01:14:16]

Nit: put '*' next to the type. Also fix lines 171-172.

[agl, 2013/11/22 17:53:49]

Done.

+    num_ciphers_ = SSL_GetNumImplementedCiphers();
+    ciphers_.reset(new uint16[num_ciphers_]);
+    memcpy(ciphers_.get(), all_ciphers, sizeof(uint16)*num_ciphers_);
+
+    if (CiphersRemove(chacha_ciphers, ciphers_.get(), num_ciphers_) &&
+        CiphersRemove(aes_gcm_ciphers, ciphers_.get(), num_ciphers_)) {

[wtc, 2013/11/22 01:14:16]

Nit: We can pass the sizes of chacha_ciphers and aes_gcm_ciphers (excluding the
zero terminators) to CiphersRemove so that CiphersRemove doesn't need to
calculate the size of its |to_remove| input.

[agl, 2013/11/22 17:53:49]

By reordering the for loops in CiphersRemove, as you suggested, the function no
longer needs to calculate the length beforehand.

+      CiphersCompact(ciphers_.get(), num_ciphers_);
+
+      const uint16 *preference_ciphers = chacha_ciphers;
+      const uint16 *other_ciphers = aes_gcm_ciphers;
+      if (base::CPU().has_aesni()) {
+        preference_ciphers = aes_gcm_ciphers;
+        other_ciphers = chacha_ciphers;
+      }
+      unsigned i = CiphersCopy(preference_ciphers, ciphers_.get());
+      CiphersCopy(other_ciphers, &ciphers_[i]);
+    } else {
+      ciphers_.reset();
+      num_ciphers_ = 0;
+    }
+
     // All other SSL options are set per-session by SSLClientSocket and
     // SSLServerSocket.
   }
 
+  const uint16* GetNSSCipherOrder(size_t* out_length) {
+    *out_length = num_ciphers_;
+    return ciphers_.get();
+  }
+
   ~NSSSSLInitSingleton() {
     // Have to clear the cache, or NSS_Shutdown fails with SEC_ERROR_BUSY.
     SSL_ClearSessionCache();
   }
+
+ private:
+  scoped_ptr<uint16[]> ciphers_;
+  size_t num_ciphers_;
 };
 
 static base::LazyInstance<NSSSSLInitSingleton> g_nss_ssl_init_singleton =
     LAZY_INSTANCE_INITIALIZER;
 
 // Initialize the NSS SSL library if it isn't already initialized.  This must
 // be called before any other NSS SSL functions.  This function is
 // thread-safe, and the NSS SSL library will only ever be initialized once.
 // The NSS SSL library will be properly shut down on program exit.
 void EnsureNSSSSLInit() {
   // Initializing SSL causes us to do blocking IO.
   // Temporarily allow it until we fix
   //   http://code.google.com/p/chromium/issues/detail?id=59847
   base::ThreadRestrictions::ScopedAllowIO allow_io;
 
   g_nss_ssl_init_singleton.Get();
 }
 
+const uint16* GetNSSCipherOrder(size_t* out_length) {
+  return g_nss_ssl_init_singleton.Get().GetNSSCipherOrder(out_length);
+}
+
 // Map a Chromium net error code to an NSS error code.
 // See _MD_unix_map_default_error in the NSS source
 // tree for inspiration.
 PRErrorCode MapErrorToNSS(int result) {
   if (result >=0)
     return result;
 
   switch (result) {
     case ERR_IO_PENDING:
       return PR_WOULD_BLOCK_ERROR;
@@ skipping 151 matching lines @@
                           const char* param) {
   DCHECK(function);
   DCHECK(param);
   net_log.AddEvent(
       NetLog::TYPE_SSL_NSS_ERROR,
       base::Bind(&NetLogSSLFailedNSSFunctionCallback,
                  function, param, PR_GetError()));
 }
 
 }  // namespace net