Bug 1492943 - Part 1 - Visually distinguish potentially dangerous certificate errors. r=nhnt11
authorJohann Hofmann <jhofmann@mozilla.com>
Tue, 16 Oct 2018 18:50:38 +0000
changeset 499973 dc57e840695b4534ce98f817f3bf2a42c35fd6d4
parent 499972 ed09a03420aef61b84d91d1977ec969fb01cf94a
child 499974 a552ab852b35f17b53441f1ce51f95a215f6329f
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnhnt11
bugs1492943
milestone64.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 1492943 - Part 1 - Visually distinguish potentially dangerous certificate errors. r=nhnt11 Differential Revision: https://phabricator.services.mozilla.com/D8253
browser/actors/NetErrorChild.jsm
browser/base/content/test/about/browser_aboutCertError.js
browser/themes/shared/aboutNetError-new.css
--- a/browser/actors/NetErrorChild.jsm
+++ b/browser/actors/NetErrorChild.jsm
@@ -9,16 +9,18 @@ ChromeUtils.import("resource://gre/modul
 ChromeUtils.import("resource://gre/modules/Services.jsm");
 ChromeUtils.import("resource://gre/modules/ActorChild.jsm");
 
 ChromeUtils.defineModuleGetter(this, "BrowserUtils",
                                "resource://gre/modules/BrowserUtils.jsm");
 ChromeUtils.defineModuleGetter(this, "WebNavigationFrames",
                                "resource://gre/modules/WebNavigationFrames.jsm");
 
+XPCOMUtils.defineLazyGlobalGetters(this, ["URL"]);
+
 XPCOMUtils.defineLazyGetter(this, "gPipNSSBundle", function() {
   return Services.strings.createBundle("chrome://pipnss/locale/pipnss.properties");
 });
 XPCOMUtils.defineLazyGetter(this, "gBrandBundle", function() {
   return Services.strings.createBundle("chrome://branding/locale/brand.properties");
 });
 XPCOMUtils.defineLazyPreferenceGetter(this, "newErrorPagesEnabled",
   "browser.security.newcerterrorpage.enabled");
@@ -76,16 +78,24 @@ class NetErrorChild extends ActorChild {
   isAboutNetError(doc) {
     return doc.documentURI.startsWith("about:neterror");
   }
 
   isAboutCertError(doc) {
     return doc.documentURI.startsWith("about:certerror");
   }
 
+  getParams(doc) {
+    let searchParams = new URL(doc.documentURI).searchParams;
+    return {
+      cssClass: searchParams.get("s"),
+      error: searchParams.get("e"),
+    };
+  }
+
   _getCertValidityRange(docShell) {
     let {securityInfo} = docShell.failedChannel;
     securityInfo.QueryInterface(Ci.nsITransportSecurityInfo);
     let notBefore = 0;
     let notAfter = Number.MAX_SAFE_INTEGER;
     for (let cert of securityInfo.failedCertChain.getEnumerator()) {
       notBefore = Math.max(notBefore, cert.validity.notBefore);
       notAfter = Math.min(notAfter, cert.validity.notAfter);
@@ -93,19 +103,17 @@ class NetErrorChild extends ActorChild {
     // nsIX509Cert reports in PR_Date terms, which uses microseconds. Convert:
     notBefore /= 1000;
     notAfter /= 1000;
     return {notBefore, notAfter};
   }
 
   _setTechDetails(input, doc) {
     // CSS class and error code are set from nsDocShell.
-    let searchParams = new URLSearchParams(doc.documentURI.split("?")[1]);
-    let cssClass = searchParams.get("s");
-    let error = searchParams.get("e");
+    let {cssClass, error} = this.getParams(doc);
     let technicalInfo = doc.getElementById("badCertTechnicalInfo");
     technicalInfo.textContent = "";
 
     let uri = Services.io.newURI(input.data.url);
     let hostString = uri.host;
     if (uri.port != 443 && uri.port != -1) {
       hostString = uri.hostPort;
     }
@@ -348,16 +356,19 @@ class NetErrorChild extends ActorChild {
     this._setTechDetails(msg, doc);
     let learnMoreLink = doc.getElementById("learnMoreLink");
     let baseURL = Services.urlFormatter.formatURLPref("app.support.baseURL");
     let errWhatToDo = doc.getElementById("es_nssBadCert_" + msg.data.codeString);
     let es = doc.getElementById("errorWhatToDoText");
     let errWhatToDoTitle = doc.getElementById("edd_nssBadCert");
     let est = doc.getElementById("errorWhatToDoTitleText");
 
+    // This is set to true later if the user's system clock is at fault for this error.
+    let clockSkew = false;
+
     switch (msg.data.code) {
       case SSL_ERROR_BAD_CERT_DOMAIN:
       case SEC_ERROR_OCSP_INVALID_SIGNING_CERT:
       case SEC_ERROR_UNKNOWN_ISSUER:
         if (!newErrorPagesEnabled) {
           break;
         }
         if (es) {
@@ -402,17 +413,16 @@ class NetErrorChild extends ActorChild {
       case SEC_ERROR_EXPIRED_CERTIFICATE:
       case SEC_ERROR_EXPIRED_ISSUER_CERTIFICATE:
       case SEC_ERROR_OCSP_FUTURE_RESPONSE:
       case SEC_ERROR_OCSP_OLD_RESPONSE:
       case MOZILLA_PKIX_ERROR_NOT_YET_VALID_CERTIFICATE:
       case MOZILLA_PKIX_ERROR_NOT_YET_VALID_ISSUER_CERTIFICATE:
 
         learnMoreLink.href = baseURL + "time-errors";
-        let clockSkew = false;
         // We check against the remote-settings server time first if available, because that allows us
         // to give the user an approximation of what the correct time is.
         let difference = Services.prefs.getIntPref(PREF_SERVICES_SETTINGS_CLOCK_SKEW_SECONDS, 0);
         let lastFetched = Services.prefs.getIntPref(PREF_SERVICES_SETTINGS_LAST_FETCHED, 0) * 1000;
 
         let now = Date.now();
         let certRange = this._getCertValidityRange(docShell);
 
@@ -503,16 +513,32 @@ class NetErrorChild extends ActorChild {
               // eslint-disable-next-line no-unsanitized/property
               est.textContent = errWhatToDoTitle.textContent;
               est.style.fontWeight = "bold";
             }
             updateContainerPosition();
         }
         break;
     }
+
+    // Add slightly more alarming UI unless there are indicators that
+    // show that the error is harmless or can not be skipped anyway.
+    if (newErrorPagesEnabled) {
+      let {cssClass} = this.getParams(doc);
+      // Don't alarm users when they can't continue to the website anyway...
+      if (cssClass != "badStsCert" &&
+          // Errors in iframes can't be skipped either...
+          doc.ownerGlobal.parent == doc.ownerGlobal &&
+          // Also don't bother if it's just the user's clock being off...
+          !clockSkew &&
+          // Symantec distrust is likely harmless as well.
+          msg.data.code != MOZILLA_PKIX_ERROR_ADDITIONAL_POLICY_CONSTRAINT_FAILED) {
+        doc.body.classList.add("caution");
+      }
+    }
   }
 
   handleEvent(aEvent) {
     // Documents have a null ownerDocument.
     let doc = aEvent.originalTarget.ownerDocument || aEvent.originalTarget;
 
     switch (aEvent.type) {
     case "AboutNetErrorLoad":
--- a/browser/base/content/test/about/browser_aboutCertError.js
+++ b/browser/base/content/test/about/browser_aboutCertError.js
@@ -488,16 +488,41 @@ add_task(async function checkUnknownIssu
       return learnMoreLink.href;
     });
     ok(href.endsWith("security-error"), "security-error in the Learn More URL");
 
     BrowserTestUtils.removeTab(gBrowser.selectedTab);
   }
 });
 
+add_task(async function checkCautionClass() {
+  info("Checking that are potentially more dangerous get a 'caution' class");
+  for (let useFrame of [false, true]) {
+    let tab = await openErrorPage(UNKNOWN_ISSUER, useFrame);
+    let browser = tab.linkedBrowser;
+
+    await ContentTask.spawn(browser, {frame: useFrame}, async function({frame}) {
+      let doc = frame ? content.document.querySelector("iframe").contentDocument : content.document;
+      is(doc.body.classList.contains("caution"), !frame, `Cert error body has ${frame ? "no" : ""} caution class`);
+    });
+
+    BrowserTestUtils.removeTab(gBrowser.selectedTab);
+
+    tab = await openErrorPage(BAD_STS_CERT, useFrame);
+    browser = tab.linkedBrowser;
+
+    await ContentTask.spawn(browser, {frame: useFrame}, async function({frame}) {
+      let doc = frame ? content.document.querySelector("iframe").contentDocument : content.document;
+      ok(!doc.body.classList.contains("caution"), "Cert error body has no caution class");
+    });
+
+    BrowserTestUtils.removeTab(gBrowser.selectedTab);
+  }
+});
+
 function getCertChain(securityInfoAsString) {
   let certChain = "";
   const serhelper = Cc["@mozilla.org/network/serialization-helper;1"]
                        .getService(Ci.nsISerializationHelper);
   let securityInfo = serhelper.deserializeObject(securityInfoAsString);
   securityInfo.QueryInterface(Ci.nsITransportSecurityInfo);
   for (let cert of securityInfo.failedCertChain.getEnumerator()) {
     certChain += getPEMString(cert);
--- a/browser/themes/shared/aboutNetError-new.css
+++ b/browser/themes/shared/aboutNetError-new.css
@@ -4,21 +4,24 @@
 
 @import url("chrome://browser/skin/error-pages.css");
 
 :root {
   --exception-button-container-background: #F5F5F7;
 }
 
 body.certerror {
+  width: 100%;
+  justify-content: normal;
+}
+
+body.caution {
   border-style: solid;
   border-color: #ffe900;
   border-width: 16px;
-  width: 100%;
-  justify-content: normal;
 }
 
 body.captiveportal .title {
   background-image: url("wifi.svg");
 }
 
 body.certerror .title {
   background-image: url("cert-error-new.svg");