Masatoshi Kimura [:emk] Assignee
	Description • 10 years ago

Chrome will drop the support since version 44 which will be released in July 2015.

	Masatoshi Kimura [:emk] Assignee
	Comment 1 • 9 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6acec7bc4e2a

	Masatoshi Kimura [:emk] Assignee
	Comment 2 • 9 years ago

This patch should be landed after the aurora merge.

	Masatoshi Kimura [:emk] Assignee
	Comment 3 • 9 years ago

Removed more dead code.
I didn't remove strings in case we have to backout on future branches.

	Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)
	Comment 4 • 9 years ago

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.

	Masatoshi Kimura [:emk] Assignee
	Comment 5 • 9 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.

	Masatoshi Kimura [:emk] Assignee
	Comment 7 • 9 years ago

More removal needed.

	Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)
	Comment 8 • 9 years ago

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)

	Masatoshi Kimura [:emk] Assignee
	Comment 9 • 9 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fc786b154f1b

	Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)
	Comment 10 • 9 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.

	Masatoshi Kimura [:emk] Assignee
	Comment 11 • 9 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

	Lawrence Mandel [:lmandel] (use needinfo)
	Comment 12 • 9 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?

	Matt Wobensmith [:mwobensmith][:matt:]
	Comment 13 • 9 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.

	Masatoshi Kimura [:emk] Assignee
	Comment 14 • 9 years ago

To remove a footgun. As we removed SSLv2.

	Masatoshi Kimura [:emk] Assignee
	Comment 15 • 9 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

	Masatoshi Kimura [:emk] Assignee
	Comment 16 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/e78c98cefda7
https://hg.mozilla.org/integration/mozilla-inbound/rev/8572d3e909a3

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 17 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/e78c98cefda7
https://hg.mozilla.org/mozilla-central/rev/8572d3e909a3

	Matt Wobensmith [:mwobensmith][:matt:]
	Comment 18 • 9 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.

	Liz Henry (:lizzard) (relman/hg->git project)
	Comment 19 • 9 years ago

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.

	Masatoshi Kimura [:emk] Assignee
	Comment 20 • 9 years ago

I've confirmed Chrome 44 dropped SSLv3 entirely.
https://code.google.com/p/chromium/issues/detail?id=487730

	Seb A
	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!

	rsx11m
	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).

	ipatrol
	Comment 23 • 8 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.

	ipatrol
	Comment 24 • 8 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.

	Dave Garrett
	Comment 25 • 8 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).