1303284 - "Restore default settings" SSL pref reset button doesn't appear for the "interrupted" case in non-English builds

Status: NEW

(Firefox :: Security, defect, P3)

Description

[:Cykesiopka]

Bug 1252068 added functionality to reset key SSL prefs to their defaults when encountering an error likely caused by a changed pref.
Unfortunately, the feature doesn't fully work on non-English builds.

STR:
1. Download a non-English build such as https://archive.mozilla.org/pub/firefox/nightly/2016/09/2016-09-15-03-04-17-mozilla-central-l10n/firefox-51.0a1.zh-TW.win64.installer.exe
2. In a clean profile, set the "security.tls.version.max" pref to 1.
3. Visit https://brasstacks.mozilla.com/orangefactor/

ER:
1. The "安全連線失敗" ("Secure Connection Failed") page should be encountered.
   The text below should mention "載入頁面時與 brasstacks.mozilla.com 的連線中斷。"
   ("The connection to brasstacks.mozilla.com was interrupted while the page was loading.").
2. There should be a button that's labelled "還原預設設定" ("Restore default settings").

AR:
1. As ER.
2. There is no button.

Comment 1

[:Cykesiopka]

The bug likely arises because the "likely caused by" part is accomplished like this:
https://hg.mozilla.org/mozilla-central/file/8a494adbc5cced90a4edf0c98cffde906bf7f3ae/browser/base/content/aboutNetError.xhtml#l333
> const hasPrefStyleError = [
>   "interrupted", // This happens with subresources that are above the max tls
>   "SSL_ERROR_PROTOCOL_VERSION_ALERT",
>   "SSL_ERROR_UNSUPPORTED_VERSION",
>   "SSL_ERROR_NO_CYPHER_OVERLAP",
>   "SSL_ERROR_NO_CIPHERS_SUPPORTED"
> ].some((substring) => getDescription().includes(substring));
> // If it looks like an error that is user config based
> if (getErrorCode() == "nssFailure2" && hasPrefStyleError && options && options.changedCertPrefs) {
>   showPrefChangeContainer();
> }

While the "SSL_ERROR_*" strings are guaranteed to be non-localised, "interrupted" is not protected by the same guarantee.

Comment 2

[:Gijs (he/him)]

Ugh, I am disappointed with myself for r+'ing that. :-\

That said, it looks like we don't get a detailed NSS error - all we have is nssFailure2 which could be a whole host of things.

Maybe we should add a param to about:neterror/about:certerror that has the more detailed error code that the message corresponds to.

Comment 3

[Erin Lancaster [:elan]]

Platform sec might also be able to fix this ticket.

Comment 4

[Jonathan Kingston [:jkt] he/him]

Happy to look into adding the param, I caused it and I wasn't confident enough at the time to add that.
:Gijs is right this was like this as there wasn't a way of cleanly specifying this.

Comment 5

[Jonathan Kingston [:jkt] he/him]

Attachment: Bug 1303284 - implement a secure page interrupt flag for netError pages to fix i18n issue of reset button

Review commit: https://reviewboard.mozilla.org/r/79892/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/79892/

Comment 6

[Jonathan Kingston [:jkt] he/him]

Comment on attachment 8793117 [details]
Bug 1303284 - implement a secure page interrupt flag for netError pages to fix i18n issue of reset button

Hey Gijs,

Could you review this like last time?

Thanks
Jonathan

Comment 7

[:Gijs (he/him)]

Comment on attachment 8793117 [details]
Bug 1303284 - implement a secure page interrupt flag for netError pages to fix i18n issue of reset button

https://reviewboard.mozilla.org/r/79892/#review79386

I think this should pass the error code to the page instead (ie aError). It can be a number, and we can compare with Components.results in the JS. Then we can also get rid of the substring matching generally, I think. Someone other than me should review the docshell changes though. Maybe :ckerschb can help.