Comment 1 Martin Thomson [:mt] 2014-10-02 10:30:30 PDT

Just checked details:

The patches on bug 1036737 and bug 1072382 do not require alteration in order to land on beta, so aurora should be fine.  I just need to know how to request and gain approval without alerting people.  Should I attach them here?

Comments on bug 1053565 and commit logs show that beta already has the right version of NSS.

Comment 2 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2014-10-02 10:40:28 PDT

I suggest that you also consider implementing the fix for bug 689814; that is, don't do non-secure fallback to SSL 3.0 at all. There are lots of reasons to move forward with that anyway, regardless of this vulnerability (e.g. Must-Staple, because it requires TLS 1.0+), and this is the best opportunity you'll have to kill the non-secure fallback to SSL 3.0. Note that servers could still negotiate SSL 3.0 in the secure way, even if you disable the non-secure fallback.

Enabling only RC4 during the fallback is also OK, but doing so pinpoints the vulnerability. Disabling SSL 3.0 non-secure fallback completely just telegraphs that there is *some* problem with SSL 3.0.

Comment 3 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2014-10-02 10:41:48 PDT

By the way, when you did the "see also" for bug 1075991, you've already publicly hinted that there is some problem with non-secure version fallback, because bug 1075991 was updated to refer to a "secure bug".

Comment 4 Martin Thomson [:mt] 2014-10-02 10:53:03 PDT

Damn, I had assumed that Bugzilla wouldn't do that to me.

Comment 5 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2014-10-02 10:59:27 PDT

When BEAST happened, Microsoft, Google, and Mozilla agreed on doing the exact-same fix at about the same time. That fix had negative compatibility impact, but the coordination meant that nobody was at an unfair disadvantage for doing the right thing. I recommend repeating that here, by trying to get agreement with MS and Google on the exact mitigation. I don't think relying on the downgrade SCSV is a reasonable solution to the problem, because it is too new, and it would take time for enough servers to deploy it. So, I recommend asking MS and Google to bite the bullet with you and disable the non-secure fallback to SSL 3.0.

Comment 6 Martin Thomson [:mt] 2014-10-02 11:04:09 PDT

Telemetry suggests we're at around 0.26% on SSL 3.0.  Is that low enough to consider the losses tolerable?  I can run a survey to see what sites this would break if that would help.  That would take a day or so to complete.

Comment 7 Camilo Viecco (:cviecco) 2014-10-02 11:06:10 PDT

The last time Matt Wobensmith did compatibility tests he found .45% of connections with problems(late July), with the only big site being "https://www.citibank.com" (this redirects to a non sslv3  "https://online.citibank.com" other domains of citi where not affected). Can we talk to them (including maybe the other 5 broken domains) and notify them about an SSLv3 security issue and our plan to remove SSLv3 by default by the embargo date? (if we agree this is a good idea or as a fallback is no agreement with MS and Google)

Comment 8 Martin Thomson [:mt] 2014-10-02 11:08:07 PDT

(In reply to Brian Smith (:briansmith, :bsmith, use NEEDINFO?) from comment #5)
> So, I
> recommend asking MS and Google to bite the bullet with you and disable the
> non-secure fallback to SSL 3.0.

I'll see what they say.  I don't know who they are sharing this with, so I'm going to ask them to coordinate.  But a coordinated response would be best.

I do want to determine if we are willing to kill SSL 3.0 in principle still.

Comment 9 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2014-10-02 11:16:56 PDT

(In reply to Martin Thomson [:mt] from comment #8)
> (In reply to Brian Smith (:briansmith, :bsmith, use NEEDINFO?) from comment #5)
> > So, I recommend asking MS and Google to bite the bullet with you and disable
> > the non-secure fallback to SSL 3.0.
> 
> I'll see what they say.  I don't know who they are sharing this with, so I'm
> going to ask them to coordinate.  But a coordinated response would be best.

We coordinated everything through Bugzilla for BEAST, by CC'ing people from Google and MS (and Oracle and others). I recommend CC'ing the Googlers onto this bug.

> I do want to determine if we are willing to kill SSL 3.0 in principle still.

I think I know what the problem is already. And, if so, you may be able to solve it in the following way:

-	    if (!isTLS) {
-		good &= SECStatusToMask(ssl_RemoveSSLv3CBCPadding(
-			    plaintext, blockSize, macSize));
-	    } else {
-		good &= SECStatusToMask(ssl_RemoveTLSCBCPadding(
-			    plaintext, macSize));
-	    }
+	    good &= SECStatusToMask(ssl_RemoveTLSCBCPadding(plaintext, macSize));

This proposed solution relies on the fact that many SSL 3.0 implementations are likely doing padding in the same way as TLS 1.0 implementations. This would be the case, for example, for any server that is "TLS extensions intolerant" but not "TLS intolerant." For SSL 3.0 servers that aren't doing the padding the TLS way, this proposed solution would effectively be equivalent to disabling SSL 3.0, but with lower compatibility risk than disabling SSL 3.0 completely. It would be a more complete solution than disabling the non-secure fallback to SSL 3.0. (Though, I still recommend you do that anyway.)

Comment 10 Martin Thomson [:mt] 2014-10-02 11:31:02 PDT

(+Adam and Bodo.)

You guessed correctly.

The problem is that it's the server that needs to apply that change to properly address the threat.  Having the client verify padding more strictly doesn't help if the attack is on data provided by the client to the server.  This only shows that NSS-based servers would be vulnerable, sadly.

Comment 11 Adam Langley 2014-10-02 11:55:44 PDT

I have a patch prepared that removes SSLv3 fallback in Chrome. (It doesn't remove SSLv3 if the server correctly negotiates it, just fallback to SSLv3.) However, it's too much to drop into stable. It would have to go via the dev and beta process.

Another vendor has indicated that they are considering removing CBC-mode ciphersuites when doing SSLv3 fallback and depending on RC4.

I also have an implementation for something called "anti-POODLE record splitting" which I can explain if you want.

The TLS_FALLBACK_SCSV is simple and has many other benefits, which is why we're recommending that as the fix. Removing SSLv3 fallback likewise has benefits and I'm certainly interesting in trying to make that stick.

Removing CBC for SSLv3 solves the problem with some compat risks but ... you end up with RC4 which doesn't seem like a great answer.

Anti-POODLE record splitting is likewise a dreary response. It also doesn't protect the server->client direction, which could likewise be attacked. (And, if the client and server are to be updated, we would rather just deploy TLS_FALLBACK_SCSV.)

Comment 12 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2014-10-02 12:05:26 PDT

There are actually other likely padding schemes for SSL 3.0, including in particular PKCS#7 and zero padding or other fixed padding. A true pre-TLS-1.0 implementation is more likely to use those schemes than the TLS scheme. To minimize compatibility problems, it is worth considering tweaking the vulnerable side to also accept those other padding schemes, if it really needs to continue talking SSL 3.0. It depends on the exact details of the attack, which I don't know.

I would be interested in seeing the record splitting idea, if you don't mind sharing it.

Comment 13 Adam Langley 2014-10-02 12:14:31 PDT

Please see below the commit message (not public) for this change. I'm not yet sure whether we'll actually do this. It only protects the client->server direction and it's complexity that might be better directed towards killing SSLv3 fallback.

Please note that this has not been reviewed yet. I might have completely screwed it up and it might not work at all.

-------
Implement anti-POODLE record splitting.

This change causes CBC-encrypted, SSLv3 records to be split as needed to
avoid a record having less than six fixed bytes in the final block. This
forces the POODLE attacker to have to try and match 48 bits at once,
which should be sufficiently hard that RC4 is the bigger problem.

The value six comes from the fact that we still need to do 1/n-1 record
splitting for the original BEAST attack and, assuming SHA-1 as the MAC
function(*), the "1" record has 1 (the plaintext) + 20 (for SHA-1) + 1
(for the padding length byte) % block_size = 6 fixed bytes in the final
block.

If a record would normally have fewer than six fixed byte then we remove
the minimum number of bytes from the end in order for the record to have
zero padding bytes. Then we repeat the process.

This works because we'll only ever have to remove 1..5 bytes from the
end thus we'll only ever end up with 1..5 bytes left over to send. For
AES (16 byte block), this always results in an acceptable number of
fixed bytes when encrypting this remainder:
>>> for x in range(1, 6):
...   print x, (x + 20 + 1) % 16
...
1 6
2 7
3 8
4 9
5 10

For 3DES (8 byte block) the situation is more complex because it doesn't
work for 4 and 5 bytes removed:
>>> for x in range(1, 6):
...   print x, (x + 20 + 1) % 8
...
1 6
2 7
3 0
4 1
5 2

However, for the 4 and 5 byte case, when we repeat the process there
will be 3 bytes and 1 or 2 bytes left over. And 1 and 2 bytes works out
fine. Thus the splitting completes in, at most, two splits.

(*) SHA-1 is always the MAC function for CBC-mode cipher suites in SSLv3
because the only exception is TLS_RSA_EXPORT_WITH_RC2_CBC_40_MD5, which
we don't support.

Comment 15 Adam Langley 2014-10-02 12:38:38 PDT

In respect to the anti-POODLE record splitting, Håvard Molland notes that one can use the unsplit Finished message to get only 5 fixed bytes when 3DES is used. Thus the requirement for 6 fixed bytes only applies to AES.

Comment 17 Martin Thomson [:mt] 2014-10-02 13:20:16 PDT

I think that the splitting changes will be rendered unnecessary if we disable SSL 3.0 outright.  It seems that we do have the stomach for it, so I'd rather go hard on that than take increasingly large changes on for other changes.  I'll try to collect some more data to support that position.

The fallback to that is that we take bug 689814, which is also a small change.  That being the case, we'd want to have all the secondary protections as well, to the extent possible: downgrade SCSV and the record splitting are probably necessary there.

Brian, there's no real chance of pretending we want to collect stats: the next build off mozilla-beta should be a release.  I'll ask anyway.

Comment 18 Adam Langley 2014-10-02 13:21:51 PDT

Our current thoughts are to disable fallback to SSLv3 but not (yet) SSLv3 itself. We do not have any data about how viable disabling SSLv3 fallback is and won't be landing any patches until the 16th.

Comment 19 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2014-10-02 13:41:52 PDT

(In reply to Martin Thomson [:mt] from comment #17)
> I think that the splitting changes will be rendered unnecessary if we
> disable SSL 3.0 outright.  It seems that we do have the stomach for it, so
> I'd rather go hard on that than take increasingly large changes on for other
> changes.  I'll try to collect some more data to support that position.

The previous discussion of doing that is in bug 1042811. I think it is a good idea to disable SSL 3.0 completely as long as you are content with the affects of the compatibility problems it would introduce.

> The fallback to that is that we take bug 689814, which is also a small
> change.  That being the case, we'd want to have all the secondary
> protections as well, to the extent possible: downgrade SCSV and the record
> splitting are probably necessary there.

Because of bug 1072382, where Firefox's implementation of the downgrade SCSV doesn't always work correctly with GFE (GMail, etc.), and in particular because the downgrade SCSV affects a lot more sites than completely removing fallback to SSL 3.0 would, I think Mozilla relying on the downgrade SCSV mechanism for Firefox 33 would be quite risky.

Reducing the cipher suites offers to just RC4-based cipher suites for SSL 3.0 handshakes is not a good solution. It would be abused as the stepping stone to force many/most websites to use RC4 for future RC4 attacks.

Martin, could you set the assignee of this bug to yourself, or whoever it is assigned to?

Comment 20 Martin Thomson [:mt] 2014-10-02 16:24:42 PDT

I'm going to take this for now.  I will attach a patch for the drastic measure.  I'm working on a patch that stops us from falling back to SSL 3.0.

Contrary to my intuition, telemetry seems to indicates that a good proportion of our completed SSL 3.0 handshakes (56%) are due to version fallback, most through a connection reset.  That indicate that the difference between these two choices isn't as favourable as I thought.  Those numbers at least would seem to favour Brian's position.  That is, unless you care about the distinction between "site cares enough to deploy TLS" and "site doesn't care, so deploys SSL 3.0", as opposed to just "this is secure" vs. "this isn't".

I'm collecting a list of affected sites; that might help us make a more informed decision.

Comment 21 Martin Thomson [:mt] 2014-10-02 16:42:16 PDT

Created attachment 8499243 [details] [diff] [review]
Option 1: Disable SSL 3.0 entirely

Drastic measures.  (Though on release, the other is looking almost as bad based on Telemetry: this would fail 61% of our completed SSL 3.0 handshakes.)

Comment 22 Martin Thomson [:mt] 2014-10-02 16:43:07 PDT

Created attachment 8499244 [details] [diff] [review]
Option 2: Add a pref to limit how far we fall back

By default, stop at TLS 1.0

Comment 23 Camilo Viecco (:cviecco) 2014-10-03 08:58:02 PDT

Comment on attachment 8499243 [details] [diff] [review]
Option 1: Disable SSL 3.0 entirely

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

you will also need (as briansmith noted) to update int32_t PSM_DEFAULT_MIN_TLS_VERSION to the new value (currently  http://mxr.mozilla.org/mozilla-central/source/security/manager/ssl/src/nsNSSComponent.cpp#843). This only for non-release branches

Comment 24 Martin Thomson [:mt] 2014-10-03 10:50:37 PDT

Created attachment 8499713 [details] [diff] [review]
Option 1: Disable SSL 3.0 entirely

Comment 25 Martin Thomson [:mt] 2014-10-03 11:19:02 PDT

Created attachment 8499729 [details] [diff] [review]
0001-Bug-1076983-Disabling-fallback-to-SSL-3.0-w-pref.patch

From discussions, I think that this is the best plan for the initial reaction to this.  It means that we can't be forced into using SSL 3.0, unless that is all the server can do.  If agl can get us a patch for record splitting, then I think that we'll take that too.

However, there is lots of support for the idea of removing SSL 3.0 support entirely.  We can also use this event as a trigger to announce that we're removing it.  A change to the default preference values would suffice initially (i.e., the other patch here).  I'm sure that we'll have a good discussion about removing the code from NSS once we can talk about this more openly.

Comment 26 Martin Thomson [:mt] 2014-10-03 11:37:44 PDT

Comment on attachment 8499729 [details] [diff] [review]
0001-Bug-1076983-Disabling-fallback-to-SSL-3.0-w-pref.patch

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

::: security/manager/ssl/tests/gtest/TLSIntoleranceTest.cpp
@@ +109,5 @@
>  {
>    ASSERT_TRUE(helpers.rememberIntolerantAtVersion(HOST, PORT,
>                                                    SSL_LIBRARY_VERSION_3_0,
> +                                                  SSL_LIBRARY_VERSION_TLS_1_1));
> +  helpers.rememberTolerantAtVersion(HOST, PORT, SSL_LIBRARY_VERSION_TLS_1_1);

I had to bump the versions on this test and the following tests by one.  This is because the minimum version is (by default) one higher.

Comment 27 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2014-10-03 11:51:03 PDT

Adam, I looked at your commit message. I understand what it is trying to accomplish and it seems reasonable, but I can't really give you useful feedback on it because I don't know the actual attack.

Questions:

1. Is this is a problem that is practically problematic only for clients that run untrusted code, like web browsers, just like BEAST, or does this have severely negative effects on even straightforward clients?

2. Are servers going to be recommended to do strict checks on padding for SSL 3.0 as a mitigation for this bug? As we saw with the BEAST record splitting, there's a lot of compatibility risk with any kind of record splitting. Hopefully servers that matter were adjusted to account for any arbitrary splitting, but there's significant compatibility risk there. It seems like you and other people who have access to GFE may be able to collect useful data about what kinds of padding are used by non-browser SSL 3.0 clients (TLS 1.0? PKCS#7? All zeros? other?) such that a recommendation could be given to servers if they want to mitigate it on their end. If not, I know some other people that could probably collect that data, if y'all feel comfortable with them knowing that there is an issue here.

3. To what extent, if at all, would it be useful for clients to do the padding check I mentioned?

Comment 28 Martin Thomson [:mt] 2014-10-03 13:15:52 PDT

Created attachment 8499785 [details]
domains.diff.errors

The attached list demonstrates the impact of disabling SSL 3.0 entirely, from the 200k sites in our (short) compatibility list.  This seems like a very small set (122, including noise), so it needs to be re-run to rule out errors on my part.

Comment 29 Adam Langley 2014-10-03 18:11:50 PDT

(In reply to Brian Smith (:briansmith, :bsmith, use NEEDINFO?) from comment #27)
> 1. Is this is a problem that is practically problematic only for clients
> that run untrusted code, like web browsers, just like BEAST, or does this
> have severely negative effects on even straightforward clients?

It's very like BEAST. In short, if you can arrange the message to be the correct length then the last block is 15 arbitrary bytes and the padding length (15). Then you arrange an interesting byte to be in the last position of a different block and duplicate that block to the end. If the record is accepted, then you know what the last byte contained because it decrypted to 15.

Thus the attacker needs to be able to control some of the plaintext in order to align things in the messages and needs to be able to burn lots of connections (256 per byte, roughly). Thus a secret needs to be repeated in connection after connection (i.e. a cookie).

> 2. Are servers going to be recommended to do strict checks on padding for
> SSL 3.0 as a mitigation for this bug? As we saw with the BEAST record
> splitting, there's a lot of compatibility risk with any kind of record
> splitting.

Agreed. We won't be dropping anti-POODLE record splitting right into a stable release. Both this and disabling SSLv3 fallback will be trialed on canary/dev.

> Hopefully servers that matter were adjusted to account for any
> arbitrary splitting, but there's significant compatibility risk there. It
> seems like you and other people who have access to GFE may be able to
> collect useful data about what kinds of padding are used by non-browser SSL
> 3.0 clients (TLS 1.0? PKCS#7? All zeros? other?) such that a recommendation
> could be given to servers if they want to mitigate it on their end. If not,
> I know some other people that could probably collect that data, if y'all
> feel comfortable with them knowing that there is an issue here.

I'm assuming that the padding is probably pretty variable, but I haven't checked yet. I'll try to do so on Monday and report back.

> 3. To what extent, if at all, would it be useful for clients to do the
> padding check I mentioned?

If servers could be relied on to do deterministic padding then it would be helpful because you can run the attack in the opposite direction, although there aren't cookie-like secrets that are sent so easily in from server to client.

Comment 30 David Keeler [:keeler] (use needinfo?) 2014-10-06 16:12:22 PDT

Comment on attachment 8499729 [details] [diff] [review]
0001-Bug-1076983-Disabling-fallback-to-SSL-3.0-w-pref.patch

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

Looks good to me with comments addressed.

::: security/manager/ssl/src/nsNSSIOLayer.cpp
@@ +1614,5 @@
>                                   "security.ssl.warn_missing_rfc5746");
>    Preferences::AddStrongObserver(mPrefObserver,
>                                   "security.ssl.false_start.require-npn");
>    Preferences::AddStrongObserver(mPrefObserver,
>                                   "security.ssl.false_start.require-forward-secrecy");

Looks like we need to have an entry here for "security.tls.version.fallback-limit" as well.

@@ +1626,5 @@
> +  int32_t limit = 1;   // 1 = TLS 1.0
> +  Preferences::GetInt("security.tls.version.fallback-limit", &limit);
> +  limit += SSL_LIBRARY_VERSION_3_0;
> +  mVersionFallbackLimit = (uint16_t)limit;
> +  if (limit != (int32_t)mVersionFallbackLimit) { // overflow check

I think we should just map { 0, 1, 2, 3 } to { SSL 3.0, TLS 1.0, TLS 1.1, TLS 1.2 } and use the default (1) if the pref value is outside that range.

@@ +1631,5 @@
> +    mVersionFallbackLimit = SSL_LIBRARY_VERSION_TLS_1_0;
> +  }
> +}
> +
> +

nit: unnecessary blank line

::: security/manager/ssl/tests/gtest/TLSIntoleranceTest.cpp
@@ +79,5 @@
>      ASSERT_EQ(SSL_LIBRARY_VERSION_TLS_1_2, range.max);
>    }
>  }
>  
> +TEST_F(TLSIntoleranceTest, Test_Fallback_Limit_1_2)

maybe something like "Test_Disable_Fallback_With_High_Limit"?

@@ +98,5 @@
> +
> +TEST_F(TLSIntoleranceTest, Test_Fallback_Limit_Default)
> +{
> +  // the default limit prevents SSL 3.0 fallback
> +  ASSERT_EQ(helpers.mVersionFallbackLimit, SSL_LIBRARY_VERSION_TLS_1_0);

I think we should add this here:
ASSERT_TRUE(helpers.rememberIntolerantAtVersion(HOST, PORT, 
                                                SSL_LIBRARY_VERSION_3_0,
                                                SSL_LIBRARY_VERSION_TLS_1_1));

@@ +103,5 @@
> +  ASSERT_FALSE(helpers.rememberIntolerantAtVersion(HOST, PORT,
> +                                                   SSL_LIBRARY_VERSION_3_0,
> +                                                   SSL_LIBRARY_VERSION_TLS_1_0));
> +}
> +

I think we should also have a test where the fallback limit is actually less than the minimum version passed in to rememberIntolerantAtVersion.

Comment 31 Martin Thomson [:mt] 2014-10-07 13:06:03 PDT

I've been advised that some vendors have requested an extension to the embargo.

Comment 32 Richard Barnes [:rbarnes] 2014-10-07 16:27:48 PDT

Created attachment 8501428 [details]
alexa-ssl3.csv

tl;dr: 0.97% of the Alexa top 1M domains require SSLv3.

I used some data provided by Alex Halderman to try to estimate the prevalence of SSLv3 on the Web.  Credit goes to him and his student David Adrian for conducting the scan for IPv4 hosts responding on port 443 with SSLv3.  (Note: I did not tell them why I had an interest in SSLv3, beyond its generally being old.)

Methodology:
1. Attempt to connect to all IPv4 addresses on port 443 [UMich]
2. Collect certs from those that negotiate SSLv3 [UMich]
3. Extract names from certs
4. Select names formatted as FQDNs, with valid TLDs
5. Select names that are subdomains of the Alexa top 1M
6. Connect to each name and send a ClientHello offering TLS1.2
7. Record version number indicated in ServerHello

Findings:
30,992,616  Hosts completed a handshake on port 443
   927,330  Hosts offered SSLv3 (3%)
   731,535  Hosts provided a well-formed cert
   274,008  Unique names collected from CN/SAN
    63,834  Unique names formatted as FQDNs with valid TLDs
    22,669  Unique names matching Alexa top 1M
     1,196    Exact matches to Alexa 1M domains
    21,473    Proper subdomains
    10,923  Responded with a valid TLS message
       551    TLS 1.2
        34    TLS 1.1
       648    TLS 1.0
      9690    SSL 3.0
      ~~~~

The full list of SSL3 sites is in the attached CSV.  The columns are as follows:
1 - name extracted from the certificate
2 - number of certificates containing this name
3 - name from the Alexa list that matched this one
4 - rank in the top 1M
5 - whether the match was exact=1 / subdomain=0
6 - first three octets returned by the server (XX0300 == SSLv3)

Comment 33 Martin Thomson [:mt] 2014-10-07 22:13:57 PDT

Created attachment 8501518 [details] [diff] [review]
0001-Bug-1076983-Adding-pref-to-control-TLS-version-fallb.patch

Rebased for m-c, addresses nits from keeler's review, sans the following:

I've maintained pref handling in a manner consistent with the existing version prefs.  I do think that this handling is ugly and can be improved, but I'd like to factor it more cleanly.  So perhaps we should consider a separate change to clean that up.  I'm happy to do that (tomorrow perhaps).

Comment 34 Martin Thomson [:mt] 2014-10-07 22:20:36 PDT

Created attachment 8501520 [details] [diff] [review]
0001-Bug-689814-Adding-pref-to-control-TLS-version-fallba.patch

Carrying r+

Note that I realized just now that we can point the commit to bug 689814.  This will look like I'm just finally doing the work.

Comment 35 Martin Thomson [:mt] 2014-10-09 16:25:50 PDT

As a TODO, we need to ensure that https://wiki.mozilla.org/Security/Server_Side_TLS is updated.  Currently, it recommends the use of SSLv3 for backward compatibility, favouring 3DES (a block cipher that is otherwise fine) over RC4.

Comment 36 Reed Loden [:reed] 2014-10-09 17:09:10 PDT

(In reply to Martin Thomson [:mt] from comment #35)
> As a TODO, we need to ensure that
> https://wiki.mozilla.org/Security/Server_Side_TLS is updated.  Currently, it
> recommends the use of SSLv3 for backward compatibility, favouring 3DES (a
> block cipher that is otherwise fine) over RC4.

Looks like it's already been mostly updated by :ulfr over the last week or so.

Comment 37 Julien Vehent [:ulfr] (use needinfo) 2014-10-11 21:04:23 PDT

:mt and :rbarnes - Hubert Kario has SSLv3 stats that closely match yours, with 0.7% of HTTPS sites in Alexa's top 1M that will only negotiate SSLv3 [1] (3,108 out of 402,742). 9 months earlier, that percentage was 0.98%. It is slowly decreasing.

Internally, OpSec has been in touch with the major operation teams asking them to disable SSLv3 everywhere. No details were shared, we simply asked them to match the intermediate configuration from our guidelines [2].

A handful of sites will still require SSLv3 because of a small audience stuck on IE6 (~5% of firefox downloads). www.mozilla.org and download.mozilla.org are on that list. For these, a decision must be made: prioritize RC4 for *everyone* (since the ciphersuite is not configurable per-protocol), or accept that clients negotiating SSLv3 are at risk. I'm interested in what this group thinks, and would happily draft a public recommendation for the Server Side TLS wiki page.

Please keep in mind that, from an operation point of view, we need to propose a fix that is usable in production. In 90% of cases, that means OpenSSL provided by linux distributions. The rest of the time, it's appliances that are even harder to upgrade. We do not have the ability to run custom TLS stacks on Mozilla's sites at the moment.

side note: I am currently evaluating 350 Mozilla sites to check their TLS quality. Anything not intermediate or modern has SSLv3 enabled. As you can see [3], that is roughly 76% of our sites at the moment...

[1] https://securitypitfalls.wordpress.com/2014/09/29/scan-results-for-september-2014/
[2] https://wiki.mozilla.org/Security/Server_Side_TLS#Intermediate_compatibility_.28default.29
[3] (internal network, requires VPN) http://mozdefqa1.private.scl3.mozilla.com:9090/index.html#/dashboard/elasticsearch/Mozilla%20Cipherscan

Comment 38 Martin Thomson [:mt] 2014-10-11 22:49:28 PDT

I think that our download servers are OK permitting SSLv3 as long as we treat them as been compromised.  That is, we don't put anything secret on them.  I don't see anything particularly dangerous on either site with respect to confidentiality, like a login.

The primary security concern with downloads is integrity, and that is not affected by this attack.  I wouldn't recommend that any site keep SSLv3 enabled ordinarily, but this is a case where it does little harm.  In fact, making a secure browser available to people who are stuck with IE6 is more of a good thing.  The risk is worse if we lock them out.

I think we should hold off on major changes to the wiki until the 21st, to avoid drawing attention, but at that time, I think we should just say that SSLv3 is broken, along with SSLv2.  We can use the above to explain our apparent hypocrisy regarding download servers if anyone cares to ask.

Comment 39 Julien Vehent [:ulfr] (use needinfo) 2014-10-12 06:57:46 PDT

For now, disabling SSLv3 on these sites is not an option. Like you said, we want to keep serving FF downloads to IE6 users, so we must support SSLv3.

We could change the ciphersuite to prefer RC4, and remove all CBC ciphers, but there are two problems:
  1. that would move *all* users to RC4, regardless of the age of their client, because we do not have the flexibility to serve a different ciphersuite depending on the client hello.
  2. all other ciphers beside RC4 are CBC. Removing them means accepting only one cipher on a major site like mozilla.org, and potentially breaking tons of clients that don't support RC4 (I don't have numbers).

This alternative seems dangerous to me. So unless we think that this vulnerability requires drastic measures, I'm inclined to keep the old ciphersuite as it is today:

prio  ciphersuite         protocols            pfs_keysize
1     DHE-RSA-AES128-SHA  SSLv3,TLSv1,TLSv1.1  DH,1024bits
2     DHE-RSA-AES256-SHA  SSLv3,TLSv1,TLSv1.1  DH,1024bits
3     AES128-SHA          SSLv3,TLSv1,TLSv1.1
4     AES256-SHA          SSLv3,TLSv1,TLSv1.1
5     DES-CBC3-SHA        SSLv3,TLSv1,TLSv1.1

And move all sites that don't need that kind of compatibility away from SSLv3.

Comment 40 Reed Loden [:reed] 2014-10-12 14:23:54 PDT

A lot of chatter on Twitter today. https://twitter.com/search?f=realtime&q=sslv3

Of note are the claims that the problem would be announced this week, so I just hope all the vendors are aware of the new embargo date (and don't release early)...

Comment 41 Martin Thomson [:mt] 2014-10-13 09:48:09 PDT

:ulfr as I said, I don't see any value in fixing download.m.o or www.m.o.  We can acknowledge the fact that these sites run broken crypto because we consider it more important that people can get a secure browser than it is to secure these sites.  This is OK as long as we don't have anything we actually need to secure on those sites.

And DH,1024bits?  Seriously?  That is pretty weak.  The accepted minimum is 2048, but even that is borderline.

We should do something about upgrading to support TLS 1.2 too.  It's 6 years old already.

Comment 42 Marc Schifer [mschifer] 2014-10-13 12:02:00 PDT

Do have a plan for a release vehicle for this patch? Do I need to prepare for a dot release to handle this?

Comment 43 Julien Vehent [:ulfr] (use needinfo) 2014-10-13 12:19:09 PDT

> And DH,1024bits?  Seriously?  That is pretty weak.  The accepted minimum is
> 2048, but even that is borderline.

It breaks backward compatibility with old clients: https://bugzilla.mozilla.org/show_bug.cgi?id=914065#c18

> We should do something about upgrading to support TLS 1.2 too.  It's 6 years
> old already.

TLS1.2 just got released with the latest version in our main load balancer. It's on the upgrade path internally.

Here's a good tracker for those things: https://bugzilla.mozilla.org/showdependencytree.cgi?id=901393&hide_resolved=0

Comment 44 Martin Thomson [:mt] 2014-10-13 14:16:27 PDT

(In reply to Marc Schifer [mschifer] from comment #42)
> Do have a plan for a release vehicle for this patch? Do I need to prepare
> for a dot release to handle this?

The current plan is to have this ride to 34 release on an accelerated path.  :lmandel was reluctant to push this to 33.1 given its severity.

Comment 45 Martin Thomson [:mt] 2014-10-13 14:17:35 PDT

Comment on attachment 8499713 [details] [diff] [review]
Option 1: Disable SSL 3.0 entirely

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

I just realized that I neglected to request review for this.  If this is our primary plan, it's probably best to have it ready.

Comment 46 David Keeler [:keeler] (use needinfo?) 2014-10-14 10:36:07 PDT

Comment on attachment 8499713 [details] [diff] [review]
Option 1: Disable SSL 3.0 entirely

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

This looks good. In terms of testing, here's what I was thinking:

In an xpcshell test in security/manager/ssl/tests/unit/, set environment variables like MOZ_TLS_SERVER_MIN/MAX_TLS_VERSION to 0. Then, in TLSServer.cpp, check for/read those variables and call SSL_VersionRangeSetDefault similar to how nsNSSComponent.cpp does it (although instead of calling SSL_VersionRangeSetDefault twice, I would just range-check the inputs and call it once). Back in the xpcshell test, ensure that a connection attempt results in the expected error.

Comment 47 Kai Engert (:kaie) 2014-10-14 11:17:36 PDT

I just learned about this bug minutes ago. Thanks Reed.
My earlier public discussion was based on the twitter rumours.
https://twitter.com/briankrebs/status/521301468192968704

Comment 48 Martin Thomson [:mt] 2014-10-14 11:22:34 PDT

Thanks for keeping a lid on this one Kai.  We're trying not to fuel the rumours.  Please respect the embargo.

Comment 49 Kai Engert (:kaie) 2014-10-14 11:40:11 PDT

(In reply to Martin Thomson [:mt] from comment #48)
> Thanks for keeping a lid on this one Kai.  We're trying not to fuel the
> rumours.  Please respect the embargo.

I will of course keep my mouth shut from now on.
(I'm simply justifying why I wasn't silent earlier, when other Mozilla security people gave me the impression that Mozilla was aware of this issue yet.)

Comment 50 Kai Engert (:kaie) 2014-10-14 11:40:49 PDT

"wasn't aware" (sorry for the spam)

Comment 51 Reed Loden [:reed] 2014-10-14 14:45:05 PDT

Thomas Pornin has commented on what he thinks the attack is, and it sounds pretty similar (if not the exact problem). This is effectively public, as it was part of a chat done via Stack Exchange (http://chat.stackexchange.com/transcript/151?m=18151930#18151930). It's also being posted all over Twitter right now.

Comment 52 Martin Thomson [:mt] 2014-10-14 15:16:32 PDT

Yes, it appears as though this is leaking out all over.  But the information that is out there is still not precise enough to mount an attack.  We're still waiting guidance from Google on what their plans are.  They are keeping us well informed.  Until we hear otherwise, we should be respecting the embargo.

Comment 53 Reed Loden [:reed] 2014-10-14 15:23:19 PDT

Just to confirm, the embargo is 10/21, right? So many comments on Twitter saying to expect an announcement this week (which would be incorrect, as per that date).

Comment 54 Thai Duong 2014-10-14 15:28:28 PDT

I think we're going to release now because Thomas Pornin has actually figured out the vulnerability.

Comment 55 Reed Loden [:reed] 2014-10-14 15:30:28 PDT

Indeed, http://www.cnbc.com/id/102078872 was just posted.

Comment 56 Reed Loden [:reed] 2014-10-14 15:34:11 PDT

http://googleonlinesecurity.blogspot.com/2014/10/this-poodle-bites-exploiting-ssl-30.html

Comment 57 Martin Thomson [:mt] 2014-10-14 15:40:01 PDT

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

Comment 58 Martin Thomson [:mt] 2014-10-14 15:41:35 PDT

We're going to try to land on m-i as soon as possible (I don't want to cause a rash of oranges as a result of stuff that I couldn't test off C-I).  Tomorrow's nightly should have SSLv3 disabled.

Comment 59 Reed Loden [:reed] 2014-10-14 15:53:41 PDT

(In reply to Martin Thomson [:mt] from comment #44)
> (In reply to Marc Schifer [mschifer] from comment #42)
> > Do have a plan for a release vehicle for this patch? Do I need to prepare
> > for a dot release to handle this?
> 
> The current plan is to have this ride to 34 release on an accelerated path. 
> :lmandel was reluctant to push this to 33.1 given its severity.

Considering this moved up a week, is that still the plan?

Comment 60 Martin Thomson [:mt] 2014-10-14 15:55:57 PDT

(In reply to Reed Loden [:reed] from comment #59)
> (In reply to Martin Thomson [:mt] from comment #44)
> > The current plan is to have this ride to 34 release on an accelerated path. 
> > :lmandel was reluctant to push this to 33.1 given its severity.
> 
> Considering this moved up a week, is that still the plan?

I will confirm.

Comment 61 Wan-Teh Chang 2014-10-14 16:41:34 PDT

Comment on attachment 8499713 [details] [diff] [review]
Option 1: Disable SSL 3.0 entirely

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

::: security/manager/ssl/src/nsNSSComponent.cpp
@@ +833,4 @@
>    mDefaultCertVerifier = new SharedCertVerifier(odc, osc, ogc, pinningMode);
>  }
>  
>  // Enable the TLS versions given in the prefs, defaulting to SSL 3.0 (min

Just wanted to note that in the patch you pushed to the try server (https://hg.mozilla.org/try/rev/ff35684a051d), you updated this comment incorrectly. I think you should delete ", minimum SSL 3.0" from that comment.

Comment 62 Martin Thomson [:mt] 2014-10-14 17:19:37 PDT

Created attachment 8505139 [details] [diff] [review]
0001-Bug-1076983-Disabling-SSL-3.0-with-pref.patch

Carrying r=keeler, now with updated bug number and fixed comments

Comment 63 Martin Thomson [:mt] 2014-10-14 17:20:41 PDT

https://hg.mozilla.org/integration/mozilla-inbound/rev/3f41967081d5

Comment 64 Martin Thomson [:mt] 2014-10-14 17:28:16 PDT

Comment on attachment 8505139 [details] [diff] [review]
0001-Bug-1076983-Disabling-SSL-3.0-with-pref.patch

Approval Request Comment
[Feature/regressing bug #]: 1076983
[User impact if declined]: Users will be exposed to POODLE
[Describe test coverage new/current, TBPL]: Tested on TBPL, no signs of test regressions.  Tested against 200k+ sites.
[Risks and why]: __THIS WILL REGRESS SITES__ around 1k sites are affected out of those tested and maybe more on the long tail.  Those sites will become unreachable to Firefox users.  We've notifying people about this flaw, and have been quietly already for major sites, but this is going to hurt those who haven't upgraded their TLS stack in the past 15 years.
[String/UUID change made/needed]: None needed.

Note: plan is to uplift into Aurora quickly.  Lag that by a couple of days for Beta and then hit Release before 34 is built (but not before 33.1).

Comment 65 Martin Thomson [:mt] 2014-10-14 17:29:42 PDT

Let's open this bug to the world now that Google has announced their intentions.  :dveditz ?

Comment 66 Martin Thomson [:mt] 2014-10-14 19:15:48 PDT

*** Bug 689814 has been marked as a duplicate of this bug. ***

Comment 67 Martin Thomson [:mt] 2014-10-14 19:18:29 PDT

*** Bug 1042811 has been marked as a duplicate of this bug. ***

Comment 68 Yuhong Bao 2014-10-15 00:01:41 PDT

(In reply to Julien Vehent [:ulfr] (use needinfo) from comment #39)
> For now, disabling SSLv3 on these sites is not an option. Like you said, we
> want to keep serving FF downloads to IE6 users, so we must support SSLv3.

Keep in mind however that it is easy for IE6 users to enable TLSv1, though I agree that for a browser download site it is probably not worth the effort.

Comment 69 Ryan VanderMeulen [:RyanVM UTC-4] 2014-10-15 11:42:39 PDT

https://hg.mozilla.org/mozilla-central/rev/3f41967081d5

Comment 70 Ryan VanderMeulen [:RyanVM UTC-4] 2014-10-16 08:16:29 PDT

https://hg.mozilla.org/releases/mozilla-aurora/rev/2763449ffb01

We need this nominated for b2g30 and b2g32 as well, don't we?

Comment 71 sjw 2014-10-16 08:38:32 PDT

Will this patch being uplifted to Firefox ESR?
Thunderbird is also affected.

Comment 72 Martin Thomson [:mt] 2014-10-16 10:34:06 PDT

Comment on attachment 8505139 [details] [diff] [review]
0001-Bug-1076983-Disabling-SSL-3.0-with-pref.patch

[Approval Request Comment]

See comment 64
https://bugzilla.mozilla.org/show_bug.cgi?id=1076983#c64

Comment 73 Al Billings [:abillings] 2014-10-16 10:39:20 PDT

(In reply to sjw from comment #71)
> Will this patch being uplifted to Firefox ESR?
> Thunderbird is also affected.

The 'tracking-firefox-esr31: 34+' flag indicates that this is planned to be fixed in ESR31.3, which goes out when Firefox 34 does.

Comment 74 Martin Thomson [:mt] 2014-10-16 22:50:53 PDT

Comment on attachment 8501520 [details] [diff] [review]
0001-Bug-689814-Adding-pref-to-control-TLS-version-fallba.patch

Moving attachment 8501520 [details] [diff] [review] to bug 1083058 (one fix per bug now that everything is open).

Comment 75 Matt Wobensmith 2014-10-17 15:17:44 PDT

I have verified in both Fx35 and Fx36, 2014-10-17, that the preference for "security.tls.version.min" is now equal to 1. 

I also decided to do a sanity test to make sure we are truly not supporting SSLv3. I used a list of around a thousand sites known to use SSLv3 as of a few months ago. I ran all sites through the these new Fx builds while capturing traffic in Wireshark. I then filtered Wireshark's network capture by protocol and whether application data went over SSLv3. I found, indeed, no case where SSLv3 was able to convey application data, and the sites in my test that did have successful data connections themselves had upgraded to TLS1.0 or higher to do so. Clearly these sites have upgraded their servers since this list was created.

I have various annotated screen captures of a Wireshark test to show how browsers treat one particular site that requests SSLv3. I also have a pcapng file (of a thousand site requests) from Fx36 that you can view in Wireshark. Sort by "ssl.app_data" to see for yourself that the above conclusions are correct. Those files are large so not posted here. Contact me directly if you'd like them.

Any queestions on the above, please get in touch. Thank you.

Comment 76 Yuhong Bao 2014-10-17 15:44:19 PDT

(In reply to Matt Wobensmith from comment #75)
> I have verified in both Fx35 and Fx36, 2014-10-17, that the preference for
> "security.tls.version.min" is now equal to 1. 
> 
> I also decided to do a sanity test to make sure we are truly not supporting
> SSLv3. I used a list of around a thousand sites known to use SSLv3 as of a
> few months ago. I ran all sites through the these new Fx builds while
> capturing traffic in Wireshark. I then filtered Wireshark's network capture
> by protocol and whether application data went over SSLv3. I found, indeed,
> no case where SSLv3 was able to convey application data, and the sites in my
> test that did have successful data connections themselves had upgraded to
> TLS1.0 or higher to do so. Clearly these sites have upgraded their servers
> since this list was created.

The fact that POODLE is in the news seems to help quite a bit, though for things like IBM Domino it may take quite a while.

Comment 77 Lawrence Mandel [:lmandel] (use needinfo) 2014-10-22 09:12:22 PDT

(In reply to Martin Thomson [:mt] from comment #64)
> Note: plan is to uplift into Aurora quickly.  Lag that by a couple of days
> for Beta and then hit Release before 34 is built (but not before 33.1).

Martin - This change has been on Aurora for 6 days now. Are you ready to uplift for beta3 (go to build is Thu)?

Comment 78 Martin Thomson [:mt] 2014-10-22 12:01:12 PDT

Yep, let's do this.  There has been much less screaming about this than I anticipated.  Beta should generate more noise.

Comment 79 Lawrence Mandel [:lmandel] (use needinfo) 2014-10-22 12:26:17 PDT

Comment on attachment 8505139 [details] [diff] [review]
0001-Bug-1076983-Disabling-SSL-3.0-with-pref.patch

OK. Let's get this into beta3. Beta+

I don't think there is any reason to hold back on the uplift to b2g30 and b2g32. If you agree, let's get the patch landed there as well.

A note on the approval request for mozilla-release, this will remain in the nom state as we may decide to take this change in 33.1. The plan of record is to ship in 34.

Comment 80 Martin Thomson [:mt] 2014-10-22 12:49:56 PDT

Created attachment 8509746 [details] [diff] [review]
As committed to beta

https://hg.mozilla.org/releases/mozilla-beta/rev/8c9d5c14b866

This attachment is as-committed.  I had to unrot to catch a completely unrelated change in the context that caused the original to fail to apply.

Since I'm paranoid, I ran a build, and diffed the patches.  It's fine.

Comment 81 Martin Thomson [:mt] 2014-10-22 12:51:14 PDT

Oh, and yes, I'll need assistance with landing on b2gXX, since I've not done that before.

Comment 82 Ryan VanderMeulen [:RyanVM UTC-4] 2014-10-22 13:30:52 PDT

Please request esr31 approval on this. Once it lands there, I can take care of the B2G landings.

Comment 83 Martin Thomson [:mt] 2014-10-22 21:18:24 PDT

Comment on attachment 8509746 [details] [diff] [review]
As committed to beta

Adding r=keeler here; noting that this is more likely to apply cleanly.

For approval notes, see comment 64

And thanks, RyanVM

Comment 84 Ryan VanderMeulen [:RyanVM UTC-4] 2014-10-23 10:30:51 PDT

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/8c9d5c14b866

Comment 85 Ryan VanderMeulen [:RyanVM UTC-4] 2014-10-27 15:22:37 PDT

https://hg.mozilla.org/releases/mozilla-esr31/rev/a0524e2a9b06

Comment 86 Josh Cheng [:josh] 2014-10-29 02:24:56 PDT

Hi Kai-Zhen,
Could you please help to land this on 2.0M? Thanks!

Comment 87 Kai-Zhen Li [:kli][:seinlin] 2014-10-29 02:56:21 PDT

http://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/36adcb8c03d9

Comment 88 Ryan VanderMeulen [:RyanVM UTC-4] 2014-10-29 06:02:23 PDT

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/93865aea0c87
https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/019d1254a79c

(As a sec-high, this was going to land on the B2G release branches anyway...)

Comment 89 Sylvestre Ledru [:sylvestre] 2014-10-29 08:50:43 PDT

Comment on attachment 8505139 [details] [diff] [review]
0001-Bug-1076983-Disabling-SSL-3.0-with-pref.patch

For now, the plan is to take in 34 (currently beta).

Comment 90 Andrei Vaida, QA [:avaida] 2014-11-20 01:04:32 PST

This is now verified fixed on Firefox 34.0b10 (build1 / 20141117202603) as well, with 'security.tls.version.min' set to 1 by default across the following platforms: Windows 7 64-bit, Mac OS X 10.9.5 and Ubuntu 12.04 LTS 32-bit.

Comment 91 Al Billings [:abillings] 2014-11-24 11:29:36 PST

Lawrence, can you please add that we're disabling sslv3 in Firefox 34 to the relnotes?

Comment 92 Al Billings [:abillings] 2014-11-24 11:30:14 PST

ESR31 has disabled it as well.

Comment 93 Lawrence Mandel [:lmandel] (use needinfo) 2014-11-24 15:33:43 PST

Release noted as "Disabled SSLv3".