Bug 1253771 - Add previous state info to mixed content callback. r=tanvi r=florian

MozReview-Commit-ID: 5msNz97psok

--HG--
extra : rebase_source : 640f86c3cc0b9b5a842a0c104cb269915b727b4b
extra : histedit_source : 6ca75dac152d5135089f9053eb91440058b124e4

browser/base/content/browser.js

@@ -6848,33 +6848,31 @@ var gIdentityHandler = {
         this._identityBox.classList.add("mixedActiveBlocked");
       }
 
-      // If it's identified, then we can populate the dialog with credentials
+      if (!this._isCertUserOverridden) {
-      let iData = this.getIdentityData();
+        // If it's identified, then we can populate the dialog with credentials
-      tooltip = gNavigatorBundle.getFormattedString("identity.identified.verifier",
+        let iData = this.getIdentityData();
-                                                    [iData.caOrg]);
+        tooltip = gNavigatorBundle.getFormattedString("identity.identified.verifier",
-      icon_label = iData.subjectOrg;
+                                                      [iData.caOrg]);
-      if (iData.country)
+        icon_label = iData.subjectOrg;
-        icon_country_label = "(" + iData.country + ")";
+        if (iData.country)
+          icon_country_label = "(" + iData.country + ")";
 
-      // If the organization name starts with an RTL character, then
+        // If the organization name starts with an RTL character, then
-      // swap the positions of the organization and country code labels.
+        // swap the positions of the organization and country code labels.
-      // The Unicode ranges reflect the definition of the UCS2_CHAR_IS_BIDI
+        // The Unicode ranges reflect the definition of the UCS2_CHAR_IS_BIDI
-      // macro in intl/unicharutil/util/nsBidiUtils.h. When bug 218823 gets
+        // macro in intl/unicharutil/util/nsBidiUtils.h. When bug 218823 gets
-      // fixed, this test should be replaced by one adhering to the
+        // fixed, this test should be replaced by one adhering to the
-      // Unicode Bidirectional Algorithm proper (at the paragraph level).
+        // Unicode Bidirectional Algorithm proper (at the paragraph level).
-      icon_labels_dir = /^[\u0590-\u08ff\ufb1d-\ufdff\ufe70-\ufefc]/.test(icon_label) ?
+        icon_labels_dir = /^[\u0590-\u08ff\ufb1d-\ufdff\ufe70-\ufefc]/.test(icon_label) ?
-                        "rtl" : "ltr";
+                          "rtl" : "ltr";
+      }
 
     } else if (this._uriHasHost && this._isSecure) {
       this._identityBox.className = "verifiedDomain";
       if (this._isMixedActiveContentBlocked) {
         this._identityBox.classList.add("mixedActiveBlocked");
       }
-      if (this._isCertUserOverridden) {
+      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");
-      } else {
         // It's a normal cert, verifier is the CA Org.
         tooltip = gNavigatorBundle.getFormattedString("identity.identified.verifier",
                                                       [this.getIdentityData().caOrg]);
@@ -6900,6 +6898,12 @@ var gIdentityHandler = {
       tooltip = gNavigatorBundle.getString("identity.unknown.tooltip");
     }
 
+    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 (SitePermissions.hasGrantedPermissions(this._uri)) {
       this._identityBox.classList.add("grantedPermissions");
     }
@@ -7063,7 +7067,7 @@ var gIdentityHandler = {
     }
 
     // Fill in the CA name if we have a valid TLS certificate.
-    if (this._isSecure) {
+    if (this._isSecure || this._isCertUserOverridden) {
       verifier = this._identityBox.tooltipText;
     }
 

browser/base/content/test/general/browser.ini

@@ -328,6 +328,7 @@ skip-if = os != "win" # The Fitts Law menu button is only supported on Windows (
 subsuite = clipboard
 [browser_minimize.js]
 [browser_misused_characters_in_strings.js]
+[browser_mixed_content_cert_override.js]
 [browser_mixedcontent_securityflags.js]
 tags = mcb
 [browser_offlineQuotaNotification.js]

browser/base/content/test/general/browser_addCertException.js

@@ -9,49 +9,15 @@
 // using the button contained therein to load the certificate exception
 // dialog, using that to add an exception, and finally successfully visiting
 // the site, including showing the right identity box and control center icons.
-function test() {
+add_task(function* () {
-  waitForExplicitFinish();
+  yield BrowserTestUtils.openNewForegroundTab(gBrowser);
-  whenNewTabLoaded(window, loadBadCertPage);
+  yield loadBadCertPage("https://expired.example.com");
-}
-
-// Attempt to load https://expired.example.com (which has an expired cert).
-function loadBadCertPage() {
-  Services.obs.addObserver(certExceptionDialogObserver,
-                           "cert-exception-ui-ready", false);
-  let startedLoad = BrowserTestUtils.loadURI(gBrowser.selectedBrowser,
-                                             "https://expired.example.com");
-  startedLoad.then(() => promiseErrorPageLoaded(gBrowser.selectedBrowser)).then(function() {
-    ContentTask.spawn(gBrowser.selectedBrowser, null, function*() {
-      content.document.getElementById("exceptionDialogButton").click();
-    });
-  });
-}
-
-// When the certificate exception dialog has opened, click the button to add
-// an exception.
-const EXCEPTION_DIALOG_URI = "chrome://pippki/content/exceptionDialog.xul";
-var certExceptionDialogObserver = {
-  observe: function(aSubject, aTopic, aData) {
-    if (aTopic == "cert-exception-ui-ready") {
-      Services.obs.removeObserver(this, "cert-exception-ui-ready");
-      let certExceptionDialog = getDialog(EXCEPTION_DIALOG_URI);
-      ok(certExceptionDialog, "found exception dialog");
-      executeSoon(function() {
-        BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser).then(realPageLoaded);
-        certExceptionDialog.documentElement.getButton("extra1").click();
-      });
-    }
-  }
-};
-
-// Finally, we should successfully load https://expired.example.com.
-function realPageLoaded() {
   checkControlPanelIcons();
   let certOverrideService = Cc["@mozilla.org/security/certoverride;1"]
                               .getService(Ci.nsICertOverrideService);
   certOverrideService.clearValidityOverride("expired.example.com", -1);
-  BrowserTestUtils.removeTab(gBrowser.selectedTab).then(finish);
+  yield BrowserTestUtils.removeTab(gBrowser.selectedTab);
-};
+});
 
 // Check for the correct icons in the identity box and control center.
 function checkControlPanelIcons() {
@@ -82,30 +48,3 @@ function checkControlPanelIcons() {
   gIdentityHandler._identityPopup.hidden = true;
 }
 
-// Utility function to get a handle on the certificate exception dialog.
-// Modified from toolkit/components/passwordmgr/test/prompt_common.js
-function getDialog(aLocation) {
-  let wm = Cc["@mozilla.org/appshell/window-mediator;1"]
-             .getService(Ci.nsIWindowMediator);
-  let enumerator = wm.getXULWindowEnumerator(null);
-
-  while (enumerator.hasMoreElements()) {
-    let win = enumerator.getNext();
-    let windowDocShell = win.QueryInterface(Ci.nsIXULWindow).docShell;
-
-    let containedDocShells = windowDocShell.getDocShellEnumerator(
-                                      Ci.nsIDocShellTreeItem.typeChrome,
-                                      Ci.nsIDocShell.ENUMERATE_FORWARDS);
-    while (containedDocShells.hasMoreElements()) {
-      // Get the corresponding document for this docshell
-      let childDocShell = containedDocShells.getNext();
-      let childDoc = childDocShell.QueryInterface(Ci.nsIDocShell)
-                                  .contentViewer
-                                  .DOMDocument;
-
-      if (childDoc.location.href == aLocation) {
-        return childDoc;
-      }
-    }
-  }
-}

browser/base/content/test/general/browser_mixed_content_cert_override.js

@@ -0,0 +1,51 @@
+/*
+ * Bug 1253771 - check mixed content blocking in combination with overriden certificates
+ */
+
+"use strict";
+
+const MIXED_CONTENT_URL = "https://self-signed.example.com/browser/browser/base/content/test/general/test-mixedcontent-securityerrors.html";
+
+function getConnectionState() {
+  return document.getElementById("identity-popup").getAttribute("connection");
+}
+
+function getPopupContentVerifier() {
+  return document.getElementById("identity-popup-content-verifier");
+}
+
+function getConnectionIcon() {
+  return window.getComputedStyle(document.getElementById("connection-icon")).listStyleImage;
+}
+
+function checkIdentityPopup(icon) {
+  gIdentityHandler.refreshIdentityPopup();
+  is(getConnectionIcon(), `url("chrome://browser/skin/${icon}.svg")`);
+  is(getConnectionState(), "secure-cert-user-overridden");
+  isnot(getPopupContentVerifier().style.display, "none", "Overridden certificate warning is shown");
+  ok(getPopupContentVerifier().textContent.includes("security exception"), "Text shows overridden certificate warning.");
+}
+
+add_task(function* () {
+
+  // check that a warning is shown when loading a page with mixed content and an overridden certificate
+  yield loadBadCertPage(MIXED_CONTENT_URL);
+  checkIdentityPopup("identity-mixed-passive-loaded");
+
+  // check that the crossed out icon is shown when disabling mixed content protection
+  gIdentityHandler.disableMixedContentProtection();
+  yield BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser);
+
+  checkIdentityPopup("identity-mixed-active-loaded");
+
+  // check that a warning is shown even without mixed content
+  yield BrowserTestUtils.loadURI(gBrowser.selectedBrowser, "https://self-signed.example.com");
+  yield BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser);
+  checkIdentityPopup("identity-mixed-passive-loaded");
+
+  // remove cert exception
+  let certOverrideService = Cc["@mozilla.org/security/certoverride;1"]
+                              .getService(Ci.nsICertOverrideService);
+  certOverrideService.clearValidityOverride("self-signed.example.com", -1);
+});
+

browser/base/content/test/general/head.js

@@ -1100,3 +1100,62 @@ function promiseErrorPageLoaded(browser) {
     }, false, true);
   });
 }
+
+function* loadBadCertPage(url) {
+  const EXCEPTION_DIALOG_URI = "chrome://pippki/content/exceptionDialog.xul";
+  let exceptionDialogResolved = new Promise(function(resolve) {
+    // When the certificate exception dialog has opened, click the button to add
+    // an exception.
+    let certExceptionDialogObserver = {
+      observe: function(aSubject, aTopic, aData) {
+        if (aTopic == "cert-exception-ui-ready") {
+          Services.obs.removeObserver(this, "cert-exception-ui-ready");
+          let certExceptionDialog = getCertExceptionDialog(EXCEPTION_DIALOG_URI);
+          ok(certExceptionDialog, "found exception dialog");
+          executeSoon(function() {
+            certExceptionDialog.documentElement.getButton("extra1").click();
+            resolve();
+          });
+        }
+      }
+    };
+
+    Services.obs.addObserver(certExceptionDialogObserver,
+                             "cert-exception-ui-ready", false);
+  });
+
+  yield BrowserTestUtils.loadURI(gBrowser.selectedBrowser, url);
+  yield promiseErrorPageLoaded(gBrowser.selectedBrowser);
+  yield ContentTask.spawn(gBrowser.selectedBrowser, null, function*() {
+    content.document.getElementById("exceptionDialogButton").click();
+  });
+  yield exceptionDialogResolved;
+  yield BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser);
+}
+
+// Utility function to get a handle on the certificate exception dialog.
+// Modified from toolkit/components/passwordmgr/test/prompt_common.js
+function getCertExceptionDialog(aLocation) {
+  let enumerator = Services.wm.getXULWindowEnumerator(null);
+
+  while (enumerator.hasMoreElements()) {
+    let win = enumerator.getNext();
+    let windowDocShell = win.QueryInterface(Ci.nsIXULWindow).docShell;
+
+    let containedDocShells = windowDocShell.getDocShellEnumerator(
+                                      Ci.nsIDocShellTreeItem.typeChrome,
+                                      Ci.nsIDocShell.ENUMERATE_FORWARDS);
+    while (containedDocShells.hasMoreElements()) {
+      // Get the corresponding document for this docshell
+      let childDocShell = containedDocShells.getNext();
+      let childDoc = childDocShell.QueryInterface(Ci.nsIDocShell)
+                                  .contentViewer
+                                  .DOMDocument;
+
+      if (childDoc.location.href == aLocation) {
+        return childDoc;
+      }
+    }
+  }
+}
+

browser/themes/shared/identity-block/identity-block.inc.css

@@ -141,6 +141,11 @@
   visibility: visible;
 }
 
+#urlbar[pageproxystate="valid"] > #identity-box.certUserOverridden > #connection-icon {
+  list-style-image: url(chrome://browser/skin/identity-mixed-passive-loaded.svg);
+  visibility: visible;
+}
+
 #urlbar[pageproxystate="valid"] > #identity-box.insecureLoginForms > #connection-icon,
 #urlbar[pageproxystate="valid"] > #identity-box.mixedActiveContent > #connection-icon {
   list-style-image: url(chrome://browser/skin/identity-mixed-active-loaded.svg);
@@ -153,8 +158,3 @@
   list-style-image: url(chrome://browser/skin/identity-mixed-passive-loaded.svg);
   visibility: visible;
 }
-
-#urlbar[pageproxystate="valid"] > #identity-box.certUserOverridden > #connection-icon {
-  list-style-image: url(chrome://browser/skin/identity-mixed-passive-loaded.svg);
-  visibility: visible;
-}

dom/security/nsMixedContentBlocker.cpp

@@ -118,17 +118,18 @@ public:
         // Bug 1182551 - before changing the security state to broken, check
         // that the root is actually secure.
         if (mRootHasSecureConnection) {
+          // reset state security flag
+          state = state >> 4 << 4;
+          // set state security flag to broken, since there is mixed content
+          state |= nsIWebProgressListener::STATE_IS_BROKEN;
+
           // If mixed display content is loaded, make sure to include that in the state.
           if (rootDoc->GetHasMixedDisplayContentLoaded()) {
-            eventSink->OnSecurityChange(mContext,
+            state |= nsIWebProgressListener::STATE_LOADED_MIXED_DISPLAY_CONTENT;
-                                        (nsIWebProgressListener::STATE_IS_BROKEN |
-                                         nsIWebProgressListener::STATE_LOADED_MIXED_ACTIVE_CONTENT |
-                                         nsIWebProgressListener::STATE_LOADED_MIXED_DISPLAY_CONTENT));
-          } else {
-            eventSink->OnSecurityChange(mContext,
-                                        (nsIWebProgressListener::STATE_IS_BROKEN |
-                                         nsIWebProgressListener::STATE_LOADED_MIXED_ACTIVE_CONTENT));
           }
+
+          eventSink->OnSecurityChange(mContext,
+                                      (state | nsIWebProgressListener::STATE_LOADED_MIXED_ACTIVE_CONTENT));
         } else {
           // root not secure, mixed active content loaded in an https subframe
           if (NS_SUCCEEDED(stateRV)) {
@@ -149,16 +150,18 @@ public:
         // Bug 1182551 - before changing the security state to broken, check
         // that the root is actually secure.
         if (mRootHasSecureConnection) {
-        // If mixed active content is loaded, make sure to include that in the state.
+          // reset state security flag
+          state = state >> 4 << 4;
+          // set state security flag to broken, since there is mixed content
+          state |= nsIWebProgressListener::STATE_IS_BROKEN;
+
+          // If mixed active content is loaded, make sure to include that in the state.
           if (rootDoc->GetHasMixedActiveContentLoaded()) {
-            eventSink->OnSecurityChange(mContext,
+            state |= nsIWebProgressListener::STATE_LOADED_MIXED_ACTIVE_CONTENT;
-                                        (nsIWebProgressListener::STATE_IS_BROKEN |
-                                         nsIWebProgressListener::STATE_LOADED_MIXED_DISPLAY_CONTENT |
-                                         nsIWebProgressListener::STATE_LOADED_MIXED_ACTIVE_CONTENT));
-          } else {
-            eventSink->OnSecurityChange(mContext, (nsIWebProgressListener::STATE_IS_BROKEN |
-                                                   nsIWebProgressListener::STATE_LOADED_MIXED_DISPLAY_CONTENT));
           }
+
+          eventSink->OnSecurityChange(mContext,
+                                      (state | nsIWebProgressListener::STATE_LOADED_MIXED_DISPLAY_CONTENT));
         } else {
           // root not secure, mixed display content loaded in an https subframe
           if (NS_SUCCEEDED(stateRV)) {
@@ -841,23 +844,24 @@ nsMixedContentBlocker::ShouldLoad(bool aHadInsecureImageRedirect,
       rootDoc->SetHasMixedDisplayContentLoaded(true);
 
       if (rootHasSecureConnection) {
+        // reset state security flag
+        state = state >> 4 << 4;
+        // set state security flag to broken, since there is mixed content
+        state |= nsIWebProgressListener::STATE_IS_BROKEN;
+
+        // If mixed active content is loaded, make sure to include that in the state.
         if (rootDoc->GetHasMixedActiveContentLoaded()) {
-          // If mixed active content is loaded, make sure to include that in the state.
+          state |= nsIWebProgressListener::STATE_LOADED_MIXED_ACTIVE_CONTENT;
-          eventSink->OnSecurityChange(aRequestingContext,
-                                      (nsIWebProgressListener::STATE_IS_BROKEN |
-                                       nsIWebProgressListener::STATE_LOADED_MIXED_DISPLAY_CONTENT |
-                                       nsIWebProgressListener::STATE_LOADED_MIXED_ACTIVE_CONTENT));
-        } else {
-          eventSink->OnSecurityChange(aRequestingContext,
-                                      (nsIWebProgressListener::STATE_IS_BROKEN |
-                                       nsIWebProgressListener::STATE_LOADED_MIXED_DISPLAY_CONTENT));
         }
+
+        eventSink->OnSecurityChange(aRequestingContext,
+                                    (state | nsIWebProgressListener::STATE_LOADED_MIXED_DISPLAY_CONTENT));
       } else {
         // User has overriden the pref and the root is not https;
         // mixed display content was allowed on an https subframe.
         if (NS_SUCCEEDED(stateRV)) {
           eventSink->OnSecurityChange(aRequestingContext, (state | nsIWebProgressListener::STATE_LOADED_MIXED_DISPLAY_CONTENT));
-         }
+        }
       }
     } else {
       *aDecision = nsIContentPolicy::REJECT_REQUEST;
@@ -882,18 +886,19 @@ nsMixedContentBlocker::ShouldLoad(bool aHadInsecureImageRedirect,
       rootDoc->SetHasMixedActiveContentLoaded(true);
 
       if (rootHasSecureConnection) {
-        // User has decided to override the pref and the root is https, so change the Security State.
+        // reset state security flag
+        state = state >> 4 << 4;
+        // set state security flag to broken, since there is mixed content
+        state |= nsIWebProgressListener::STATE_IS_BROKEN;
+
+        // If mixed display content is loaded, make sure to include that in the state.
         if (rootDoc->GetHasMixedDisplayContentLoaded()) {
-          // If mixed display content is loaded, make sure to include that in the state.
+          state |= nsIWebProgressListener::STATE_LOADED_MIXED_DISPLAY_CONTENT;
-          eventSink->OnSecurityChange(aRequestingContext,
-                                      (nsIWebProgressListener::STATE_IS_BROKEN |
-                                       nsIWebProgressListener::STATE_LOADED_MIXED_ACTIVE_CONTENT |
-                                       nsIWebProgressListener::STATE_LOADED_MIXED_DISPLAY_CONTENT));
-        } else {
-          eventSink->OnSecurityChange(aRequestingContext,
-                                      (nsIWebProgressListener::STATE_IS_BROKEN |
-                                       nsIWebProgressListener::STATE_LOADED_MIXED_ACTIVE_CONTENT));
         }
+
+        eventSink->OnSecurityChange(aRequestingContext,
+                                    (state | nsIWebProgressListener::STATE_LOADED_MIXED_ACTIVE_CONTENT));
+
         return NS_OK;
       } else {
         // User has already overriden the pref and the root is not https;

security/manager/ssl/nsSecureBrowserUIImpl.cpp

@@ -226,9 +226,6 @@ nsSecureBrowserUIImpl::MapInternalToExternalState(uint32_t* aState, lockIconStat
     if (ev) {
       *aState |= nsIWebProgressListener::STATE_IDENTITY_EV_TOPLEVEL;
     }
-    if (mCertUserOverridden) {
-      *aState |= nsIWebProgressListener::STATE_CERT_USER_OVERRIDDEN;
-    }
   }
   // * If so, the state should be broken or insecure; overriding the previous
   // state set by the lock parameter.
@@ -251,6 +248,10 @@ nsSecureBrowserUIImpl::MapInternalToExternalState(uint32_t* aState, lockIconStat
                 nsIWebProgressListener::STATE_LOADED_MIXED_DISPLAY_CONTENT;
   }
 
+  if (mCertUserOverridden) {
+    *aState |= nsIWebProgressListener::STATE_CERT_USER_OVERRIDDEN;
+  }
+
   // Has Mixed Content Been Blocked in nsMixedContentBlocker?
   if (docShell->GetHasMixedActiveContentBlocked())
     *aState |= nsIWebProgressListener::STATE_BLOCKED_MIXED_ACTIVE_CONTENT;