+++ This bug was initially created as a clone of Bug #920248 +++ From bug 920248 comment #19: > Note that in this patch I changed the default to require perfect forward > secrecy, and I removed false start support for RC4. > > I verified that we negotiate AES-GCM cipher suites when connecting to > Twitter, Facebook, CloudFlare, and Google. > > I also modified the telemetry so that we have more precise measurements for > why we don't false start. So far in my limited testing, I've found that > requiring forward secrecy and not allowing false start for RC4 have no > effect on how often we false start. There appear to be two primary reasons: > (1) The NPN requirement seems to subsume these requirements; that is, sites > that support NPN seem to negotiate non-RC4 cipher suites with ephemeral key > exchange already, and (2) The false start callback isn't called in many > cases, presumably because the Finished message arrived before the > certificate was successfully verified. Consequently, based on my limited > testing, it seems like the primary improvements we need to make to improve > the frequency of false starting are to remove the NPN requirement and to > make certificate verification faster (i.e. stop doing OCSP requests). > > Let's see if the telemetry we gather on Nightly and Aurora users matches my > personal measurements.

I forgot to mention that I also removed 3DES from the set of cipher suites for which we false start, and I added Camellia. This is based on my research of the security merits and also on the telemetry that shows that 3DES is basically never negotiated in Nightly: http://telemetry.mozilla.org/#path=nightly/28/SSL_SYMMETRIC_CIPHER

(In reply to Brian Smith (:briansmith, was :bsmith; Please NEEDINFO? me if you want a response) from comment #0) it seems like the primary improvements we need to make to improve > > the frequency of false starting are to remove the NPN requirement and to > > make certificate verification faster (i.e. stop doing OCSP requests). I share that expectation

I think this patch is likely fine - thanks! I've read it but want to test it out before I stick an r+ on it. I'll do that tomorrow. I've added a second patch that just deletes a bunch of code now that we don't use need to track rc4 history in the permission manager anymore. I want to actually test that patch before I put the r? flag on it :)

(In reply to Patrick McManus [:mcmanus] from comment #4) > I've added a second patch that just deletes a bunch of code now that we > don't use need to track rc4 history in the permission manager anymore. I > want to actually test that patch before I put the r? flag on it :) Are you sure we should do that now? If the code isn't in the way, I'd rather keep it. We might need it for CBC mode or we may need it if we decide that blocking downgrade to RC4 is still too drastic. I think the telemetry my patch adds will help inform that decision.

(In reply to Brian Smith (:briansmith, was :bsmith; Please NEEDINFO? me if you want a response) from comment #5) > (In reply to Patrick McManus [:mcmanus] from comment #4) > > I've added a second patch that just deletes a bunch of code now that we > > don't use need to track rc4 history in the permission manager anymore. I > > want to actually test that patch before I put the r? flag on it :) > > Are you sure we should do that now? If the code isn't in the way, I'd rather > keep it. We might need it for CBC mode or we may need it if we decide that > blocking downgrade to RC4 is still too drastic. I think the telemetry my > patch adds will help inform that decision. I think so - it tracks rc4 specifically with the permission manager which is definitely something I would prefer not to have if we didn't need it.. I'm skeptical of databases :) We have it as a separate patch so it should be easy to add back if it serves a purpose again and I would be cool with that if the need arose.

Comment on attachment 8337599 [details] [diff] [review] re-enable-false-start.patch Review of attachment 8337599 [details] [diff] [review]: ----------------------------------------------------------------- lgtm. I manually verified with facebook to see the right behavior wrt OCSP. It was really nice to see a false-started tls 1.2 TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 connection! ::: security/manager/ssl/src/nsNSSCallbacks.cpp @@ +963,4 @@ > } > > + // XXX: This assumes that all TLS_DH_* and TLS_ECDH_* cipher suites > + // are disabled. whitespace nit

I will check these in after fixing the whitespace nit.

https://hg.mozilla.org/integration/mozilla-inbound/rev/e2f70138f5aa https://hg.mozilla.org/integration/mozilla-inbound/rev/288b02c2e5d1

https://hg.mozilla.org/mozilla-central/rev/e2f70138f5aa https://hg.mozilla.org/mozilla-central/rev/288b02c2e5d1

FWIW, Apple's MacOSX/Safari criteria are very similar to ours, except that they don't require NPN and they allow RC4. They require AES and they require ephemeral key exchange. http://opensource.apple.com/source/Security/Security-55471/libsecurity_ssl/lib/sslTransport.c

Brian, does this need any QA testing before we release Firefox 28?

(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #13) > Brian, does this need any QA testing before we release Firefox 28? No, I don't know of any good way for QA to test it. We (Patrick and I) have tested it manually quite a few times already.