Bug 1297630 - Make certificate error pages work properly in iframes. r=Gijs
☠☠ backed out by 72f6a225e5e6 ☠ ☠
authorJohann Hofmann <jhofmann@mozilla.com>
Thu, 15 Mar 2018 13:55:13 +0100
changeset 408610 a09f27b6dceb0b3f8f43468489ee3eac2c4e5a86
parent 408609 1f9f56aeb495482135d58d6edf8bf4a29841cb50
child 408611 8d44271e5973cad25b6a8b8153fe958cadaba33c
push id100996
push userbtara@mozilla.com
push dateSat, 17 Mar 2018 10:37:43 +0000
treeherdermozilla-inbound@97160a734959 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs
bugs1297630, 1446319
milestone61.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 1297630 - Make certificate error pages work properly in iframes. r=Gijs This adds workarounds to ensure that messages passed from browser.js to content.js in the context of certerror pages always contain a frameId which can be used to identify the frame that is supposed to receive them. This fix is really meant to be temporary until we come up with a good replacement for chrome - content communication, which probably boils down to finding a middle ground between nsAboutCapabilities, RemotePageManager and WebChannel. I did not update communication for Captive Portal pages, since those require one-way broadcasting from chrome to content, which is not supported in this model. This is tracked in bug 1446319. I did also not change the behavior of the "Go Back" button, which still navigates away the top level page, because I consider changing that behavior out of scope for this bug (and in my personal opinion we should not change the behavior). MozReview-Commit-ID: GrM6PFys6Cu
browser/base/content/browser.js
browser/base/content/content.js
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -3003,17 +3003,18 @@ var BrowserOnClick = {
     }
   },
 
   receiveMessage(msg) {
     switch (msg.name) {
       case "Browser:CertExceptionError":
         this.onCertError(msg.target, msg.data.elementId,
                          msg.data.isTopFrame, msg.data.location,
-                         msg.data.securityInfoAsString);
+                         msg.data.securityInfoAsString,
+                         msg.data.frameId);
       break;
       case "Browser:OpenCaptivePortalPage":
         CaptivePortalWatcher.ensureCaptivePortalTab();
       break;
       case "Browser:SiteBlockedError":
         this.onAboutBlocked(msg.data.elementId, msg.data.reason,
                             msg.data.isTopFrame, msg.data.location,
                             msg.data.blockedInfo);
@@ -3048,17 +3049,17 @@ var BrowserOnClick = {
           .add(reportStatus);
       break;
       case "Browser:SSLErrorGoBack":
         goBackFromErrorPage();
       break;
     }
   },
 
-  onCertError(browser, elementId, isTopFrame, location, securityInfoAsString) {
+  onCertError(browser, elementId, isTopFrame, location, securityInfoAsString, frameId) {
     let secHistogram = Services.telemetry.getHistogramById("SECURITY_UI");
     let securityInfo;
 
     switch (elementId) {
       case "exceptionDialogButton":
         if (isTopFrame) {
           secHistogram.add(Ci.nsISecurityUITelemetry.WARNING_BAD_CERT_TOP_CLICK_ADD_EXCEPTION);
         }
@@ -3099,19 +3100,20 @@ var BrowserOnClick = {
       case "advancedButton":
         if (isTopFrame) {
           secHistogram.add(Ci.nsISecurityUITelemetry.WARNING_BAD_CERT_TOP_UNDERSTAND_RISKS);
         }
 
         securityInfo = getSecurityInfo(securityInfoAsString);
         let errorInfo = getDetailedCertErrorInfo(location,
                                                  securityInfo);
-        browser.messageManager.sendAsyncMessage( "CertErrorDetails", {
+        browser.messageManager.sendAsyncMessage("CertErrorDetails", {
             code: securityInfo.errorCode,
-            info: errorInfo
+            info: errorInfo,
+            frameId,
         });
         break;
 
       case "copyToClipboard":
         const gClipboardHelper = Cc["@mozilla.org/widget/clipboardhelper;1"]
                                     .getService(Ci.nsIClipboardHelper);
         securityInfo = getSecurityInfo(securityInfoAsString);
         let detailedInfo = getDetailedCertErrorInfo(location,
--- a/browser/base/content/content.js
+++ b/browser/base/content/content.js
@@ -229,40 +229,45 @@ var AboutNetAndCertErrorListener = {
     addMessageListener("CertErrorDetails", this);
     addMessageListener("Browser:CaptivePortalFreed", this);
     chromeGlobal.addEventListener("AboutNetErrorLoad", this, false, true);
     chromeGlobal.addEventListener("AboutNetErrorOpenCaptivePortal", this, false, true);
     chromeGlobal.addEventListener("AboutNetErrorSetAutomatic", this, false, true);
     chromeGlobal.addEventListener("AboutNetErrorResetPreferences", this, false, true);
   },
 
-  get isAboutNetError() {
-    return content.document.documentURI.startsWith("about:neterror");
+  isAboutNetError(doc) {
+    return doc.documentURI.startsWith("about:neterror");
   },
 
-  get isAboutCertError() {
-    return content.document.documentURI.startsWith("about:certerror");
+  isAboutCertError(doc) {
+    return doc.documentURI.startsWith("about:certerror");
   },
 
   receiveMessage(msg) {
-    if (!this.isAboutCertError) {
-      return;
-    }
+    if (msg.name == "CertErrorDetails") {
+      let frameDocShell = WebNavigationFrames.findDocShell(msg.data.frameId, docShell);
+      // We need nsIWebNavigation to access docShell.document.
+      frameDocShell && frameDocShell.QueryInterface(Ci.nsIWebNavigation);
+      if (!frameDocShell || !this.isAboutCertError(frameDocShell.document)) {
+        return;
+      }
 
-    switch (msg.name) {
-      case "CertErrorDetails":
-        this.onCertErrorDetails(msg);
-        break;
-      case "Browser:CaptivePortalFreed":
-        this.onCaptivePortalFreed(msg);
-        break;
+      this.onCertErrorDetails(msg, frameDocShell);
+    } else if (msg.name == "Browser:CaptivePortalFreed") {
+      // TODO: This check is not correct for frames.
+      if (!this.isAboutCertError(content.document)) {
+        return;
+      }
+
+      this.onCaptivePortalFreed(msg);
     }
   },
 
-  _getCertValidityRange() {
+  _getCertValidityRange(docShell) {
     let {securityInfo} = docShell.failedChannel;
     securityInfo.QueryInterface(Ci.nsITransportSecurityInfo);
     let certs = securityInfo.failedCertChain.getEnumerator();
     let notBefore = 0;
     let notAfter = Number.MAX_SAFE_INTEGER;
     while (certs.hasMoreElements()) {
       let cert = certs.getNext();
       cert.QueryInterface(Ci.nsIX509Cert);
@@ -270,20 +275,22 @@ var AboutNetAndCertErrorListener = {
       notAfter = Math.min(notAfter, cert.validity.notAfter);
     }
     // nsIX509Cert reports in PR_Date terms, which uses microseconds. Convert:
     notBefore /= 1000;
     notAfter /= 1000;
     return {notBefore, notAfter};
   },
 
-  onCertErrorDetails(msg) {
-    let div = content.document.getElementById("certificateErrorText");
+  onCertErrorDetails(msg, docShell) {
+    let doc = docShell.document;
+
+    let div = doc.getElementById("certificateErrorText");
     div.textContent = msg.data.info;
-    let learnMoreLink = content.document.getElementById("learnMoreLink");
+    let learnMoreLink = doc.getElementById("learnMoreLink");
     let baseURL = Services.urlFormatter.formatURLPref("app.support.baseURL");
 
     switch (msg.data.code) {
       case SEC_ERROR_UNKNOWN_ISSUER:
         learnMoreLink.href = baseURL + "security-error";
         break;
 
       // In case the certificate expired we make sure the system clock
@@ -296,41 +303,36 @@ var AboutNetAndCertErrorListener = {
       case MOZILLA_PKIX_ERROR_NOT_YET_VALID_ISSUER_CERTIFICATE:
 
         // We check against Kinto 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_BLOCKLIST_CLOCK_SKEW_SECONDS, 0);
         let lastFetched = Services.prefs.getIntPref(PREF_BLOCKLIST_LAST_FETCHED, 0) * 1000;
 
         let now = Date.now();
-        let certRange = this._getCertValidityRange();
+        let certRange = this._getCertValidityRange(docShell);
 
         let approximateDate = now - difference * 1000;
         // If the difference is more than a day, we last fetched the date in the last 5 days,
         // and adjusting the date per the interval would make the cert valid, warn the user:
         if (Math.abs(difference) > 60 * 60 * 24 && (now - lastFetched) <= 60 * 60 * 24 * 5 &&
             certRange.notBefore < approximateDate && certRange.notAfter > approximateDate) {
           let formatter = new Services.intl.DateTimeFormat(undefined, {
             dateStyle: "short"
           });
           let systemDate = formatter.format(new Date());
           // negative difference means local time is behind server time
           approximateDate = formatter.format(new Date(approximateDate));
 
-          content.document.getElementById("wrongSystemTime_URL")
-            .textContent = content.document.location.hostname;
-          content.document.getElementById("wrongSystemTime_systemDate")
-            .textContent = systemDate;
-          content.document.getElementById("wrongSystemTime_actualDate")
-            .textContent = approximateDate;
+          doc.getElementById("wrongSystemTime_URL").textContent = doc.location.hostname;
+          doc.getElementById("wrongSystemTime_systemDate").textContent = systemDate;
+          doc.getElementById("wrongSystemTime_actualDate").textContent = approximateDate;
 
-          content.document.getElementById("errorShortDesc")
-            .style.display = "none";
-          content.document.getElementById("wrongSystemTimePanel")
-            .style.display = "block";
+          doc.getElementById("errorShortDesc").style.display = "none";
+          doc.getElementById("wrongSystemTimePanel").style.display = "block";
 
         // If there is no clock skew with Kinto servers, check against the build date.
         // (The Kinto ping could have happened when the time was still right, or not at all)
         } else {
           let appBuildID = Services.appinfo.appBuildID;
 
           let year = parseInt(appBuildID.substr(0, 4), 10);
           let month = parseInt(appBuildID.substr(4, 2), 10) - 1;
@@ -343,38 +345,43 @@ var AboutNetAndCertErrorListener = {
           // as it is of course almost certain that it is now later than the build date,
           // so we shouldn't exclude the possibility that the cert has become valid
           // since the build date.
           if (buildDate > systemDate && new Date(certRange.notAfter) > buildDate) {
             let formatter = new Services.intl.DateTimeFormat(undefined, {
               dateStyle: "short"
             });
 
-            content.document.getElementById("wrongSystemTimeWithoutReference_URL")
-              .textContent = content.document.location.hostname;
-            content.document.getElementById("wrongSystemTimeWithoutReference_systemDate")
+            doc.getElementById("wrongSystemTimeWithoutReference_URL")
+              .textContent = doc.location.hostname;
+            doc.getElementById("wrongSystemTimeWithoutReference_systemDate")
               .textContent = formatter.format(systemDate);
 
-            content.document.getElementById("errorShortDesc")
-              .style.display = "none";
-            content.document.getElementById("wrongSystemTimeWithoutReferencePanel")
-              .style.display = "block";
+            doc.getElementById("errorShortDesc").style.display = "none";
+            doc.getElementById("wrongSystemTimeWithoutReferencePanel").style.display = "block";
           }
         }
         learnMoreLink.href = baseURL + "time-errors";
         break;
     }
   },
 
   onCaptivePortalFreed(msg) {
     content.dispatchEvent(new content.CustomEvent("AboutNetErrorCaptivePortalFreed"));
   },
 
   handleEvent(aEvent) {
-    if (!this.isAboutNetError && !this.isAboutCertError) {
+    let doc;
+    if (aEvent.originalTarget instanceof Ci.nsIDOMDocument) {
+      doc = aEvent.originalTarget;
+    } else {
+      doc = aEvent.originalTarget.ownerDocument;
+    }
+
+    if (!this.isAboutNetError(doc) && !this.isAboutCertError(doc)) {
       return;
     }
 
     switch (aEvent.type) {
     case "AboutNetErrorLoad":
       this.onPageLoad(aEvent);
       break;
     case "AboutNetErrorOpenCaptivePortal":
@@ -401,24 +408,27 @@ var AboutNetAndCertErrorListener = {
 
     return false;
   },
 
   onPageLoad(evt) {
     // Values for telemtery bins: see TLS_ERROR_REPORT_UI in Histograms.json
     const TLS_ERROR_REPORT_TELEMETRY_UI_SHOWN = 0;
 
-    if (this.isAboutCertError) {
-      let originalTarget = evt.originalTarget;
-      let ownerDoc = originalTarget.ownerDocument;
-      ClickEventHandler.onCertError(originalTarget, ownerDoc);
+    let originalTarget = evt.originalTarget;
+    let ownerDoc = originalTarget.ownerDocument;
+
+    if (this.isAboutCertError(ownerDoc)) {
+      ClickEventHandler.onCertError(originalTarget);
     }
 
+    let win = originalTarget.ownerGlobal;
+
     let automatic = Services.prefs.getBoolPref("security.ssl.errorReporting.automatic");
-    content.dispatchEvent(new content.CustomEvent("AboutNetErrorOptions", {
+    win.dispatchEvent(new win.CustomEvent("AboutNetErrorOptions", {
       detail: JSON.stringify({
         enabled: Services.prefs.getBoolPref("security.ssl.errorReporting.enabled"),
         changedCertPrefs: this.changedCertPrefs(),
         automatic
       })
     }));
 
     sendAsyncMessage("Browser:SSLErrorReportTelemetry",
@@ -437,19 +447,17 @@ var AboutNetAndCertErrorListener = {
   onSetAutomatic(evt) {
     sendAsyncMessage("Browser:SetSSLErrorReportAuto", {
       automatic: evt.detail
     });
 
     // If we're enabling reports, send a report for this failure.
     if (evt.detail) {
       let win = evt.originalTarget.ownerGlobal;
-      let docShell = win.QueryInterface(Ci.nsIInterfaceRequestor)
-                        .getInterface(Ci.nsIWebNavigation)
-                        .QueryInterface(Ci.nsIDocShell);
+      let docShell = win.document.docShell;
 
       let {securityInfo} = docShell.failedChannel;
       securityInfo.QueryInterface(Ci.nsITransportSecurityInfo);
       let {host, port} = win.document.mozDocumentURIIfNotForErrorPages;
 
       let errorReporter = Cc["@mozilla.org/securityreporter;1"]
                             .getService(Ci.nsISecurityReporter);
       errorReporter.reportTLSError(securityInfo, host, port);
@@ -473,23 +481,23 @@ var ClickEventHandler = {
     let originalTarget = event.originalTarget;
     let ownerDoc = originalTarget.ownerDocument;
     if (!ownerDoc) {
       return;
     }
 
     // Handle click events from about pages
     if (event.button == 0) {
-      if (ownerDoc.documentURI.startsWith("about:certerror")) {
-        this.onCertError(originalTarget, ownerDoc);
+      if (AboutNetAndCertErrorListener.isAboutCertError(ownerDoc)) {
+        this.onCertError(originalTarget);
         return;
       } else if (ownerDoc.documentURI.startsWith("about:blocked")) {
         this.onAboutBlocked(originalTarget, ownerDoc);
         return;
-      } else if (ownerDoc.documentURI.startsWith("about:neterror")) {
+      } else if (AboutNetAndCertErrorListener.isAboutNetError(ownerDoc)) {
         this.onAboutNetError(event, ownerDoc.documentURI);
         return;
       }
     }
 
     let [href, node, principal] = this._hrefAndLinkNodeForClickEvent(event);
 
     // get referrer attribute from clicked link and parse it
@@ -533,19 +541,17 @@ var ClickEventHandler = {
         }
       }
       json.noReferrer = BrowserUtils.linkHasNoReferrer(node);
 
       // Check if the link needs to be opened with mixed content allowed.
       // Only when the owner doc has |mixedContentChannel| and the same origin
       // should we allow mixed content.
       json.allowMixedContent = false;
-      let docshell = ownerDoc.defaultView.QueryInterface(Ci.nsIInterfaceRequestor)
-                             .getInterface(Ci.nsIWebNavigation)
-                             .QueryInterface(Ci.nsIDocShell);
+      let docshell = ownerDoc.docShell;
       if (docShell.mixedContentChannel) {
         const sm = Services.scriptSecurityManager;
         try {
           let targetURI = Services.io.newURI(href);
           sm.checkSameOriginURI(docshell.mixedContentChannel.URI, targetURI, false);
           json.allowMixedContent = true;
         } catch (e) {}
       }
@@ -557,24 +563,25 @@ var ClickEventHandler = {
     }
 
     // This might be middle mouse navigation.
     if (event.button == 1) {
       sendAsyncMessage("Content:Click", json);
     }
   },
 
-  onCertError(targetElement, ownerDoc) {
-    let docShell = ownerDoc.defaultView.QueryInterface(Ci.nsIInterfaceRequestor)
-                                       .getInterface(Ci.nsIWebNavigation)
-                                       .QueryInterface(Ci.nsIDocShell);
+  onCertError(targetElement) {
+    let win = targetElement.ownerGlobal;
+    let frameId = WebNavigationFrames.getFrameId(win);
+    let docShell = win.document.docShell;
     sendAsyncMessage("Browser:CertExceptionError", {
-      location: ownerDoc.location.href,
+      frameId,
+      location: win.document.location.href,
       elementId: targetElement.getAttribute("id"),
-      isTopFrame: (ownerDoc.defaultView.parent === ownerDoc.defaultView),
+      isTopFrame: (win.parent === win),
       securityInfoAsString: getSerializedSecurityInfo(docShell),
     });
   },
 
   onAboutBlocked(targetElement, ownerDoc) {
     var reason = "phishing";
     if (/e=malwareBlocked/.test(ownerDoc.documentURI)) {
       reason = "malware";