Bug 1567826 - Don't mark any secureContext pages as insecure. r=nhnt11,keeler,Ehsan

Differential Revision: https://phabricator.services.mozilla.com/D39012

--HG--
extra : moz-landing-system : lando

browser/base/content/browser-siteIdentity.js

@@ -35,6 +35,13 @@ var gIdentityHandler = {
    */
   _isSecureInternalUI: false,
 
+  /**
+   * Whether the content window is considered a "secure context". This
+   * includes "potentially trustworthy" origins such as file:// URLs or localhost.
+   * https://w3c.github.io/webappsec-secure-contexts/#is-origin-trustworthy
+   */
+  _isSecureContext: false,
+
   /**
    * nsITransportSecurityInfo metadata provided by gBrowser.securityUI the last
    * time the identity UI was updated, or null if the connection is not secure.
@@ -52,11 +59,23 @@ var gIdentityHandler = {
    */
   _secureInternalUIWhitelist: /^(?:accounts|addons|cache|config|crashes|downloads|license|logins|preferences|protections|rights|sessionrestore|support|welcomeback)(?:[?#]|$)/i,
 
-  get _isBroken() {
+  /**
+   * Whether the established HTTPS connection is considered "broken".
+   * This could have several reasons, such as mixed content or weak
+   * cryptography. If this is true, _isSecureConnection is false.
+   */
+  get _isBrokenConnection() {
     return this._state & Ci.nsIWebProgressListener.STATE_IS_BROKEN;
   },
 
-  get _isSecure() {
+  /**
+   * Whether the connection to the current site was done via secure
+   * transport. Note that this attribute is not true in all cases that
+   * the site was accessed via HTTPS, i.e. _isSecureConnection will
+   * be false when _isBrokenConnection is true, even though the page
+   * was loaded over HTTPS.
+   */
+  get _isSecureConnection() {
     // 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
@@ -457,6 +476,7 @@ var gIdentityHandler = {
     // the documentation of the individual properties for details.
     this.setURI(uri);
     this._secInfo = gBrowser.securityUI.secInfo;
+    this._isSecureContext = gBrowser.securityUI.isSecureContext;
 
     // Then, update the user interface with the available data.
     this.refreshIdentityBlock();
@@ -586,7 +606,7 @@ var gIdentityHandler = {
     if (this._uriHasHost && this._isEV) {
       return "verifiedIdentity";
     }
-    if (this._uriHasHost && this._isSecure) {
+    if (this._uriHasHost && this._isSecureConnection) {
       return "verifiedDomain";
     }
     return "unknownIdentity";
@@ -620,10 +640,12 @@ var gIdentityHandler = {
     let icon_labels_dir = "ltr";
 
     if (this._isSecureInternalUI) {
+      // This is a secure internal Firefox page.
       this._identityBox.className = "chromeUI";
       let brandBundle = document.getElementById("bundle_brand");
       icon_label = brandBundle.getString("brandShorterName");
     } else if (this._uriHasHost && this._isEV) {
+      // This is a secure connection with EV.
       this._identityBox.className = "verifiedIdentity";
       if (this._isMixedActiveContentBlocked) {
         this._identityBox.classList.add("mixedActiveBlocked");
@@ -654,13 +676,15 @@ var gIdentityHandler = {
           : "ltr";
       }
     } else if (this._pageExtensionPolicy) {
+      // This is a WebExtension page.
       this._identityBox.className = "extensionPage";
       let extensionName = this._pageExtensionPolicy.name;
       icon_label = gNavigatorBundle.getFormattedString(
         "identity.extension.label",
         [extensionName]
       );
-    } else if (this._uriHasHost && this._isSecure) {
+    } else if (this._uriHasHost && this._isSecureConnection) {
+      // This is a secure connection.
       this._identityBox.className = "verifiedDomain";
       if (this._isMixedActiveContentBlocked) {
         this._identityBox.classList.add("mixedActiveBlocked");
@@ -672,46 +696,45 @@ var gIdentityHandler = {
           [this.getIdentityData().caOrg]
         );
       }
-    } else if (!this._uriHasHost) {
+    } else if (this._isBrokenConnection) {
+      // This is a secure connection, but something is wrong.
       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");
+      }
     } else if (
-      gBrowser.selectedBrowser.documentURI &&
-      (gBrowser.selectedBrowser.documentURI.scheme == "about" ||
-        gBrowser.selectedBrowser.documentURI.scheme == "chrome")
+      this._isSecureContext ||
+      (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 is a local resource (and shouldn't be marked insecure).
       this._identityBox.className = "unknownIdentity";
     } else {
-      if (this._isBroken) {
-        this._identityBox.className = "unknownIdentity";
+      // This is an insecure connection.
+      let warnOnInsecure =
+        this._insecureConnectionIconEnabled ||
+        (this._insecureConnectionIconPBModeEnabled &&
+          PrivateBrowsingUtils.isWindowPrivate(window));
+      let className = warnOnInsecure ? "notSecure" : "unknownIdentity";
+      this._identityBox.className = className;
 
-        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");
-        }
-      } else {
-        let warnOnInsecure =
-          this._insecureConnectionIconEnabled ||
-          (this._insecureConnectionIconPBModeEnabled &&
-            PrivateBrowsingUtils.isWindowPrivate(window));
-        let className = warnOnInsecure ? "notSecure" : "unknownIdentity";
-        this._identityBox.className = className;
-
-        let warnTextOnInsecure =
-          this._insecureConnectionTextEnabled ||
-          (this._insecureConnectionTextPBModeEnabled &&
-            PrivateBrowsingUtils.isWindowPrivate(window));
-        if (warnTextOnInsecure) {
-          icon_label = gNavigatorBundle.getString("identity.notSecure.label");
-          this._identityBox.classList.add("notSecureText");
-        }
+      let warnTextOnInsecure =
+        this._insecureConnectionTextEnabled ||
+        (this._insecureConnectionTextPBModeEnabled &&
+          PrivateBrowsingUtils.isWindowPrivate(window));
+      if (warnTextOnInsecure) {
+        icon_label = gNavigatorBundle.getString("identity.notSecure.label");
+        this._identityBox.classList.add("notSecureText");
       }
       if (this._hasInsecureLoginForms) {
         // Insecure login forms can only be present on "unknown identity"
@@ -856,7 +879,7 @@ var gIdentityHandler = {
       connection = "secure-ev";
     } else if (this._isCertUserOverridden) {
       connection = "secure-cert-user-overridden";
-    } else if (this._isSecure) {
+    } else if (this._isSecureConnection) {
       connection = "secure";
       customRoot = this._hasCustomRoot();
     }
@@ -884,7 +907,7 @@ var gIdentityHandler = {
     // cipher.
     let ciphers = "";
     if (
-      this._isBroken &&
+      this._isBrokenConnection &&
       !this._isMixedActiveContentLoaded &&
       !this._isMixedPassiveContentLoaded
     ) {
@@ -908,7 +931,7 @@ var gIdentityHandler = {
       updateAttribute(element, "loginforms", loginforms);
       updateAttribute(element, "ciphers", ciphers);
       updateAttribute(element, "mixedcontent", mixedcontent);
-      updateAttribute(element, "isbroken", this._isBroken);
+      updateAttribute(element, "isbroken", this._isBrokenConnection);
       updateAttribute(element, "customroot", customRoot);
     }
 
@@ -919,7 +942,7 @@ var gIdentityHandler = {
     let owner = "";
 
     // Fill in the CA name if we have a valid TLS certificate.
-    if (this._isSecure || this._isCertUserOverridden) {
+    if (this._isSecureConnection || this._isCertUserOverridden) {
       verifier = this._identityIconLabels.tooltipText;
     }
 

browser/base/content/test/siteIdentity/browser_identityIcon_img_url.js

@@ -11,6 +11,11 @@ const kBaseURI = getRootDirectory(gTestPath).replace(
   "https://example.com"
 );
 
+const kBaseURILocalhost = getRootDirectory(gTestPath).replace(
+  "chrome://mochitests/content",
+  "http://127.0.0.1"
+);
+
 const TEST_CASES = [
   {
     type: "http",
@@ -54,9 +59,42 @@ const TEST_CASES = [
     // change it to show a not secure lock icon in Bug 1566813.
     img_url: `url("chrome://browser/skin/identity-icon.svg")`,
   },
+  {
+    type: "localhost",
+    testURL: "http://127.0.0.1",
+    img_url: `url("chrome://browser/skin/identity-icon.svg")`,
+  },
+  {
+    type: "localhost + http frame",
+    testURL: kBaseURILocalhost + "file_csp_block_all_mixedcontent.html",
+    img_url: `url("chrome://browser/skin/identity-icon.svg")`,
+  },
+  {
+    type: "data URI",
+    testURL: "data:text/html,<div>",
+    img_url: `url("chrome://browser/skin/connection-mixed-active-loaded.svg")`,
+  },
+  {
+    type: "view-source HTTP",
+    testURL: "view-source:http://example.com/",
+    img_url: `url("chrome://browser/skin/connection-mixed-active-loaded.svg")`,
+  },
+  {
+    type: "view-source HTTPS",
+    testURL: "view-source:https://example.com/",
+    // TODO this will get a secure treatment with bug 1496844.
+    img_url: `url("chrome://browser/skin/identity-icon.svg")`,
+  },
 ];
 
 add_task(async function test() {
+  await SpecialPowers.pushPrefEnv({
+    set: [
+      // By default, proxies don't apply to 127.0.0.1. We need them to for this test, though:
+      ["network.proxy.allow_hijacking_localhost", true],
+    ],
+  });
+
   for (let testData of TEST_CASES) {
     info(`Testing for ${testData.type}`);
     // Open the page for testing.

browser/base/content/test/siteIdentity/browser_identity_UI.js

@@ -104,14 +104,7 @@ async function runTest(i, forward) {
     "location matches for test " + testDesc
   );
   // getEffectiveHost can't be called for all modes
-  if (currentTest.effectiveHost === null) {
-    let identityBox = document.getElementById("identity-box");
-    ok(
-      identityBox.className == "unknownIdentity" ||
-        identityBox.className == "chromeUI",
-      "mode matched"
-    );
-  } else {
+  if (currentTest.effectiveHost !== null) {
     is(
       gIdentityHandler.getEffectiveHost(),
       currentTest.effectiveHost,

netwerk/base/nsISecureBrowserUI.idl

@@ -16,6 +16,7 @@ interface nsISecureBrowserUI : nsISupports
 
     readonly attribute unsigned long state;
     readonly attribute unsigned long contentBlockingEvent;
+    readonly attribute bool isSecureContext;
     readonly attribute nsITransportSecurityInfo secInfo;
 };
 

security/manager/ssl/nsSecureBrowserUIImpl.cpp

@@ -13,6 +13,7 @@
 #include "nsIChannel.h"
 #include "nsDocShell.h"
 #include "nsIDocShellTreeItem.h"
+#include "nsGlobalWindow.h"
 #include "nsIInterfaceRequestorUtils.h"
 #include "nsITransportSecurityInfo.h"
 #include "nsIWebProgress.h"
@@ -21,7 +22,8 @@ using namespace mozilla;
 
 LazyLogModule gSecureBrowserUILog("nsSecureBrowserUI");
 
-nsSecureBrowserUIImpl::nsSecureBrowserUIImpl() : mState(0), mEvent(0) {
+nsSecureBrowserUIImpl::nsSecureBrowserUIImpl()
+    : mState(0), mEvent(0), mIsSecureContext(false) {
   MOZ_ASSERT(NS_IsMainThread());
 }
 
@@ -73,6 +75,15 @@ nsSecureBrowserUIImpl::GetState(uint32_t* aState) {
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsSecureBrowserUIImpl::GetIsSecureContext(bool* aIsSecureContext) {
+  MOZ_ASSERT(NS_IsMainThread());
+  NS_ENSURE_ARG(aIsSecureContext);
+
+  *aIsSecureContext = mIsSecureContext;
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 nsSecureBrowserUIImpl::GetContentBlockingEvent(uint32_t* aEvent) {
   MOZ_ASSERT(NS_IsMainThread());
@@ -389,6 +400,7 @@ nsSecureBrowserUIImpl::OnLocationChange(nsIWebProgress* aWebProgress,
 
   mState = 0;
   mEvent = 0;
+  mIsSecureContext = false;
   mTopLevelSecurityInfo = nullptr;
 
   if (aFlags & LOCATION_CHANGE_ERROR_PAGE) {
@@ -412,6 +424,19 @@ nsSecureBrowserUIImpl::OnLocationChange(nsIWebProgress* aWebProgress,
         mTopLevelSecurityInfo = nullptr;
       }
     }
+
+    nsCOMPtr<mozIDOMWindowProxy> domWindow;
+    aWebProgress->GetDOMWindow(getter_AddRefs(domWindow));
+    if (domWindow) {
+      nsPIDOMWindowOuter* outerWindow = nsPIDOMWindowOuter::From(domWindow);
+      if (outerWindow) {
+        nsGlobalWindowInner* innerWindow =
+            nsGlobalWindowInner::Cast(outerWindow->GetCurrentInnerWindow());
+        if (innerWindow) {
+          mIsSecureContext = innerWindow->IsSecureContext();
+        }
+      }
+    }
   }
 
   nsCOMPtr<nsIDocShell> docShell = do_QueryReferent(mDocShell);

security/manager/ssl/nsSecureBrowserUIImpl.h

@@ -52,6 +52,7 @@ class nsSecureBrowserUIImpl : public nsISecureBrowserUI,
 
   uint32_t mState;
   uint32_t mEvent;
+  bool mIsSecureContext;
   nsWeakPtr mDocShell;
   nsWeakPtr mWebProgress;
   nsCOMPtr<nsITransportSecurityInfo> mTopLevelSecurityInfo;

toolkit/modules/RemoteSecurityUI.jsm

@@ -9,6 +9,7 @@ function RemoteSecurityUI() {
   this._secInfo = null;
   this._state = 0;
   this._event = 0;
+  this._isSecureContext = false;
 }
 
 RemoteSecurityUI.prototype = {
@@ -24,10 +25,14 @@ RemoteSecurityUI.prototype = {
   get secInfo() {
     return this._secInfo;
   },
+  get isSecureContext() {
+    return this._isSecureContext;
+  },
 
-  _update(aSecInfo, aState) {
+  _update(aSecInfo, aState, aIsSecureContext) {
     this._secInfo = aSecInfo;
     this._state = aState;
+    this._isSecureContext = aIsSecureContext;
   },
   _updateContentBlockingEvent(aEvent) {
     this._event = aEvent;

toolkit/modules/RemoteWebProgress.jsm

@@ -69,7 +69,7 @@ class RemoteWebProgressManager {
     );
   }
 
-  _fixSecInfoAndState(aSecInfo, aState) {
+  _fixSecInfo(aSecInfo) {
     let deserialized = null;
     if (aSecInfo) {
       let helper = Cc["@mozilla.org/network/serialization-helper;1"].getService(
@@ -80,7 +80,7 @@ class RemoteWebProgressManager {
       deserialized.QueryInterface(Ci.nsITransportSecurityInfo);
     }
 
-    return [deserialized, aState];
+    return deserialized;
   }
 
   setCurrentURI(aURI) {
@@ -216,17 +216,17 @@ class RemoteWebProgressManager {
 
     switch (aMessage.name) {
       case "Content:SecurityChange":
-        let [secInfo, state] = this._fixSecInfoAndState(
-          json.secInfo,
-          json.state
-        );
+        let state = json.state;
 
         if (isTopLevel) {
+          let secInfo = this._fixSecInfo(json.secInfo);
+          let isSecureContext = json.isSecureContext;
+
           // Invoking this getter triggers the generation of the underlying object,
           // which we need to access with ._securityUI, because .securityUI returns
           // a wrapper that makes _update inaccessible.
           void this._browser.securityUI;
-          this._browser._securityUI._update(secInfo, state);
+          this._browser._securityUI._update(secInfo, state, isSecureContext);
         }
 
         this.onSecurityChange(webProgress, request, state);

toolkit/modules/WebProgressChild.jsm

@@ -94,7 +94,11 @@ class WebProgressChild {
     let json = this._setupJSON(aWebProgress, aRequest);
 
     json.state = aState;
-    json.secInfo = this.getSecInfoAsString();
+
+    if (aWebProgress.isTopLevel) {
+      json.secInfo = this.getSecInfoAsString();
+      json.isSecureContext = this.mm.content.isSecureContext;
+    }
 
     this._send("Content:SecurityChange", json);
   }