diff --git 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 index 2fc222da104f..cae2a6174e49 100644 --- 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 @@ - Bug 1454242: Setting samesite cookie should not rely on NS_IsSameSiteForeign + Bug 1454242: Setting samesite cookie should not rely on CookieCommons::IsSameSiteForeign diff --git a/netwerk/base/nsNetUtil.cpp b/netwerk/base/nsNetUtil.cpp index b11f2998dfb6..31b5f7907cc4 100644 --- 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 loadInfo = aChannel->LoadInfo(); - if (loadInfo->GetExternalContentPolicyType() != - nsIContentPolicy::TYPE_DOCUMENT) { - return false; - } - return NS_IsSafeMethodNav(aChannel); -} - bool NS_IsSafeMethodNav(nsIChannel* aChannel) { RefPtr 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 loadInfo = aChannel->LoadInfo(); - // Do not treat loads triggered by web extensions as foreign - nsCOMPtr channelURI; - NS_GetFinalChannelURI(aChannel, getter_AddRefs(channelURI)); - RefPtr 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 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 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); diff --git a/netwerk/base/nsNetUtil.h b/netwerk/base/nsNetUtil.h index 03b38ec1a650..f9cb59a14dd4 100644 --- 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. diff --git a/netwerk/cookie/CookieCommons.cpp b/netwerk/cookie/CookieCommons.cpp index e1733d660992..f2c9270f7b94 100644 --- 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 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 loadInfo = aChannel->LoadInfo(); + // Do not treat loads triggered by web extensions as foreign + nsCOMPtr channelURI; + NS_GetFinalChannelURI(aChannel, getter_AddRefs(channelURI)); + RefPtr 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 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 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, diff --git a/netwerk/cookie/CookieCommons.h b/netwerk/cookie/CookieCommons.h index d2a16f91a216..55a6a2068a9a 100644 --- 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 diff --git a/netwerk/cookie/CookieService.cpp b/netwerk/cookie/CookieService.cpp index dbac613c25b9..8857d422c118 100644 --- 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 foundCookieList; GetCookiesForURI( diff --git a/netwerk/cookie/CookieServiceChild.cpp b/netwerk/cookie/CookieServiceChild.cpp index 59160ec022d5..ef23c4b56e7f 100644 --- 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), diff --git a/netwerk/cookie/CookieServiceParent.cpp b/netwerk/cookie/CookieServiceParent.cpp index 7c7450310fff..1efa548a5fc9 100644 --- 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 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);