I didn't implement this in bug 1202489.

Ah, bug 1207137.

Probably I have no time to implement this before the next merge. Let's land strings first so that we can uplift later.

Missed the train, but we can just remove the link.

Comment on attachment 8679989 [details] [diff] [review] String changes for infobar Approval Request Comment [Feature/regressing bug #]: N/A [User impact if declined]: Users will not be warned enough when sites are using weak crypto. [Describe test coverage new/current, TreeHerder]: Build locally, string change only. [Risks and why]: Very low, string change only. [String/UUID change made/needed]: string change (only) missed due to the merge day shift.

Similar question as in bug 1219109 comment 9: What's the ETA for the actual patch, and what's the risk for the actual patch using the strings here?

Comment on attachment 8679989 [details] [diff] [review] String changes for infobar Canceling the uplift request until the string is finalized.

Backout: https://hg.mozilla.org/integration/fx-team/rev/5db171f726b2

This patch includes string changes.

Comment on attachment 8682282 [details] [diff] [review] Show infobar when RC4 override is enabled Review of attachment 8682282 [details] [diff] [review]: ----------------------------------------------------------------- I don't think I'm the best reviewer for this, one note though ::: browser/base/content/browser.js @@ +7286,5 @@ > this._identityIconLabel.parentNode.style.direction = icon_labels_dir; > // Hide completely if the organization label is empty > this._identityIconLabel.parentNode.collapsed = icon_label ? false : true; > }, > + Nit: trailing whitespace @@ +7293,5 @@ > + */ > + showWeakCryptoInfoBar() { > + if (!this._uriHasHost || !this._isBroken || !this._sslStatus.cipherName || > + this._sslStatus.cipherName.indexOf("_RC4_") < 0) { > + return; Should an existing notification get removed if a security change happens that doesn't fit this criteria (i.e. we've navigated to a new page that doesn't need to notification)?

Comment on attachment 8682282 [details] [diff] [review] Show infobar when RC4 override is enabled Review of attachment 8682282 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/base/content/browser.js @@ +7322,5 @@ > + callback: function (aNotification, aButton) { > + try { > + let weakCryptoOverride = Cc["@mozilla.org/security/weakcryptooverride;1"] > + .getService(Ci.nsIWeakCryptoOverride); > + Cu.reportError(host + ":" + port); Is this reporting necessary? I already see the more elaborate error message in the Browser Console from TransportSecurityInfo and this doesn't add any more value. @@ +7325,5 @@ > + .getService(Ci.nsIWeakCryptoOverride); > + Cu.reportError(host + ":" + port); > + weakCryptoOverride.removeWeakCryptoOverride(host, port, > + PrivateBrowsingUtils.isBrowserPrivate(gBrowser.selectedBrowser)); > + BrowserReloadWithFlags(nsIWebNavigation.LOAD_FLAGS_BYPASS_CACHE); This callback causes an ssl_error_illegal_parameter_alert error to appear first and then I have to click "Try Again" to actually visit the page with the override properly removed. Can we tweak the reload action somehow to avoid this? @@ +7333,5 @@ > + } > + }]; > + > + const priority = notificationBox.PRIORITY_WARNING_MEDIUM; > + notificationBox.appendNotification(message, "popup-blocked", null, You meant to use "weak-crypto" here, not "popup-blocked".

(In reply to Panos Astithas [:past] from comment #11) > ::: browser/base/content/browser.js > @@ +7322,5 @@ > > + callback: function (aNotification, aButton) { > > + try { > > + let weakCryptoOverride = Cc["@mozilla.org/security/weakcryptooverride;1"] > > + .getService(Ci.nsIWeakCryptoOverride); > > + Cu.reportError(host + ":" + port); > > Is this reporting necessary? I already see the more elaborate error message > in the Browser Console from TransportSecurityInfo and this doesn't add any > more value. Vestigial debug output. Removed. > @@ +7325,5 @@ > > + .getService(Ci.nsIWeakCryptoOverride); > > + Cu.reportError(host + ":" + port); > > + weakCryptoOverride.removeWeakCryptoOverride(host, port, > > + PrivateBrowsingUtils.isBrowserPrivate(gBrowser.selectedBrowser)); > > + BrowserReloadWithFlags(nsIWebNavigation.LOAD_FLAGS_BYPASS_CACHE); > > This callback causes an ssl_error_illegal_parameter_alert error to appear > first and then I have to click "Try Again" to actually visit the page with > the override properly removed. Can we tweak the reload action somehow to > avoid this? Oops, I forgot to attach a PSM change. No workaround is necessary with the PSM change. > @@ +7333,5 @@ > > + } > > + }]; > > + > > + const priority = notificationBox.PRIORITY_WARNING_MEDIUM; > > + notificationBox.appendNotification(message, "popup-blocked", null, > > You meant to use "weak-crypto" here, not "popup-blocked". Fixed.

Comment on attachment 8685385 [details] [diff] [review] Show infobar when RC4 override is enabled, v2 Review of attachment 8685385 [details] [diff] [review]: ----------------------------------------------------------------- This looks great, thanks! ::: browser/base/content/browser.js @@ +7286,5 @@ > this._identityIconLabel.parentNode.style.direction = icon_labels_dir; > // Hide completely if the organization label is empty > this._identityIconLabel.parentNode.collapsed = icon_label ? false : true; > }, > + Please remove this trailing whitespace.

Comment on attachment 8685384 [details] [diff] [review] Clear the session cache when a weak crypto override is revoked Review of attachment 8685384 [details] [diff] [review]: ----------------------------------------------------------------- ::: security/manager/ssl/WeakCryptoOverride.cpp @@ +59,5 @@ > const nsPromiseFlatCString& host = PromiseFlatCString(aHostName); > sharedState->IOLayerHelpers().removeInsecureFallbackSite(host, aPort); > > + // SharedSSLState is available only if NSS is initialized. > + MOZ_ASSERT(NSS_IsInitialized()); This seems unnecessary.

https://hg.mozilla.org/integration/fx-team/rev/fc4a5bfc904e3fe1e35804de8663e5bb26aad1b4 Bug 1219088 - Clear the session cache when a weak crypto override is revoked. r=keeler https://hg.mozilla.org/integration/fx-team/rev/52dc9d73dcd3fc033ea01137377c82fd1240a4f9 Bug 1219088 - Show infobar when RC4 override is enabled. r=past

https://hg.mozilla.org/mozilla-central/rev/fc4a5bfc904e https://hg.mozilla.org/mozilla-central/rev/52dc9d73dcd3

I tested this on https://rc4.badssl.com/. 1. While the notification bar is displayed, reloading the page makes it disappear. 2. Even if I dismiss the notification bar, it will still appear if I load the page in a new tab. Are these expected?

No, filing a follow-up bug. (But I couldn't reproduce the latter.)

Marking as verified fixed on FF 45.0a1 (2015-11-16) Win 7, Ubuntu 14.04, OS X 10.11, considering the follow-up bug 1225299.