Closed
Bug 1250568
Opened 9 years ago
Closed 9 years ago
Allow enabling TLS 1.3
Categories
(Core :: Security: PSM, defect)
Core
Security: PSM
Tracking
()
RESOLVED
FIXED
mozilla49
Tracking | Status | |
---|---|---|
firefox49 | --- | fixed |
People
(Reporter: mt, Unassigned)
References
Details
Attachments
(5 files, 10 obsolete files)
This will eventually need a bit more infrastructure, but for the moment, expanding the allowed range of values for SSL/TLS versions should allow the brave to enable draft versions of TLS 1.3. It should also let us get a head-start on compatibility testing.
Comment 1•9 years ago
|
||
Is NSS_ENABLE_TLS_1_3 defined by default? Do we have to handle the error gracefully in case the system NSS does not set NSS_ENABLE_TLS_1_3?
Reporter | ||
Comment 2•9 years ago
|
||
The code detects what version of TLS is supported by NSS. But apparently, despite what I've been told, we don't set NSS_ENABLE_TLS_1_3, so nothing has changed here.
Reporter | ||
Comment 3•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/36367/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/36367/
Attachment #8723124 -
Flags: review?(dkeeler)
Reporter | ||
Comment 4•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/36369/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/36369/
Attachment #8723125 -
Flags: review?(ekr)
Comment 5•9 years ago
|
||
https://reviewboard.mozilla.org/r/36367/#review32977
FWIW, it looks like you accidentally typo'ed bsmedberg as bsmegberg in your r? syntax, so he might not have noticed the review.
Reporter | ||
Updated•9 years ago
|
Attachment #8723124 -
Flags: review?(benjamin)
Reporter | ||
Comment 6•9 years ago
|
||
Cykesiopka, thanks.
Comment 7•9 years ago
|
||
Comment on attachment 8723124 [details]
MozReview Request: Bug 1250568 - Add support for TLS1.3 in prefs and telemetry, r=bsmedberg?keeler
https://reviewboard.mozilla.org/r/36367/#review32999
data-review=me with that change
::: toolkit/components/telemetry/Histograms.json:1334
(Diff revision 1)
> "SSL_HANDSHAKE_VERSION": {
This histogram as well as the new ones below need the following data which is now required:
"bug_numbers"
"alert_emails"
Attachment #8723124 -
Flags: review?(benjamin) → review+
Reporter | ||
Comment 8•9 years ago
|
||
Comment on attachment 8723124 [details]
MozReview Request: Bug 1250568 - Add support for TLS1.3 in prefs and telemetry, r=bsmedberg?keeler
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/36367/diff/1-2/
Attachment #8723124 -
Attachment description: MozReview Request: Bug 1250568 - Add support for TLS1.3 in prefs and telemetry, r?bsmegberg,keeler → MozReview Request: Bug 1250568 - Add support for TLS1.3 in prefs and telemetry, r=bsmedberg?keeler
Attachment #8723124 -
Flags: review?(dkeeler)
Reporter | ||
Comment 9•9 years ago
|
||
Comment on attachment 8723125 [details]
MozReview Request: Bug 1250568 - Enable building of TLS 1.3, r?ekr
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/36369/diff/1-2/
Reporter | ||
Comment 10•9 years ago
|
||
Comment on attachment 8723124 [details]
MozReview Request: Bug 1250568 - Add support for TLS1.3 in prefs and telemetry, r=bsmedberg?keeler
I don't know why Mozreview in its infinite wisdom decided to drop the review request. It still stands. BTW, I've added alert_emails entries for all the histograms that looked security-relevant. I hope that the alerts aren't too spammy.
Attachment #8723124 -
Flags: review?(dkeeler)
Comment 11•9 years ago
|
||
Comment on attachment 8723124 [details]
MozReview Request: Bug 1250568 - Add support for TLS1.3 in prefs and telemetry, r=bsmedberg?keeler
https://reviewboard.mozilla.org/r/36367/#review33041
LGTM.
::: security/manager/ssl/nsNSSIOLayer.cpp:1676
(Diff revision 2)
> - SSL_LIBRARY_VERSION_TLS_1_2 };
> + SSL_LIBRARY_VERSION_TLS_1_3 };
I don't think this should change, actually. If 'security.tls.version.fallback-limit' is set to 4 and NSS is compiled with TLS 1.3 support, this should already do what we want.
::: toolkit/components/telemetry/Histograms.json:1340
(Diff revision 2)
> - "description": "SSL Version (1=tls1, 2=tls1.1, 3=tls1.2)"
> + "description": "SSL Version (1=tls1, 2=tls1.1, 3=tls1.2, 4=tls1.3)"
Might be nice to update the comment at https://dxr.mozilla.org/mozilla-central/rev/d848a5628d801a460a7244cbcdea22d328d8b310/security/manager/ssl/nsNSSCallbacks.cpp#1149 as well.
::: toolkit/components/telemetry/Histograms.json:8014
(Diff revision 2)
> "SSL_AUTH_DSA_KEY_SIZE_FULL": {
We don't enable these cipher suites any more, so this can be removed (or we can do it in a follow-up).
Attachment #8723124 -
Flags: review?(dkeeler) → review+
Comment 12•9 years ago
|
||
(In reply to David Keeler [:keeler] (use needinfo?) from comment #11)
> I don't think this should change, actually. If
> 'security.tls.version.fallback-limit' is set to 4 and NSS is compiled with
> TLS 1.3 support, this should already do what we want.
Then we don't fallback to TLS 1.2? I don't think it's possible, given that ~1% servers are TLS 1.3 intolerant.
Luckily, TLS 1.3 has a built-in anti-downgrade mechanism, so we don't have to worry about the downgrade attack when we fallback to 1.2.
Comment 13•9 years ago
|
||
The desired behavior is that we offer TLS 1.3 and then fall back to TLS 1.2 if it fails.
Comment 14•9 years ago
|
||
(In reply to David Keeler [:keeler] (use needinfo?) from comment #11)
> ::: security/manager/ssl/nsNSSIOLayer.cpp:1676
> (Diff revision 2)
> > - SSL_LIBRARY_VERSION_TLS_1_2 };
> > + SSL_LIBRARY_VERSION_TLS_1_3 };
Ah, but this should not be changed. David is right. (MozReview should really show more context by default...)
Reporter | ||
Comment 15•9 years ago
|
||
https://reviewboard.mozilla.org/r/36367/#review33041
> I don't think this should change, actually. If 'security.tls.version.fallback-limit' is set to 4 and NSS is compiled with TLS 1.3 support, this should already do what we want.
Quite right. We want this to fall back to 1.2 by default.
> We don't enable these cipher suites any more, so this can be removed (or we can do it in a follow-up).
bug 1251133
Reporter | ||
Comment 16•9 years ago
|
||
Comment on attachment 8723124 [details]
MozReview Request: Bug 1250568 - Add support for TLS1.3 in prefs and telemetry, r=bsmedberg?keeler
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/36367/diff/2-3/
Reporter | ||
Comment 17•9 years ago
|
||
Comment on attachment 8723125 [details]
MozReview Request: Bug 1250568 - Enable building of TLS 1.3, r?ekr
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/36369/diff/2-3/
Reporter | ||
Comment 18•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/44005/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/44005/
Attachment #8737554 -
Flags: review?(dkeeler)
Attachment #8737554 -
Flags: review?(benjamin)
Attachment #8737555 -
Flags: review?(ekr)
Attachment #8737556 -
Flags: review?(dkeeler)
Attachment #8737557 -
Flags: review?(dkeeler)
Reporter | ||
Comment 19•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/44007/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/44007/
Reporter | ||
Comment 20•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/44009/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/44009/
Reporter | ||
Comment 21•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/44011/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/44011/
Reporter | ||
Updated•9 years ago
|
Attachment #8723124 -
Attachment is obsolete: true
Reporter | ||
Updated•9 years ago
|
Attachment #8723125 -
Attachment is obsolete: true
Attachment #8723125 -
Flags: review?(ekr)
Reporter | ||
Comment 22•9 years ago
|
||
Comment on attachment 8737554 [details]
MozReview Request: Bug 1250568 - Add support for TLS1.3 in prefs and telemetry, r?bsmedberg,keeler
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/44005/diff/1-2/
Reporter | ||
Comment 23•9 years ago
|
||
Comment on attachment 8737555 [details]
MozReview Request: Bug 1250568 - Enable building of TLS 1.3, r?ekr
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/44007/diff/1-2/
Reporter | ||
Comment 24•9 years ago
|
||
Comment on attachment 8737556 [details]
MozReview Request: Bug 1250568 - Adding ECDHE_PSK suites, r?keeler
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/44009/diff/1-2/
Reporter | ||
Comment 25•9 years ago
|
||
Comment on attachment 8737557 [details]
MozReview Request: Bug 1250568 - Adding TLS 1.3 to nsISSLStatus, r?keeler
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/44011/diff/1-2/
Reporter | ||
Comment 26•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/44025/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/44025/
Attachment #8737595 -
Flags: review?(dkeeler)
Attachment #8737595 -
Flags: review?(benjamin)
Attachment #8737596 -
Flags: review?(ekr)
Attachment #8737597 -
Flags: review?(dkeeler)
Attachment #8737598 -
Flags: review?(dkeeler)
Reporter | ||
Comment 27•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/44027/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/44027/
Reporter | ||
Comment 28•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/44029/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/44029/
Reporter | ||
Comment 29•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/44031/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/44031/
Reporter | ||
Updated•9 years ago
|
Attachment #8737554 -
Attachment is obsolete: true
Attachment #8737554 -
Flags: review?(dkeeler)
Attachment #8737554 -
Flags: review?(benjamin)
Reporter | ||
Updated•9 years ago
|
Attachment #8737555 -
Attachment is obsolete: true
Attachment #8737555 -
Flags: review?(ekr)
Reporter | ||
Updated•9 years ago
|
Attachment #8737556 -
Attachment is obsolete: true
Attachment #8737556 -
Flags: review?(dkeeler)
Reporter | ||
Updated•9 years ago
|
Attachment #8737557 -
Attachment is obsolete: true
Attachment #8737557 -
Flags: review?(dkeeler)
Comment 30•9 years ago
|
||
Comment on attachment 8737595 [details]
MozReview Request: Bug 1250568 - Add support for TLS1.3 in prefs and telemetry, r?bsmedberg,keeler
https://reviewboard.mozilla.org/r/44025/#review40745
Attachment #8737595 -
Flags: review?(dkeeler) → review+
Comment 31•9 years ago
|
||
Comment on attachment 8737597 [details]
MozReview Request: Bug 1250568 - Adding ECDHE_PSK suites, r?keeler
https://reviewboard.mozilla.org/r/44029/#review40749
Attachment #8737597 -
Flags: review?(dkeeler) → review+
Comment 32•9 years ago
|
||
Comment on attachment 8737598 [details]
MozReview Request: Bug 1250568 - Adding TLS 1.3 to nsISSLStatus, r?keeler
https://reviewboard.mozilla.org/r/44031/#review40753
LGTM.
::: devtools/shared/webconsole/network-helper.js:529
(Diff revision 1)
> * * "secure": the connection was properly secured.
> * If state == broken:
> * - errorMessage: full error message from
> * nsITransportSecurityInfo.
> * If state == secure:
> * - protocolVersion: one of TLSv1, TLSv1.1, TLSv1.2.
Let's add TLSv1.3 here.
::: devtools/shared/webconsole/network-helper.js:713
(Diff revision 1)
> * description.
> *
> * @param Number version
> * One of nsISSLStatus version constants.
> * @return string
> * One of TLSv1, TLSv1.1, TLSv1.2 if @param version is valid,
nit: update documentation
::: devtools/shared/webconsole/network-helper.js:724
(Diff revision 1)
> return "TLSv1";
> case Ci.nsISSLStatus.TLS_VERSION_1_1:
> return "TLSv1.1";
> case Ci.nsISSLStatus.TLS_VERSION_1_2:
> return "TLSv1.2";
> + case Ci.nsISSLStatus.TLS_VERSION_1_3:
I guess there's a test for this function we should probably update: test_security-info-protocol-version.js
::: security/manager/ssl/nsISSLStatus.idl:23
(Diff revision 1)
>
> const short SSL_VERSION_3 = 0;
> const short TLS_VERSION_1 = 1;
> const short TLS_VERSION_1_1 = 2;
> const short TLS_VERSION_1_2 = 3;
> + const short TLS_VERSION_1_3 = 4;
As a heads-up, it looks like both nsITLSServerSocket.idl and nsISSLSocketControl.idl also define TLS_VERSION_1\* - it might be relevant to add 1_3 to those as well (then again, they really only seem to use 1_2).
Attachment #8737598 -
Flags: review?(dkeeler) → review+
Reporter | ||
Comment 33•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/44171/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/44171/
Attachment #8737907 -
Flags: review?(dkeeler)
Attachment #8737907 -
Flags: review?(benjamin)
Attachment #8737908 -
Flags: review?(ekr)
Attachment #8737909 -
Flags: review?(dkeeler)
Attachment #8737910 -
Flags: review?(dkeeler)
Reporter | ||
Comment 34•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/44173/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/44173/
Reporter | ||
Comment 35•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/44175/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/44175/
Reporter | ||
Comment 36•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/44177/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/44177/
Reporter | ||
Updated•9 years ago
|
Attachment #8737595 -
Attachment is obsolete: true
Attachment #8737595 -
Flags: review?(benjamin)
Reporter | ||
Updated•9 years ago
|
Attachment #8737596 -
Attachment is obsolete: true
Attachment #8737596 -
Flags: review?(ekr)
Reporter | ||
Updated•9 years ago
|
Attachment #8737597 -
Attachment is obsolete: true
Reporter | ||
Updated•9 years ago
|
Attachment #8737598 -
Attachment is obsolete: true
Comment 37•9 years ago
|
||
Comment on attachment 8737907 [details]
MozReview Request: Bug 1250568 - Add support for TLS1.3 in prefs and telemetry, r=bsmedberg,keeler
https://reviewboard.mozilla.org/r/44171/#review41021
::: toolkit/components/telemetry/Histograms.json:8059
(Diff revision 1)
> "description": "The number of errors using the HomeProvider due to a locked DB. *** No longer needed (bug 1156565). Delete histogram and accumulation code! ***"
> },
> + "SSL_TLS13_INTOLERANCE_REASON_PRE": {
> + "alert_emails": ["seceng-telemetry@mozilla.com"],
> + "bug_numbers": [1250568],
> + "expires_in_version": "never",
Why is this "never"? If this is an operational metric, should this be opt-out instead of opt-in? Also how will you be monitoring this? Current automated alerting doesn't work with enumerated histograms in a useful way.
::: toolkit/components/telemetry/Histograms.json:8062
(Diff revision 1)
> + "alert_emails": ["seceng-telemetry@mozilla.com"],
> + "bug_numbers": [1250568],
> + "expires_in_version": "never",
> + "kind": "enumerated",
> + "n_values": 64,
> + "description": "detected symptom of TLS 1.3 intolerance, before considering historical info"
This needs more detail. What do these 64 values mean? You either need to list them or reference some other enumeration value in the code.
Attachment #8737907 -
Flags: review?(benjamin)
Updated•9 years ago
|
Attachment #8737907 -
Flags: feedback-
Comment 38•9 years ago
|
||
Comment on attachment 8737907 [details]
MozReview Request: Bug 1250568 - Add support for TLS1.3 in prefs and telemetry, r=bsmedberg,keeler
https://reviewboard.mozilla.org/r/44171/#review41071
Did this change since the last review? Why isn't there an interdiff? If it hasn't changed, r=me carries over.
Attachment #8737907 -
Flags: review?(dkeeler) → review+
Comment 39•9 years ago
|
||
Comment on attachment 8737909 [details]
MozReview Request: Bug 1250568 - Adding ECDHE_PSK suites, r=keeler
https://reviewboard.mozilla.org/r/44175/#review41073
Attachment #8737909 -
Flags: review?(dkeeler) → review+
Comment 40•9 years ago
|
||
Comment on attachment 8737910 [details]
MozReview Request: Bug 1250568 - Adding TLS 1.3 to nsISSLStatus, r=keeler
https://reviewboard.mozilla.org/r/44177/#review41077
LGTM.
Attachment #8737910 -
Flags: review?(dkeeler) → review+
Reporter | ||
Comment 41•9 years ago
|
||
So, I get an r+, address the nits, mozreview forgets the r+ and asks for review again, then I end up with f-. Great.
I will address the concerns, but that might take some time.
Updated•9 years ago
|
Attachment #8737908 -
Flags: review?(ekr) → review+
Comment 42•9 years ago
|
||
Comment on attachment 8737908 [details]
MozReview Request: Bug 1250568 - Enable building of TLS 1.3, r=ekr
https://reviewboard.mozilla.org/r/44173/#review43515
LGTM
Reporter | ||
Comment 43•9 years ago
|
||
https://reviewboard.mozilla.org/r/44171/#review41021
> Why is this "never"? If this is an operational metric, should this be opt-out instead of opt-in? Also how will you be monitoring this? Current automated alerting doesn't work with enumerated histograms in a useful way.
Actually, I just copied the existing ones on the assumption that whoever put them there knew what they were doing. However, I will say that this is something we look at whenever we make changes to the TLS implementation, and while some of the existing metrics are now less useful to the point they are probably due to be removed, they have been necessary to drive decisions about how we manage upgrades to the stack. Since we are looking to ship TLS 1.3 some time this year, this will be critical in doing that. And it will be useful for some time after that in determining whether we need all the nasty fallback logic we carry.
never: I don't believe that we need real-time alerting for this, it's primarily of strategic value.
opt-out: I don't have a ready rule-of-thumb for making the privacy trade-off here. In terms of utility, we currently get only a low volume of measurements on the equivalent opt-out metrics, we could certainly use more measurements if you think the cost acceptable. In terms of privacy cost, this metric is predominantly about sites rather than users.
expires_in_version: I can't give you an end date for that experiment because I currently can't give you a start date. I hope that we'll be done before Firefox 60, but can't be certain.
> This needs more detail. What do these 64 values mean? You either need to list them or reference some other enumeration value in the code.
I've modified this, and all the existing values to include a better pointer.
Reporter | ||
Comment 44•9 years ago
|
||
Comment on attachment 8737907 [details]
MozReview Request: Bug 1250568 - Add support for TLS1.3 in prefs and telemetry, r=bsmedberg,keeler
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/44171/diff/1-2/
Attachment #8737907 -
Flags: feedback- → review?(benjamin)
Reporter | ||
Comment 45•9 years ago
|
||
Comment on attachment 8737908 [details]
MozReview Request: Bug 1250568 - Enable building of TLS 1.3, r=ekr
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/44173/diff/1-2/
Reporter | ||
Comment 46•9 years ago
|
||
Comment on attachment 8737909 [details]
MozReview Request: Bug 1250568 - Adding ECDHE_PSK suites, r=keeler
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/44175/diff/1-2/
Reporter | ||
Comment 47•9 years ago
|
||
Comment on attachment 8737910 [details]
MozReview Request: Bug 1250568 - Adding TLS 1.3 to nsISSLStatus, r=keeler
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/44177/diff/1-2/
Comment 48•9 years ago
|
||
Comment on attachment 8737907 [details]
MozReview Request: Bug 1250568 - Add support for TLS1.3 in prefs and telemetry, r=bsmedberg,keeler
data-review=me
I'm a little uncomfortable with the "never", so expect me to ping you every 6 months to make sure that you're actually using this data.
Attachment #8737907 -
Flags: review?(benjamin) → feedback+
Reporter | ||
Comment 49•9 years ago
|
||
Challenge accepted. Honestly, I want to remove the whole lot of these. The data we get from this one should hopefully put us in a place to delete a whole bunch of fragile and complicated code that is doing nasty things in the name of interoperability over security (and performance). If we get a year of this and it allows us to remove all 8 telemetry probes and all that code, I think that this will have paid off.
Reporter | ||
Comment 50•9 years ago
|
||
Comment on attachment 8737907 [details]
MozReview Request: Bug 1250568 - Add support for TLS1.3 in prefs and telemetry, r=bsmedberg,keeler
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/44171/diff/2-3/
Attachment #8737907 -
Attachment description: MozReview Request: Bug 1250568 - Add support for TLS1.3 in prefs and telemetry, r?bsmedberg,keeler → MozReview Request: Bug 1250568 - Add support for TLS1.3 in prefs and telemetry, r=bsmedberg,keeler
Attachment #8737908 -
Attachment description: MozReview Request: Bug 1250568 - Enable building of TLS 1.3, r?ekr → MozReview Request: Bug 1250568 - Enable building of TLS 1.3, r=ekr
Attachment #8737909 -
Attachment description: MozReview Request: Bug 1250568 - Adding ECDHE_PSK suites, r?keeler → MozReview Request: Bug 1250568 - Adding ECDHE_PSK suites, r=keeler
Attachment #8737910 -
Attachment description: MozReview Request: Bug 1250568 - Adding TLS 1.3 to nsISSLStatus, r?keeler → MozReview Request: Bug 1250568 - Adding TLS 1.3 to nsISSLStatus, r=keeler
Attachment #8737907 -
Flags: feedback+
Reporter | ||
Comment 51•9 years ago
|
||
Comment on attachment 8737908 [details]
MozReview Request: Bug 1250568 - Enable building of TLS 1.3, r=ekr
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/44173/diff/2-3/
Reporter | ||
Comment 52•9 years ago
|
||
Comment on attachment 8737909 [details]
MozReview Request: Bug 1250568 - Adding ECDHE_PSK suites, r=keeler
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/44175/diff/2-3/
Reporter | ||
Comment 53•9 years ago
|
||
Comment on attachment 8737910 [details]
MozReview Request: Bug 1250568 - Adding TLS 1.3 to nsISSLStatus, r=keeler
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/44177/diff/2-3/
Comment 54•9 years ago
|
||
Comment 55•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/33f8fafd7dcb
https://hg.mozilla.org/mozilla-central/rev/df4fa2bbf8db
https://hg.mozilla.org/mozilla-central/rev/600ad888b875
https://hg.mozilla.org/mozilla-central/rev/d30839422ea9
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Comment 56•9 years ago
|
||
With TLS 1.3 turned on in Nightly, we appear to have 27 broken sites.
Two of them appear superficially as certificate errors (interscope.com, vzlom-igr.com) but that's because this client change prevented a connection to their primary servers, and forced them to connect to alternate servers with bad configs.
To review the report:
* Go to this URL: https://tlscanary.mozilla.org/runs/2016-05-23-09-13-21/
* Click on the heading for the column "error.message" to sort
* Click on the error "ssl_error_bad_cert_domain" - choose "remove all" to eliminate these from view (these are caused by the BR workaround changes in bug 1245280)
List of broken sites attached.
Reporter | ||
Comment 57•9 years ago
|
||
Honestly, I'm surprised that this is so few. Given the small number of sites, maybe we could ask someone to reach out to them.
I see the same problem regarding primary servers on some of those that you filtered out. For example, feelinwow.com responds with a *.shophippo.com certificate. https://bdnog.org/ fails in Nightly for me as well.
Comment 58•9 years ago
|
||
I'm curious why the insecure fallback to TLS 1.2 didn't work on (the most of) those servers. no_cypher_overlap is a fallback reason.
Comment 59•9 years ago
|
||
Hmm, if I load ampli.fr from the command line with an NSPR log, the page loads fine. I could reproduce the no_cypher_overlap error without the log...
Comment 60•9 years ago
|
||
My bad. I launched a different binary from the command line.
Filing a follow-up bug to fix the issue.
Comment 61•9 years ago
|
||
(In reply to Martin Thomson [:mt:] from comment #57)
> Honestly, I'm surprised that this is so few. Given the small number of
> sites, maybe we could ask someone to reach out to them.
>
> I see the same problem regarding primary servers on some of those that you
> filtered out. For example, feelinwow.com responds with a *.shophippo.com
> certificate. https://bdnog.org/ fails in Nightly for me as well.
I just kicked off a canary run on a list of sites taken from the Google CT list - over 2.6 million. That's 10x as big as the sample I used before. I should have results in a few days.
You need to log in
before you can comment on or make changes to this bug.
Description
•