Bug 1640405 - SameSite=lax/strict cookies should be sent to TYPE_SAVEAS_DOWNLOAD content-type, r=mayhemer

Differential Revision: https://phabricator.services.mozilla.com/D77298
2020-06-02 12:23:00 +00:00 · 2020-06-02 12:23:00 +00:00 · 2a98a3d66d
--- a/dom/security/test/general/test_same_site_cookies_toplevel_set_cookie.html
+++ b/dom/security/test/general/test_same_site_cookies_toplevel_set_cookie.html
@ -1,7 +1,7 @@
 <!DOCTYPE HTML>
 <html>
 <head>
-  <title>Bug 1454242: Setting samesite cookie should not rely on NS_IsSameSiteForeign</title>
+  <title>Bug 1454242: Setting samesite cookie should not rely on CookieCommons::IsSameSiteForeign</title>
  <script src="/tests/SimpleTest/SimpleTest.js"></script>
  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
 </head>
--- a/netwerk/base/nsNetUtil.cpp
+++ b/netwerk/base/nsNetUtil.cpp
@ -2056,18 +2056,6 @@ bool NS_HasBeenCrossOrigin(nsIChannel* aChannel, bool aReport) {
  return NS_FAILED(res);
 }

-bool NS_IsSafeTopLevelNav(nsIChannel* aChannel) {
-  if (!aChannel) {
-    return false;
-  }
-  nsCOMPtr<nsILoadInfo> loadInfo = aChannel->LoadInfo();
-  if (loadInfo->GetExternalContentPolicyType() !=
-      nsIContentPolicy::TYPE_DOCUMENT) {
-    return false;
-  }
-  return NS_IsSafeMethodNav(aChannel);
-}
-
 bool NS_IsSafeMethodNav(nsIChannel* aChannel) {
  RefPtr<HttpBaseChannel> baseChan = do_QueryObject(aChannel);
  if (!baseChan) {
@ -2080,75 +2068,6 @@ bool NS_IsSafeMethodNav(nsIChannel* aChannel) {
  return requestHead->IsSafeMethod();
 }

-bool NS_IsSameSiteForeign(nsIChannel* aChannel, nsIURI* aHostURI) {
-  if (!aChannel) {
-    return false;
-  }
-  nsCOMPtr<nsILoadInfo> loadInfo = aChannel->LoadInfo();
-  // Do not treat loads triggered by web extensions as foreign
-  nsCOMPtr<nsIURI> channelURI;
-  NS_GetFinalChannelURI(aChannel, getter_AddRefs(channelURI));
-  RefPtr<BasePrincipal> triggeringPrincipal =
-      BasePrincipal::Cast(loadInfo->TriggeringPrincipal());
-  if (triggeringPrincipal->AddonPolicy() &&
-      triggeringPrincipal->AddonAllowsLoad(channelURI)) {
-    return false;
-  }
-
-  bool isForeign = true;
-  nsresult rv;
-  if (loadInfo->GetExternalContentPolicyType() ==
-      nsIContentPolicy::TYPE_DOCUMENT) {
-    // for loads of TYPE_DOCUMENT we query the hostURI from the
-    // triggeringPrincipal which returns the URI of the document that caused the
-    // navigation.
-    rv = triggeringPrincipal->IsThirdPartyChannel(aChannel, &isForeign);
-  } else {
-    nsCOMPtr<mozIThirdPartyUtil> thirdPartyUtil =
-        do_GetService(THIRDPARTYUTIL_CONTRACTID);
-    if (!thirdPartyUtil) {
-      return true;
-    }
-    rv = thirdPartyUtil->IsThirdPartyChannel(aChannel, aHostURI, &isForeign);
-  }
-  // if we are dealing with a cross origin request, we can return here
-  // because we already know the request is 'foreign'.
-  if (NS_FAILED(rv) || isForeign) {
-    return true;
-  }
-
-  // for loads of TYPE_SUBDOCUMENT we have to perform an additional test,
-  // because a cross-origin iframe might perform a navigation to a same-origin
-  // iframe which would send same-site cookies. Hence, if the iframe navigation
-  // was triggered by a cross-origin triggeringPrincipal, we treat the load as
-  // foreign.
-  if (loadInfo->GetExternalContentPolicyType() ==
-      nsIContentPolicy::TYPE_SUBDOCUMENT) {
-    rv = loadInfo->TriggeringPrincipal()->IsThirdPartyChannel(aChannel,
-                                                              &isForeign);
-    if (NS_FAILED(rv) || isForeign) {
-      return true;
-    }
-  }
-
-  // for the purpose of same-site cookies we have to treat any cross-origin
-  // redirects as foreign. E.g. cross-site to same-site redirect is a problem
-  // with regards to CSRF.
-
-  nsCOMPtr<nsIPrincipal> redirectPrincipal;
-  for (nsIRedirectHistoryEntry* entry : loadInfo->RedirectChain()) {
-    entry->GetPrincipal(getter_AddRefs(redirectPrincipal));
-    if (redirectPrincipal) {
-      rv = redirectPrincipal->IsThirdPartyChannel(aChannel, &isForeign);
-      // if at any point we encounter a cross-origin redirect we can return.
-      if (NS_FAILED(rv) || isForeign) {
-        return true;
-      }
-    }
-  }
-  return isForeign;
-}
-
 bool NS_ShouldCheckAppCache(nsIPrincipal* aPrincipal) {
  uint32_t privateBrowsingId = 0;
  nsresult rv = aPrincipal->GetPrivateBrowsingId(&privateBrowsingId);
--- a/netwerk/base/nsNetUtil.h
+++ b/netwerk/base/nsNetUtil.h
@ -596,23 +596,11 @@ bool NS_UsePrivateBrowsing(nsIChannel* channel);
 */
 bool NS_HasBeenCrossOrigin(nsIChannel* aChannel, bool aReport = false);

-/**
- * Returns true if the channel is a safe top-level navigation.
- */
-bool NS_IsSafeTopLevelNav(nsIChannel* aChannel);
-
 /**
 * Returns true if the channel has a safe method.
 */
 bool NS_IsSafeMethodNav(nsIChannel* aChannel);

-/**
- * Returns true if the channel is a foreign with respect to the host-uri.
- * For loads of TYPE_DOCUMENT, this function returns true if it's a
- * cross origin navigation.
- */
-bool NS_IsSameSiteForeign(nsIChannel* aChannel, nsIURI* aHostURI);
-
 // Unique first-party domain for separating the safebrowsing cookie.
 // Note if this value is changed, code in test_cookiejars_safebrowsing.js and
 // nsUrlClassifierHashCompleter.js should also be changed.
--- a/netwerk/cookie/CookieCommons.cpp
+++ b/netwerk/cookie/CookieCommons.cpp
@ -17,6 +17,7 @@
 #include "nsICookiePermission.h"
 #include "nsICookieService.h"
 #include "nsIEffectiveTLDService.h"
+#include "nsIRedirectHistoryEntry.h"
 #include "nsScriptSecurityManager.h"

 constexpr auto CONSOLE_SCHEMEFUL_CATEGORY =
@ -460,6 +461,91 @@ bool CookieCommons::ShouldIncludeCrossSiteCookieForDocument(Cookie* aCookie) {
  return sameSiteAttr == nsICookie::SAMESITE_NONE;
 }

+bool CookieCommons::IsSafeTopLevelNav(nsIChannel* aChannel) {
+  if (!aChannel) {
+    return false;
+  }
+  nsCOMPtr<nsILoadInfo> loadInfo = aChannel->LoadInfo();
+  if (loadInfo->GetExternalContentPolicyType() !=
+          nsIContentPolicy::TYPE_DOCUMENT &&
+      loadInfo->GetExternalContentPolicyType() !=
+          nsIContentPolicy::TYPE_SAVEAS_DOWNLOAD) {
+    return false;
+  }
+  return NS_IsSafeMethodNav(aChannel);
+}
+
+bool CookieCommons::IsSameSiteForeign(nsIChannel* aChannel, nsIURI* aHostURI) {
+  if (!aChannel) {
+    return false;
+  }
+  nsCOMPtr<nsILoadInfo> loadInfo = aChannel->LoadInfo();
+  // Do not treat loads triggered by web extensions as foreign
+  nsCOMPtr<nsIURI> channelURI;
+  NS_GetFinalChannelURI(aChannel, getter_AddRefs(channelURI));
+  RefPtr<BasePrincipal> triggeringPrincipal =
+      BasePrincipal::Cast(loadInfo->TriggeringPrincipal());
+  if (triggeringPrincipal->AddonPolicy() &&
+      triggeringPrincipal->AddonAllowsLoad(channelURI)) {
+    return false;
+  }
+
+  bool isForeign = true;
+  nsresult rv;
+  if (loadInfo->GetExternalContentPolicyType() ==
+          nsIContentPolicy::TYPE_DOCUMENT ||
+      loadInfo->GetExternalContentPolicyType() ==
+          nsIContentPolicy::TYPE_SAVEAS_DOWNLOAD) {
+    // for loads of TYPE_DOCUMENT we query the hostURI from the
+    // triggeringPrincipal which returns the URI of the document that caused the
+    // navigation.
+    rv = triggeringPrincipal->IsThirdPartyChannel(aChannel, &isForeign);
+  } else {
+    nsCOMPtr<mozIThirdPartyUtil> thirdPartyUtil =
+        do_GetService(THIRDPARTYUTIL_CONTRACTID);
+    if (!thirdPartyUtil) {
+      return true;
+    }
+    rv = thirdPartyUtil->IsThirdPartyChannel(aChannel, aHostURI, &isForeign);
+  }
+  // if we are dealing with a cross origin request, we can return here
+  // because we already know the request is 'foreign'.
+  if (NS_FAILED(rv) || isForeign) {
+    return true;
+  }
+
+  // for loads of TYPE_SUBDOCUMENT we have to perform an additional test,
+  // because a cross-origin iframe might perform a navigation to a same-origin
+  // iframe which would send same-site cookies. Hence, if the iframe navigation
+  // was triggered by a cross-origin triggeringPrincipal, we treat the load as
+  // foreign.
+  if (loadInfo->GetExternalContentPolicyType() ==
+      nsIContentPolicy::TYPE_SUBDOCUMENT) {
+    rv = loadInfo->TriggeringPrincipal()->IsThirdPartyChannel(aChannel,
+                                                              &isForeign);
+    if (NS_FAILED(rv) || isForeign) {
+      return true;
+    }
+  }
+
+  // for the purpose of same-site cookies we have to treat any cross-origin
+  // redirects as foreign. E.g. cross-site to same-site redirect is a problem
+  // with regards to CSRF.
+
+  nsCOMPtr<nsIPrincipal> redirectPrincipal;
+  for (nsIRedirectHistoryEntry* entry : loadInfo->RedirectChain()) {
+    entry->GetPrincipal(getter_AddRefs(redirectPrincipal));
+    if (redirectPrincipal) {
+      rv = redirectPrincipal->IsThirdPartyChannel(aChannel, &isForeign);
+      // if at any point we encounter a cross-origin redirect we can return.
+      if (NS_FAILED(rv) || isForeign) {
+        return true;
+      }
+    }
+  }
+  return isForeign;
+}
+
 namespace {

 bool MaybeCompareSchemeInternal(Cookie* aCookie,
--- a/netwerk/cookie/CookieCommons.h
+++ b/netwerk/cookie/CookieCommons.h
@ -121,6 +121,15 @@ class CookieCommons final {
  static nsICookie::schemeType PrincipalToSchemeType(nsIPrincipal* aPrincipal);

  static nsICookie::schemeType SchemeToSchemeType(const nsACString& aScheme);
+
+  // Returns true if the channel is a safe top-level navigation or if it's a
+  // download request
+  static bool IsSafeTopLevelNav(nsIChannel* aChannel);
+
+  // Returns true if the channel is a foreign with respect to the host-uri.
+  // For loads of TYPE_DOCUMENT, this function returns true if it's a cross
+  // origin navigation.
+  static bool IsSameSiteForeign(nsIChannel* aChannel, nsIURI* aHostURI);
 };

 }  // namespace net
--- a/netwerk/cookie/CookieService.cpp
+++ b/netwerk/cookie/CookieService.cpp
@ -412,8 +412,8 @@ CookieService::GetCookieStringFromHttp(nsIURI* aHostURI, nsIChannel* aChannel,
  StoragePrincipalHelper::GetOriginAttributes(
      aChannel, attrs, StoragePrincipalHelper::eStorageAccessPrincipal);

-  bool isSafeTopLevelNav = NS_IsSafeTopLevelNav(aChannel);
-  bool isSameSiteForeign = NS_IsSameSiteForeign(aChannel, aHostURI);
+  bool isSafeTopLevelNav = CookieCommons::IsSafeTopLevelNav(aChannel);
+  bool isSameSiteForeign = CookieCommons::IsSameSiteForeign(aChannel, aHostURI);

  AutoTArray<Cookie*, 8> foundCookieList;
  GetCookiesForURI(
--- a/netwerk/cookie/CookieServiceChild.cpp
+++ b/netwerk/cookie/CookieServiceChild.cpp
@ -135,8 +135,8 @@ void CookieServiceChild::TrackCookieLoad(nsIChannel* aChannel) {
  StoragePrincipalHelper::PrepareEffectiveStoragePrincipalOriginAttributes(
      aChannel, attrs);

-  bool isSafeTopLevelNav = NS_IsSafeTopLevelNav(aChannel);
-  bool isSameSiteForeign = NS_IsSameSiteForeign(aChannel, uri);
+  bool isSafeTopLevelNav = CookieCommons::IsSafeTopLevelNav(aChannel);
+  bool isSameSiteForeign = CookieCommons::IsSameSiteForeign(aChannel, uri);
  SendPrepareCookieList(
      uri, result.contains(ThirdPartyAnalysis::IsForeign),
      result.contains(ThirdPartyAnalysis::IsThirdPartyTrackingResource),
--- a/netwerk/cookie/CookieServiceParent.cpp
+++ b/netwerk/cookie/CookieServiceParent.cpp
@ -3,6 +3,7 @@
 * License, v. 2.0. If a copy of the MPL was not distributed with this
 * file, You can obtain one at http://mozilla.org/MPL/2.0/. */

+#include "CookieCommons.h"
 #include "mozilla/net/CookieService.h"
 #include "mozilla/net/CookieServiceParent.h"
 #include "mozilla/net/NeckoParent.h"
@ -80,8 +81,8 @@ void CookieServiceParent::TrackCookieLoad(nsIChannel* aChannel) {

  nsCOMPtr<nsILoadInfo> loadInfo = aChannel->LoadInfo();
  OriginAttributes attrs = loadInfo->GetOriginAttributes();
-  bool isSafeTopLevelNav = NS_IsSafeTopLevelNav(aChannel);
-  bool aIsSameSiteForeign = NS_IsSameSiteForeign(aChannel, uri);
+  bool isSafeTopLevelNav = CookieCommons::IsSafeTopLevelNav(aChannel);
+  bool aIsSameSiteForeign = CookieCommons::IsSameSiteForeign(aChannel, uri);

  StoragePrincipalHelper::PrepareEffectiveStoragePrincipalOriginAttributes(
      aChannel, attrs);