Bug 1310842 - Pref with insecure only connection icons r?johannh draft
authorJonathan Kingston <jkt@mozilla.com>
Tue, 13 Feb 2018 16:11:11 +0000
changeset 787189 171d068f0847dbf0921cd49730ecd6e10b642d23
parent 787142 26e53729a10976f52e75efa44e17b5e054969fec
push id107674
push userbmo:jkt@mozilla.com
push dateTue, 24 Apr 2018 14:08:01 +0000
reviewersjohannh
bugs1310842
milestone61.0a1
Bug 1310842 - Pref with insecure only connection icons r?johannh MozReview-Commit-ID: EiJR5vZxibP
browser/app/profile/firefox.js
browser/base/content/browser-siteIdentity.js
browser/base/content/test/performance/browser_preferences_usage.js
browser/base/content/test/siteIdentity/browser_check_identity_state.js
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -1319,16 +1319,20 @@ pref("sidebar.position_start", true);
 pref("security.mixed_content.block_active_content", true);
 
 // Show degraded UI for http pages with password fields.
 pref("security.insecure_password.ui.enabled", true);
 
 // Show in-content login form warning UI for insecure login fields
 pref("security.insecure_field_warning.contextual.enabled", true);
 
+// Show positive UI for https pages; disabling forces insecure_connection_* true
+// unless insecure pages already have an insecure icon or text pref set.
+pref("security.secure_connection_icon.enabled", true);
+
 // Show degraded UI for http pages; disabled for now
 pref("security.insecure_connection_icon.enabled", false);
 // Show degraded UI for http pages in private mode only for Nightly: Bug 1434626
 #if defined(NIGHTLY_BUILD)
 pref("security.insecure_connection_icon.pbmode.enabled", true);
 #else
 pref("security.insecure_connection_icon.pbmode.enabled", false);
 #endif
--- a/browser/base/content/browser-siteIdentity.js
+++ b/browser/base/content/browser-siteIdentity.js
@@ -1,14 +1,37 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 
 /**
+ * Provides an object for lazily loading prefs values.
+ * Observes the prefs after the first access to provide a quick update.
+ * eg:
+ * let prefs = setupPrefObserver({
+ *   secureIcon: "security.secure_connection_icon.enabled"
+ * });
+ * console.log("Has secure icon", prefs.secureIcon);
+ *
+ * @param prefNames {Object}
+ *        The keys are used to access the pref values and values that are preference names.
+ *
+ * @return object which has getters for the prefs
+ */
+function setupPrefObserver(prefNames) {
+  let prefObserver = {
+  };
+  for (let propName in prefNames) {
+    XPCOMUtils.defineLazyPreferenceGetter(prefObserver, propName, prefNames[propName]);
+  }
+  return prefObserver;
+}
+
+/**
  * Utility object to handle manipulations of the identity indicators in the UI
  */
 var gIdentityHandler = {
   /**
    * nsIURI for which the identity UI is displayed. This has been already
    * processed by nsIURIFixup.createExposableURI.
    */
   _uri: null,
@@ -52,16 +75,25 @@ var gIdentityHandler = {
   _popupTriggeredByKeyboard: false,
 
   /**
    * RegExp used to decide if an about url should be shown as being part of
    * the browser UI.
    */
   _secureInternalUIWhitelist: /^(?:accounts|addons|cache|config|crashes|customizing|downloads|healthreport|license|permissions|preferences|rights|searchreset|sessionrestore|support|welcomeback)(?:[?#]|$)/i,
 
+  _prefs: setupPrefObserver({
+    secureIcon: "security.secure_connection_icon.enabled",
+    insecureIcon: "security.insecure_connection_icon.enabled",
+    insecureIconPbmode: "security.insecure_connection_icon.pbmode.enabled",
+    insecureText: "security.insecure_connection_text.enabled",
+    insecureTextPbmode: "security.insecure_connection_text.pbmode.enabled",
+    insecurePassword: "security.insecure_password.ui.enabled"
+  }),
+
   get _isBroken() {
     return this._state & Ci.nsIWebProgressListener.STATE_IS_BROKEN;
   },
 
   get _isSecure() {
     // If a <browser> is included within a chrome document, then this._state
     // will refer to the security state for the <browser> and not the top level
     // document. In this case, don't upgrade the security state in the UI
@@ -96,17 +128,17 @@ var gIdentityHandler = {
   get _isCertDistrustImminent() {
     return this._state & Ci.nsIWebProgressListener.STATE_CERT_DISTRUST_IMMINENT;
   },
 
   get _hasInsecureLoginForms() {
     // checks if the page has been flagged for an insecure login. Also checks
     // if the pref to degrade the UI is set to true
     return LoginManagerParent.hasInsecureLoginForms(gBrowser.selectedBrowser) &&
-           Services.prefs.getBoolPref("security.insecure_password.ui.enabled");
+           this._prefs.insecurePassword;
   },
 
   // smart getters
   get _identityPopup() {
     delete this._identityPopup;
     return this._identityPopup = document.getElementById("identity-popup");
   },
   get _identityBox() {
@@ -407,33 +439,107 @@ var gIdentityHandler = {
     }
     if (this._uriHasHost && this._isSecure) {
       return "verifiedDomain";
     }
     return "unknownIdentity";
   },
 
   /**
+   * Should show warning when pref matches.
+   * Checks pbmode variant when the window is private.
+   *
+   * @param prefPrefix
+   *        A string that is either "insecureIcon" or "insecureText"
+   * @return boolean
+   */
+  _warnOnInsecure(prefPrefix) {
+    return this._prefs[prefPrefix] ||
+           (this._prefs[`${prefPrefix}Pbmode`] &&
+           PrivateBrowsingUtils.isWindowPrivate(window));
+  },
+
+  /**
+   * Should show icon (strike through lock) warning for insecure pages.
+   *
+   * @return boolean
+   */
+  get _shouldShowInsecureIcon() {
+    return this._warnOnInsecure("insecureIcon") ||
+           // If we don't have secure indicators and no other insecure indicator
+           (!this._prefs.secureIcon && !this._warnOnInsecure("insecureText"));
+  },
+
+  /**
+   * Should show text warning for insecure pages.
+   *
+   * @return boolean
+   */
+  get _shouldShowInsecureText() {
+    return this._warnOnInsecure("insecureText") ||
+           // If we don't have secure indicators and no other insecure indicator
+           (!this._prefs.secureIcon && !this._warnOnInsecure("insecureIcon"));
+  },
+
+  /**
+   * Treat identity block user interface as an insecure page.
+   * This is separated out from refreshIdentityBlock due to there being multiple cases
+   * where this would be called.
+   * This should represent pages that should only ever be considered insecure:
+   *   HTTP Pages, Cert overrides and net/cert errors
+   *
+   * @return string of icon_label
+   */
+  _showInsecureIdentityBlock() {
+    let icon_label = "";
+    this._identityBox.className = this._shouldShowInsecureIcon ? "notSecure" : "unknownIdentity";
+
+    if (this._shouldShowInsecureText) {
+      icon_label = gNavigatorBundle.getString("identity.notSecure.label");
+      this._identityBox.classList.add("notSecureText");
+    }
+
+    if (this._isBroken) {
+      if (this._isMixedActiveContentLoaded) {
+        this._identityBox.classList.add("mixedActiveContent");
+      } else if (this._isMixedActiveContentBlocked) {
+        this._identityBox.classList.add("mixedDisplayContentLoadedActiveBlocked");
+      } else if (this._isMixedPassiveContentLoaded) {
+        this._identityBox.classList.add("mixedDisplayContent");
+      } else {
+        this._identityBox.classList.add("weakCipher");
+      }
+    }
+
+    if (this._hasInsecureLoginForms) {
+      // Insecure login forms can only be present on "unknown identity"
+      // pages, either already insecure or with mixed active content loaded.
+      this._identityBox.classList.add("insecureLoginForms");
+    }
+    return icon_label;
+  },
+
+  /**
    * Updates the identity block user interface with the data from this object.
    */
   refreshIdentityBlock() {
     if (!this._identityBox) {
       return;
     }
 
     let icon_label = "";
     let tooltip = "";
     let icon_country_label = "";
     let icon_labels_dir = "ltr";
 
     if (this._isSecureInternalUI) {
       this._identityBox.className = "chromeUI";
       let brandBundle = document.getElementById("bundle_brand");
       icon_label = brandBundle.getString("brandShorterName");
-    } else if (this._uriHasHost && this._isEV) {
+    } else if (this._uriHasHost && this._isEV && this._prefs.secureIcon) {
       this._identityBox.className = "verifiedIdentity";
       if (this._isMixedActiveContentBlocked) {
         this._identityBox.classList.add("mixedActiveBlocked");
       }
 
       if (!this._isCertUserOverridden) {
         // If it's identified, then we can populate the dialog with credentials
         let iData = this.getIdentityData();
@@ -453,71 +559,53 @@ var gIdentityHandler = {
                           "rtl" : "ltr";
       }
     } else if (this._pageExtensionPolicy) {
       this._identityBox.className = "extensionPage";
       let extensionName = this._pageExtensionPolicy.name;
       icon_label = gNavigatorBundle.getFormattedString(
         "identity.extension.label", [extensionName]);
     } else if (this._uriHasHost && this._isSecure) {
-      this._identityBox.className = "verifiedDomain";
-      if (this._isMixedActiveContentBlocked) {
-        this._identityBox.classList.add("mixedActiveBlocked");
-      }
-      if (!this._isCertUserOverridden) {
-        // It's a normal cert, verifier is the CA Org.
-        tooltip = gNavigatorBundle.getFormattedString("identity.identified.verifier",
-                                                      [this.getIdentityData().caOrg]);
+      this._identityBox.className = this._prefs.secureIcon ? "verifiedDomain" : "unknownIdentity";
+      if (this._prefs.secureIcon) {
+        this._identityBox.className = "verifiedDomain";
+        if (this._isMixedActiveContentBlocked) {
+          this._identityBox.classList.add("mixedActiveBlocked");
+        }
+        if (!this._isCertUserOverridden) {
+          // It's a normal cert, verifier is the CA Org.
+          tooltip = gNavigatorBundle.getFormattedString("identity.identified.verifier",
+                                                        [this.getIdentityData().caOrg]);
+        }
+      } else {
+        this._identityBox.className = "unknownIdentity";
       }
     } else if (!this._uriHasHost) {
       this._identityBox.className = "unknownIdentity";
     } else if (gBrowser.selectedBrowser.documentURI &&
                (gBrowser.selectedBrowser.documentURI.scheme == "about" ||
                gBrowser.selectedBrowser.documentURI.scheme == "chrome")) {
-        // For net errors we should not show notSecure as it's likely confusing
-      this._identityBox.className = "unknownIdentity";
-    } else {
-      if (this._isBroken) {
-        this._identityBox.className = "unknownIdentity";
-
-        if (this._isMixedActiveContentLoaded) {
-          this._identityBox.classList.add("mixedActiveContent");
-        } else if (this._isMixedActiveContentBlocked) {
-          this._identityBox.classList.add("mixedDisplayContentLoadedActiveBlocked");
-        } else if (this._isMixedPassiveContentLoaded) {
-          this._identityBox.classList.add("mixedDisplayContent");
-        } else {
-          this._identityBox.classList.add("weakCipher");
-        }
+      // For net errors we should not show notSecure as it's likely confusing
+      if (gBrowser.selectedBrowser.documentURI.spec.startsWith("about:certerror")) {
+        icon_label = this._showInsecureIdentityBlock();
       } else {
-        let warnOnInsecure = Services.prefs.getBoolPref("security.insecure_connection_icon.enabled") ||
-                             (Services.prefs.getBoolPref("security.insecure_connection_icon.pbmode.enabled") &&
-                             PrivateBrowsingUtils.isWindowPrivate(window));
-        let className = warnOnInsecure ? "notSecure" : "unknownIdentity";
-        this._identityBox.className = className;
-
-        let warnTextOnInsecure = Services.prefs.getBoolPref("security.insecure_connection_text.enabled") ||
-                                 (Services.prefs.getBoolPref("security.insecure_connection_text.pbmode.enabled") &&
-                                 PrivateBrowsingUtils.isWindowPrivate(window));
-        if (warnTextOnInsecure) {
-          icon_label = gNavigatorBundle.getString("identity.notSecure.label");
-          this._identityBox.classList.add("notSecureText");
-        }
+        this._identityBox.className = "unknownIdentity";
       }
-      if (this._hasInsecureLoginForms) {
-        // Insecure login forms can only be present on "unknown identity"
-        // pages, either already insecure or with mixed active content loaded.
-        this._identityBox.classList.add("insecureLoginForms");
-      }
+    } else {
+      icon_label = this._showInsecureIdentityBlock();
     }
 
     if (this._isCertUserOverridden) {
-      this._identityBox.classList.add("certUserOverridden");
-      // Cert is trusted because of a security exception, verifier is a special string.
-      tooltip = gNavigatorBundle.getString("identity.identified.verified_by_you");
+      if (!this._prefs.secureIcon) {
+        icon_label = this._showInsecureIdentityBlock();
+      } else {
+        this._identityBox.classList.add("certUserOverridden");
+        // Cert is trusted because of a security exception, verifier is a special string.
+        tooltip = gNavigatorBundle.getString("identity.identified.verified_by_you");
+      }
     }
 
     let permissionAnchors = this._permissionAnchors;
 
     // hide all permission icons
     for (let icon of Object.values(permissionAnchors)) {
       icon.removeAttribute("showing");
     }
@@ -682,16 +770,18 @@ var gIdentityHandler = {
     if (this._isSecure || this._isCertUserOverridden) {
       verifier = this._identityIconLabels.tooltipText;
     }
 
     // Fill in organization information if we have a valid EV certificate.
     if (this._isEV) {
       let iData = this.getIdentityData();
       host = owner = iData.subjectOrg;
+      if (iData.country && !this._prefs.secureIcon)
+        host += " (" + iData.country + ")";
       verifier = this._identityIconLabels.tooltipText;
 
       // Build an appropriate supplemental block out of whatever location data we have
       if (iData.city)
         supplemental += iData.city + "\n";
       if (iData.state && iData.country)
         supplemental += gNavigatorBundle.getFormattedString("identity.identified.state_and_country",
                                                             [iData.state, iData.country]);
--- a/browser/base/content/test/performance/browser_preferences_usage.js
+++ b/browser/base/content/test/performance/browser_preferences_usage.js
@@ -114,32 +114,16 @@ add_task(async function open_10_tabs() {
   let whitelist = {
     "layout.css.dpi": {
       max: 35,
     },
     "browser.zoom.full": {
       min: 10,
       max: 25,
     },
-    "security.insecure_connection_icon.pbmode.enabled": {
-      min: 17,
-      max: 25,
-    },
-    "security.insecure_connection_icon.enabled": {
-      min: 17,
-      max: 25,
-    },
-    "security.insecure_connection_text.enabled": {
-      min: 17,
-      max: 25,
-    },
-    "security.insecure_connection_text.pbmode.enabled": {
-      min: 17,
-      max: 25,
-    },
     "dom.ipc.processCount": {
       min: 10,
       max: 15,
     },
     "media.autoplay.enabled": {
       min: 10,
       max: 30,
     },
--- a/browser/base/content/test/siteIdentity/browser_check_identity_state.js
+++ b/browser/base/content/test/siteIdentity/browser_check_identity_state.js
@@ -1,15 +1,16 @@
 /*
  * Test the identity mode UI for a variety of page types
  */
 
 "use strict";
 
 const DUMMY = "browser/browser/base/content/test/siteIdentity/dummy_page.html";
+const SECURE_ICON_PREF = "security.secure_connection_icon.enabled";
 const INSECURE_ICON_PREF = "security.insecure_connection_icon.enabled";
 const INSECURE_TEXT_PREF = "security.insecure_connection_text.enabled";
 const INSECURE_PBMODE_ICON_PREF = "security.insecure_connection_icon.pbmode.enabled";
 
 function loadNewTab(url) {
   return BrowserTestUtils.openNewForegroundTab(gBrowser, url, true);
 }
 
@@ -137,37 +138,50 @@ async function blankPageTest(secureCheck
   await SpecialPowers.popPrefEnv();
 }
 
 add_task(async function test_blank() {
   await blankPageTest(true);
   await blankPageTest(false);
 });
 
-async function secureTest(secureCheck) {
+async function secureTest(secureCheck, secureEnabledCheck) {
   let oldTab = gBrowser.selectedTab;
-  await SpecialPowers.pushPrefEnv({set: [[INSECURE_ICON_PREF, secureCheck]]});
+  await SpecialPowers.pushPrefEnv({set: [
+    [INSECURE_ICON_PREF, secureCheck],
+    [SECURE_ICON_PREF, secureEnabledCheck],
+  ]});
 
   let newTab = await loadNewTab("https://example.com/" + DUMMY);
-  is(getIdentityMode(), "verifiedDomain", "Identity should be verified");
+  if (secureEnabledCheck) {
+    is(getIdentityMode(), "verifiedDomain", "Identity should be verified");
+  } else {
+    is(getIdentityMode(), "unknownIdentity", "Identity should be unknown");
+  }
 
   gBrowser.selectedTab = oldTab;
   is(getIdentityMode(), "unknownIdentity", "Identity should be unknown");
 
   gBrowser.selectedTab = newTab;
-  is(getIdentityMode(), "verifiedDomain", "Identity should be verified");
+  if (secureEnabledCheck) {
+    is(getIdentityMode(), "verifiedDomain", "Identity should be verified");
+  } else {
+    is(getIdentityMode(), "unknownIdentity", "Identity should be unknown");
+  }
 
   gBrowser.removeTab(newTab);
 
   await SpecialPowers.popPrefEnv();
 }
 
-add_task(async function test_secure_enabled() {
-  await secureTest(true);
-  await secureTest(false);
+add_task(async function test_secure_disabled() {
+  await secureTest(true, true);
+  await secureTest(false, true);
+  await secureTest(true, false);
+  await secureTest(false, false);
 });
 
 async function insecureTest(secureCheck) {
   let oldTab = gBrowser.selectedTab;
   await SpecialPowers.pushPrefEnv({set: [[INSECURE_ICON_PREF, secureCheck]]});
 
   let newTab = await loadNewTab("http://example.com/" + DUMMY);
   if (secureCheck) {
@@ -263,43 +277,60 @@ async function resourceUriTest(secureChe
   await SpecialPowers.popPrefEnv();
 }
 
 add_task(async function test_resource_uri() {
   await resourceUriTest(true);
   await resourceUriTest(false);
 });
 
-async function noCertErrorTest(secureCheck) {
+async function noCertErrorTest(secureCheck, secureEnabledCheck) {
   let oldTab = gBrowser.selectedTab;
-  await SpecialPowers.pushPrefEnv({set: [[INSECURE_ICON_PREF, secureCheck]]});
+  await SpecialPowers.pushPrefEnv({set: [
+    [INSECURE_ICON_PREF, secureCheck],
+    [SECURE_ICON_PREF, secureEnabledCheck],
+  ]});
   let newTab = BrowserTestUtils.addTab(gBrowser);
   gBrowser.selectedTab = newTab;
 
   let promise = BrowserTestUtils.waitForErrorPage(gBrowser.selectedBrowser);
   gBrowser.loadURI("https://nocert.example.com/");
   await promise;
-  is(getIdentityMode(), "unknownIdentity", "Identity should be unknown");
+  if (!secureEnabledCheck && !secureCheck) {
+    is(getIdentityMode(), "notSecure notSecureText", "Identity should be not secure");
+  } else if (secureCheck) {
+    is(getIdentityMode(), "notSecure", "Identity should be not secure");
+  } else {
+    is(getIdentityMode(), "unknownIdentity", "Identity should be unknown");
+  }
   is(getConnectionState(), "not-secure", "Connection should be file");
 
   gBrowser.selectedTab = oldTab;
   is(getIdentityMode(), "unknownIdentity", "Identity should be unknown");
 
   gBrowser.selectedTab = newTab;
-  is(getIdentityMode(), "unknownIdentity", "Identity should be unknown");
+  if (!secureEnabledCheck && !secureCheck) {
+    is(getIdentityMode(), "notSecure notSecureText", "Identity should be not secure");
+  } else if (secureCheck) {
+    is(getIdentityMode(), "notSecure", "Identity should be not secure");
+  } else {
+    is(getIdentityMode(), "unknownIdentity", "Identity should be unknown");
+  }
   is(getConnectionState(), "not-secure", "Connection should be file");
 
   gBrowser.removeTab(newTab);
 
   await SpecialPowers.popPrefEnv();
 }
 
 add_task(async function test_about_net_error_uri() {
-  await noCertErrorTest(true);
-  await noCertErrorTest(false);
+  await noCertErrorTest(true, true);
+  await noCertErrorTest(false, true);
+  await noCertErrorTest(true, false);
+  await noCertErrorTest(false, false);
 });
 
 async function noCertErrorFromNavigationTest(secureCheck) {
   await SpecialPowers.pushPrefEnv({set: [[INSECURE_ICON_PREF, secureCheck]]});
   let newTab = await loadNewTab("http://example.com/" + DUMMY);
 
   let promise = BrowserTestUtils.waitForErrorPage(gBrowser.selectedBrowser);
   await ContentTask.spawn(gBrowser.selectedBrowser, {}, function() {
@@ -307,17 +338,21 @@ async function noCertErrorFromNavigation
   });
   await promise;
   await ContentTask.spawn(gBrowser.selectedBrowser, {}, function() {
     is(content.window.location.href, "https://nocert.example.com/", "Should be the cert error URL");
   });
 
 
   is(newTab.linkedBrowser.documentURI.spec.startsWith("about:certerror?"), true, "Should be an about:certerror");
-  is(getIdentityMode(), "unknownIdentity", "Identity should be unknown");
+  if (secureCheck) {
+    is(getIdentityMode(), "notSecure", "Identity should be not secure");
+  } else {
+    is(getIdentityMode(), "unknownIdentity", "Identity should be unknown");
+  }
   is(getConnectionState(), "not-secure", "Connection should be file");
 
   gBrowser.removeTab(newTab);
 
   await SpecialPowers.popPrefEnv();
 }
 
 add_task(async function test_about_net_error_uri_from_navigation_tab() {