Closed Bug 947149 Opened 11 years ago Closed 10 years ago

Connection information claims RC4 is "high grade"

Categories

(Core Graveyard :: Security: UI, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla36

People

(Reporter: noloader, Assigned: emk)

References

Details

Attachments

(2 files, 3 obsolete files)

Attached image firefox-high-grade.png (deleted) —
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.
Component: Untriaged → Security
Product: Firefox → Core
OS: Mac OS X → All
Hardware: x86 → All
Severity: normal → enhancement
Component: Security → Security: UI
Version: 25 Branch → Trunk
Attached patch Don't treat RC4 as a strong cipher (obsolete) (deleted) — Splinter Review
Attachment #8511583 - Flags: review?(dkeeler)
Assignee: nobody → VYV03354
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
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.
Attachment #8511583 - Flags: review?(dkeeler) → review-
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.
Attachment #8511583 - Attachment is obsolete: true
Attachment #8512612 - Flags: review?(dkeeler)
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.
Attachment #8512612 - Flags: review?(dkeeler) → review+
(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 :)
Attachment #8513038 - Flags: review?(philipp)
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 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.)
Attachment #8513038 - Flags: review?(philipp) → review+
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.
(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
I would like to make RC4 broken (in another bug), but only after bug 1088915. Because otherwise too many servers (>= 15%) will negotiate RC4.
Attached patch patch for checkin (deleted) — Splinter Review
Attachment #8512612 - Attachment is obsolete: true
Attachment #8513038 - Attachment is obsolete: true
Attachment #8514621 - Flags: review+
Keywords: checkin-needed
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
(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.
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: