net: don't do TLS False Start with RC4.

net: don't do TLS False Start with RC4.

In light of recent analysis of RC4, this seems like a prudent measure.

BUG=none

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

[Ryan Sleevi, 2013-04-15 18:08:35]

So this LGTM, but a few comment nits.

Also, I would encourage you to chime in on
https://bugzilla.mozilla.org/show_bug.cgi?id=713933 while Pat and Brian are
working through similar issues.

https://codereview.chromium.org/13985023/diff/1/net/third_party/nss/patches/n...
File net/third_party/nss/patches/norc4falsestart.patch (right):

https://codereview.chromium.org/13985023/diff/1/net/third_party/nss/patches/n...
net/third_party/nss/patches/norc4falsestart.patch:12: +         * don't allow
False Start with RC4. */
In 5 years (as NSS code easily lives on), this will no longer be "recent".

Perhaps reword

/* Due to biases in the RC4 keystream, do not allow False Start with RC4 */

https://codereview.chromium.org/13985023/diff/1/net/third_party/nss/ssl/ssl3c...
File net/third_party/nss/ssl/ssl3con.c (right):

https://codereview.chromium.org/13985023/diff/1/net/third_party/nss/ssl/ssl3c...
net/third_party/nss/ssl/ssl3con.c:6421: * don't allow False Start with RC4. */
Perhaps this should be integrated with the comments on 6410-6414 to provide more
meaningful context on why RC4 isn't allowed.

[agl, 2013-04-16 14:59:29]

https://codereview.chromium.org/13985023/diff/1/net/third_party/nss/patches/n...
File net/third_party/nss/patches/norc4falsestart.patch (right):

https://codereview.chromium.org/13985023/diff/1/net/third_party/nss/patches/n...
net/third_party/nss/patches/norc4falsestart.patch:12: +         * don't allow
False Start with RC4. */
On 2013/04/15 18:08:35, Ryan Sleevi wrote:
> In 5 years (as NSS code easily lives on), this will no longer be "recent".
> 
> Perhaps reword
> 
> /* Due to biases in the RC4 keystream, do not allow False Start with RC4 */

Good point. Done.

https://codereview.chromium.org/13985023/diff/1/net/third_party/nss/ssl/ssl3c...
File net/third_party/nss/ssl/ssl3con.c (right):

https://codereview.chromium.org/13985023/diff/1/net/third_party/nss/ssl/ssl3c...
net/third_party/nss/ssl/ssl3con.c:6421: * don't allow False Start with RC4. */
On 2013/04/15 18:08:35, Ryan Sleevi wrote:
> Perhaps this should be integrated with the comments on 6410-6414 to provide
more
> meaningful context on why RC4 isn't allowed.

Done.

[wtc, 2013-04-16 19:06:14]

This CL implies we consider RC4_128 cipher suites to be
less secure than AES_128 cipher suites, so it should be
accompanied by two other changes to be complete:
1. In ClientHello, RC4_128 cipher suites should be listed
after AES_128 cipher suites.
2. Our servers should prefer AES_128 cipher suites over
RC4_128 cipher suites.

[agl, 2013-04-16 19:14:26]

On Tue, Apr 16, 2013 at 3:06 PM,  <wtc@chromium.org> wrote:
> This CL implies we consider RC4_128 cipher suites to be
> less secure than AES_128 cipher suites, so it should be
> accompanied by two other changes to be complete:
> 1. In ClientHello, RC4_128 cipher suites should be listed
> after AES_128 cipher suites.
> 2. Our servers should prefer AES_128 cipher suites over
> RC4_128 cipher suites.

I don't think it's that simple. AES-CBC presents implementation issues
that are not nearly as important with False Start. False Start
specifically depends on the uniformity of the cipher over many
connections, which is where RC4 is weak.


Cheers

AGL

[wtc, 2013-04-16 21:06:45]

Patch set 2 LGTM.

https://codereview.chromium.org/13985023/diff/6001/net/third_party/nss/ssl/ss...
File net/third_party/nss/ssl/ssl3con.c (right):

https://codereview.chromium.org/13985023/diff/6001/net/third_party/nss/ssl/ss...
net/third_party/nss/ssl/ssl3con.c:6418: * Because of this, RC4 is not suitable
for False Start. */

agl wrote:
> False Start specifically depends on the uniformity of the
> cipher over many connections, which is where RC4 is weak.

Thank you for the explanation. I think I got it now. It would
be good to add this to the comment. Note: your use of
"uniformity" is not clear to me. (I believe you meant
"lack of biases".)  Could you reword it? Thanks.

[agl, 2013-04-19 14:49:39]

https://codereview.chromium.org/13985023/diff/6001/net/third_party/nss/ssl/ss...
File net/third_party/nss/ssl/ssl3con.c (right):

https://codereview.chromium.org/13985023/diff/6001/net/third_party/nss/ssl/ss...
net/third_party/nss/ssl/ssl3con.c:6418: * Because of this, RC4 is not suitable
for False Start. */
On 2013/04/16 21:06:45, wtc wrote:
> Could you reword it? Thanks.

Done.

[commit-bot: I haz the power, 2013-04-19 14:49:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/agl@chromium.org/13985023/23001

[commit-bot: I haz the power, 2013-04-19 17:27:44]

Change committed as 195216