Bug 1297630 - Make certificate error pages work properly in iframes. r=Gijs draft
authorJohann Hofmann <jhofmann@mozilla.com>
Thu, 15 Mar 2018 13:55:13 +0100
changeset 768781 135cd9abe570736b80587a018ba5caba59247e10
parent 768549 90390afd294cb7254ff3c8249fa84a43ad6a517d
push id102979
push userjhofmann@mozilla.com
push dateFri, 16 Mar 2018 20:38:04 +0000
reviewersGijs
bugs1297630, 1446319
milestone61.0a1
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
@@ -3002,17 +3002,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);
@@ -3047,17 +3048,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);
         }
@@ -3098,19 +3099,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,44 +345,49 @@ 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);
+      this.onPageLoad(aEvent.originalTarget, doc.defaultView);
       break;
     case "AboutNetErrorOpenCaptivePortal":
       this.openCaptivePortalPage(aEvent);
       break;
     case "AboutNetErrorSetAutomatic":
       this.onSetAutomatic(aEvent);
       break;
     case "AboutNetErrorResetPreferences":
@@ -397,28 +404,26 @@ var AboutNetAndCertErrorListener = {
       if (Services.prefs.prefHasUserValue(prefName)) {
         return true;
       }
     }
 
     return false;
   },
 
-  onPageLoad(evt) {
+  onPageLoad(originalTarget, win) {
     // 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);
+    if (this.isAboutCertError(win.document)) {
+      ClickEventHandler.onCertError(originalTarget, win);
     }
 
     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 +442,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 +476,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, ownerDoc.defaultView);
         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 +536,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 +558,23 @@ 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, win) {
+    let docShell = win.document.docShell;
     sendAsyncMessage("Browser:CertExceptionError", {
-      location: ownerDoc.location.href,
+      frameId: WebNavigationFrames.getFrameId(win),
+      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";