SSL Error page for domain mismatch should hyperlink to correct site (sometimes). b=402210 r=gavin r=kengert r=axel ui-r=beltzner moa/sr=biesi a=beltzner Significant chunks of p=timeless
authorjohnath@mozilla.com
Wed, 30 Apr 2008 13:10:22 -0700
changeset 14821 984ab19fafd635ddac3373ae78975477b9a81d17
parent 14820 d0f5d0a69f133d4c6596dc843abcfd324b087cbf
child 14822 16dd53c68d5396e5adab78e9e88b833641c67145
push id18
push userbsmedberg@mozilla.com
push dateThu, 01 May 2008 13:52:17 +0000
treeherdermozilla-central@22552b22344c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgavin, kengert, axel, beltzner, beltzner
bugs402210
milestone1.9pre
SSL Error page for domain mismatch should hyperlink to correct site (sometimes). b=402210 r=gavin r=kengert r=axel ui-r=beltzner moa/sr=biesi a=beltzner Significant chunks of p=timeless
docshell/resources/content/netError.xhtml
docshell/test/Makefile.in
docshell/test/test_bug402210.html
security/manager/locales/en-US/chrome/pipnss/pipnss.properties
--- a/docshell/resources/content/netError.xhtml
+++ b/docshell/resources/content/netError.xhtml
@@ -123,29 +123,16 @@
 
         buttonEl.disabled = true;
       }
 
       function initPage()
       {
         var err = getErrorCode();
         
-        if (err == "nssBadCert") {
-          // Remove the "Try again" button for security exceptions, since it's
-          // almost certainly useless.
-          document.getElementById("errorTryAgain").style.display = "none";
-          document.getElementById("errorPageContainer").setAttribute("class", "certerror");
-        }
-        else {
-          // Remove the override block for non-certificate errors.  CSS-hiding
-          // isn't good enough here, because of bug 39098
-          var secOverride = document.getElementById("securityOverrideDiv");
-          secOverride.parentNode.removeChild(secOverride);
-        }
-
         // if it's an unknown error or there's no title or description
         // defined, get the generic message
         var errTitle = document.getElementById("et_" + err);
         var errDesc  = document.getElementById("ed_" + err);
         if (!errTitle || !errDesc)
         {
           errTitle = document.getElementById("et_generic");
           errDesc  = document.getElementById("ed_generic");
@@ -185,23 +172,87 @@
           // favicon.  In order to trigger the browser to repaint though, we
           // need to remove/add the link element. 
           var favicon = document.getElementById("favicon");
           var faviconParent = favicon.parentNode;
           faviconParent.removeChild(favicon);
           favicon.setAttribute("href", "chrome://global/skin/icons/" + className + "_favicon.png");
           faviconParent.appendChild(favicon);
         }
+        
+        if (err == "nssBadCert") {
+          // Remove the "Try again" button for security exceptions, since it's
+          // almost certainly useless.
+          document.getElementById("errorTryAgain").style.display = "none";
+          document.getElementById("errorPageContainer").setAttribute("class", "certerror");
+          addDomainErrorLink();
+        }
+        else {
+          // Remove the override block for non-certificate errors.  CSS-hiding
+          // isn't good enough here, because of bug 39098
+          var secOverride = document.getElementById("securityOverrideDiv");
+          secOverride.parentNode.removeChild(secOverride);
+        }
       }
       
       function showSecuritySection() {
         // Swap link out, content in
         document.getElementById('securityOverrideContent').style.display = '';
         document.getElementById('securityOverrideLink').style.display = 'none';
       }
+      
+      /* In the case of SSL error pages about domain mismatch, see if
+         we can hyperlink the user to the correct site.  We don't want
+         to do this generically since it allows MitM attacks to redirect
+         users to a site under attacker control, but in certain cases
+         it is safe (and helpful!) to do so.  Bug 402210
+      */
+      function addDomainErrorLink() {
+        // Rather than textContent, we need to treat description as HTML
+        var sd = document.getElementById("errorShortDescText");
+        if (sd)
+          sd.innerHTML = getDescription();
+
+        var link = document.getElementById('cert_domain_link');
+        if (!link)
+          return;
+        
+        var okHost = link.getAttribute("title");
+        var thisHost = document.location.hostname;
+        var proto = document.location.protocol;
+
+        /* case #1: 
+         * example.com uses an invalid security certificate.
+         *
+         * The certificate is only valid for www.example.com
+         *
+         * Make sure to include the "." ahead of thisHost so that
+         * a MitM attack on paypal.com doesn't hyperlink to "notpaypal.com"
+         *
+         * We'd normally just use a RegExp here except that we lack a
+         * library function to escape them properly (bug 248062), and
+         * domain names are famous for having '.' characters in them,
+         * which would allow spurious and possibly hostile matches.
+         */
+        if (endsWith(okHost, "." + thisHost))
+          link.href = proto + okHost;
+
+        /* case #2:
+         * browser.garage.maemo.org uses an invalid security certificate.
+         *
+         * The certificate is only valid for garage.maemo.org
+         */
+        if (endsWith(thisHost, "." + okHost))
+          link.href = proto + okHost;
+      }
+      
+      function endsWith(haystack, needle) {
+        return haystack.slice(-needle.length) == needle;
+      }
+
     ]]></script>
   </head>
 
   <body dir="&locale.dir;">
 
     <!-- ERROR ITEM CONTAINER (removed during loading to avoid bug 39098) -->
     <div id="errorContainer">
       <div id="errorTitlesContainer">
--- a/docshell/test/Makefile.in
+++ b/docshell/test/Makefile.in
@@ -61,12 +61,13 @@ include $(topsrcdir)/config/rules.mk
 		bug369814.zip       \
 		test_bug384014.html \
 		test_bug387979.html \
 		test_bug404548.html \
 		bug404548-subframe.html \
 		test_bug413310.html \
 		bug413310-subframe.html \
 		bug413310-post.sjs \
+		test_bug402210.html \
 		$(NULL)
 
 libs:: $(_TEST_FILES)
 	$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)
new file mode 100644
--- /dev/null
+++ b/docshell/test/test_bug402210.html
@@ -0,0 +1,51 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+While working on bug 402210, it came up that the code was doing
+
+a.href = proto + host
+
+which technically produces "https:host" instead of "https://host" and
+that the code was relying on href's setting having fixup behaviour
+for this kind of thing.
+
+If we rely on it, we might as well test for it, even if it isn't the
+problem 402210 was meant to fix.
+
+https://bugzilla.mozilla.org/show_bug.cgi?id=402210
+-->
+<head>
+  <title>Test for Bug 402210</title>
+  <script type="text/javascript" src="/MochiKit/MochiKit.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=402210">Mozilla Bug 402210</a>
+<p id="display">
+  <a id="testlink">Test Link</a>
+</p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+
+SimpleTest.waitForExplicitFinish();
+
+function runTest() {
+  $("testlink").href = "https:example.com";
+  is($("testlink").href, "https://example.com/", "Setting href on an anchor tag should fixup missing slashes after https protocol");
+  
+  $("testlink").href = "ftp:example.com";
+  is($("testlink").href, "ftp://example.com/", "Setting href on an anchor tag should fixup missing slashes after non-http protocol");
+  
+  SimpleTest.finish();
+}
+
+addLoadEvent(runTest);
+</script>
+</pre>
+</body>
+</html>
+
--- a/security/manager/locales/en-US/chrome/pipnss/pipnss.properties
+++ b/security/manager/locales/en-US/chrome/pipnss/pipnss.properties
@@ -344,17 +344,18 @@ certErrorIntro=%S uses an invalid securi
 certErrorTrust_SelfSigned=The certificate is not trusted because it is self signed.
 certErrorTrust_UnknownIssuer=The certificate is not trusted because the issuer certificate is unknown.
 certErrorTrust_CaInvalid=The certificate is not trusted because it was issued by an invalid CA certificate.
 certErrorTrust_Issuer=The certificate is not trusted because the issuer certificate is not trusted.
 certErrorTrust_ExpiredIssuer=The certificate is not trusted because the issuer certificate has expired.
 certErrorTrust_Untrusted=The certificate does not come from a trusted source.
 
 certErrorMismatch=The certificate is not valid for the name %S.
-certErrorMismatchSingle2=The certificate is only valid for %S.
+# LOCALIZATION NOTE (certErrorMismatchSingle2): Do not translate <a id="cert_domain_link" title="%1$S">%1$S</a>
+certErrorMismatchSingle2=The certificate is only valid for <a id="cert_domain_link" title="%1$S">%1$S</a>
 certErrorMismatchMultiple=The certificate is only valid for the following names:
 certErrorMismatchNoNames=The certificate is not valid for any server names.
 
 certErrorExpired=The certificate expired on %S.
 certErrorNotYetValid=The certificate will not be valid until %S.
 
 certErrorCodePrefix=(Error code: %S)