Closed
Bug 1106470
Opened 10 years ago
Closed 10 years ago
Drop SSLv3 support entirely
Categories
(Core :: Security: PSM, defect)
Core
Security: PSM
Tracking
()
VERIFIED
FIXED
mozilla39
People
(Reporter: emk, Assigned: emk)
References
Details
(Keywords: dev-doc-needed, site-compat)
Attachments
(2 files, 3 obsolete files)
(deleted),
patch
|
keeler
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
vporof
:
review+
|
Details | Diff | Splinter Review |
Chrome will drop the support since version 44 which will be released in July 2015.
Assignee | ||
Updated•10 years ago
|
Updated•10 years ago
|
OS: Windows 7 → All
Hardware: x86 → All
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8567850 -
Flags: review?(dkeeler)
Assignee | ||
Comment 2•10 years ago
|
||
This patch should be landed after the aurora merge.
Assignee | ||
Comment 3•10 years ago
|
||
Removed more dead code.
I didn't remove strings in case we have to backout on future branches.
Attachment #8567850 -
Attachment is obsolete: true
Attachment #8567850 -
Flags: review?(dkeeler)
Attachment #8568473 -
Flags: review?(dkeeler)
Comment on attachment 8568473 [details] [diff] [review]
patch v2
Review of attachment 8568473 [details] [diff] [review]:
-----------------------------------------------------------------
In general, looks good. I found more things to remove, however. Also, we should keep a very close eye on this. If it doesn't look like SSL 3 is going to be similarly removed from Chrome 44 or if that release date is not close enough to the release date for Firefox 39 (looks like 30 June 2015), we should hold this back at least one release.
security/manager/ssl/tests/gtest/TLSIntoleranceTest.cpp should probably be updated to reflect the new minimum supported version.
I found some remaining uses of STATE_USES_SSL_3 in toolkit/devtools/webconsole/network-helper.js and toolkit/devtools/webconsole/test/unit/test_security-info-weakness-reasons.js (we should probably have someone from devtools look at these changes as well, since they just added support for showing when a connection uses SSL 3).
I think I should have another look, so r- for now.
::: browser/devtools/netmonitor/test/browser_net_security-warnings.js
@@ +35,5 @@
> ]}, resolve);
> });
>
> let cipher = $("#security-warning-cipher");
> let sslv3 = $("#security-warning-sslv3");
Looks like this and related things should be removed, too.
::: netwerk/protocol/http/nsHttpChannel.cpp
@@ -1218,5 @@
> if (securityInfo &&
> NS_SUCCEEDED(securityInfo->GetSecurityState(&state)) &&
> (state & nsIWebProgressListener::STATE_IS_BROKEN)) {
> // Send weak crypto warnings to the web console
> - if (state & nsIWebProgressListener::STATE_USES_SSL_3) {
Looks like there is still code that sets this flag in nsNSSIOLayer.cpp (around line 1212).
Also, it should be removed from uriloader/base/nsIWebProgressListener.idl and docshell/base/nsDocShell.cpp
::: security/manager/ssl/src/nsNSSIOLayer.cpp
@@ -1297,5 @@
> post = Telemetry::SSL_TLS10_INTOLERANCE_REASON_POST;
> break;
> - case SSL_LIBRARY_VERSION_3_0:
> - pre = Telemetry::SSL_SSL30_INTOLERANCE_REASON_PRE;
> - post = Telemetry::SSL_SSL30_INTOLERANCE_REASON_POST;
These probes should be removed from toolkit/components/telemetry/Histograms.json as well.
Attachment #8568473 -
Flags: review?(dkeeler) → review-
Assignee | ||
Comment 5•10 years ago
|
||
(In reply to David Keeler [:keeler] (use needinfo?) from comment #4)
> I found some remaining uses of STATE_USES_SSL_3 in
> toolkit/devtools/webconsole/network-helper.js and
> toolkit/devtools/webconsole/test/unit/test_security-info-weakness-reasons.js
> (we should probably have someone from devtools look at these changes as
> well, since they just added support for showing when a connection uses SSL
> 3).
I divided the devtools part to another patch.
> ::: netwerk/protocol/http/nsHttpChannel.cpp
> @@ -1218,5 @@
> > if (securityInfo &&
> > NS_SUCCEEDED(securityInfo->GetSecurityState(&state)) &&
> > (state & nsIWebProgressListener::STATE_IS_BROKEN)) {
> > // Send weak crypto warnings to the web console
> > - if (state & nsIWebProgressListener::STATE_USES_SSL_3) {
>
> Looks like there is still code that sets this flag in nsNSSIOLayer.cpp
> (around line 1212).
This flag is repurposed to indicate an error when the server tried to negotiate SSLv3 (and we refused). So we can't remove this until we remove the dedicated error page.
> Also, it should be removed from uriloader/base/nsIWebProgressListener.idl
> and docshell/base/nsDocShell.cpp
Even if the flag is no longer used, I will not change the idl to make the late backout possible, just like strings. In theory, constant-only change should not require the iid bump, but now a server-side hook will refuse to push the commit without changing the iid.
Attachment #8568473 -
Attachment is obsolete: true
Attachment #8569820 -
Flags: review?(dkeeler)
Assignee | ||
Comment 6•10 years ago
|
||
Attachment #8569821 -
Flags: review?(vporof)
Assignee | ||
Comment 7•10 years ago
|
||
More removal needed.
Attachment #8569821 -
Attachment is obsolete: true
Attachment #8569821 -
Flags: review?(vporof)
Attachment #8569853 -
Flags: review?(vporof)
Comment on attachment 8569820 [details] [diff] [review]
patch v2, PSM part
Review of attachment 8569820 [details] [diff] [review]:
-----------------------------------------------------------------
Ok - r=me.
::: security/manager/ssl/tests/gtest/TLSIntoleranceTest.cpp
@@ +20,5 @@
> };
>
> TEST_F(TLSIntoleranceTest, Test_Full_Fallback_Process)
> {
> + helpers.mVersionFallbackLimit = SSL_LIBRARY_VERSION_TLS_1_0;
Let's remove this line and assert that we're expecting the fallback limit to be TLS 1.0.
@@ +113,5 @@
> SSL_LIBRARY_VERSION_TLS_1_0,
> 0));
> }
>
> TEST_F(TLSIntoleranceTest, Test_Fallback_Limit_Default)
Hmmm - I'm not sure this test is testing anything important anymore (i.e. I think Test_Full_Fallback_Process now covers what this test does, particularly if the line asserting what the default limit is gets moved to that test)
Attachment #8569820 -
Flags: review?(dkeeler) → review+
Assignee | ||
Comment 9•10 years ago
|
||
We should probably post to dev.platform about this change. :emk, since you're spearheading this, I thought you might want to be the one to tell everyone the good news. If not, I can.
Assignee | ||
Comment 11•10 years ago
|
||
(In reply to David Keeler [:keeler] (use needinfo?) from comment #10)
> We should probably post to dev.platform about this change. :emk, since
> you're spearheading this, I thought you might want to be the one to tell
> everyone the good news. If not, I can.
Sent a mail:
https://lists.mozilla.org/pipermail/dev-platform/2015-February/008785.html
Updated•10 years ago
|
Keywords: dev-doc-needed,
site-compat
Comment 12•10 years ago
|
||
Release Note Request (optional, but appreciated)
[Why is this notable]: Dropping SSLv3 seems like a notable security change.
[Suggested wording]: Removed support for SSLv3
[Links (documentation, blog post, etc)]:
Does this warrant a blog post from the sec team?
relnote-firefox:
--- → ?
Comment 13•10 years ago
|
||
Question for anyone: if this is disabled by default since Fx34, why do we need to remove it entirely?
I'm trying to assess the testing impact as well, if any.
Thanks.
Assignee | ||
Comment 14•10 years ago
|
||
To remove a footgun. As we removed SSLv2.
Assignee | ||
Comment 15•10 years ago
|
||
Also we can simplify some logic because we can assume SSLv3 will never be negotiated. An example:
https://mxr.mozilla.org/mozilla-central/source/security/manager/ssl/src/nsNSSIOLayer.cpp?rev=5801ebeeb3b6&mark=1260-1261,1264-1273#1258
Updated•10 years ago
|
Attachment #8569853 -
Flags: review?(vporof) → review+
Assignee | ||
Comment 16•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/e78c98cefda7
https://hg.mozilla.org/integration/mozilla-inbound/rev/8572d3e909a3
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/e78c98cefda7
https://hg.mozilla.org/mozilla-central/rev/8572d3e909a3
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox39:
--- → fixed
Resolution: --- → FIXED
Comment 18•10 years ago
|
||
Just to make sure, I ran a compatibility pass for this change on builds of Fx39 both before and after the code was removed. I ran the Pulse top sites list. No regressions were found - hence, marking verified.
Status: RESOLVED → VERIFIED
Relnote added with the wording: "Removed support for SSLv3".
If there's dev documentation or a blog post on this please need-info me or email me so I can add a link to the release note.
Assignee | ||
Comment 20•10 years ago
|
||
I've confirmed Chrome 44 dropped SSLv3 entirely.
https://code.google.com/p/chromium/issues/detail?id=487730
Comment 21•9 years ago
|
||
2 things:
1. Removing SSLv3 meant I spent about hours back in July or August trying to connect to a Dell Remote Access Controller (DRAC/MC) for a blade chassis that only supports SSL and not TLS. It used to work in Firefox, but I always had issues with IE already, but then I tried to get it working on IE and Java was a pain about certificates and you have to know the exact URL to add an exeception, which you don't because it's hidden, and these DRACs are pretty unreliable and unresponsive when it comes to connecting to them anyway - so thanks for wasting my time by removing SSLv3 rather than just leaving it disabled by default. (And, no, Dell are not providing an update for this equipment to support TLS, even though it is not that old.)
2. SSLv3 was removed in Firefox 39.0 (in plain English, for the benefit of people trying to work this out to work out which old or ASR version to download). I'll try Firefox 38.4.0 ESR and see if I can finally connect to this chassis!
Comment 22•9 years ago
|
||
As far as I can tell, this didn't check into mozilla-esr38 and thus the current 38.x ESR releases should still support SSL 3.0, so that should be a viable workaround. Inevitably, this issue will come up again for you once support for that branch is no longer provided (in about 30 weeks from now).
Comment 23•9 years ago
|
||
(In reply to rsx11m from comment #22)
> As far as I can tell, this didn't check into mozilla-esr38 and thus the
> current 38.x ESR releases should still support SSL 3.0, so that should be a
> viable workaround. Inevitably, this issue will come up again for you once
> support for that branch is no longer provided (in about 30 weeks from now).
Which, as of the time of writing, is fast approaching.
The DD-Wrt project still cannot fit TLS onto routers with less than 4MB of storage, of which there are a good number of in operation. The logic of completely disabling SSLv3 for so long as plain HTTP is supported escapes me, unless Mozilla plans to deprecate that too.
Comment 24•9 years ago
|
||
> The logic of
> completely disabling SSLv3 for so long as plain HTTP is supported escapes
> me, unless Mozilla plans to deprecate that too.
It appears I spoke too soon:
https://blog.mozilla.org/security/2015/04/30/deprecating-non-secure-http/
Lovely.
Comment 25•9 years ago
|
||
(In reply to ipatrol from comment #23)
> (In reply to rsx11m from comment #22)
> > As far as I can tell, this didn't check into mozilla-esr38 and thus the
> > current 38.x ESR releases should still support SSL 3.0, so that should be a
> > viable workaround. Inevitably, this issue will come up again for you once
> > support for that branch is no longer provided (in about 30 weeks from now).
>
> Which, as of the time of writing, is fast approaching.
>
> The DD-Wrt project still cannot fit TLS onto routers with less than 4MB of
> storage, of which there are a good number of in operation.
If you're continuing to use obsolete hardware that requires an obsolete and insecure security protocol that is now 20 years old, it is not exactly a big ask to require an obsolete browser to continue to access it. Have a separate profile for obsolete access and use the last ESR that is capable of doing so, just like we still sadly have the need for old versions of IE for obsolete sites/services in some instances (and my guess is, a router or two).
> The logic of
> completely disabling SSLv3 for so long as plain HTTP is supported escapes
> me, unless Mozilla plans to deprecate that too.
It's a 20 year old obsolete insecure security protocol, with shockingly wide continued support on servers where continuing to play along is downright dangerous. It needs to be nuked from orbit and everyone needs to finally join the 21st century. The same is true for insecure HTTP, but that is a much much longer road.
Further comments here are not productive. If you wish to discuss interop with obsolete systems, please do so on a Mozilla forum/group/mailinglist and not here (it's a legitimate topic of discussion, but this issue here is closed). If you have a specific RFE or other issue that is appropriate for this system, please file a new bug (taking into account that nobody will be re-adding support for SSL).
You need to log in
before you can comment on or make changes to this bug.
Description
•