From dcae496bc99a7e4a1e3bddb49b0465109be14f58 Mon Sep 17 00:00:00 2001 From: Andrea Marchesini Date: Tue, 21 Apr 2020 08:53:33 +0000 Subject: [PATCH] Bug 1631444 - Cookies should follow the CookieJarSettings, r=dimi Differential Revision: https://phabricator.services.mozilla.com/D71594 --- netwerk/cookie/CookieCommons.cpp | 78 +++++++++++++++++++++++ netwerk/cookie/CookieCommons.h | 3 + netwerk/cookie/CookiePermission.cpp | 85 +------------------------- netwerk/cookie/CookiePermission.h | 5 -- netwerk/cookie/CookieService.cpp | 40 ++++-------- netwerk/cookie/CookieService.h | 2 - netwerk/cookie/CookieServiceChild.cpp | 36 ++++------- netwerk/cookie/CookieServiceChild.h | 2 - netwerk/cookie/nsICookiePermission.idl | 39 ------------ 9 files changed, 106 insertions(+), 184 deletions(-) diff --git a/netwerk/cookie/CookieCommons.cpp b/netwerk/cookie/CookieCommons.cpp index 8db1f69d1d40..14e749bcf4d7 100644 --- a/netwerk/cookie/CookieCommons.cpp +++ b/netwerk/cookie/CookieCommons.cpp @@ -6,7 +6,10 @@ #include "Cookie.h" #include "CookieCommons.h" #include "mozilla/ContentBlockingNotifier.h" +#include "nsICookiePermission.h" +#include "nsICookieService.h" #include "nsIEffectiveTLDService.h" +#include "nsScriptSecurityManager.h" namespace mozilla { namespace net { @@ -179,5 +182,80 @@ bool CookieCommons::CheckHttpValue(const CookieStruct& aCookieData) { return aCookieData.value().FindCharInSet(illegalCharacters, 0) == -1; } +// static +bool CookieCommons::CheckCookiePermission(nsIChannel* aChannel, + CookieStruct& aCookieData) { + if (!aChannel) { + // No channel, let's assume this is a system-principal request. + return true; + } + + nsCOMPtr loadInfo = aChannel->LoadInfo(); + nsCOMPtr cookieJarSettings; + nsresult rv = + loadInfo->GetCookieJarSettings(getter_AddRefs(cookieJarSettings)); + if (NS_WARN_IF(NS_FAILED(rv))) { + return true; + } + + nsIScriptSecurityManager* ssm = + nsScriptSecurityManager::GetScriptSecurityManager(); + MOZ_ASSERT(ssm); + + nsCOMPtr channelPrincipal; + rv = ssm->GetChannelURIPrincipal(aChannel, getter_AddRefs(channelPrincipal)); + if (NS_WARN_IF(NS_FAILED(rv))) { + return false; + } + + if (!channelPrincipal->GetIsContentPrincipal()) { + return true; + } + + uint32_t cookiePermission = nsICookiePermission::ACCESS_DEFAULT; + rv = cookieJarSettings->CookiePermission(channelPrincipal, &cookiePermission); + if (NS_WARN_IF(NS_FAILED(rv))) { + return true; + } + + if (cookiePermission == nsICookiePermission::ACCESS_ALLOW) { + return true; + } + + if (cookiePermission == nsICookiePermission::ACCESS_SESSION) { + aCookieData.isSession() = true; + return true; + } + + if (cookiePermission == nsICookiePermission::ACCESS_DENY) { + return false; + } + + // Here we can have any legacy permission value. + + // now we need to figure out what type of accept policy we're dealing with + // if we accept cookies normally, just bail and return + if (StaticPrefs::network_cookie_lifetimePolicy() == + nsICookieService::ACCEPT_NORMALLY) { + return true; + } + + // declare this here since it'll be used in all of the remaining cases + int64_t currentTime = PR_Now() / PR_USEC_PER_SEC; + int64_t delta = aCookieData.expiry() - currentTime; + + // We are accepting the cookie, but, + // if it's not a session cookie, we may have to limit its lifetime. + if (!aCookieData.isSession() && delta > 0) { + if (StaticPrefs::network_cookie_lifetimePolicy() == + nsICookieService::ACCEPT_SESSION) { + // limit lifetime to session + aCookieData.isSession() = true; + } + } + + return true; +} + } // namespace net } // namespace mozilla diff --git a/netwerk/cookie/CookieCommons.h b/netwerk/cookie/CookieCommons.h index 804f9e0b457d..e511c1d8f203 100644 --- a/netwerk/cookie/CookieCommons.h +++ b/netwerk/cookie/CookieCommons.h @@ -64,6 +64,9 @@ class CookieCommons final { static bool CheckName(const CookieStruct& aCookieData); static bool CheckHttpValue(const CookieStruct& aCookieData); + + static bool CheckCookiePermission(nsIChannel* aChannel, + CookieStruct& aCookieData); }; } // namespace net diff --git a/netwerk/cookie/CookiePermission.cpp b/netwerk/cookie/CookiePermission.cpp index dde54c0f4d00..bcfebfea3903 100644 --- a/netwerk/cookie/CookiePermission.cpp +++ b/netwerk/cookie/CookiePermission.cpp @@ -5,23 +5,8 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ #include "mozilla/net/CookiePermission.h" - -#include "Cookie.h" -#include "mozilla/StaticPrefs_network.h" -#include "nsICookie.h" -#include "nsICookieService.h" -#include "nsNetUtil.h" -#include "nsIInterfaceRequestorUtils.h" -#include "nsIURI.h" -#include "nsIChannel.h" -#include "nsString.h" -#include "nsCRT.h" -#include "nsIScriptObjectPrincipal.h" -#include "nsNetCID.h" -#include "prtime.h" #include "mozilla/StaticPtr.h" #include "mozilla/ClearOnShutdown.h" -#include "nsContentUtils.h" /**************************************************************** ************************ CookiePermission ********************** @@ -30,8 +15,6 @@ namespace mozilla { namespace net { -static const bool kDefaultPolicy = true; - namespace { StaticRefPtr gSingleton; } // namespace @@ -47,73 +30,7 @@ already_AddRefed CookiePermission::GetOrCreate() { return do_AddRef(gSingleton); } -bool CookiePermission::Init() { - // Initialize PermissionManager and fetch relevant prefs. This is only - // required for some methods on nsICookiePermission, so it should be done - // lazily. - - mPermMgr = PermissionManager::GetInstance(); - return mPermMgr != nullptr; -} - -NS_IMETHODIMP -CookiePermission::CanSetCookie(nsIURI* aURI, nsIChannel* /*aChannel*/, - nsICookie* aCookie, bool* aIsSession, - int64_t* aExpiry, bool* aResult) { - NS_ASSERTION(aURI, "null uri"); - - *aResult = kDefaultPolicy; - - // Lazily initialize ourselves - if (!EnsureInitialized()) { - return NS_ERROR_UNEXPECTED; - } - - auto* cookie = static_cast(aCookie); - uint32_t perm; - mPermMgr->LegacyTestPermissionFromURI(aURI, &cookie->OriginAttributesRef(), - NS_LITERAL_CSTRING("cookie"), &perm); - switch (perm) { - case nsICookiePermission::ACCESS_SESSION: - *aIsSession = true; - [[fallthrough]]; - - case nsICookiePermission::ACCESS_ALLOW: - *aResult = true; - break; - - case nsICookiePermission::ACCESS_DENY: - *aResult = false; - break; - - default: - // Here we can have any legacy permission value. - - // now we need to figure out what type of accept policy we're dealing with - // if we accept cookies normally, just bail and return - if (StaticPrefs::network_cookie_lifetimePolicy() == - nsICookieService::ACCEPT_NORMALLY) { - *aResult = true; - return NS_OK; - } - - // declare this here since it'll be used in all of the remaining cases - int64_t currentTime = PR_Now() / PR_USEC_PER_SEC; - int64_t delta = *aExpiry - currentTime; - - // We are accepting the cookie, but, - // if it's not a session cookie, we may have to limit its lifetime. - if (!*aIsSession && delta > 0) { - if (StaticPrefs::network_cookie_lifetimePolicy() == - nsICookieService::ACCEPT_SESSION) { - // limit lifetime to session - *aIsSession = true; - } - } - } - - return NS_OK; -} +bool CookiePermission::Init() { return true; } } // namespace net } // namespace mozilla diff --git a/netwerk/cookie/CookiePermission.h b/netwerk/cookie/CookiePermission.h index 47bfb8897b4e..2196482b7f37 100644 --- a/netwerk/cookie/CookiePermission.h +++ b/netwerk/cookie/CookiePermission.h @@ -6,7 +6,6 @@ #define mozilla_net_CookiePermission_h #include "nsICookiePermission.h" -#include "mozilla/PermissionManager.h" namespace mozilla { namespace net { @@ -23,10 +22,6 @@ class CookiePermission final : public nsICookiePermission { private: ~CookiePermission() = default; - - bool EnsureInitialized() { return (mPermMgr != nullptr) || Init(); }; - - RefPtr mPermMgr; }; } // namespace net diff --git a/netwerk/cookie/CookieService.cpp b/netwerk/cookie/CookieService.cpp index 20b7373eb043..ae5a3d4a697a 100644 --- a/netwerk/cookie/CookieService.cpp +++ b/netwerk/cookie/CookieService.cpp @@ -177,8 +177,6 @@ nsresult CookieService::Init() { os->AddObserver(this, "profile-do-change", true); os->AddObserver(this, "last-pb-context-exited", true); - mPermissionService = CookiePermission::GetOrCreate(); - return NS_OK; } @@ -1065,6 +1063,17 @@ bool CookieService::SetCookieInternal(CookieStorage* aStorage, nsIURI* aHostURI, return newCookie; } + // check permissions from site permission list. + if (!CookieCommons::CheckCookiePermission(aChannel, cookieData)) { + COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, savedCookieHeader, + "cookie rejected by permission manager"); + CookieCommons::NotifyRejected( + aHostURI, aChannel, + nsIWebProgressListener::STATE_COOKIES_BLOCKED_BY_PERMISSION, + OPERATION_WRITE); + return newCookie; + } + int64_t currentTimeInUsec = PR_Now(); // create a new Cookie and copy attributes RefPtr cookie = Cookie::Create( @@ -1073,32 +1082,7 @@ bool CookieService::SetCookieInternal(CookieStorage* aStorage, nsIURI* aHostURI, Cookie::GenerateUniqueCreationTime(currentTimeInUsec), cookieData.isSession(), cookieData.isSecure(), cookieData.isHttpOnly(), aOriginAttributes, cookieData.sameSite(), cookieData.rawSameSite()); - if (!cookie) { - return newCookie; - } - - // check permissions from site permission list, or ask the user, - // to determine if we can set the cookie - if (mPermissionService) { - bool permission; - mPermissionService->CanSetCookie( - aHostURI, aChannel, - static_cast(static_cast(cookie)), - &cookieData.isSession(), &cookieData.expiry(), &permission); - if (!permission) { - COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, savedCookieHeader, - "cookie rejected by permission manager"); - CookieCommons::NotifyRejected( - aHostURI, aChannel, - nsIWebProgressListener::STATE_COOKIES_BLOCKED_BY_PERMISSION, - OPERATION_WRITE); - return newCookie; - } - - // update isSession and expiry attributes, in case they changed - cookie->SetIsSession(cookieData.isSession()); - cookie->SetExpiry(cookieData.expiry()); - } + MOZ_ASSERT(cookie); // add the cookie to the list. AddCookie() takes care of logging. // we get the current time again here, since it may have changed during diff --git a/netwerk/cookie/CookieService.h b/netwerk/cookie/CookieService.h index 89128689b7e4..83eaed41e08b 100644 --- a/netwerk/cookie/CookieService.h +++ b/netwerk/cookie/CookieService.h @@ -8,7 +8,6 @@ #include "nsICookieService.h" #include "nsICookieManager.h" -#include "nsICookiePermission.h" #include "nsIObserver.h" #include "nsWeakReference.h" @@ -185,7 +184,6 @@ class CookieService final : public nsICookieService, const nsTArray& aParams); // cached members. - nsCOMPtr mPermissionService; nsCOMPtr mThirdPartyUtil; nsCOMPtr mTLDService; nsCOMPtr mIDNService; diff --git a/netwerk/cookie/CookieServiceChild.cpp b/netwerk/cookie/CookieServiceChild.cpp index eeaced6aae8e..0f41e3b7d7ff 100644 --- a/netwerk/cookie/CookieServiceChild.cpp +++ b/netwerk/cookie/CookieServiceChild.cpp @@ -79,8 +79,6 @@ CookieServiceChild::CookieServiceChild() { mTLDService = do_GetService(NS_EFFECTIVETLDSERVICE_CONTRACTID); NS_ASSERTION(mTLDService, "couldn't get TLDService"); - mPermissionService = CookiePermission::GetOrCreate(); - // Init our prefs and observer. nsCOMPtr prefBranch = do_GetService(NS_PREFSERVICE_CONTRACTID); NS_WARNING_ASSERTION(prefBranch, "no prefservice"); @@ -413,34 +411,24 @@ nsresult CookieServiceChild::SetCookieStringInternal( continue; } + // check permissions from site permission list. + if (!CookieCommons::CheckCookiePermission(aChannel, cookieData)) { + COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, aCookieString, + "cookie rejected by permission manager"); + CookieCommons::NotifyRejected( + aHostURI, aChannel, + nsIWebProgressListener::STATE_COOKIES_BLOCKED_BY_PERMISSION, + OPERATION_WRITE); + continue; + } + RefPtr cookie = Cookie::Create( cookieData.name(), cookieData.value(), cookieData.host(), cookieData.path(), cookieData.expiry(), currentTimeInUsec, Cookie::GenerateUniqueCreationTime(currentTimeInUsec), cookieData.isSession(), cookieData.isSecure(), cookieData.isHttpOnly(), attrs, cookieData.sameSite(), cookieData.rawSameSite()); - - // check permissions from site permission list, or ask the user, - // to determine if we can set the cookie - if (mPermissionService) { - bool permission; - mPermissionService->CanSetCookie(aHostURI, aChannel, cookie, - &cookieData.isSession(), - &cookieData.expiry(), &permission); - if (!permission) { - COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, aCookieString, - "cookie rejected by permission manager"); - CookieCommons::NotifyRejected( - aHostURI, aChannel, - nsIWebProgressListener::STATE_COOKIES_BLOCKED_BY_PERMISSION, - OPERATION_WRITE); - continue; - } - - // update isSession and expiry attributes, in case they changed - cookie->SetIsSession(cookieData.isSession()); - cookie->SetExpiry(cookieData.expiry()); - } + MOZ_ASSERT(cookie); RecordDocumentCookie(cookie, attrs); cookiesToSend.AppendElement(cookieData); diff --git a/netwerk/cookie/CookieServiceChild.h b/netwerk/cookie/CookieServiceChild.h index daf6f08db93b..39e0aa2281d0 100644 --- a/netwerk/cookie/CookieServiceChild.h +++ b/netwerk/cookie/CookieServiceChild.h @@ -16,7 +16,6 @@ #include "nsWeakReference.h" #include "nsThreadUtils.h" -class nsICookiePermission; class nsIEffectiveTLDService; class nsILoadInfo; @@ -82,7 +81,6 @@ class CookieServiceChild final : public PCookieServiceChild, CookiesMap mCookiesMap; nsCOMPtr mCookieTimer; - nsCOMPtr mPermissionService; nsCOMPtr mThirdPartyUtil; nsCOMPtr mTLDService; }; diff --git a/netwerk/cookie/nsICookiePermission.idl b/netwerk/cookie/nsICookiePermission.idl index 5742f0190f50..dc9b2b0a1bdd 100644 --- a/netwerk/cookie/nsICookiePermission.idl +++ b/netwerk/cookie/nsICookiePermission.idl @@ -4,11 +4,6 @@ #include "nsISupports.idl" -interface nsICookie; -interface nsIURI; -interface nsIChannel; -interface nsIPrincipal; - typedef long nsCookieAccess; /** @@ -37,38 +32,4 @@ interface nsICookiePermission : nsISupports * and ACCESS_LIMIT_THIRD_PARTY, now removed, but maybe still stored in some * ancient user profiles. */ - - /** - * canSetCookie - * - * this method is called to test whether or not the given URI/channel may - * set a specific cookie. this method is always preceded by a call to - * canAccess. it may modify the isSession and expiry attributes of the - * cookie via the aIsSession and aExpiry parameters, in order to limit - * or extend the lifetime of the cookie. this is useful, for instance, to - * downgrade a cookie to session-only if it fails to meet certain criteria. - * - * @param aURI - * the URI trying to set the cookie - * @param aChannel - * the channel corresponding to aURI - * @param aCookie - * the cookie being added to the cookie database - * @param aIsSession - * when canSetCookie is invoked, this is the current isSession attribute - * of the cookie. canSetCookie may leave this value unchanged to - * preserve this attribute of the cookie. - * @param aExpiry - * when canSetCookie is invoked, this is the current expiry time of - * the cookie. canSetCookie may leave this value unchanged to - * preserve this attribute of the cookie. - * - * @return true if the cookie can be set. - */ - boolean canSetCookie(in nsIURI aURI, - in nsIChannel aChannel, - in nsICookie aCookie, - inout boolean aIsSession, - inout int64_t aExpiry); }; -