Closed
Bug 549641
Opened 15 years ago
Closed 15 years ago
Firefox raises alarm (in error console) about SSL servers being vulnerable to CVE-2009-3555
Categories
(Core :: Security: PSM, defect)
Core
Security: PSM
Tracking
()
RESOLVED
FIXED
People
(Reporter: BenB, Assigned: KaiE)
References
Details
(Keywords: dev-doc-complete)
Attachments
(2 files)
7.84 KB,
patch
|
rrelyea
:
review+
mayhemer
:
review+
dveditz
:
approval1.9.2.5-
dveditz
:
approval1.9.1.11-
|
Details | Diff | Splinter Review |
1004 bytes,
patch
|
KaiE
:
review+
dveditz
:
approval1.9.2.7+
dveditz
:
approval1.9.1.11+
|
Details | Diff | Splinter Review |
Firefox claims the SSL servers are vulnerable to CVE-2009-3555, even if the server is not vulnerable (or even if TLS renegotiation is entirely turned off?). This is a clearly false alarm. It's qualified by the word "potential", but that does little to mitigate the false alarm, as can be seen in bug 549109. False alarms are damaging in the long term, esp. in security matters, as most of us know. Therefore: 1) IMHO, Firefox should not show a warning unless it has reason to believe the server is indeed vulnerable. If that warning appears even if the most obvious (and often only) fix for servers is applied, namely showing even if TLS renegotiation is disabled, I think it should not show at all. The word "potential" does very little to mitigate the false alarm. 2) At the absolute minimum, please make the warning more specific. Please mention that we expect servers to implement RFC 5746. The warning as-is without that reference achieves very little other than raising alarm and making admins confused, as you can see in the bug 549109. Bug 549109 shows that the alarm did help to secure a critical server which before was unsecured, so I think the warning is very good and useful in general, but it should be more targetted and not show for servers where admins took appropriate steps to secure the server.
Comment 2•15 years ago
|
||
A Firefox bug filed in the Thunderbird product? ;-) Moving to PSM which is what's generating the warnings. It's not a false alarm in that we positively know that the server does not support RFC 5746, and that clients cannot be made secure until they stop connecting to such servers. It doesn't matter if server-A doesn't do renegotiation -- that helps server-A but doesn't help the user when he surfs to server-B and gets pwned. I'm certainly open to changing the text to something else, like "Warning: server does not support RFC 5746 (see CVE-2009-3555)" > Bug 549109 shows that the alarm did help to secure a critical server which > before was unsecured, so I think the warning is very good and useful in > general, but it should be more targetted and not show for servers where admins > took appropriate steps to secure the server. There is no way for the client to know the admin has taken appropriate steps to secure the server other than the server implementing the new renegotiation spec. I suppose the client could probe such servers to see if a renegotiation fails--at the cost of making SSL handshakes even more expensive than they are already!--but that's not foolproof because they might do renegotiation elsewhere on the site.
Assignee: nobody → kaie
Component: Security → Security: PSM
Product: Thunderbird → Core
QA Contact: thunderbird → psm
Comment 3•15 years ago
|
||
The appropriate level of warning should be discussed in the security or crypto newsgroups -- it doesn't seem very productive to hash it out in a bug.
Assignee | ||
Comment 4•15 years ago
|
||
Just wanted to clarify, the prober you suggest won't be sufficient to feel secure. A server may behave differently, based on which side of the connection attempt to initiate the negotiation. In other words, a server may have disabled client initiated renegotiations, and the test performed by the client may succeed. But still, the server may initiate renegotiations on its own, only when the server sees a need to (e.g. when a specific resource gets requested, which the prober doesn't know about). In my understanding there is no way for a prober to know that. The only fix is to implement RFC 5746, both in the client and the server.
Comment 5•15 years ago
|
||
Thanks, that's what I was trying to say with the "that's not foolproof" bit. I fully support some warning message when encountering such hosts but wouldn't mind explaining things differently if that helps. <site> : no RFC 5746 support, potentially vulnerable to CVE-2009-3555 is that any better? If not let's stick with what we've got and see what people say.
Reporter | ||
Comment 6•15 years ago
|
||
That message would make a wrong causal link between "no RFC 5746 support" and "vulnerable". That link may exist from our perspective, but not from the admin's. Therefore, I'd propose: "potentially vulnerable to CVE-2009-3555. Site should implement RFC 5746 support. <http://www.mozilla.org/.../tls-renegotiation>" (Note: not a wiki page!) On that page, explain the situation, for users and admins, and maybe also publish that you may in the future not support SSL servers which don't support that RFC.
Assignee | ||
Comment 7•15 years ago
|
||
... although your proposal would triple the number of bytes we dump into the error console. I don't personally mind, but today we dump this for a lot of sites, so trying to keep it should short would be good. Could we use a minimal text and point people to the help page, using a tinyurl?
Reporter | ||
Comment 8•15 years ago
|
||
While I agree that short is good, I don't think there's a particular need to save bytes. And all URLs we publish should be stable and under our exclusive control. Removing anything would remove critical information bits (CVE, RFC, no causal link, URL), e.g. why I filed this bug. How about: "<site> possibly vulnerable to CVE-2009-3555. Implement RFC 5746 <http://www.mozilla.org/tls-renegotiation>" I don't think we can or should make it any shorter.
Comment 9•15 years ago
|
||
(In reply to comment #6) > > <site> : no RFC 5746 support, potentially vulnerable to CVE-2009-3555 > > That message would make a wrong causal link between "no RFC 5746 support" and > "vulnerable". That link may exist from our perspective, but not from the > admin's. There absolutely is a link. If you don't support RI you ARE vulnerable if your site allows renegotiation. Your site is ONLY safe if you have taken proactive steps to disable renegotiation because renegotiation is on by default. Depending on your server software you might even have had to upgrade your server to be able to turn it off. The message is, you ARE vulnerable to CVE-2009-3555 unless you have taken steps to do something; implementing rfc 5746 is the recommended way of doing something because then people communicating with you can tell that you've done it. But you must do SOMEthing. If it makes people happy to swap s/potentially/possibly/ then fine, they mean pretty much the same to me. I really don't want to see a huge message because it's already going to clutter up the console. Admins only need to find the link once, but it's on the console every visit. All we need is enough information so admins can yahoo what the message means. Personally I'm fine with the current short message -- it does the job -- but we should put but a MDC page about what it means.
Keywords: dev-doc-needed
Comment 10•15 years ago
|
||
This warning actually comes up when you access MDC. I'm not clear on what it means. :)
Reporter | ||
Comment 11•15 years ago
|
||
> Your site is ONLY safe if you have taken proactive > steps to disable renegotiation because renegotiation is on by default. Exactly. The mozilla.org sites are safe (bug 549109), yet we say that they are "potentially vulnerable". This is particularly embarrassing for versioncheck.mozilla.org (which regularly happens and spams the error console) and addons.mozilla.org. The message suggests a problem which doesn't exist. With the security release upcoming, can we please fix this before we spam millions of users (who check the error console) and frighten them about a problem which may not even exist? We should at least lead them the right way, meaning reference "RFC 5746" in the message and give a link with explanations.
Comment 14•15 years ago
|
||
Is this bug about removing the info message altogether? If not how can we make it so? The current situation is not appropriate.
Assignee | ||
Comment 15•15 years ago
|
||
We believe it's useful to have a warning by default, it has already proven to be helpful, several servers have been upgraded as a consequence. Regarding the frequency of the message and being distracted by it, please note there are three classes of messages in the error console: - errors - warnings - messages The message about CVE-2009-3555 is shown as a plain message. If you click one of the icons that will filter only errors, or only warnings, you won't see this informational message. Also, we'd like to avoid adding code that would reduce the warning to once per session and site, as it would add unnecessary complexity on the stable branches, where we try to be conservative with new logic. Still, we'd like to address two concerns that have been raised, and the following compromise is being suggested. - instead of saying "potentially vulnerable to CVE-2009-3555" this patch changes it to say: "server does not support RFC 5746, see CVE-2009-3555" - for developers who experience trouble caused by the amount of additional console log message, we want to offer an additional pref. This patch adds pref security.ssl.warn_missing_rfc5746 It will have the value "1" by default, which means "show the warnings". Developers can set it to "0" to turn the warning off.
Attachment #436588 -
Flags: review?(rrelyea)
Comment 16•15 years ago
|
||
(In reply to comment #15) > Created an attachment (id=436588) [details] > Patch v1 > > We believe it's useful to have a warning by default, it has already proven to > be helpful, several servers have been upgraded as a consequence. As a matter of principle, this is still ridiculous. Creating some success on your agenda does not justify inflicting these messages on everyone else. > > Regarding the frequency of the message and being distracted by it, please note > there are three classes of messages in the error console: > - errors > - warnings > - messages > > The message about CVE-2009-3555 is shown as a plain message. If you click one > of the icons that will filter only errors, or only warnings, you won't see this > informational message. Your claim these messages are insignificant misses the reality that the Firefox Error Console is not the only client of nsIConsoleService. For example, the primary mechanism we have for debugging Javascript debuggers is logging from our application including feeds from nsIConsoleService. > > Also, we'd like to avoid adding code that would reduce the warning to once per > session and site, as it would add unnecessary complexity on the stable > branches, where we try to be conservative with new logic. > > Still, we'd like to address two concerns that have been raised, and the > following compromise is being suggested. > > - instead of saying "potentially vulnerable to CVE-2009-3555" > this patch changes it to say: > "server does not support RFC 5746, see CVE-2009-3555" That message provides no useful information to anyone other than the people reading this bug report. Your message should provide a URL to a mozilla maintained page providing specific instructions that will cause the message to be prevented. Otherwise you are just wasting more of everyone's time. > > - for developers who experience trouble caused by the amount of additional > console log message, we want to offer an additional pref. > This patch adds pref > security.ssl.warn_missing_rfc5746 > It will have the value "1" by default, which means "show the warnings". > Developers can set it to "0" to turn the warning off. Thank you, I will use it as soon as it becomes available.
Assignee | ||
Comment 17•15 years ago
|
||
For developers who would like to filter the message related to CVE-2009-3555, I'd like to make you aware of a tool that could help you. I found Firefox add-on Console² available from addons.mozilla.org It offers four ways to filter away the message: - you can click "messages" to filter out any informational messages and see all errors and all alerts or - you chould click "chrome" to filter out any messages that are produced from Mozilla internally (not produced by content of websites) or - you could enable "Options / Hide Duplicates" to filter repeated messages or - add -CVE-2009-3555 to search field
Comment 18•15 years ago
|
||
> security.ssl.warn_missing_rfc5746
> It will have the value "1" by default, which means "show the warnings".
Why an int pref instead of a boolean?
Assignee | ||
Comment 19•15 years ago
|
||
(In reply to comment #18) > > security.ssl.warn_missing_rfc5746 > > It will have the value "1" by default, which means "show the warnings". > > Why an int pref instead of a boolean? This would allow us to have different warning settings in the future, without adding yet another pref.
Reporter | ||
Comment 20•15 years ago
|
||
> This would allow us to have different warning settings in the future, without
> adding yet another pref.
Do you expect to introduce a level 2 which is even more chatty than now?
Assignee | ||
Comment 21•15 years ago
|
||
(In reply to comment #20) > > This would allow us to have different warning settings in the future, without > > adding yet another pref. > > Do you expect to introduce a level 2 which is even more chatty than now? I don't know yet what we might want to do, but when we discussed this a couple of days ago, we realized it's unfortunate that all the other prefs we had introduced recently were bools, not int, which makes it necessary to introduce yet another pref to suppress the warnings.
Comment 22•15 years ago
|
||
(In reply to comment #4) > Just wanted to clarify, the prober you suggest won't be sufficient to feel > secure. [...] > But still, the server may initiate renegotiations on its own, only when the > server sees a need to (e.g. when a specific resource gets requested, which the > prober doesn't know about). > > In my understanding there is no way for a prober to know that. Let me clarify this point, for I almost missed it. An attacker would request a resource, let the server initiate renegotiation, and then hand over to the client to complete what the client thinks is an initial handshake. Thus, having the client complain when it sees the server initiate renegotiation does not close the hole.
Comment 23•15 years ago
|
||
(In reply to comment #2) > I suppose the client could probe such servers to see if a renegotiation > fails--at the cost of making SSL handshakes even more expensive than they are > already!--but that's not foolproof because they might do renegotiation > elsewhere on the site. That probe would still be useful: if the renegotiation succeeds, I can tell the server admin that their server is actually vulnerable, and not merely failing to advertise its invulnerability. Currently, I am doing a separate test with "openssl s_client" (http://blog.ivanristic.com/2009/12/testing-for-ssl-renegotiation.html).
Reporter | ||
Comment 24•15 years ago
|
||
Matt, this bug is not about a general discussion about server vulnerability and its detection. Please do that on the newsgroups. This bug is merely about how (or whether) we *report* the facts no the error console.
Reporter | ||
Comment 25•15 years ago
|
||
> "server does not support RFC 5746, see CVE-2009-3555" Sounds good to me, esp. if you add ..." and <http://www.mozilla.org/tlsrenego>" or ..." and <http://www.mozilla.org/rfc5746>"
Assignee | ||
Comment 26•15 years ago
|
||
Comment on attachment 436588 [details] [diff] [review] Patch v1 Asking Honza for additional review, as Bob didn't have time yet. Honza, FYI, I consider file netwerk/base/public/security-prefs.js to be part of PSM, even though the file lives in netwerk (if we can we might want to move that file to PSM some day)
Attachment #436588 -
Flags: review?(honzab.moz)
Assignee | ||
Updated•15 years ago
|
Summary: Firefox raises false alarm (in error console) about SSL servers being vulnerable to CVE-2009-3555 → Firefox raises alarm (in error console) about SSL servers being vulnerable to CVE-2009-3555
Comment 27•15 years ago
|
||
Is IT/webdev aware that several of our own servers result in this warning? It's quite disturbing to see it coming up on mozilla sites. :)
![]() |
||
Comment 30•15 years ago
|
||
Comment on attachment 436588 [details] [diff] [review] Patch v1 > +++ b/security/manager/ssl/src/nsNSSCallbacks.cpp >@@ -871,25 +871,30 @@ void PR_CALLBACK HandshakeCallback(PRFil > nsNSSSocketInfo* infoObject = (nsNSSSocketInfo*) fd->higher->secret; >- nsCOMPtr<nsIConsoleService> console(do_GetService(NS_CONSOLESERVICE_CONTRACTID)); >- if (infoObject && console) { >+ nsCOMPtr<nsIConsoleService> console; >+ if (wantWarning) { >+ console = do_GetService(NS_CONSOLESERVICE_CONTRACTID); >+ } >+ if (wantWarning && infoObject && console) { > nsXPIDLCString hostName; > infoObject->GetHostName(getter_Copies(hostName)); > > nsAutoString msg; > msg.Append(NS_ConvertASCIItoUTF16(hostName)); >- msg.Append(NS_LITERAL_STRING(" : potentially vulnerable to CVE-2009-3555")); >+ msg.Append(NS_LITERAL_STRING(" : server does not support RFC 5746, see CVE-2009-3555")); > > console->LogStringMessage(msg.get()); > } Wouldn't be nicer to simply put this code block to a branch if (wantWarning) { } ?
Attachment #436588 -
Flags: review?(honzab.moz) → review+
Comment 31•15 years ago
|
||
Comment on attachment 436588 [details] [diff] [review] Patch v1 r+.. one style issue which Honras already pointed out. The current style is easier for current reviewers. Future reviewers would probably expect the nesting. But I leave it to you decide if this is acceptable PSM style. bob
Attachment #436588 -
Flags: review?(rrelyea) → review+
Comment 33•15 years ago
|
||
In http://gsfn.us/t/106av Thunderbird users are discovering that their outgoing email is broken because of CVE-2009-3555.
Comment 34•15 years ago
|
||
(In reply to comment #33) > In http://gsfn.us/t/106av Thunderbird users are discovering that their outgoing > email is broken because of CVE-2009-3555. Roland's reply (http://getsatisfaction.com/mozilla_messaging/topics/thunderbird_cannot_send_email#reply_2313669) clearly says "The CVE-2009-3555 error is not related to this issue.", so your above comment seems wrong.
Reporter | ||
Comment 35•15 years ago
|
||
Correct, their impression is wrong, but nevertheless that's the conclusion they draw. That's what this bug is about: the false impression.
Assignee | ||
Comment 36•15 years ago
|
||
(In reply to comment #30) > > Wouldn't be nicer to simply put this code block to a branch if (wantWarning) { > } ? I'll switch to nested "if"s and add one more indent level. nsCOMPtr<nsIConsoleService> console; if (infoObject && wantWarning) { console = do_GetService(NS_CONSOLESERVICE_CONTRACTID); if (console) { nsXPIDLCString hostName; infoObject->GetHostName(getter_Copies(hostName)); nsAutoString msg; msg.Append(NS_ConvertASCIItoUTF16(hostName)); msg.Append(NS_LITERAL_STRING(" : server does not support RFC 5746, see CVE-2009-3555")); console->LogStringMessage(msg.get()); } }
Assignee | ||
Comment 37•15 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/ccd19364cb13
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 38•15 years ago
|
||
Comment on attachment 436588 [details] [diff] [review] Patch v1 Requesting approval for log message improvement (and pref to disable) for stable branches.
Attachment #436588 -
Flags: approval1.9.2.5?
Attachment #436588 -
Flags: approval1.9.1.11?
Comment 40•14 years ago
|
||
Comment on attachment 436588 [details] [diff] [review] Patch v1 not approved for the stable branches, a lot of code churn for what is effectively a simple string change for 99.99% of our users.
Attachment #436588 -
Flags: approval1.9.2.5?
Attachment #436588 -
Flags: approval1.9.2.5-
Attachment #436588 -
Flags: approval1.9.1.11?
Attachment #436588 -
Flags: approval1.9.1.11-
Assignee | ||
Comment 41•14 years ago
|
||
(In reply to comment #40) > (From update of attachment 436588 [details] [diff] [review]) > not approved for the stable branches, a lot of code churn for what is > effectively a simple string change for 99.99% of our users. The string change is a single line, if you'd like that, only. The other 99% of the patch are the option to suppress the warning.
Comment 42•14 years ago
|
||
For the branches, yes please. Anyone actually bothered enough to find out about the pref can download a trunk beta by time we get 1.9.2.6 released.
Assignee | ||
Comment 43•14 years ago
|
||
This is a subset of the patch which changes the string, only, intended for stable branches. Obvious single line change to use the identical string as used on trunk. rs=me Requesting branch approval
Attachment #451590 -
Flags: review+
Attachment #451590 -
Flags: approval1.9.2.6?
Attachment #451590 -
Flags: approval1.9.1.11?
Comment 44•14 years ago
|
||
Comment on attachment 451590 [details] [diff] [review] Branch patch, string change only Thanks! Approved for 1.9.2.6 and 1.9.1.11, a=dveditz for release-drivers
Attachment #451590 -
Flags: approval1.9.2.6?
Attachment #451590 -
Flags: approval1.9.2.6+
Attachment #451590 -
Flags: approval1.9.1.11?
Attachment #451590 -
Flags: approval1.9.1.11+
Assignee | ||
Comment 45•14 years ago
|
||
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/bc5ae6a0542e
status1.9.1:
--- → .11-fixed
Assignee | ||
Comment 46•14 years ago
|
||
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/7e4fa2fc106d
status1.9.2:
--- → .6-fixed
Comment 47•14 years ago
|
||
Now documented: https://developer.mozilla.org/en/Preferences/Mozilla_preferences_for_uber-geeks
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•