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

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

We want to use ChaCha20 based cipher suites on machines that don't have AES-NI
and AES-GCM on those that do.

Note: this was https://codereview.chromium.org/75663004/ and the review took
place there. But codereview broke and I had to reupload as a different issue.

BUG=310768
[email protected]

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=238113

[wtc, 2013-11-27 18:01:51]

Patch set 1 LGTM.

You can ignore the nits if you have submitted the CL to the commit queue.

https://codereview.chromium.org/91913002/diff/1/base/cpu.cc
File base/cpu.cc (right):

https://codereview.chromium.org/91913002/diff/1/base/cpu.cc#newcode41
base/cpu.cc:41: has_aesni_(false),

Nit: move this member initializer up.

https://codereview.chromium.org/91913002/diff/1/base/cpu.cc#newcode124
base/cpu.cc:124: has_aesni_ = (cpu_info[2] & 0x02000000) != 0;

Nit: this should ideally be set after the has_avx_ member is set.

https://codereview.chromium.org/91913002/diff/1/base/cpu.h
File base/cpu.h (right):

https://codereview.chromium.org/91913002/diff/1/base/cpu.h#newcode83
base/cpu.h:83: bool has_aesni_;

Nit: move this member up, to be right after has_avx_hardware_.

https://codereview.chromium.org/91913002/diff/1/net/socket/nss_ssl_util.cc
File net/socket/nss_ssl_util.cc (right):

https://codereview.chromium.org/91913002/diff/1/net/socket/nss_ssl_util.cc#ne...
net/socket/nss_ssl_util.cc:86: : num_ciphers_(0) {

Nit: the initializer list can be on the previous line.

https://codereview.chromium.org/91913002/diff/1/net/socket/ssl_client_socket_...
File net/socket/ssl_client_socket_nss.cc (right):

https://codereview.chromium.org/91913002/diff/1/net/socket/ssl_client_socket_...
net/socket/ssl_client_socket_nss.cc:3145: }

Nit: omit the curly braces.

[agl, 2013-11-27 20:50:57]

(I hit more 500s while trying to upload a new version so I've no idea what state
the review is in now.)

https://codereview.chromium.org/91913002/diff/1/base/cpu.cc
File base/cpu.cc (right):

https://codereview.chromium.org/91913002/diff/1/base/cpu.cc#newcode41
base/cpu.cc:41: has_aesni_(false),
On 2013/11/27 18:01:51, wtc wrote:
> 
> Nit: move this member initializer up.

Done.

https://codereview.chromium.org/91913002/diff/1/base/cpu.cc#newcode124
base/cpu.cc:124: has_aesni_ = (cpu_info[2] & 0x02000000) != 0;
On 2013/11/27 18:01:51, wtc wrote:
> 
> Nit: this should ideally be set after the has_avx_ member is set.

Done.

https://codereview.chromium.org/91913002/diff/1/base/cpu.h
File base/cpu.h (right):

https://codereview.chromium.org/91913002/diff/1/base/cpu.h#newcode83
base/cpu.h:83: bool has_aesni_;
On 2013/11/27 18:01:51, wtc wrote:
> 
> Nit: move this member up, to be right after has_avx_hardware_.

Done.

https://codereview.chromium.org/91913002/diff/1/net/socket/nss_ssl_util.cc
File net/socket/nss_ssl_util.cc (right):

https://codereview.chromium.org/91913002/diff/1/net/socket/nss_ssl_util.cc#ne...
net/socket/nss_ssl_util.cc:86: : num_ciphers_(0) {
On 2013/11/27 18:01:51, wtc wrote:
> 
> Nit: the initializer list can be on the previous line.

Done.

[wtc, 2013-11-27 22:30:35]

Patch set 4 LGTM.

[agl, 2013-12-02 16:15:09]

Committed patchset #4 manually as r238113.

[wtc, 2014-01-06 19:18:18]

https://codereview.chromium.org/91913002/diff/60001/net/socket/nss_ssl_util.cc
File net/socket/nss_ssl_util.cc (right):

https://codereview.chromium.org/91913002/diff/60001/net/socket/nss_ssl_util.c...
net/socket/nss_ssl_util.cc:156: TLS_DHE_RSA_WITH_AES_128_GCM_SHA256,

While testing a CL of mine today, I noticed that TLS_RSA_WITH_AES_128_GCM_SHA256
is not in this array. I just wanted to make sure this is because
TLS_RSA_WITH_AES_128_GCM_SHA256 is not forward-secret.

[agl, 2014-01-06 19:26:19]

https://codereview.chromium.org/91913002/diff/60001/net/socket/nss_ssl_util.cc
File net/socket/nss_ssl_util.cc (right):

https://codereview.chromium.org/91913002/diff/60001/net/socket/nss_ssl_util.c...
net/socket/nss_ssl_util.cc:156: TLS_DHE_RSA_WITH_AES_128_GCM_SHA256,
On 2014/01/06 19:18:18, wtc wrote:
> 
> While testing a CL of mine today, I noticed that
TLS_RSA_WITH_AES_128_GCM_SHA256
> is not in this array. I just wanted to make sure this is because
> TLS_RSA_WITH_AES_128_GCM_SHA256 is not forward-secret.

That's correct. No desire to bump non-forward-secret ciphersuites up over
forward-secret ones.