947149 - Connection information claims RC4 is "high grade"

Status: RESOLVED FIXED

(Core Graveyard :: Security: UI, enhancement)

Description

[Jeffrey Walton]

Attachment: firefox-high-grade.png

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:25.0) Gecko/20100101 Firefox/25.0 (Beta/Release)
Build ID: 20131112160018

Steps to reproduce:

Visited a site (https://community.qualys.com) where negotiated cipher suite was RSA_WITH_RC4_128_SHA.



Expected results:

There is nothing "high grade" about RC4. It was the lesser of the evils in 2011 when Duong and Rizzo's BEAST was making headlines.

See, for example, "On the Security of RC4 in TLS and WPA", http://cr.yp.to/streamciphers/rc4biases-20130708.pdf:

    ... While the RC4 algorithm is known to have a
    variety of cryptographic weaknesses (see [26]
    for an excellent survey), it has not been previously
    explored how these weaknesses can be exploited
    in the context of TLS. Here we show that new and
    recently discovered biases in the RC4 keystream
    do create serious vulnerabilities in TLS when using
    RC4 as its encryption algorithm.

If Firefox wants a stream cipher, use GCM mode or the upcoming Salsa20.

Comment 1

[Dave Garrett]

See also bug 956744.

Comment 2

[Masatoshi Kimura [:emk]]

Attachment: Don't treat RC4 as a strong cipher

Comment 3

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Comment on attachment 8511583 [details] [diff] [review]
Don't treat RC4 as a strong cipher

Review of attachment 8511583 [details] [diff] [review]:
-----------------------------------------------------------------

Honestly, the distinction here is meaningless. Saying "high-grade" vs. "low-grade" isn't actionable. First, you've already visited the page using weak encryption to see this dialog, so your cookies have already been stolen. Second, the (vanishingly small number) of people who would (or indeed, could) use this information to inform whether or not they should continue using a site know how to decipher a string like "TLS_RSA_WITH_RC4_128_SHA", so "high-grade" vs. "low-grade" doesn't tell them anything they don't already know. Finally, even if the primary load of a page was delivered using a strong cipher, everything else on the page could have been delivered with a weak cipher, so presenting the information in this way is misleading at best in the first place. Really what we should do is expose this information per-load in the devtools. In the meantime, I would r+ a patch that removes the distinction here.

On the other hand, if we do expose what cipher suite each resource load used in the devtools, we probably would want to be able to classify each one according to best practices (including various things like perfect forward secrecy, key strength, etc. - see also bug 956744). So, I would also be interested in a patch that introduces a more general framework for classifying cipher suites.

Comment 4

[Masatoshi Kimura [:emk]]

Attachment: Remove useless and even misleading word and dead code

Currently all supported cipher suites have >=90 bit keys. So the "weak" variants of strings are already dead.
I also remove the word "high-grade" which is useless and even misleading.

Comment 5

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Comment on attachment 8512612 [details] [diff] [review]
Remove useless and even misleading word and dead code

Review of attachment 8512612 [details] [diff] [review]:
-----------------------------------------------------------------

This looks great. I just noted a couple nits. You should probably get a review from an actual Firefox (as in, the front-end) reviewer and sign-off from someone from UX (e.g. :phlsa) before landing this.

::: browser/base/content/pageinfo/security.js
@@ +256,5 @@
>      hdr = pkiBundle.getString("pageInfo_MixedContent");
>      msg1 = pkiBundle.getString("pageInfo_Privacy_Mixed1");
>      msg2 = pkiBundle.getString("pageInfo_Privacy_None2");
>    }
>    else if (info.encryptionStrength >= 90) {

Hmmm - should this be changed to info.encryptionStrength > 0 now?

@@ -271,5 @@
> -                                        [info.encryptionAlgorithm,
> -                                         info.encryptionStrength + "",
> -                                         info.version]);
> -    msg1 = pkiBundle.getFormattedString("pageInfo_Privacy_Weak1", [info.hostName]);
> -    msg2 = pkiBundle.getString("pageInfo_Privacy_Weak2");

Huh - looks like there was a bug here where security._cert wouldn't get set in the "weak encryption" case (although maybe that was on purpose - who knows).

::: security/manager/locales/en-US/chrome/pippki/pippki.properties
@@ +80,2 @@
>  pageInfo_Privacy_Strong1=The page you are viewing was encrypted before being transmitted over the Internet.
>  pageInfo_Privacy_Strong2=Encryption makes it very difficult for unauthorized people to view information traveling between computers. It is therefore very unlikely that anyone read this page as it traveled across the network.

Now that I'm really picking nits, I would get rid of the "very"s in this sentence, but it's not a big deal.

Comment 6

[Masatoshi Kimura [:emk]]

Attachment: Remove useless and even misleading word and dead code, v2

(In reply to David Keeler (:keeler) [use needinfo?] from comment #5)
> Now that I'm really picking nits, I would get rid of the "very"s in this
> sentence, but it's not a big deal.

OK, I've got the reason to rename the key :)

Comment 7

[Dave Garrett]

Comment on attachment 8513038 [details] [diff] [review]
Remove useless and even misleading word and dead code, v2

>+pageInfo_Privacy_Encrypted1=The page you are viewing was encrypted before being transmitted over the Internet.
>+pageInfo_Privacy_Encrypted2=Encryption makes it difficult for unauthorized people to view information traveling between computers. It is therefore unlikely that anyone read this page as it traveled across the network.

I suggest just dropping the second sentence of pageInfo_Privacy_Encrypted2 entirely. It doesn't really add any information and just pads the text out. There's also the technicality of PFS being required for it to stay true.

Comment 8

[Justin Dolske [:Dolske]]

Comment on attachment 8513038 [details] [diff] [review]
Remove useless and even misleading word and dead code, v2

Review of attachment 8513038 [details] [diff] [review]:
-----------------------------------------------------------------

*yoink* I think this is fine, and doesn't even need explicit ui-review.

But I did just want to explicitly confirm that comment 4 ("Currently all supported cipher suites have >=90 bit keys.") is true, which makes this is a no-brainer.

Looks so from https://www.ssllabs.com/ssltest/viewMyClient.html... I know there's still some MD5/3DES/RC4 in there that we'd like to get rid of, but that's a whole 'nuther discussion not suitable for this bug. (And, really, if we thought it was important enough to try and explain, we should really just treat them as broken SSL.)

Comment 9

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Thanks, Dolske. Here's the list of ciphers enabled in firefox: https://bugzilla.mozilla.org/show_bug.cgi?id=947149 (so yes, as far as I can tell, there are none that have <= 90-bit keys). See also bug 1088915 where we're exploring only using RC4 as a fallback.

Comment 10

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

(In reply to David Keeler (:keeler) [use needinfo?] from comment #9)
> Thanks, Dolske. Here's the list of ciphers enabled in firefox:
> https://bugzilla.mozilla.org/show_bug.cgi?id=947149

Er, I meant this: http://hg.mozilla.org/mozilla-central/annotate/675913ddbb55/security/manager/ssl/src/nsNSSComponent.cpp#l631

Comment 11

[Masatoshi Kimura [:emk]]

I would like to make RC4 broken (in another bug), but only after bug 1088915.
Because otherwise too many servers (>= 15%) will negotiate RC4.

Comment 12

[Masatoshi Kimura [:emk]]

Attachment: patch for checkin

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7277fa721a3f

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7277fa721a3f

Comment 16

[Yuhong Bao]

(In reply to David Keeler (:keeler) [use needinfo?] from comment #3)
> Honestly, the distinction here is meaningless. Saying "high-grade" vs.
> "low-grade" isn't actionable. First, you've already visited the page using
> weak encryption to see this dialog, so your cookies have already been
> stolen. 
I think Mozilla browsers used to be able to warn when a "low-grade" (export) cipher suite is used with a modal dialog.