Bug 1617987 - Fix URLs by prefixing www. when users encounter bad cert domain errors. r=nika,keeler

Differential Revision: https://phabricator.services.mozilla.com/D82024

build/pgo/certs/badCertDomain.certspec

@@ -0,0 +1,3 @@
+subject:www.badcertdomain.example.com
+issuer:printableString/CN=Temporary Certificate Authority/O=Mozilla Testing/OU=Profile Guided Optimization
+extension:subjectAlternativeName:www.badcertdomain.example.com

build/pgo/server-locations.txt

@@ -296,6 +296,13 @@ https://bad.include-subdomains.pinning-dynamic.example.com:443    privileged,cer
 https://badchain.include-subdomains.pinning.example.com:443       privileged,cert=staticPinningBad
 https://fail-handshake.example.com:443                            privileged,failHandshake
 
+# Host for bad cert domain fixup test
+https://badcertdomain.example.com:443                             privileged,cert=badCertDomain
+https://www.badcertdomain.example.com:443                         privileged,cert=badCertDomain
+https://127.0.0.3:433                                             privileged,cert=badCertDomain
+https://badcertdomain.example.com:82                              privileged,cert=badCertDomain
+https://mismatch.badcertdomain.example.com:443                    privileged,cert=badCertDomain
+
 # Hosts for sha1 console warning tests
 https://sha1ee.example.com:443                                    privileged,cert=sha1_end_entity
 https://sha256ee.example.com:443                                  privileged,cert=sha256_end_entity

docshell/base/nsDocShell.cpp

@@ -81,6 +81,7 @@
 #include "mozilla/net/DocumentChannel.h"
 #include "mozilla/net/ParentChannelWrapper.h"
 #include "mozilla/net/UrlClassifierFeatureFactory.h"
+#include "mozilla/dom/RTCCertificate.h"
 #include "ReferrerInfo.h"
 
 #include "nsIApplicationCacheChannel.h"
@@ -160,6 +161,7 @@
 #include "nsIWidget.h"
 #include "nsIWindowWatcher.h"
 #include "nsIWritablePropertyBag2.h"
+#include "nsIX509Cert.h"
 
 #include "nsCommandManager.h"
 #include "nsPIDOMWindow.h"
@@ -208,6 +210,7 @@
 #include "nsStructuredCloneContainer.h"
 #include "nsSubDocumentFrame.h"
 #include "nsURILoader.h"
+#include "nsURLHelper.h"
 #include "nsView.h"
 #include "nsViewManager.h"
 #include "nsViewSourceHandler.h"
@@ -216,11 +219,14 @@
 #include "nsWidgetsCID.h"
 #include "nsXULAppAPI.h"
 
+#include "BRNameMatchingPolicy.h"
 #include "GeckoProfiler.h"
 #include "mozilla/NullPrincipal.h"
 #include "Navigator.h"
 #include "prenv.h"
 #include "URIUtils.h"
+#include "sslerr.h"
+#include "mozpkix/pkix.h"
 
 #include "timeline/JavascriptTimelineMarker.h"
 #include "nsDocShellTelemetryUtils.h"
@@ -5669,6 +5675,137 @@ already_AddRefed<nsIURIFixupInfo> nsDocShell::KeywordToURI(
   return info.forget();
 }
 
+/* static */
+already_AddRefed<nsIURI> nsDocShell::MaybeFixBadCertDomainErrorURI(
+    nsIChannel* aChannel, nsIURI* aUrl) {
+  if (!aChannel) {
+    return nullptr;
+  }
+
+  nsresult rv = NS_OK;
+  nsAutoCString host;
+  rv = aUrl->GetAsciiHost(host);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return nullptr;
+  }
+
+  // No point in going further if "www." is included in the hostname
+  // already. That is the only hueristic we're applying in this function.
+  if (StringBeginsWith(host, "www."_ns)) {
+    return nullptr;
+  }
+
+  // Return if fixup enable pref is turned off.
+  if (!mozilla::StaticPrefs::security_bad_cert_domain_error_url_fix_enabled()) {
+    return nullptr;
+  }
+
+  // Return if scheme is not HTTPS.
+  if (!SchemeIsHTTPS(aUrl)) {
+    return nullptr;
+  }
+
+  nsCOMPtr<nsILoadInfo> info = aChannel->LoadInfo();
+  if (!info) {
+    return nullptr;
+  }
+
+  // Skip doing the fixup if our channel was redirected, because we
+  // shouldn't be guessing things about the post-redirect URI.
+  if (!info->RedirectChain().IsEmpty()) {
+    return nullptr;
+  }
+
+  int32_t port = 0;
+  rv = aUrl->GetPort(&port);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return nullptr;
+  }
+
+  // Don't fix up hosts with ports.
+  if (port != -1) {
+    return nullptr;
+  }
+
+  // Don't fix up localhost url.
+  if (host == "localhost") {
+    return nullptr;
+  }
+
+  // Don't fix up hostnames with IP address.
+  if (net_IsValidIPv4Addr(host) || net_IsValidIPv6Addr(host)) {
+    return nullptr;
+  }
+
+  nsAutoCString userPass;
+  rv = aUrl->GetUserPass(userPass);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return nullptr;
+  }
+
+  // Security - URLs with user / password info should NOT be modified.
+  if (!userPass.IsEmpty()) {
+    return nullptr;
+  }
+
+  nsCOMPtr<nsISupports> securityInfo;
+  rv = aChannel->GetSecurityInfo(getter_AddRefs(securityInfo));
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return nullptr;
+  }
+
+  nsCOMPtr<nsITransportSecurityInfo> tsi = do_QueryInterface(securityInfo);
+  if (NS_WARN_IF(!tsi)) {
+    return nullptr;
+  }
+
+  nsCOMPtr<nsIX509Cert> cert;
+  rv = tsi->GetServerCert(getter_AddRefs(cert));
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return nullptr;
+  }
+
+  nsTArray<uint8_t> certBytes;
+  rv = cert->GetRawDER(certBytes);
+  if (NS_FAILED(rv)) {
+    return nullptr;
+  }
+
+  mozilla::pkix::Input serverCertInput;
+  mozilla::pkix::Result rv1 =
+      serverCertInput.Init(certBytes.Elements(), certBytes.Length());
+  if (rv1 != mozilla::pkix::Success) {
+    return nullptr;
+  }
+
+  nsAutoCString newHost("www."_ns);
+  newHost.Append(host);
+
+  mozilla::pkix::Input newHostInput;
+  rv1 = newHostInput.Init(
+      BitwiseCast<const uint8_t*, const char*>(newHost.BeginReading()),
+      newHost.Length());
+  if (rv1 != mozilla::pkix::Success) {
+    return nullptr;
+  }
+
+  // Check if adding a "www." prefix to the request's hostname will
+  // cause the response's certificate to match.
+  mozilla::psm::BRNameMatchingPolicy nameMatchingPolicy(
+      mozilla::psm::BRNameMatchingPolicy::Mode::Enforce);
+  rv1 = mozilla::pkix::CheckCertHostname(serverCertInput, newHostInput,
+                                         nameMatchingPolicy);
+  if (rv1 != mozilla::pkix::Success) {
+    return nullptr;
+  }
+
+  nsCOMPtr<nsIURI> newURI;
+  Unused << NS_MutateURI(aUrl).SetHost(newHost).Finalize(
+      getter_AddRefs(newURI));
+
+  return newURI.forget();
+}
+
 /* static */
 already_AddRefed<nsIURI> nsDocShell::AttemptURIFixup(
     nsIChannel* aChannel, nsresult aStatus,
@@ -5676,7 +5813,9 @@ already_AddRefed<nsIURI> nsDocShell::AttemptURIFixup(
     bool aIsTopFrame, bool aAllowKeywordFixup, bool aUsePrivateBrowsing,
     bool aNotifyKeywordSearchLoading, nsIInputStream** aNewPostData) {
   if (aStatus != NS_ERROR_UNKNOWN_HOST && aStatus != NS_ERROR_NET_RESET &&
-      aStatus != NS_ERROR_CONNECTION_REFUSED) {
+      aStatus != NS_ERROR_CONNECTION_REFUSED &&
+      aStatus !=
+          mozilla::psm::GetXPCOMFromNSSError(SSL_ERROR_BAD_CERT_DOMAIN)) {
     return nullptr;
   }
 
@@ -5841,6 +5980,15 @@ already_AddRefed<nsIURI> nsDocShell::AttemptURIFixup(
     }
   }
 
+  // If we have a SSL_ERROR_BAD_CERT_DOMAIN error, try prefixing the domain name
+  // with www. to see if we can avoid showing the cert error page. For example,
+  // https://example.com -> https://www.example.com.
+  if (aStatus ==
+      mozilla::psm::GetXPCOMFromNSSError(SSL_ERROR_BAD_CERT_DOMAIN)) {
+    newPostData = nullptr;
+    newURI = MaybeFixBadCertDomainErrorURI(aChannel, url);
+  }
+
   // Did we make a new URI that is different to the old one? If so
   // load it.
   //

docshell/base/nsDocShell.h

@@ -444,6 +444,9 @@ class nsDocShell final : public nsDocLoader,
       bool aNotifyKeywordSearchLoading = false,
       nsIInputStream** aNewPostData = nullptr);
 
+  static already_AddRefed<nsIURI> MaybeFixBadCertDomainErrorURI(
+      nsIChannel* aChannel, nsIURI* aUrl);
+
   // Takes aStatus and filters out results that should not display
   // an error page.
   // If this returns a failed result, then we should display an error

docshell/test/browser/browser.ini

@@ -160,3 +160,4 @@ skip-if = (os == 'linux' && bits == 64 && os_version == '18.04') # Bug 1604237
 [browser_browsing_context_discarded.js]
 [browser_fall_back_to_https.js]
 skip-if = (os == 'mac')
+[browser_badCertDomainFixup.js]

docshell/test/browser/browser_badCertDomainFixup.js

@@ -0,0 +1,88 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// This test checks if we are correctly fixing https URLs by prefixing
+// with www. when we encounter a SSL_ERROR_BAD_CERT_DOMAIN error.
+// For example, https://example.com -> https://www.example.com.
+
+const PREF_BAD_CERT_DOMAIN_FIX_ENABLED =
+  "security.bad_cert_domain_error.url_fix_enabled";
+
+const BAD_CERT_DOMAIN_ERROR_URL = "https://badcertdomain.example.com:443";
+const FIXED_URL = "https://www.badcertdomain.example.com/";
+
+const BAD_CERT_DOMAIN_ERROR_URL2 =
+  "https://mismatch.badcertdomain.example.com:443";
+const IPV4_ADDRESS = "https://127.0.0.3:433";
+const BAD_CERT_DOMAIN_ERROR_PORT = "https://badcertdomain.example.com:82";
+
+async function verifyErrorPage(errorPageURL) {
+  let certErrorLoaded = BrowserTestUtils.waitForErrorPage(
+    gBrowser.selectedBrowser
+  );
+  BrowserTestUtils.loadURI(gBrowser, errorPageURL);
+  await certErrorLoaded;
+
+  await SpecialPowers.spawn(gBrowser.selectedBrowser, [], async function() {
+    let ec;
+    await ContentTaskUtils.waitForCondition(() => {
+      ec = content.document.getElementById("errorCode");
+      return ec.textContent;
+    }, "Error code has been set inside the advanced button panel");
+    is(
+      ec.textContent,
+      "SSL_ERROR_BAD_CERT_DOMAIN",
+      "Correct error code is shown"
+    );
+  });
+}
+
+// Test that "www." is prefixed to a https url when we encounter a bad cert domain
+// error if the "www." form is included in the certificate's subjectAltNames.
+add_task(async function prefixBadCertDomain() {
+  // Turn off the pref and ensure that we show the error page as expected.
+  Services.prefs.setBoolPref(PREF_BAD_CERT_DOMAIN_FIX_ENABLED, false);
+
+  gBrowser.selectedTab = BrowserTestUtils.addTab(gBrowser);
+  await verifyErrorPage(BAD_CERT_DOMAIN_ERROR_URL);
+  info("Cert error is shown as expected when the fixup pref is disabled");
+
+  // Turn on the pref and test that we fix the HTTPS URL.
+  Services.prefs.setBoolPref(PREF_BAD_CERT_DOMAIN_FIX_ENABLED, true);
+  gBrowser.selectedTab = BrowserTestUtils.addTab(gBrowser);
+  let loadSuccessful = BrowserTestUtils.browserLoaded(
+    gBrowser.selectedBrowser,
+    false,
+    FIXED_URL
+  );
+  BrowserTestUtils.loadURI(gBrowser, BAD_CERT_DOMAIN_ERROR_URL);
+  await loadSuccessful;
+
+  info("The URL was fixed as expected");
+
+  BrowserTestUtils.removeTab(gBrowser.selectedTab);
+  BrowserTestUtils.removeTab(gBrowser.selectedTab);
+});
+
+// Test that we don't prefix "www." to a https url when we encounter a bad cert domain
+// error under certain conditions.
+add_task(async function ignoreBadCertDomain() {
+  Services.prefs.setBoolPref(PREF_BAD_CERT_DOMAIN_FIX_ENABLED, true);
+  gBrowser.selectedTab = BrowserTestUtils.addTab(gBrowser);
+
+  // Test for when "www." form is not present in the certificate.
+  await verifyErrorPage(BAD_CERT_DOMAIN_ERROR_URL2);
+  info("Certificate error was shown as expected");
+
+  // Test that urls with IP addresses are not fixed.
+  await verifyErrorPage(IPV4_ADDRESS);
+  info("Certificate error was shown as expected for an IP address");
+
+  // Test that urls with ports are not fixed.
+  await verifyErrorPage(BAD_CERT_DOMAIN_ERROR_PORT);
+  info("Certificate error was shown as expected for a host with port");
+
+  BrowserTestUtils.removeTab(gBrowser.selectedTab);
+});

modules/libpref/init/StaticPrefList.yaml

@@ -8790,6 +8790,11 @@
   value: false
   mirror: always
 
+- name: security.bad_cert_domain_error.url_fix_enabled
+  type: bool
+  value: true
+  mirror: always
+
 - name: security.csp.enable
   type: bool
   value: true