From 903768fcbf9f32f244e448a6e04a940cdeb3aa80 Mon Sep 17 00:00:00 2001 From: Dan Witte Date: Mon, 7 Jun 2010 12:32:12 -0700 Subject: [PATCH] Bug 565475 - Allow 3rd party cookies for session only. r=sdwilsh --- browser/app/profile/firefox.js | 2 - extensions/cookie/test/unit/test_cookies.js | 73 +++++++++++++-------- modules/libpref/src/init/all.js | 1 + netwerk/cookie/src/nsCookieService.cpp | 67 +++++++++++-------- netwerk/cookie/src/nsCookieService.h | 20 +++++- 5 files changed, 103 insertions(+), 60 deletions(-) diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js index 8900f30835c..f14a90ba75c 100644 --- a/browser/app/profile/firefox.js +++ b/browser/app/profile/firefox.js @@ -422,8 +422,6 @@ pref("privacy.sanitize.migrateFx3Prefs", false); pref("network.proxy.share_proxy_settings", false); // use the same proxy settings for all protocols -pref("network.cookie.cookieBehavior", 0); // 0-Accept, 1-dontAcceptForeign, 2-dontUse - // l12n and i18n pref("intl.accept_languages", "chrome://global/locale/intl.properties"); pref("intl.charsetmenu.browser.static", "chrome://global/locale/intl.properties"); diff --git a/extensions/cookie/test/unit/test_cookies.js b/extensions/cookie/test/unit/test_cookies.js index ccda2a72bee..9172f5a6410 100644 --- a/extensions/cookie/test/unit/test_cookies.js +++ b/extensions/cookie/test/unit/test_cookies.js @@ -11,39 +11,56 @@ function run_test() { var ios = Cc["@mozilla.org/network/io-service;1"].getService(Ci.nsIIOService); var prefs = Cc["@mozilla.org/preferences-service;1"].getService(Ci.nsIPrefBranch); - var spec = "http://foo.com/dribble.html"; - var uri = ios.newURI(spec, null, null); - var channel = ios.newChannelFromURI(uri); + // Create URIs and channels pointing to foo.com and bar.com. + // We will use these to put foo.com into first and third party contexts. + var spec1 = "http://foo.com/foo.html"; + var spec2 = "http://bar.com/bar.html"; + var uri1 = ios.newURI(spec1, null, null); + var uri2 = ios.newURI(spec2, null, null); + var channel1 = ios.newChannelFromURI(uri1); + var channel2 = ios.newChannelFromURI(uri2); // test with cookies enabled prefs.setIntPref("network.cookie.cookieBehavior", 0); - // without channel - cs.setCookieString(uri, null, "oh=hai", null); - do_check_eq(cs.countCookiesFromHost("foo.com"), 1); - // with channel - cs.setCookieString(uri, null, "can=has", channel); - do_check_eq(cs.countCookiesFromHost("foo.com"), 2); - // without channel, from http - cs.setCookieStringFromHttp(uri, null, null, "cheez=burger", null, null); - do_check_eq(cs.countCookiesFromHost("foo.com"), 3); - // with channel, from http - cs.setCookieStringFromHttp(uri, null, null, "hot=dog", null, channel); - do_check_eq(cs.countCookiesFromHost("foo.com"), 4); - cs.removeAll(); + run_cookie_test(cs, uri1, channel1, [1, 2, 3, 4]); + run_cookie_test(cs, uri1, channel2, [1, 2, 3, 4]); // test with third party cookies blocked prefs.setIntPref("network.cookie.cookieBehavior", 1); - // without channel - cs.setCookieString(uri, null, "oh=hai", null); - do_check_eq(cs.countCookiesFromHost("foo.com"), 0); - // with channel - cs.setCookieString(uri, null, "can=has", channel); - do_check_eq(cs.countCookiesFromHost("foo.com"), 0); - // without channel, from http - cs.setCookieStringFromHttp(uri, null, null, "cheez=burger", null, null); - do_check_eq(cs.countCookiesFromHost("foo.com"), 0); - // with channel, from http - cs.setCookieStringFromHttp(uri, null, null, "hot=dog", null, channel); - do_check_eq(cs.countCookiesFromHost("foo.com"), 0); + run_cookie_test(cs, uri1, channel1, [0, 0, 0, 0]); + run_cookie_test(cs, uri1, channel2, [0, 0, 0, 0]); + + // Force the channel URI to be used when determining the originating URI of + // the channel. + var httpchannel1 = channel1.QueryInterface(Ci.nsIHttpChannelInternal); + var httpchannel2 = channel1.QueryInterface(Ci.nsIHttpChannelInternal); + httpchannel1.forceAllowThirdPartyCookie = true; + httpchannel2.forceAllowThirdPartyCookie = true; + + // test with cookies enabled + prefs.setIntPref("network.cookie.cookieBehavior", 0); + run_cookie_test(cs, uri1, channel1, [1, 2, 3, 4]); + run_cookie_test(cs, uri1, channel2, [1, 2, 3, 4]); + + // test with third party cookies blocked + prefs.setIntPref("network.cookie.cookieBehavior", 1); + run_cookie_test(cs, uri1, channel1, [0, 1, 1, 2]); + run_cookie_test(cs, uri1, channel2, [0, 0, 0, 0]); +} + +function run_cookie_test(cs, uri, channel, expected) { + // without channel + cs.setCookieString(uri, null, "oh=hai", null); + do_check_eq(cs.countCookiesFromHost("foo.com"), expected[0]); + // with channel + cs.setCookieString(uri, null, "can=has", channel); + do_check_eq(cs.countCookiesFromHost("foo.com"), expected[1]); + // without channel, from http + cs.setCookieStringFromHttp(uri, null, null, "cheez=burger", null, null); + do_check_eq(cs.countCookiesFromHost("foo.com"), expected[2]); + // with channel, from http + cs.setCookieStringFromHttp(uri, null, null, "hot=dog", null, channel); + do_check_eq(cs.countCookiesFromHost("foo.com"), expected[3]); + cs.removeAll(); } diff --git a/modules/libpref/src/init/all.js b/modules/libpref/src/init/all.js index 287810a1a04..0a81ade406c 100644 --- a/modules/libpref/src/init/all.js +++ b/modules/libpref/src/init/all.js @@ -897,6 +897,7 @@ pref("network.proxy.no_proxies_on", "localhost, 127.0.0.1"); pref("network.proxy.failover_timeout", 1800); // 30 minutes pref("network.online", true); //online/offline pref("network.cookie.cookieBehavior", 0); // 0-Accept, 1-dontAcceptForeign, 2-dontUse +pref("network.cookie.thirdparty.sessionOnly", true); pref("network.cookie.lifetimePolicy", 0); // accept normally, 1-askBeforeAccepting, 2-acceptForSession,3-acceptForNDays pref("network.cookie.alwaysAcceptSessionCookies", false); pref("network.cookie.prefsMigrated", false); diff --git a/netwerk/cookie/src/nsCookieService.cpp b/netwerk/cookie/src/nsCookieService.cpp index 6233886adcb..7a2e3464bdd 100644 --- a/netwerk/cookie/src/nsCookieService.cpp +++ b/netwerk/cookie/src/nsCookieService.cpp @@ -111,25 +111,17 @@ static const PRUint32 kMaxCookiesPerHost = 50; static const PRUint32 kMaxBytesPerCookie = 4096; static const PRUint32 kMaxBytesPerPath = 1024; -// these constants represent a decision about a cookie based on user prefs. -static const PRUint32 STATUS_ACCEPTED = 0; -static const PRUint32 STATUS_REJECTED = 1; -// STATUS_REJECTED_WITH_ERROR indicates the cookie should be rejected because -// of an error (rather than something the user can control). this is used for -// notification purposes, since we only want to notify of rejections where -// the user can do something about it (e.g. whitelist the site). -static const PRUint32 STATUS_REJECTED_WITH_ERROR = 2; - -// behavior pref constants +// behavior pref constants static const PRUint32 BEHAVIOR_ACCEPT = 0; static const PRUint32 BEHAVIOR_REJECTFOREIGN = 1; static const PRUint32 BEHAVIOR_REJECT = 2; // pref string constants -static const char kPrefCookiesPermissions[] = "network.cookie.cookieBehavior"; +static const char kPrefCookieBehavior[] = "network.cookie.cookieBehavior"; static const char kPrefMaxNumberOfCookies[] = "network.cookie.maxNumber"; static const char kPrefMaxCookiesPerHost[] = "network.cookie.maxPerHost"; static const char kPrefCookiePurgeAge[] = "network.cookie.purgeAge"; +static const char kPrefThirdPartySession[] = "network.cookie.thirdparty.sessionOnly"; // struct for temporarily storing cookie attributes during header parsing struct nsCookieAttributes @@ -531,7 +523,8 @@ NS_IMPL_ISUPPORTS5(nsCookieService, nsCookieService::nsCookieService() : mDBState(&mDefaultDBState) - , mCookiesPermissions(BEHAVIOR_ACCEPT) + , mCookieBehavior(BEHAVIOR_ACCEPT) + , mThirdPartySession(PR_TRUE) , mMaxNumberOfCookies(kMaxNumberOfCookies) , mMaxCookiesPerHost(kMaxCookiesPerHost) , mCookiePurgeAge(kCookiePurgeAge) @@ -557,10 +550,11 @@ nsCookieService::Init() // init our pref and observer nsCOMPtr prefBranch = do_GetService(NS_PREFSERVICE_CONTRACTID); if (prefBranch) { - prefBranch->AddObserver(kPrefCookiesPermissions, this, PR_TRUE); + prefBranch->AddObserver(kPrefCookieBehavior, this, PR_TRUE); prefBranch->AddObserver(kPrefMaxNumberOfCookies, this, PR_TRUE); prefBranch->AddObserver(kPrefMaxCookiesPerHost, this, PR_TRUE); prefBranch->AddObserver(kPrefCookiePurgeAge, this, PR_TRUE); + prefBranch->AddObserver(kPrefThirdPartySession, this, PR_TRUE); PrefChanged(prefBranch); } @@ -871,6 +865,7 @@ nsCookieService::Observe(nsISupports *aSubject, nsCOMPtr prefBranch = do_QueryInterface(aSubject); if (prefBranch) PrefChanged(prefBranch); + } else if (!strcmp(aTopic, NS_PRIVATE_BROWSING_SWITCH_TOPIC)) { if (NS_LITERAL_STRING(NS_PRIVATE_BROWSING_ENTER).Equals(aData)) { if (!mPrivateDBState.hostTable.IsInitialized() && @@ -1002,14 +997,16 @@ nsCookieService::SetCookieStringInternal(nsIURI *aHostURI, } // check default prefs - PRUint32 cookieStatus = CheckPrefs(aHostURI, aOriginatingURI, baseDomain, - requireHostMatch, aCookieHeader.get()); + CookieStatus cookieStatus = CheckPrefs(aHostURI, aOriginatingURI, baseDomain, + requireHostMatch, aCookieHeader.get()); // fire a notification if cookie was rejected (but not if there was an error) switch (cookieStatus) { case STATUS_REJECTED: NotifyRejected(aHostURI); case STATUS_REJECTED_WITH_ERROR: return; + default: + break; } // parse server local time. this is not just done here for efficiency @@ -1030,7 +1027,7 @@ nsCookieService::SetCookieStringInternal(nsIURI *aHostURI, // process each cookie in the header nsDependentCString cookieHeader(aCookieHeader); while (SetCookieInternal(aHostURI, baseDomain, requireHostMatch, - cookieHeader, serverTime, aFromHttp)); + cookieStatus, cookieHeader, serverTime, aFromHttp)); } // notify observers that a cookie was rejected due to the users' prefs. @@ -1066,8 +1063,8 @@ void nsCookieService::PrefChanged(nsIPrefBranch *aPrefBranch) { PRInt32 val; - if (NS_SUCCEEDED(aPrefBranch->GetIntPref(kPrefCookiesPermissions, &val))) - mCookiesPermissions = (PRUint8) LIMIT(val, 0, 2, 0); + if (NS_SUCCEEDED(aPrefBranch->GetIntPref(kPrefCookieBehavior, &val))) + mCookieBehavior = (PRUint8) LIMIT(val, 0, 2, 0); if (NS_SUCCEEDED(aPrefBranch->GetIntPref(kPrefMaxNumberOfCookies, &val))) mMaxNumberOfCookies = (PRUint16) LIMIT(val, 1, 0xFFFF, kMaxNumberOfCookies); @@ -1077,6 +1074,10 @@ nsCookieService::PrefChanged(nsIPrefBranch *aPrefBranch) if (NS_SUCCEEDED(aPrefBranch->GetIntPref(kPrefCookiePurgeAge, &val))) mCookiePurgeAge = LIMIT(val, 0, PR_INT32_MAX, PR_INT32_MAX) * PR_USEC_PER_SEC; + + PRBool boolval; + if (NS_SUCCEEDED(aPrefBranch->GetBoolPref(kPrefThirdPartySession, &boolval))) + mThirdPartySession = boolval; } /****************************************************************************** @@ -1251,7 +1252,7 @@ nsCookieService::Read() getter_AddRefs(stmtDeleteExpired)); NS_ENSURE_SUCCESS(rv, rv); - rv = stmtDeleteExpired->BindInt64Parameter(0, PR_Now() / PR_USEC_PER_SEC); + rv = stmtDeleteExpired->BindInt64ByIndex(0, PR_Now() / PR_USEC_PER_SEC); NS_ENSURE_SUCCESS(rv, rv); PRBool hasResult; @@ -1545,13 +1546,15 @@ nsCookieService::GetCookieInternal(nsIURI *aHostURI, } // check default prefs - PRUint32 cookieStatus = CheckPrefs(aHostURI, aOriginatingURI, baseDomain, - requireHostMatch, nsnull); + CookieStatus cookieStatus = CheckPrefs(aHostURI, aOriginatingURI, baseDomain, + requireHostMatch, nsnull); // for GetCookie(), we don't fire rejection notifications. switch (cookieStatus) { case STATUS_REJECTED: case STATUS_REJECTED_WITH_ERROR: return; + default: + break; } // check if aHostURI is using an https secure protocol. @@ -1700,6 +1703,7 @@ PRBool nsCookieService::SetCookieInternal(nsIURI *aHostURI, const nsCString &aBaseDomain, PRBool aRequireHostMatch, + CookieStatus aStatus, nsDependentCString &aCookieHeader, PRInt64 aServerTime, PRBool aFromHttp) @@ -1724,6 +1728,11 @@ nsCookieService::SetCookieInternal(nsIURI *aHostURI, // calculate expiry time of cookie. cookieAttributes.isSession = GetExpiry(cookieAttributes, aServerTime, currentTimeInUsec / PR_USEC_PER_SEC); + if (aStatus == STATUS_ACCEPT_SESSION) { + // force lifetime to session. note that the expiration time, if set above, + // will still apply. + cookieAttributes.isSession = PR_TRUE; + } // reject cookie if it's over the size limit, per RFC2109 if ((cookieAttributes.name.Length() + cookieAttributes.value.Length()) > kMaxBytesPerCookie) { @@ -2246,8 +2255,8 @@ nsCookieService::GetOriginatingURI(nsIChannel *aChannel, nsIURI **aURI) { // Determine the originating URI. We only need to do this if we're - // rejecting third-party cookies. - if (mCookiesPermissions != BEHAVIOR_REJECTFOREIGN) + // rejecting or altering the lifetime of third-party cookies. + if (mCookieBehavior != BEHAVIOR_REJECTFOREIGN && !mThirdPartySession) return; if (!mPermissionService) { @@ -2258,7 +2267,7 @@ nsCookieService::GetOriginatingURI(nsIChannel *aChannel, mPermissionService->GetOriginatingURI(aChannel, aURI); } -PRUint32 +CookieStatus nsCookieService::CheckPrefs(nsIURI *aHostURI, nsIURI *aOriginatingURI, const nsCString &aBaseDomain, @@ -2296,15 +2305,19 @@ nsCookieService::CheckPrefs(nsIURI *aHostURI, } // check default prefs - if (mCookiesPermissions == BEHAVIOR_REJECT) { + if (mCookieBehavior == BEHAVIOR_REJECT) { COOKIE_LOGFAILURE(aCookieHeader ? SET_COOKIE : GET_COOKIE, aHostURI, aCookieHeader, "cookies are disabled"); return STATUS_REJECTED; + } - } else if (mCookiesPermissions == BEHAVIOR_REJECTFOREIGN) { + if (mCookieBehavior == BEHAVIOR_REJECTFOREIGN || mThirdPartySession) { // check if cookie is foreign if (!aOriginatingURI || IsForeign(aBaseDomain, aRequireHostMatch, aOriginatingURI)) { - COOKIE_LOGFAILURE(aCookieHeader ? SET_COOKIE : GET_COOKIE, aHostURI, aCookieHeader, "originating server test failed"); + if (mCookieBehavior == BEHAVIOR_ACCEPT && mThirdPartySession) + return STATUS_ACCEPT_SESSION; + + COOKIE_LOGFAILURE(aCookieHeader ? SET_COOKIE : GET_COOKIE, aHostURI, aCookieHeader, "context is third party"); return STATUS_REJECTED; } } diff --git a/netwerk/cookie/src/nsCookieService.h b/netwerk/cookie/src/nsCookieService.h index b1bc2a0d90f..9a6b5eca760 100644 --- a/netwerk/cookie/src/nsCookieService.h +++ b/netwerk/cookie/src/nsCookieService.h @@ -147,6 +147,19 @@ struct DBState nsCOMPtr stmtUpdate; }; +// these constants represent a decision about a cookie based on user prefs. +enum CookieStatus +{ + STATUS_ACCEPTED, + STATUS_ACCEPT_SESSION, + STATUS_REJECTED, + // STATUS_REJECTED_WITH_ERROR indicates the cookie should be rejected because + // of an error (rather than something the user can control). this is used for + // notification purposes, since we only want to notify of rejections where + // the user can do something about it (e.g. whitelist the site). + STATUS_REJECTED_WITH_ERROR +}; + /****************************************************************************** * nsCookieService: * class declaration @@ -182,7 +195,7 @@ class nsCookieService : public nsICookieService nsresult GetBaseDomainFromHost(const nsACString &aHost, nsCString &aBaseDomain); void GetCookieInternal(nsIURI *aHostURI, nsIURI *aOriginatingURI, PRBool aHttpBound, nsCString &aCookie); void SetCookieStringInternal(nsIURI *aHostURI, nsIURI *aOriginatingURI, const nsCString &aCookieHeader, const nsCString &aServerTime, PRBool aFromHttp); - PRBool SetCookieInternal(nsIURI *aHostURI, const nsCString& aBaseDomain, PRBool aRequireHostMatch, nsDependentCString &aCookieHeader, PRInt64 aServerTime, PRBool aFromHttp); + PRBool SetCookieInternal(nsIURI *aHostURI, const nsCString& aBaseDomain, PRBool aRequireHostMatch, CookieStatus aStatus, nsDependentCString &aCookieHeader, PRInt64 aServerTime, PRBool aFromHttp); void AddInternal(const nsCString& aBaseDomain, nsCookie *aCookie, PRInt64 aCurrentTimeInUsec, nsIURI *aHostURI, const char *aCookieHeader, PRBool aFromHttp); void RemoveCookieFromList(const nsListIter &aIter, mozIStorageBindingParamsArray *aParamsArray = NULL); PRBool AddCookieToList(const nsCString& aBaseDomain, nsCookie *aCookie, mozIStorageBindingParamsArray *aParamsArray, PRBool aWriteToDB = PR_TRUE); @@ -191,7 +204,7 @@ class nsCookieService : public nsICookieService static PRBool ParseAttributes(nsDependentCString &aCookieHeader, nsCookieAttributes &aCookie); PRBool IsForeign(const nsCString &aBaseDomain, PRBool aRequireHostMatch, nsIURI *aFirstURI); void GetOriginatingURI(nsIChannel *aChannel, nsIURI **aURI); - PRUint32 CheckPrefs(nsIURI *aHostURI, nsIURI *aOriginatingURI, const nsCString &aBaseDomain, PRBool aRequireHostMatch, const char *aCookieHeader); + CookieStatus CheckPrefs(nsIURI *aHostURI, nsIURI *aOriginatingURI, const nsCString &aBaseDomain, PRBool aRequireHostMatch, const char *aCookieHeader); PRBool CheckDomain(nsCookieAttributes &aCookie, nsIURI *aHostURI, const nsCString &aBaseDomain, PRBool aRequireHostMatch); static PRBool CheckPath(nsCookieAttributes &aCookie, nsIURI *aHostURI); static PRBool GetExpiry(nsCookieAttributes &aCookie, PRInt64 aServerTime, PRInt64 aCurrentTime); @@ -219,7 +232,8 @@ class nsCookieService : public nsICookieService DBState mPrivateDBState; // cached prefs - PRUint8 mCookiesPermissions; // BEHAVIOR_{ACCEPT, REJECTFOREIGN, REJECT} + PRUint8 mCookieBehavior; // BEHAVIOR_{ACCEPT, REJECTFOREIGN, REJECT} + PRBool mThirdPartySession; PRUint16 mMaxNumberOfCookies; PRUint16 mMaxCookiesPerHost; PRInt64 mCookiePurgeAge;