Bug 1614969 - Check download with MixedContentBlocker r=ckerschb
☠☠ backed out by 70c5a2c012c9 ☠ ☠
authorSebastian Streich <sstreich@mozilla.com>
Tue, 23 Jun 2020 11:30:28 +0000
changeset 536878 3685f83e0dc032341ab9442f90bef92dd4aa7ea2
parent 536877 18727b87e5091ceb4ac2894228e641c3fa8d41fb
child 536879 35ea60e019d1210c9b2094801549963398bb1ad3
push id37533
push userdluca@mozilla.com
push dateTue, 23 Jun 2020 21:38:40 +0000
treeherdermozilla-central@d48aa0f0aa0b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersckerschb
bugs1614969
milestone79.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 1614969 - Check download with MixedContentBlocker r=ckerschb Differential Revision: https://phabricator.services.mozilla.com/D73302
dom/locales/en-US/chrome/security/security.properties
dom/security/nsContentSecurityUtils.cpp
dom/security/nsContentSecurityUtils.h
dom/security/test/mixedcontentblocker/browser.ini
dom/security/test/mixedcontentblocker/browser_test_mixed_content_download.js
dom/security/test/mixedcontentblocker/download_page.html
dom/security/test/mixedcontentblocker/download_server.sjs
dom/security/test/moz.build
modules/libpref/init/StaticPrefList.yaml
uriloader/exthandler/nsExternalHelperAppService.cpp
--- a/dom/locales/en-US/chrome/security/security.properties
+++ b/dom/locales/en-US/chrome/security/security.properties
@@ -40,16 +40,19 @@ STSCouldNotSaveState=Strict-Transport-Se
 SHA1Sig=This site makes use of a SHA-1 Certificate; it’s recommended you use certificates with signature algorithms that use hash functions stronger than SHA-1.
 InsecurePasswordsPresentOnPage=Password fields present on an insecure (http://) page. This is a security risk that allows user login credentials to be stolen.
 InsecureFormActionPasswordsPresent=Password fields present in a form with an insecure (http://) form action. This is a security risk that allows user login credentials to be stolen.
 InsecurePasswordsPresentOnIframe=Password fields present on an insecure (http://) iframe. This is a security risk that allows user login credentials to be stolen.
 # LOCALIZATION NOTE: "%1$S" is the URI of the insecure mixed content resource
 LoadingMixedActiveContent2=Loading mixed (insecure) active content “%1$S” on a secure page
 LoadingMixedDisplayContent2=Loading mixed (insecure) display content “%1$S” on a secure page
 LoadingMixedDisplayObjectSubrequestDeprecation=Loading mixed (insecure) content “%1$S” within a plugin on a secure page is discouraged and will be blocked soon.
+# LOCALIZATION NOTE: "%S" is the URI of the insecure mixed content download
+MixedContentBlockedDownload = Blocked downloading insecure content “%1$S”.
+
 # LOCALIZATION NOTE: Do not translate "allow-scripts", "allow-same-origin", "sandbox" or "iframe"
 BothAllowScriptsAndSameOriginPresent=An iframe which has both allow-scripts and allow-same-origin for its sandbox attribute can remove its sandboxing.
 # LOCALIZATION NOTE: Do not translate "allow-top-navigation-by-user-activation", "allow-top-navigation", "sandbox" or "iframe"
 BothAllowTopNavigationAndUserActivationPresent=An iframe which has both allow-top-navigation and allow-top-navigation-by-user-activation for its sandbox attribute will permit top navigations.
 
 # Sub-Resource Integrity
 # LOCALIZATION NOTE: Do not translate "script" or "integrity". "%1$S" is the invalid token found in the attribute.
 MalformedIntegrityHash=The script element has a malformed hash in its integrity attribute: “%1$S”. The correct format is “<hash algorithm>-<hash value>”.
--- a/dom/security/nsContentSecurityUtils.cpp
+++ b/dom/security/nsContentSecurityUtils.cpp
@@ -17,16 +17,17 @@
 #  include "WinUtils.h"
 #  include <wininet.h>
 #endif
 
 #include "js/Array.h"  // JS::GetArrayLength
 #include "mozilla/ExtensionPolicyService.h"
 #include "mozilla/Logging.h"
 #include "mozilla/dom/Document.h"
+#include "LoadInfo.h"
 #include "mozilla/StaticPrefs_extensions.h"
 #include "mozilla/StaticPrefs_dom.h"
 
 // Helper function for IsConsideredSameOriginForUIR which makes
 // Principals of scheme 'http' return Principals of scheme 'https'.
 static already_AddRefed<nsIPrincipal> MakeHTTPPrincipalHTTPS(
     nsIPrincipal* aPrincipal) {
   nsCOMPtr<nsIPrincipal> principal = aPrincipal;
@@ -1031,8 +1032,83 @@ bool nsContentSecurityUtils::ValidateScr
   Telemetry::RecordEvent(eventType, mozilla::Some(fileNameTypeAndDetails.first),
                          extra);
 
   // Presently we are not enforcing any restrictions for the script filename,
   // we're only reporting Telemetry. In the future we will assert in debug
   // builds and return false to prevent execution in non-debug builds.
   return true;
 }
+
+/* static */
+void nsContentSecurityUtils::LogMessageToConsole(nsIHttpChannel* aChannel,
+                                                 const char* aMsg) {
+  nsCOMPtr<nsIURI> uri;
+  nsresult rv = aChannel->GetURI(getter_AddRefs(uri));
+  if (NS_FAILED(rv)) {
+    return;
+  }
+
+  uint64_t windowID = 0;
+  rv = aChannel->GetTopLevelContentWindowId(&windowID);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return;
+  }
+  if (!windowID) {
+    nsCOMPtr<nsILoadInfo> loadinfo;
+    aChannel->GetLoadInfo(getter_AddRefs(loadinfo));
+    loadinfo->GetInnerWindowID(&windowID);
+  }
+
+  nsAutoString localizedMsg;
+  nsAutoCString spec;
+  uri->GetSpec(spec);
+  AutoTArray<nsString, 1> params = {NS_ConvertUTF8toUTF16(spec)};
+  rv = nsContentUtils::FormatLocalizedString(
+      nsContentUtils::eSECURITY_PROPERTIES, aMsg, params, localizedMsg);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return;
+  }
+
+  nsContentUtils::ReportToConsoleByWindowID(
+      localizedMsg, nsIScriptError::warningFlag, NS_LITERAL_CSTRING("Security"),
+      windowID, uri);
+}
+
+/* static */
+bool nsContentSecurityUtils::IsDownloadAllowed(
+    nsIChannel* aChannel, const nsAutoCString& aMimeTypeGuess) {
+  MOZ_ASSERT(aChannel, "IsDownloadAllowed without channel?");
+  if (!StaticPrefs::dom_block_download_insecure()) {
+    return true;
+  }
+
+  nsCOMPtr<nsILoadInfo> loadInfo = aChannel->LoadInfo();
+  if (loadInfo->TriggeringPrincipal()->IsSystemPrincipal()) {
+    return true;
+  }
+
+  nsCOMPtr<nsIURI> contentLocation;
+  aChannel->GetURI(getter_AddRefs(contentLocation));
+
+  nsCOMPtr<nsIPrincipal> loadingPrincipal = loadInfo->GetLoadingPrincipal();
+  // Creating a fake Loadinfo that is just used for the MCB check.
+  nsCOMPtr<nsILoadInfo> secCheckLoadInfo =
+      new LoadInfo(loadingPrincipal, loadInfo->TriggeringPrincipal(), nullptr,
+                   nsILoadInfo::SEC_ONLY_FOR_EXPLICIT_CONTENTSEC_CHECK,
+                   nsIContentPolicy::TYPE_OTHER);
+
+  int16_t decission = nsIContentPolicy::ACCEPT;
+  nsMixedContentBlocker::ShouldLoad(false,  //  aHadInsecureImageRedirect
+                                    contentLocation,   //  aContentLocation,
+                                    secCheckLoadInfo,  //  aLoadinfo
+                                    aMimeTypeGuess,    //  aMimeGuess,
+                                    &decission         // aDecision
+  );
+  if (decission == nsIContentPolicy::ACCEPT) {
+    return true;
+  }
+  nsCOMPtr<nsIHttpChannel> httpChannel = do_QueryInterface(aChannel);
+  if (httpChannel) {
+    LogMessageToConsole(httpChannel, "MixedContentBlockedDownload");
+  }
+  return false;
+}
\ No newline at end of file
--- a/dom/security/nsContentSecurityUtils.h
+++ b/dom/security/nsContentSecurityUtils.h
@@ -45,16 +45,22 @@ class nsContentSecurityUtils {
       nsIChannel* aChannel, nsIHttpChannel** aHttpChannel);
 
   // Helper function which performs the following framing checks
   // * CSP frame-ancestors
   // * x-frame-options
   // If any of the two disallows framing, the channel will be cancelled.
   static void PerformCSPFrameAncestorAndXFOCheck(nsIChannel* aChannel);
 
+  // Helper function to Check if a Download is allowed;
+  static bool IsDownloadAllowed(nsIChannel* aChannel,
+                                const nsAutoCString& aMimeTypeGuess);
+  // Logs an Error Message to the Console
+  static void LogMessageToConsole(nsIHttpChannel* aChannel, const char* aMsg);
+
 #if defined(DEBUG)
   static void AssertAboutPageHasCSP(mozilla::dom::Document* aDocument);
 #endif
 
   static bool ValidateScriptFilename(const char* aFilename,
                                      bool aIsSystemRealm);
 };
 
new file mode 100644
--- /dev/null
+++ b/dom/security/test/mixedcontentblocker/browser.ini
@@ -0,0 +1,6 @@
+[DEFAULT]
+support-files =
+  download_page.html
+  download_server.sjs
+
+[browser_test_mixed_content_download.js]
new file mode 100644
--- /dev/null
+++ b/dom/security/test/mixedcontentblocker/browser_test_mixed_content_download.js
@@ -0,0 +1,98 @@
+let INSECURE_BASE_URL =
+  getRootDirectory(gTestPath).replace(
+    "chrome://mochitests/content/",
+    "http://example.com/"
+  ) + "download_page.html";
+let SECURE_BASE_URL =
+  getRootDirectory(gTestPath).replace(
+    "chrome://mochitests/content/",
+    "https://example.com/"
+  ) + "download_page.html";
+
+function shouldPromptDownload() {
+  // Waits until the download Prompt is shown
+  return new Promise((resolve, reject) => {
+    Services.wm.addListener({
+      onOpenWindow(xulWin) {
+        Services.wm.removeListener(this);
+        let win = xulWin.docShell.domWindow;
+        waitForFocus(() => {
+          if (
+            win.location ==
+            "chrome://mozapps/content/downloads/unknownContentType.xhtml"
+          ) {
+            resolve();
+            info("Trying to close window");
+            win.close();
+          } else {
+            reject();
+          }
+        }, win);
+      },
+    });
+  });
+}
+
+const CONSOLE_ERROR_MESSAGE = "was blocked because it was insecure.";
+
+function shouldConsoleError() {
+  // Waits until CONSOLE_ERROR_MESSAGE was logged
+  return new Promise((resolve, reject) => {
+    function listener(msgObj) {
+      let text = msgObj.message;
+      if (text.includes(CONSOLE_ERROR_MESSAGE)) {
+        Services.console.unregisterListener(listener);
+        resolve();
+      }
+    }
+    Services.console.registerListener(listener);
+  });
+}
+
+async function runTest(url, link, check, decscription) {
+  let tab = BrowserTestUtils.addTab(gBrowser, url);
+  gBrowser.selectedTab = tab;
+
+  let browser = gBrowser.getBrowserForTab(tab);
+  await BrowserTestUtils.browserLoaded(browser);
+
+  info("Checking: " + decscription);
+
+  let checkPromise = check();
+  // Click the Link to trigger the download
+  SpecialPowers.spawn(gBrowser.selectedBrowser, [link], contentLink => {
+    content.document.getElementById(contentLink).click();
+  });
+
+  await checkPromise;
+
+  ok(true, decscription);
+  BrowserTestUtils.removeTab(tab);
+}
+
+add_task(async function() {
+  await runTest(
+    INSECURE_BASE_URL,
+    "insecure",
+    shouldPromptDownload,
+    "Insecure -> Insecure should download"
+  );
+  await runTest(
+    INSECURE_BASE_URL,
+    "secure",
+    shouldPromptDownload,
+    "Insecure -> Secure should download"
+  );
+  await runTest(
+    SECURE_BASE_URL,
+    "insecure",
+    shouldConsoleError,
+    "Secure -> Insecure should Error"
+  );
+  await runTest(
+    SECURE_BASE_URL,
+    "secure",
+    shouldPromptDownload,
+    "Secure -> Secure should Download"
+  );
+});
new file mode 100644
--- /dev/null
+++ b/dom/security/test/mixedcontentblocker/download_page.html
@@ -0,0 +1,35 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=676619
+-->
+  <head>
+
+    <title>Test for the download attribute</title>
+
+  </head>
+  <body>
+    hi
+
+    <script>
+      const host = window.location.host;
+      const path = location.pathname.replace("download_page.html","download_server.sjs");
+
+      const secureLink = document.createElement("a");
+      secureLink.href=`https://${host}/${path}`;
+      secureLink.download="true";
+      secureLink.textContent="Secure Link";
+      
+      secureLink.id="secure";
+
+      const insecureLink = document.createElement("a");        
+      insecureLink.href=`http://${host}/${path}`;
+      insecureLink.download="true";
+      insecureLink.id="insecure";
+      insecureLink.textContent="Not secure Link";
+
+      document.body.append(secureLink);
+      document.body.append(insecureLink);
+  </script>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/dom/security/test/mixedcontentblocker/download_server.sjs
@@ -0,0 +1,9 @@
+// force the Browser to Show a Download Prompt
+
+function handleRequest(request, response)
+{
+  response.setHeader("Cache-Control", "no-cache", false);
+  response.setHeader("Content-Disposition", "attachment");
+  response.setHeader("Content-Type", "application/octet-stream");
+  response.write('🙈🙊🐵🙊');
+}
--- a/dom/security/test/moz.build
+++ b/dom/security/test/moz.build
@@ -29,9 +29,10 @@ MOCHITEST_CHROME_MANIFESTS += [
     'general/chrome.ini',
 ]
 
 BROWSER_CHROME_MANIFESTS += [
     'cors/browser.ini',
     'csp/browser.ini',
     'general/browser.ini',
     'https-only/browser.ini',
+    'mixedcontentblocker/browser.ini'
 ]
--- a/modules/libpref/init/StaticPrefList.yaml
+++ b/modules/libpref/init/StaticPrefList.yaml
@@ -1493,16 +1493,22 @@
   mirror: always
 
 # Block multiple external protocol URLs in iframes per single event.
 - name: dom.block_external_protocol_in_iframes
   type: bool
   value: true
   mirror: always
 
+# Block Insecure downloads from Secure Origins
+- name: dom.block_download_insecure
+  type: bool
+  value: @IS_NIGHTLY_BUILD@
+  mirror: always
+
 # Block multiple window.open() per single event.
 - name: dom.block_multiple_popups
   type: bool
   value: true
   mirror: always
 
 # The maximum number of popup that is allowed to be opened. Set to -1 for no
 # limit.
--- a/uriloader/exthandler/nsExternalHelperAppService.cpp
+++ b/uriloader/exthandler/nsExternalHelperAppService.cpp
@@ -41,16 +41,17 @@
 #include "nsIRequest.h"
 #include "nsDirectoryServiceDefs.h"
 #include "nsIInterfaceRequestor.h"
 #include "nsThreadUtils.h"
 #include "nsIMutableArray.h"
 #include "nsIRedirectHistoryEntry.h"
 #include "nsOSHelperAppService.h"
 #include "nsOSHelperAppServiceChild.h"
+#include "nsContentSecurityUtils.h"
 
 // used to access our datastore of user-configured helper applications
 #include "nsIHandlerService.h"
 #include "nsIMIMEInfo.h"
 #include "nsIHelperAppLauncherDialog.h"
 #include "nsIContentDispatchChooser.h"
 #include "nsNetUtil.h"
 #include "nsIPrivateBrowsingChannel.h"
@@ -1552,16 +1553,24 @@ NS_IMETHODIMP nsExternalAppHandler::OnSt
   // we want to record the start time before showing the filepicker.
   mTimeDownloadStarted = PR_Now();
 
   mRequest = request;
 
   nsCOMPtr<nsIChannel> aChannel = do_QueryInterface(request);
 
   nsresult rv;
+  nsAutoCString MIMEType;
+  mMimeInfo->GetMIMEType(MIMEType);
+
+  if (!nsContentSecurityUtils::IsDownloadAllowed(aChannel, MIMEType)) {
+    mCanceled = true;
+    request->Cancel(NS_ERROR_ABORT);
+    return NS_OK;
+  }
 
   nsCOMPtr<nsIFileChannel> fileChan(do_QueryInterface(request));
   mIsFileChannel = fileChan != nullptr;
   if (!mIsFileChannel) {
     // It's possible that this request came from the child process and the
     // file channel actually lives there. If this returns true, then our
     // mSourceUrl will be an nsIFileURL anyway.
     nsCOMPtr<dom::nsIExternalHelperAppParent> parent(
@@ -1572,17 +1581,16 @@ NS_IMETHODIMP nsExternalAppHandler::OnSt
   // Get content length
   if (aChannel) {
     aChannel->GetContentLength(&mContentLength);
   }
 
   if (mBrowsingContext) {
     mMaybeCloseWindowHelper = new MaybeCloseWindowHelper(mBrowsingContext);
     mMaybeCloseWindowHelper->SetShouldCloseWindow(mShouldCloseWindow);
-
     nsCOMPtr<nsIPropertyBag2> props(do_QueryInterface(request, &rv));
     // Determine whether a new window was opened specifically for this request
     if (props) {
       bool tmp = false;
       if (NS_SUCCEEDED(props->GetPropertyAsBool(
               NS_LITERAL_STRING("docshell.newWindowTarget"), &tmp))) {
         mMaybeCloseWindowHelper->SetShouldCloseWindow(tmp);
       }
@@ -1661,18 +1669,16 @@ NS_IMETHODIMP nsExternalAppHandler::OnSt
   // is one part of a multipart document).  Opening sniffed content in helper
   // apps by default introduces security holes that we'd rather not have.
 
   // So let's find out whether the user wants to be prompted.  If he does not,
   // check mReason and the preferred action to see what we should do.
 
   bool alwaysAsk = true;
   mMimeInfo->GetAlwaysAskBeforeHandling(&alwaysAsk);
-  nsAutoCString MIMEType;
-  mMimeInfo->GetMIMEType(MIMEType);
   if (alwaysAsk) {
     // But we *don't* ask if this mimeInfo didn't come from
     // our user configuration datastore and the user has said
     // at some point in the distant past that they don't
     // want to be asked.  The latter fact would have been
     // stored in pref strings back in the old days.
 
     bool mimeTypeIsInDatastore = false;