Last Comment Bug 733647 - Implement TLS 1.1 (RFC 4346) in Gecko (Firefox, Thunderbird), on by default
: Implement TLS 1.1 (RFC 4346) in Gecko (Firefox, Thunderbird), on by default
Status: VERIFIED FIXED
: feature, sec-want
Product: Core
Classification: Components
Component: Security: PSM (show other bugs)
: Trunk
: All All
: -- enhancement with 109 votes (vote)
: mozilla28
Assigned To: Brian Smith (:briansmith, :bsmith, use NEEDINFO?)
: Matt Wobensmith [:mwobensmith][:matt:]
Mentors:
http://www.ietf.org/rfc/rfc4346.txt
Depends on: RFC4346 733632 733641 733642 839310 861310 874622 944465
Blocks: 861266
  Show dependency treegraph
 
Reported: 2012-03-06 18:40 PST by Brian Smith (:briansmith, :bsmith, use NEEDINFO?)
Modified: 2015-03-22 09:27 PDT (History)
149 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
disabled
verified
verified
disabled
27+


Attachments
setting the defaults for TLS 1.1 (2.60 KB, patch)
2013-06-11 10:21 PDT, Meadhbh Hamrick :mhamrick
no flags Details | Diff | Splinter Review
default max TLS Version to TLS 1.2 (1.99 KB, patch)
2013-06-18 07:41 PDT, Meadhbh Hamrick :mhamrick
brian: review+
wtc: review+
Details | Diff | Splinter Review
defaults sans firefox.js comment (1020 bytes, patch)
2013-06-19 12:03 PDT, Meadhbh Hamrick :mhamrick
no flags Details | Diff | Splinter Review
1.1 max, removes firefox.js and fixes comment to refer to SSL 3.0, TLS 1.0 and TLS 1.1 (2.37 KB, patch)
2013-07-29 18:13 PDT, Meadhbh Hamrick :mhamrick
brian: review+
wtc: review+
Details | Diff | Splinter Review
removed trailing whitespace (2.36 KB, patch)
2013-07-30 12:01 PDT, Meadhbh Hamrick :mhamrick
brian: review+
wtc: review+
bajaj.bhavana: approval‑mozilla‑aurora+
lukasblakk+bugs: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2012-03-06 18:40:04 PST
+++ This bug was initially created as a clone of Bug #565047 +++

This is likely to be a 2012Q2 goal for the Necko team.
Comment 1 Kathleen Wilson 2012-08-23 10:27:29 PDT
TLS 1.1 is being added to NSS in bug #565047.

PSM work will need to be done for Firefox to use TLS 1.1:
-- Add new preference for using TLS 1.1
-- Add fall-back and retry mechanism; if TLS 1.1 fails, fall back to 1.0.
-- To start with, leave 1.0 as default.
Comment 2 Florian Bender 2013-01-31 05:42:56 PST
NSS 3.14 has TLS 1.1 support (see bug 565047 comment 118) which is included in the current Fx 19 Beta (at least) so this bug can get going now.
Comment 3 Florian Bender 2013-03-01 22:55:22 PST
Bug 733642 duplicates Bug 839644. Can someone please update the dependency list?
Comment 4 e281857 2013-03-28 20:40:32 PDT Comment hidden (advocacy)
Comment 5 Cork 2013-03-28 23:10:43 PDT Comment hidden (obsolete)
Comment 6 tu 2013-03-30 03:59:48 PDT
With RC4 being so insecure supporting TLS 1.1 is really important now.
http://blog.cryptographyengineering.com/2013/03/attack-of-week-rc4-is-kind-of-broken-in.html
Comment 7 Anthony Usman 2013-03-30 23:05:47 PDT Comment hidden (advocacy)
Comment 8 wind 2013-04-10 04:37:17 PDT Comment hidden (off-topic)
Comment 9 Kurt Roeckx 2013-05-21 10:17:09 PDT
So as I understand things, in 22 it should be possible to enable TLS 1.1 (in about:config), but there are 2 open issues that prevent us from changing the default to enable it:
- Compatibility with servers that are not tolerant to sending TLS 1.1 in ClientHello and were we need to fall back to TLS 1.0 (or SSL 3.0) (bug #839310)
- Prevent downgrade attacks (bug #861310)
Comment 10 Kurt Roeckx 2013-05-21 12:57:49 PDT
Thinking about it, I think not having prevention against downgrade attacks should block us from enabling TLS 1.1 by default.  It will not make things less secure than the current situation.
Comment 11 Alex Xu 2013-05-21 13:01:14 PDT
TLS 1.1 can be enabled manually in about:config with security.tls.version.max = 2.

However, the fallback is not implemented properly; there is undefined behavior in NSS due to the simultaneous use of SSL_VersionRangeSetDefault and SSL_OptionSet(SSL_ENABLE_TLS).
Comment 12 Meadhbh Hamrick :mhamrick 2013-06-11 10:21:49 PDT
Created attachment 761016 [details] [diff] [review]
setting the defaults for TLS 1.1

cviecco suggested i post the diff for just what Alex mentioned above. this diff changes the security.tls.version.max in netwerk/base/public/security-prefs.js as well as the PSM_DEFAULT_MAX_TLS_VERSION in security/manager/ssl/src/nsNSSCompontent.cpp. I also added a LOG message listing the hex values of min and max TLS versions. soooo... if after applying this patch you have problems attaching to TLS 1.1 sites, set NSPR_LOG_MODULES to something like "pipnss:5" and check you're getting the values you expect; it should say something like "NSS TLS Version Range. min: 0300, max: 0302".
Comment 13 Meadhbh Hamrick :mhamrick 2013-06-11 10:32:10 PDT
Also, geekboy recommended i document what i'm working on so peeps know what's coming. bsmith & i put our heads together and figured out what we _should_ be doing wrt TLS intolerance is this...

buried inside class nsSSLIOLayerHelpers (in nsNSSIOLayer.{h|cpp}) are a pair of nsTHashtables representing sites that are tolerant or intolerant of TLS. if we get an error when trying to connect to a site, we add an entry to the intolerant table indicating the TLS rev that failed. the error eventually bubbles up and a retry occurs, so when we try to connect to a site, we check to see if there's an entry in that table. if so, it represents the TLS rev that failed, so we decrement by one (modulo max/min TLS revs) before trying to connect.
Comment 14 Kurt Roeckx 2013-06-11 10:40:49 PDT
(In reply to Meadhbh Hamrick :mhamrick from comment #13)
> Also, geekboy recommended i document what i'm working on so peeps know
> what's coming. bsmith & i put our heads together and figured out what we
> _should_ be doing wrt TLS intolerance is this...

I think that should be discussed in bug #839310
Comment 15 Wan-Teh Chang 2013-06-14 17:23:01 PDT
Comment on attachment 761016 [details] [diff] [review]
setting the defaults for TLS 1.1

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

::: security/manager/ssl/src/nsNSSComponent.cpp
@@ +1132,5 @@
>  nsNSSComponent::setEnabledTLSVersions(nsIPrefBranch * prefBranch)
>  {
>    // keep these values in sync with security-prefs.js and firefox.js
>    static const PRInt32 PSM_DEFAULT_MIN_TLS_VERSION = 0;
> +  static const PRInt32 PSM_DEFAULT_MAX_TLS_VERSION = 2;

If this patch is intended for mozilla-central, you can consider
setting PSM_DEFAULT_MAX_TLS_VERSION to 3 (TLS 1.2) because the
NSS in mozilla-central now supports TLS 1.2.
Comment 16 Meadhbh Hamrick :mhamrick 2013-06-18 07:41:20 PDT
Created attachment 764166 [details] [diff] [review]
default max TLS Version to TLS 1.2

Okay... here's a patch that defaults to 1.2
Comment 17 rsx11m 2013-06-18 10:46:01 PDT
(In reply to Wan-Teh Chang from comment #15)
> If this patch is intended for mozilla-central, you can consider
> setting PSM_DEFAULT_MAX_TLS_VERSION to 3 (TLS 1.2) because the
> NSS in mozilla-central now supports TLS 1.2.

So, setting security.tls.version.max to 3 works properly in any builds (e.g., nightly trunk builds) from mozilla-central at this time already? Just wondering, given that there is still quite a bit of activity in bug 480514 on implementing TLS 1.2 support.
Comment 18 Wan-Teh Chang 2013-06-18 11:01:10 PDT
Comment on attachment 764166 [details] [diff] [review]
default max TLS Version to TLS 1.2

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

r=wtc. Please wait for Brian's review.

::: security/manager/ssl/src/nsNSSComponent.cpp
@@ +1132,5 @@
>  nsNSSComponent::setEnabledTLSVersions(nsIPrefBranch * prefBranch)
>  {
>    // keep these values in sync with security-prefs.js and firefox.js
>    static const int32_t PSM_DEFAULT_MIN_TLS_VERSION = 0;
> +  static const int32_t PSM_DEFAULT_MAX_TLS_VERSION = 3;

This comment mentions security-prefs.js and firefox.js but the patch
only modifies security-prefs.js. Is this comment wrong?
Comment 19 Alex Xu 2013-06-18 12:37:24 PDT
I'm not sure we want to land this in m-c before bug 839310...
Comment 20 rsx11m 2013-06-18 14:33:01 PDT
+1, especially if the plan is to squeeze this into the 24.0 cycle. Requesting by default TLS 1.2 without providing a fallback should come as a surprise for many ESR users; a lot of old infrastructure running intranets is still out there.
Comment 21 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2013-06-18 14:44:20 PDT
Comment on attachment 764166 [details] [diff] [review]
default max TLS Version to TLS 1.2

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

::: security/manager/ssl/src/nsNSSComponent.cpp
@@ +1132,5 @@
>  nsNSSComponent::setEnabledTLSVersions(nsIPrefBranch * prefBranch)
>  {
>    // keep these values in sync with security-prefs.js and firefox.js
>    static const int32_t PSM_DEFAULT_MIN_TLS_VERSION = 0;
> +  static const int32_t PSM_DEFAULT_MAX_TLS_VERSION = 3;

The comment should just have "and firefox.js" removed. That comment was written that way because of a misunderstanding I had with the original patch to add that preference, and I forgot to update the comment when I fixed the code for that misunderstanding.
Comment 22 Meadhbh Hamrick :mhamrick 2013-06-19 12:03:43 PDT
Created attachment 764922 [details] [diff] [review]
defaults sans firefox.js comment

okay... here's a patch on the above patch that removes the "and firefox.js" comment
Comment 23 Reed Loden [:reed] (use needinfo?) 2013-06-19 12:09:35 PDT
Comment on attachment 764922 [details] [diff] [review]
defaults sans firefox.js comment

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

::: security/manager/ssl/src/nsNSSComponent.cpp
@@ +1126,5 @@
>    SSL_ClearSessionCache();
>  }
>  
>  // Enable the TLS versions given in the prefs, defaulting to SSL 3.0 and
>  // TLS 1.0 when the prefs aren't set or when they are set to invalid values.

Can you fix this comment as well? It's now wrong, as the default is SSLv3, TLS1.0, TLS1.1, and TLS1.2 now.
Comment 24 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2013-07-07 19:33:28 PDT
Comment on attachment 764922 [details] [diff] [review]
defaults sans firefox.js comment

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

I agree with Reed that the other comment needs to be fixed too.
Comment 25 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2013-07-07 20:46:24 PDT
Please see Bug 861266 comment 7. Because of the additional complications with TLS 1.2 support, I suggest that we revert the patch back so that it enables only TLS 1.1 so that we can land and test *that* ASAP, hopefully even uplifting it to mozilla-aurora. Otherwise, the extra time required to address all the issues in bug 861266 may unnecessarily hold back the enabling of TLS 1.1.
Comment 26 Meadhbh Hamrick :mhamrick 2013-07-29 18:13:56 PDT
Created attachment 782893 [details] [diff] [review]
1.1 max, removes firefox.js and fixes comment to refer to SSL 3.0, TLS 1.0 and TLS 1.1
Comment 27 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2013-07-29 18:16:12 PDT
Comment on attachment 782893 [details] [diff] [review]
1.1 max, removes firefox.js and fixes comment to refer to SSL 3.0, TLS 1.0 and TLS 1.1

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

::: security/manager/ssl/src/nsNSSComponent.cpp
@@ +969,5 @@
>  }
>  
> +// Enable the TLS versions given in the prefs, defaulting to SSL 3.0 (min
> +// version) and TLS 1.1 (max version) when the prefs aren't set or set to
> +// invalid values. 

Nit: trailing whitespace.
Comment 28 Wan-Teh Chang 2013-07-29 18:24:31 PDT
Comment on attachment 782893 [details] [diff] [review]
1.1 max, removes firefox.js and fixes comment to refer to SSL 3.0, TLS 1.0 and TLS 1.1

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

r=wtc.
Comment 29 Meadhbh Hamrick :mhamrick 2013-07-30 12:01:12 PDT
Created attachment 783283 [details] [diff] [review]
removed trailing whitespace
Comment 30 rsx11m 2013-07-30 12:10:14 PDT
Meadhbh, since attachment 782893 [details] [diff] [review] received r+ from both reviewers already (subject to removal of the trailing space per comment #27), I think you don't need any further reviews for the updated patch with this nit fixed. However, I don't know if this is supposed to land only along with bug 839310 to fix the fallback issue, thus I'm reluctant to recommend that you get your patch checked in at this time.
Comment 31 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2013-07-30 12:36:05 PDT
Comment on attachment 783283 [details] [diff] [review]
removed trailing whitespace

FYI, when you get a r+ from a peer and you are asked to make a trivial change like this, you can mark the updated patch r+ yourself. You do not need to ask for re-review.
Comment 32 Johnny Iguna 2013-10-15 02:08:55 PDT
The latest version of Opera, Safari, Chrome and even INTERNET EXPLORER
support TLS 1.1 + TLS 1.2.
Firefox is being left behind, which is rare.
What work needs to be done for TLS 1.1 to be enabled?
Comment 33 Zoltán Halassy 2013-10-15 02:17:14 PDT
(In reply to Johnny Iguna from comment #32)
> The latest version of Opera, Safari, Chrome and even INTERNET EXPLORER
> support TLS 1.1 + TLS 1.2.

So does stable Firefox.

> Firefox is being left behind, which is rare.

Not true.

> What work needs to be done for TLS 1.1 to be enabled?

Your work.
You can enable it in about:config :
set security.tls.version.max to 2 or 3 to enable TLSv1.1 (3 enables TLSv1.2 too)
Comment 34 4994650 2013-10-15 11:48:47 PDT
(In reply to Zoltán Halassy from comment #33)
> (In reply to Johnny Iguna from comment #32)
> > What work needs to be done for TLS 1.1 to be enabled?
> 
> Your work.
> You can enable it in about:config :

I do believe he was referring to TLSv1.1 (and TLSv1.2) being enabled by default for all users
Comment 35 Justin Tracey 2013-10-15 15:12:45 PDT
(In reply to Johnny Iguna from comment #32)
> The latest version of Opera, Safari, Chrome and even INTERNET EXPLORER
> support TLS 1.1 + TLS 1.2.
> Firefox is being left behind, which is rare.
> What work needs to be done for TLS 1.1 to be enabled?

You can see the blocking bugs where it says "Depends on:"
Assuming that remains accurate, it would appear the largest thing blocking 1.1 (and 1.2) from being on by default is that there is no fallback implemented, so that if 1.1 or 1.2 were enabled, any server that does not have 1.1 or 1.2 enabled would fail to make an SSL connection.
To quote http://kb.mozillazine.org/Security.tls.version.* :
"There is currently no fallback from TLS 1.1/1.2 to earlier protocols. Thus, selecting security.tls.version.max = 2 (or 3) for TLS 1.1 (or 1.2) support results in the connection failing when the server connected to doesn't support that version. Once the fallback is implemented, the default for security.tls.version.max will be changed to 3 to utilize the most recent TLS 1.2 version by default."

(In reply to Zoltán Halassy from comment #33)
> You can enable it in about:config :
> set security.tls.version.max to 2 or 3 to enable TLSv1.1 (3 enables TLSv1.2
> too)
That's not entirely accurate, as I descibe above. For example, right now, setting it to 3 does not enable TLSv1.2 "too", it *only* enables TLSv1.2. Given that's not the behavior most people want, it's not a viable solution.

This is, at least, the theory; I haven't personally replicated the bug.
https://bugzilla.mozilla.org/show_bug.cgi?id=839310
Comment 36 BoerenkoolMetWorst 2013-10-16 01:21:38 PDT
I have been to quite a few sites that don't support TLS 1.1 or 1.2 while having both enabled, so it seems the fallback works fine.

(In reply to Justin Tracey from comment #35)
> (In reply to Johnny Iguna from comment #32)
> > The latest version of Opera, Safari, Chrome and even INTERNET EXPLORER
> > support TLS 1.1 + TLS 1.2.
> > Firefox is being left behind, which is rare.
> > What work needs to be done for TLS 1.1 to be enabled?
> 
> You can see the blocking bugs where it says "Depends on:"
> Assuming that remains accurate, it would appear the largest thing blocking
> 1.1 (and 1.2) from being on by default is that there is no fallback
> implemented, so that if 1.1 or 1.2 were enabled, any server that does not
> have 1.1 or 1.2 enabled would fail to make an SSL connection.
> To quote http://kb.mozillazine.org/Security.tls.version.* :
> "There is currently no fallback from TLS 1.1/1.2 to earlier protocols. Thus,
> selecting security.tls.version.max = 2 (or 3) for TLS 1.1 (or 1.2) support
> results in the connection failing when the server connected to doesn't
> support that version. Once the fallback is implemented, the default for
> security.tls.version.max will be changed to 3 to utilize the most recent TLS
> 1.2 version by default."
> 
> (In reply to Zoltán Halassy from comment #33)
> > You can enable it in about:config :
> > set security.tls.version.max to 2 or 3 to enable TLSv1.1 (3 enables TLSv1.2
> > too)
> That's not entirely accurate, as I descibe above. For example, right now,
> setting it to 3 does not enable TLSv1.2 "too", it *only* enables TLSv1.2.
> Given that's not the behavior most people want, it's not a viable solution.
Setting it to 3 does enable TLS 1.2 "too", only if you also so set security.tls.version.MIN to 3, it will be TLS 1.2 only. See here for more info:
http://kb.mozillazine.org/Security.tls.version.*
Comment 37 Zoltán Halassy 2013-10-16 01:22:43 PDT
(In reply to Justin Tracey from comment #35)
> (In reply to Zoltán Halassy from comment #33)
> > You can enable it in about:config :
> > set security.tls.version.max to 2 or 3 to enable TLSv1.1 (3 enables TLSv1.2
> > too)
> That's not entirely accurate, as I descibe above. For example, right now,
> setting it to 3 does not enable TLSv1.2 "too", it *only* enables TLSv1.2.
> Given that's not the behavior most people want, it's not a viable solution.
> 
> This is, at least, the theory; I haven't personally replicated the bug.
> https://bugzilla.mozilla.org/show_bug.cgi?id=839310

That is strange. I'm using the stock 64bit version for linux (Firefox 24), and that does fall back to TLSv1.0 when TLSv1.2 or TLSv1.1 is not available. I did not test if it falls back to TLSv1.1 when that one is available and TLSv1.2 is not.
Comment 38 Florian Bender 2013-10-16 05:44:28 PDT
AFAIU, not all TLSv1 servers are intolerant to TLSv1.1 and/or TLSv1.2. So even if there is no dedicated fallback mechanism yet, some sites will work anyway. But please correct me if I'm wrong.
Comment 39 sjw 2013-10-17 08:55:36 PDT
I tested a lot of sites that only supports SSL 3 and TLS 1.0 (checked with https://www.ssllabs.com/ssltest/index.html) and they worked fine.
Sites with TLS 1.2 also works fine, but I didn't found any server that supports only TLS 1.1.
Comment 40 Kurt Roeckx 2013-10-17 09:21:57 PDT
The last statistics I saw that around 1.1% of the sites are intolerant to client announcing that they support a higher version than that the server supports. For those you need the fallback logic to be able to talk to them.

Those statistics are june 2013.  I've just asked for updated numbers.
Comment 41 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2013-10-28 18:41:28 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/998b63fe3492

NEEDINFO?brian@briansmith.org to remind myself to request uplift to mozilla-aurora and mozilla-beta.
Comment 42 Carsten Book [:Tomcat] 2013-10-29 05:46:11 PDT
https://hg.mozilla.org/mozilla-central/rev/998b63fe3492
Comment 43 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2013-11-03 23:58:22 PST
Comment on attachment 783283 [details] [diff] [review]
removed trailing whitespace

[Approval Request Comment]
Bug caused by (feature/regressing bug #): none (new feature)

User impact if declined: Firefox will not be able to communicate with any servers that require TLS 1.1. There are very few such sites. Also, I am hoping to uplift TLS 1.2 to mozilla-aurora, and it would be useful to have TLS 1.1 enabled in mozilla-beta if that happens so that we can more correctly diagnose problems that occur due to TLS 1.2 support that don't happen when TLS 1.1 is enabled.

Testing completed (on m-c, etc.): Google Chrome has been testing the NSS code for this for a few releases and they have basically worked out all the kinks for us. This has been on mozilla-central for about a week now.

Risk to taking this patch (and alternatives if risky): Little risk because, Google Chrome has basically tested this stuff for us. Also, we implemented a workaround in bug 839310 that should make the compatibility impact basically zero. Bug 839310 is already on mozilla-aurora.

String or IDL/UUID changes made by this patch: none.
Comment 44 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2013-11-03 23:58:53 PST
Clearing the NEEDINFO I set on myself.
Comment 45 Florian Bender 2013-11-04 00:55:21 PST
Just to check: In comment 41, you indicated that you wanted to request uplift to Beta, too, though you only requested Aurora. If Beta, do you intend to uplift this to B2G26 as well (AFAIK it already branched)?
Comment 46 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2013-11-04 12:45:15 PST
Comment on attachment 783283 [details] [diff] [review]
removed trailing whitespace

See previous approval request. This requires the patch for bug 839310 to be uplifted too. All of these changes are low-risk.
Comment 47 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2013-11-04 12:48:56 PST
(In reply to Florian Bender from comment #45)
> Just to check: In comment 41, you indicated that you wanted to request
> uplift to Beta, too, though you only requested Aurora. If Beta, do you
> intend to uplift this to B2G26 as well (AFAIK it already branched)?

Yes, I meant to request approval on m-b too. As for b2g26, I think it would be great to land it there, but I need to figure out what the process is. There's no b2g26 approval flag, for example.
Comment 48 Kurt Roeckx 2013-11-04 12:56:20 PST
So some stats:
June 2013:
- 169,801 no problems
- 58 servers intolerant to TLS 1.0+
- 1,691 servers intolerant to TLS 1.1+
- 4 servers intolerant only to TLS 1.1
- 56 servers intolerant only to TLS 1.2

1,809 problematic servers in total.


November 2013:

Total servers: 163,587

TLS 1.0 intolerance        9
TLS 1.1 intolerance    1,388
TLS 1.2 intolerance    1,448 (~ 0.9%)
TLS 1.3 intolerance   17,840 (~10.9%)
TLS 2.98 intolerance 122,698 (~75.0%)

Long handshake intolerance: 4,795 (~2.9%)

Numbers provided by Ivan Ristic from ssllabs.
Comment 49 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2013-11-04 13:09:02 PST
(In reply to Kurt Roeckx from comment #48)
> 
> November 2013:
> 
> Total servers: 163,587
> 
> TLS 1.0 intolerance        9
> TLS 1.1 intolerance    1,388
> TLS 1.2 intolerance    1,448 (~ 0.9%)
> TLS 1.3 intolerance   17,840 (~10.9%)
> TLS 2.98 intolerance 122,698 (~75.0%)
> 
> Long handshake intolerance: 4,795 (~2.9%)
> 
> Numbers provided by Ivan Ristic from ssllabs.

Thanks Kurt. Note that this is the type of problem that is worked around by the patch for bug 839310, and that's why that is a prerequisite for this bug.
Comment 50 Kurt Roeckx 2013-11-04 13:14:57 PST
(In reply to Brian Smith (:briansmith, was :bsmith; NEEDINFO me if you want a response) from comment #49)
> Thanks Kurt. Note that this is the type of problem that is worked around by
> the patch for bug 839310, and that's why that is a prerequisite for this bug.

That will deal with the TLS 1.0-1.2 intolerance yes.

I'm now a little concerned about the long handshake intolerance since in my experience those connections just hang, but I guess we'll just have to see whether people run into that or not.
Comment 51 bhavana bajaj [:bajaj] 2013-11-04 14:28:36 PST
(In reply to Brian Smith (:briansmith, was :bsmith; NEEDINFO me if you want a response) from comment #46)
> Comment on attachment 783283 [details] [diff] [review]
> removed trailing whitespace
> 
> See previous approval request. This requires the patch for bug 839310 to be
> uplifted too. All of these changes are low-risk.

Unclear why we are doing a feature uplift so late in the cycle ? I think this is fine to take on aurora keeping in mind where we are in the cycle but given the risk here, its a little late in the game for beta.
Comment 52 bhavana bajaj [:bajaj] 2013-11-04 14:32:36 PST
(In reply to Brian Smith (:briansmith, was :bsmith; NEEDINFO me if you want a response) from comment #43)
> Comment on attachment 783283 [details] [diff] [review]
> removed trailing whitespace
> 
> [Approval Request Comment]
> Bug caused by (feature/regressing bug #): none (new feature)
> 
> User impact if declined: Firefox will not be able to communicate with any
> servers that require TLS 1.1. There are very few such sites. Also, I am
> hoping to uplift TLS 1.2 to mozilla-aurora, and it would be useful to have
> TLS 1.1 enabled in mozilla-beta if that happens so that we can more
> correctly diagnose problems that occur due to TLS 1.2 support that don't
> happen when TLS 1.1 is enabled.
> 
> Testing completed (on m-c, etc.): Google Chrome has been testing the NSS
> code for this for a few releases and they have basically worked out all the
> kinks for us. This has been on mozilla-central for about a week now.
> 
> Risk to taking this patch (and alternatives if risky): Little risk because,
> Google Chrome has basically tested this stuff for us. Also, we implemented a
> workaround in bug 839310 that should make the compatibility impact basically
> zero. Bug 839310 is already on mozilla-aurora.

Do we plan to do any additional testing here ? +1 to the patch in 839310 to reduce incompatibilities, but did we engage any QA on getting corner-cases tested here ? Also would like to know if this impacts our esr users ?
> 
> String or IDL/UUID changes made by this patch: none.
Comment 53 bhavana bajaj [:bajaj] 2013-11-04 14:36:27 PST
(In reply to Brian Smith (:briansmith, was :bsmith; NEEDINFO me if you want a response) from comment #47)
> (In reply to Florian Bender from comment #45)
> > Just to check: In comment 41, you indicated that you wanted to request
> > uplift to Beta, too, though you only requested Aurora. If Beta, do you
> > intend to uplift this to B2G26 as well (AFAIK it already branched)?
> 
> Yes, I meant to request approval on m-b too. As for b2g26, I think it would
> be great to land it there, but I need to figure out what the process is.
> There's no b2g26 approval flag, for example.

If this is target for B2G version 1.2 then you should be setting the blocking-b2g: koi? with appropriate reasoning on why this feature is critical for B2G. Although note that for 1.2 in B2G we are way past the time where we are blocking on enhancement/new feature request unless deemed absolutely critical explaining the user impact.
Comment 54 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2013-11-10 23:11:52 PST
(In reply to bhavana bajaj [:bajaj] from comment #52)
> Do we plan to do any additional testing here ? +1 to the patch in 839310 to
> reduce incompatibilities, but did we engage any QA on getting corner-cases
> tested here ? Also would like to know if this impacts our esr users ?

We haven't had any issues with this on Nightly since it landed in mozilla-central on 2013-10-29. Also, I have been surfing the net with TLS 1.1 enabled in my Firefox Nightly literally for months without issue, even without the patch for bug 839310. Google Chrome actually has less compatibility/fallback logic than mozilla-beta will, and it has been shipping with TLS 1.1 enabled for quite a while now. I worry that if we don't uplift this to m-b before the next merge, then it will be too late to uplift it for the merge after that. If we want QA to do testing for this, then I suggest we uplift it to m-b now and do that testing while it is on m-b. If we notice any problems then we'll pull TLS 1.1 off of m-b immediately. It is controlled by a pref, so it will be easy. Also, because it is controlled by a pref, even the hotfix addon can be used to work around any issues.

Thanks for explaining the B2G 1.2 issues. I am not going to request that this be added to B2G 1.2, given what you've said.
Comment 55 bhavana bajaj [:bajaj] 2013-11-11 10:08:33 PST
Comment on attachment 783283 [details] [diff] [review]
removed trailing whitespace

Approving on aurora for now and adding :mwobensmith as QA contact here to see if he can help with any additional testing.
Comment 56 Lukas Blakk [:lsblakk] use ?needinfo 2013-11-11 10:21:45 PST
Comment on attachment 783283 [details] [diff] [review]
removed trailing whitespace

Thanks for the thoughtful comments Brian, that helps a lot.  Let's get this into today's beta build and Matt, if you can run this through its paces to help pile on to our other feedback channels that would help bring peace of mind to shipping here.
Comment 58 Ryan VanderMeulen [:RyanVM] 2013-11-11 14:54:44 PST
https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/658709366f0d
Comment 59 vulcain 2013-11-11 23:44:26 PST
Someone knows if Thunderbird 26 will have TLS 1.1 ??
Comment 60 rsx11m 2013-11-12 03:32:03 PST
It's a Core change, thus Thunderbird should pick it up on the beta channel (i.e., 26.0) unless they explicitly override it in their application-specific default preferences.
Comment 61 Ioana (away) 2013-11-13 02:37:13 PST
The Firefox Desktop QA team tested several secure sites to make sure there are no regressions caused by this fix: https://etherpad.mozilla.org/firefox26b4-exploratory.

Is there anything else (certain sites, scenarios) you think should be covered here?
Comment 62 Matt Wobensmith [:mwobensmith][:matt:] 2013-11-13 09:41:03 PST
I'm working with Brian to develop more test methodologies for QA to implement. 

For now, I think we have our bases covered, thanks to your work.
Comment 65 Florian Bender 2013-11-26 01:09:29 PST
Updating target milestone to clarify tracking. The bug landed in Gecko 28 but was uplifted to Gecko 27, Gecko 26 and B2G 26.


Please correct if you don't like that.
Comment 66 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2013-11-26 01:15:12 PST
(In reply to Florian Bender from comment #65)
> Updating target milestone to clarify tracking. The bug landed in Gecko 28
> but was uplifted to Gecko 27, Gecko 26 and B2G 26.
> 
> 
> Please correct if you don't like that.

Thanks for helping, but the target milestone needs to be the version where it got checked into mozilla-central. The "tracking flags" section keeps track of uplifts.
Comment 67 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2013-12-02 11:03:57 PST
https://hg.mozilla.org/releases/mozilla-beta/rev/d2a1d2a70d8d

TLS 1.1 was disabled for Firefox 26 because a last-minute compatibility issue was found with https://mymedicare.gov. Error code: ssl_error_bad_mac_read. This compatibility issue is being tracked in bug 944465. Bug 944465 is not public because we want to verify that it isn't a security-sensitive bug, but I expect that bug will be made public soon.
Comment 68 Ryan VanderMeulen [:RyanVM] 2013-12-02 15:23:01 PST
(In reply to Brian Smith (:briansmith, was :bsmith; Please NEEDINFO? me if you want a response) from comment #67)
> https://hg.mozilla.org/releases/mozilla-beta/rev/d2a1d2a70d8d

https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/d2a1d2a70d8d
Comment 69 Andrei Vaida, QA [:avaida] – please ni? me 2013-12-03 06:51:22 PST
(In reply to Ioana Budnar, QA [:ioana] from comment #61)
> The Firefox Desktop QA team tested several secure sites to make sure there
> are no regressions caused by this fix:
> https://etherpad.mozilla.org/firefox26b4-exploratory.
> 
> Is there anything else (certain sites, scenarios) you think should be
> covered here?

Regression testing has been performed on Fx 26 Beta 10, using the following secured websites: https://etherpad.mozilla.org/Fx26b10-TLS-SSL. This bug has been confirmed as fixed on the above stated Firefox version.
Comment 70 Andrei Vaida, QA [:avaida] – please ni? me 2013-12-03 07:13:00 PST
Please ignore the last sentence from my previous statement. We can't really verify this bug yet, so we just tested to see that the fix didn't cause any regressions.
Comment 71 Petruta Rasa [QA] [:petruta] 2013-12-17 06:15:08 PST
Regression testing has been performed on Firefox 27 Beta 2 (20131216183647) on Windows 8 64-bit, Windows 7 64-bit, Mac OSX 10.9, Ubuntu 12.04 32-bit on the following secured websites: 
https://etherpad.mozilla.org/Fx27b2-TLS-SSL

Except bug 947079, we did not find any other issues, so we are marking this as verified.
Comment 72 Florian Bender 2013-12-17 07:10:01 PST
Given comment 71, how do you feel about enabling TLS 1.1 for a limited audience on Fx 26 through a hotfix addon? The purpose would be a broader testing ground / gathering compat data before TLS 1.2 rolls out in Fx 27.
Comment 73 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2013-12-18 14:29:08 PST
(In reply to Florian Bender from comment #72)
> Given comment 71, how do you feel about enabling TLS 1.1 for a limited
> audience on Fx 26 through a hotfix addon? The purpose would be a broader
> testing ground / gathering compat data before TLS 1.2 rolls out in Fx 27.

Florian, thanks for your interest. However, we only use the hotfix addon for emergency fixes to widespread and serious problems. This doesn't qualify as that. I am going to be talking to mwobensmith again today about compatibility testing for TLS 1.2 and TLS 1.1 in Firefox 27 to minimize the odds of something like bug 944465 happening again for the Firefox 27 cycle.
Comment 74 Bogdan Maris, QA [:bogdan_maris] 2014-01-08 09:09:46 PST
Regression testing has been performed on latest Aurora 28.0a2 (buildID: 20140107004003) on Windows XP 32-bit, Windows 7 64-bit, Mac OSX 10.9.1 and Ubuntu 13.10 32-bit on the following secured websites: 
https://etherpad.mozilla.org/Fx28-0a2-TLS-SSL

We ran into "ssl_error_internal_error_alert" while trying to access some URLs using a secured connection (a bug is logged regarding this issue - bug 955315), I think this is a website issue though since on older versions of Firefox (27 beta 2) the error is thrown as well; and also ran into bug 947079 as well. Marking this as verified since I think none of the above issues are related to this implementation.

Note You need to log in before you can comment on or make changes to this bug.