Bug 565475 - Allow 3rd party cookies for session only. r=sdwilsh

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");

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";
+  // Create URIs and channels pointing to foo.com and bar.com.
-  var uri = ios.newURI(spec, null, null);
+  // We will use these to put foo.com into first and third party contexts.
-  var channel = ios.newChannelFromURI(uri);
+  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
+  run_cookie_test(cs, uri1, channel1, [1, 2, 3, 4]);
-  cs.setCookieString(uri, null, "oh=hai", null);
+  run_cookie_test(cs, uri1, channel2, [1, 2, 3, 4]);
-  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();
 
   // test with third party cookies blocked
   prefs.setIntPref("network.cookie.cookieBehavior", 1);
-  // without channel
+  run_cookie_test(cs, uri1, channel1, [0, 0, 0, 0]);
-  cs.setCookieString(uri, null, "oh=hai", null);
+  run_cookie_test(cs, uri1, channel2, [0, 0, 0, 0]);
-  do_check_eq(cs.countCookiesFromHost("foo.com"), 0);
+
-  // with channel
+  // Force the channel URI to be used when determining the originating URI of
-  cs.setCookieString(uri, null, "can=has", channel);
+  // the channel.
-  do_check_eq(cs.countCookiesFromHost("foo.com"), 0);
+  var httpchannel1 = channel1.QueryInterface(Ci.nsIHttpChannelInternal);
-  // without channel, from http
+  var httpchannel2 = channel1.QueryInterface(Ci.nsIHttpChannelInternal);
-  cs.setCookieStringFromHttp(uri, null, null, "cheez=burger", null, null);
+  httpchannel1.forceAllowThirdPartyCookie = true;
-  do_check_eq(cs.countCookiesFromHost("foo.com"), 0);
+  httpchannel2.forceAllowThirdPartyCookie = true;
-  // with channel, from http
+
-  cs.setCookieStringFromHttp(uri, null, null, "hot=dog", null, channel);
+  // test with cookies enabled
-  do_check_eq(cs.countCookiesFromHost("foo.com"), 0);
+  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();
 }
 

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);

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.
+// behavior pref constants
-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 
 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<nsIPrefBranch2> 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<nsIPrefBranch> 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,
+  CookieStatus cookieStatus = CheckPrefs(aHostURI, aOriginatingURI, baseDomain,
-                                     requireHostMatch, aCookieHeader.get());
+                                         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)))
+  if (NS_SUCCEEDED(aPrefBranch->GetIntPref(kPrefCookieBehavior, &val)))
-    mCookiesPermissions = (PRUint8) LIMIT(val, 0, 2, 0);
+    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,
+  CookieStatus cookieStatus = CheckPrefs(aHostURI, aOriginatingURI, baseDomain,
-                                     requireHostMatch, nsnull);
+                                         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.
+  // rejecting or altering the lifetime of third-party cookies.
-  if (mCookiesPermissions != BEHAVIOR_REJECTFOREIGN)
+  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;
     }
   }

netwerk/cookie/src/nsCookieService.h

@@ -147,6 +147,19 @@ struct DBState
   nsCOMPtr<mozIStorageStatement>  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;