Bug 1661593 - pageinfo: Do not show insecure connections as secure when using a proxy. r=johannh,nhnt11
authorpbz <pbz@mozilla.com>
Tue, 27 Oct 2020 16:45:25 +0000
changeset 554744 a8aadd76602fdce2443edaf9345d57967e158ac6
parent 554743 7157e71ad2891488b085dfc94fd7ec107fc6bc4c
child 554745 79261eb923741aef8cd3737f1cc7ae6feb0be44b
push id37898
push userabutkovits@mozilla.com
push dateWed, 28 Oct 2020 09:24:21 +0000
treeherdermozilla-central@83bf4fd3b1fb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjohannh, nhnt11
bugs1661593
milestone84.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1661593 - pageinfo: Do not show insecure connections as secure when using a proxy. r=johannh,nhnt11 Differential Revision: https://phabricator.services.mozilla.com/D93643
browser/base/content/pageinfo/security.js
browser/base/content/test/siteIdentity/browser_secure_transport_insecure_scheme.js
--- a/browser/base/content/pageinfo/security.js
+++ b/browser/base/content/pageinfo/security.js
@@ -56,102 +56,111 @@ var security = {
 
     var isBroken = ui.state & Ci.nsIWebProgressListener.STATE_IS_BROKEN;
     var isMixed =
       ui.state &
       (Ci.nsIWebProgressListener.STATE_LOADED_MIXED_ACTIVE_CONTENT |
         Ci.nsIWebProgressListener.STATE_LOADED_MIXED_DISPLAY_CONTENT);
     var isEV = ui.state & Ci.nsIWebProgressListener.STATE_IDENTITY_EV_TOPLEVEL;
 
-    let secInfo = await window.opener.gBrowser.selectedBrowser.browsingContext.currentWindowGlobal.getSecurityInfo();
-    if (secInfo) {
-      secInfo.QueryInterface(Ci.nsITransportSecurityInfo);
-      let cert = secInfo.serverCert;
-      let issuerName = null;
-      if (cert) {
-        issuerName = cert.issuerOrganization || cert.issuerName;
-      }
-
-      let certChainArray = [];
-      if (secInfo.succeededCertChain.length) {
-        certChainArray = secInfo.succeededCertChain;
-      } else {
-        certChainArray = secInfo.failedCertChain;
-      }
-
-      var retval = {
-        cAName: issuerName,
-        encryptionAlgorithm: undefined,
-        encryptionStrength: undefined,
-        version: undefined,
-        isBroken,
-        isMixed,
-        isEV,
-        cert,
-        certChain: certChainArray,
-        certificateTransparency: undefined,
-      };
-
-      var version;
-      try {
-        retval.encryptionAlgorithm = secInfo.cipherName;
-        retval.encryptionStrength = secInfo.secretKeyLength;
-        version = secInfo.protocolVersion;
-      } catch (e) {}
-
-      switch (version) {
-        case Ci.nsITransportSecurityInfo.SSL_VERSION_3:
-          retval.version = "SSL 3";
-          break;
-        case Ci.nsITransportSecurityInfo.TLS_VERSION_1:
-          retval.version = "TLS 1.0";
-          break;
-        case Ci.nsITransportSecurityInfo.TLS_VERSION_1_1:
-          retval.version = "TLS 1.1";
-          break;
-        case Ci.nsITransportSecurityInfo.TLS_VERSION_1_2:
-          retval.version = "TLS 1.2";
-          break;
-        case Ci.nsITransportSecurityInfo.TLS_VERSION_1_3:
-          retval.version = "TLS 1.3";
-          break;
-      }
-
-      // Select the status text to display for Certificate Transparency.
-      // Since we do not yet enforce the CT Policy on secure connections,
-      // we must not complain on policy discompliance (it might be viewed
-      // as a security issue by the user).
-      switch (secInfo.certificateTransparencyStatus) {
-        case Ci.nsITransportSecurityInfo
-          .CERTIFICATE_TRANSPARENCY_NOT_APPLICABLE:
-        case Ci.nsITransportSecurityInfo
-          .CERTIFICATE_TRANSPARENCY_POLICY_NOT_ENOUGH_SCTS:
-        case Ci.nsITransportSecurityInfo
-          .CERTIFICATE_TRANSPARENCY_POLICY_NOT_DIVERSE_SCTS:
-          retval.certificateTransparency = null;
-          break;
-        case Ci.nsITransportSecurityInfo
-          .CERTIFICATE_TRANSPARENCY_POLICY_COMPLIANT:
-          retval.certificateTransparency = "Compliant";
-          break;
-      }
-
-      return retval;
-    }
-    return {
+    let retval = {
       cAName: "",
       encryptionAlgorithm: "",
       encryptionStrength: 0,
       version: "",
       isBroken,
       isMixed,
       isEV,
       cert: null,
       certificateTransparency: null,
     };
+
+    // Only show certificate info for secure contexts. This prevents us from
+    // showing certificate data for http origins when using a proxy.
+    // https://searchfox.org/mozilla-central/rev/9c72508fcf2bba709a5b5b9eae9da35e0c707baa/security/manager/ssl/nsSecureBrowserUI.cpp#62-64
+    if (!ui.isSecureContext) {
+      return retval;
+    }
+
+    let secInfo = await window.opener.gBrowser.selectedBrowser.browsingContext.currentWindowGlobal.getSecurityInfo();
+    if (!secInfo) {
+      return retval;
+    }
+
+    secInfo.QueryInterface(Ci.nsITransportSecurityInfo);
+    let cert = secInfo.serverCert;
+    let issuerName = null;
+    if (cert) {
+      issuerName = cert.issuerOrganization || cert.issuerName;
+    }
+
+    let certChainArray = [];
+    if (secInfo.succeededCertChain.length) {
+      certChainArray = secInfo.succeededCertChain;
+    } else {
+      certChainArray = secInfo.failedCertChain;
+    }
+
+    retval = {
+      cAName: issuerName,
+      encryptionAlgorithm: undefined,
+      encryptionStrength: undefined,
+      version: undefined,
+      isBroken,
+      isMixed,
+      isEV,
+      cert,
+      certChain: certChainArray,
+      certificateTransparency: undefined,
+    };
+
+    var version;
+    try {
+      retval.encryptionAlgorithm = secInfo.cipherName;
+      retval.encryptionStrength = secInfo.secretKeyLength;
+      version = secInfo.protocolVersion;
+    } catch (e) {}
+
+    switch (version) {
+      case Ci.nsITransportSecurityInfo.SSL_VERSION_3:
+        retval.version = "SSL 3";
+        break;
+      case Ci.nsITransportSecurityInfo.TLS_VERSION_1:
+        retval.version = "TLS 1.0";
+        break;
+      case Ci.nsITransportSecurityInfo.TLS_VERSION_1_1:
+        retval.version = "TLS 1.1";
+        break;
+      case Ci.nsITransportSecurityInfo.TLS_VERSION_1_2:
+        retval.version = "TLS 1.2";
+        break;
+      case Ci.nsITransportSecurityInfo.TLS_VERSION_1_3:
+        retval.version = "TLS 1.3";
+        break;
+    }
+
+    // Select the status text to display for Certificate Transparency.
+    // Since we do not yet enforce the CT Policy on secure connections,
+    // we must not complain on policy discompliance (it might be viewed
+    // as a security issue by the user).
+    switch (secInfo.certificateTransparencyStatus) {
+      case Ci.nsITransportSecurityInfo.CERTIFICATE_TRANSPARENCY_NOT_APPLICABLE:
+      case Ci.nsITransportSecurityInfo
+        .CERTIFICATE_TRANSPARENCY_POLICY_NOT_ENOUGH_SCTS:
+      case Ci.nsITransportSecurityInfo
+        .CERTIFICATE_TRANSPARENCY_POLICY_NOT_DIVERSE_SCTS:
+        retval.certificateTransparency = null;
+        break;
+      case Ci.nsITransportSecurityInfo
+        .CERTIFICATE_TRANSPARENCY_POLICY_COMPLIANT:
+        retval.certificateTransparency = "Compliant";
+        break;
+    }
+
+    return retval;
   },
 
   // Find the secureBrowserUI object (if present)
   _getSecurityUI() {
     if (window.opener.gBrowser) {
       return window.opener.gBrowser.securityUI;
     }
     return null;
--- a/browser/base/content/test/siteIdentity/browser_secure_transport_insecure_scheme.js
+++ b/browser/base/content/test/siteIdentity/browser_secure_transport_insecure_scheme.js
@@ -2,16 +2,45 @@
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 // Test that an insecure resource routed over a secure transport is considered
 // insecure in terms of the site identity panel. We achieve this by running an
 // HTTP-over-TLS "proxy" and having Firefox request an http:// URI over it.
 
+/**
+ * Tests that the page info dialog "security" section labels a
+ * connection as unencrypted and does not show certificate.
+ * @param {string} uri - URI of the page to test with.
+ */
+async function testPageInfoNotEncrypted(uri) {
+  let pageInfo = BrowserPageInfo(uri, "securityTab");
+  await BrowserTestUtils.waitForEvent(pageInfo, "load");
+  let pageInfoDoc = pageInfo.document;
+  let securityTab = pageInfoDoc.getElementById("securityTab");
+  await TestUtils.waitForCondition(
+    () => BrowserTestUtils.is_visible(securityTab),
+    "Security tab should be visible."
+  );
+
+  let secLabel = pageInfoDoc.getElementById("security-technical-shortform");
+  await TestUtils.waitForCondition(
+    () => secLabel.value == "Connection Not Encrypted",
+    "pageInfo 'Security Details' should show not encrypted"
+  );
+
+  let viewCertBtn = pageInfoDoc.getElementById("security-view-cert");
+  ok(
+    viewCertBtn.collapsed,
+    "pageInfo 'View Cert' button should not be visible"
+  );
+  pageInfo.close();
+}
+
 // But first, a quick test that we don't incorrectly treat a
 // blob:https://example.com URI as secure.
 add_task(async function() {
   let uri =
     getRootDirectory(gTestPath).replace(
       "chrome://mochitests/content",
       "https://example.com"
     ) + "dummy_page.html";
@@ -22,16 +51,17 @@ add_task(async function() {
         type: "application/json",
       });
       let blobUri = URL.createObjectURL(blob);
       content.document.location = blobUri;
     });
     await BrowserTestUtils.browserLoaded(browser);
     let identityMode = window.document.getElementById("identity-box").className;
     is(identityMode, "localResource", "identity should be 'localResource'");
+    await testPageInfoNotEncrypted(uri);
   });
 });
 
 // This server pretends to be a HTTP over TLS proxy. It isn't really, but this
 // is sufficient for the purposes of this test.
 function startServer(cert) {
   let tlsServer = Cc["@mozilla.org/network/tls-server-socket;1"].createInstance(
     Ci.nsITLSServerSocket
@@ -158,10 +188,12 @@ add_task(async function() {
 
   // Navigate to 'http://example.com'. Our proxy settings will route this via
   // the "proxy" we just started. Even though our connection to the proxy is
   // secure, in a real situation the connection from the proxy to
   // http://example.com won't be secure, so we treat it as not secure.
   await BrowserTestUtils.withNewTab("http://example.com/", async browser => {
     let identityMode = window.document.getElementById("identity-box").className;
     is(identityMode, "notSecure", "identity should be 'not secure'");
+
+    await testPageInfoNotEncrypted("http://example.com");
   });
 });