Bug 1551798 - SameSite=lax by default and SameSite=none only if secure, r=Ehsan
authorAndrea Marchesini <amarchesini@mozilla.com>
Wed, 05 Jun 2019 12:18:52 +0000
changeset 476954 3e2cb9325361e640605146448fc6414c7653b098
parent 476953 c06f834fce851ab7224a0984ce87580b1ebd3207
child 476955 bd894a6a1b39c3e0ec68885789a2aacb552805a6
push id36112
push usermalexandru@mozilla.com
push dateWed, 05 Jun 2019 15:52:48 +0000
treeherdermozilla-central@8acc968600ff [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersEhsan
bugs1551798
milestone69.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1551798 - SameSite=lax by default and SameSite=none only if secure, r=Ehsan Differential Revision: https://phabricator.services.mozilla.com/D31215
modules/libpref/init/StaticPrefList.h
netwerk/cookie/nsCookieService.cpp
netwerk/cookie/nsCookieService.h
--- a/modules/libpref/init/StaticPrefList.h
+++ b/modules/libpref/init/StaticPrefList.h
@@ -5744,16 +5744,30 @@ VARCACHE_PREF(
   Live,
   "network.cookie.lifetimePolicy",
   network_cookie_lifetimePolicy,
   RelaxedAtomicInt32, 0
 )
 
 VARCACHE_PREF(
   Live,
+  "network.cookie.sameSite.laxByDefault",
+   network_cookie_sameSite_laxByDefault,
+  bool, false
+)
+
+VARCACHE_PREF(
+  Live,
+  "network.cookie.sameSite.noneRequiresSecure",
+   network_cookie_sameSite_noneRequiresSecure,
+  bool, false
+)
+
+VARCACHE_PREF(
+  Live,
   "network.cookie.thirdparty.sessionOnly",
    network_cookie_thirdparty_sessionOnly,
   bool, false
 )
 
 VARCACHE_PREF(
   Live,
   "network.cookie.thirdparty.nonsecureSessionOnly",
--- a/netwerk/cookie/nsCookieService.cpp
+++ b/netwerk/cookie/nsCookieService.cpp
@@ -3164,17 +3164,22 @@ bool nsCookieService::CanSetCookie(nsIUR
   // aCookieHeader is an in/out param to point to the next cookie, if
   // there is one. Save the present value for logging purposes
   nsCString savedCookieHeader(aCookieHeader);
 
   // newCookie says whether there are multiple cookies in the header;
   // so we can handle them separately.
   nsAutoCString expires;
   nsAutoCString maxage;
-  bool newCookie = ParseAttributes(aCookieHeader, aCookieData, expires, maxage);
+  bool acceptedByParser = false;
+  bool newCookie = ParseAttributes(aCookieHeader, aCookieData, expires, maxage,
+                                   acceptedByParser);
+  if (!acceptedByParser) {
+    return newCookie;
+  }
 
   // Collect telemetry on how often secure cookies are set from non-secure
   // origins, and vice-versa.
   //
   // 0 = nonsecure and "http:"
   // 1 = nonsecure and "https:"
   // 2 = secure and "http:"
   // 3 = secure and "https:"
@@ -3712,37 +3717,44 @@ bool nsCookieService::GetTokenValue(nsAC
   return false;
 }
 
 // Parses attributes from cookie header. expires/max-age attributes aren't
 // folded into the cookie struct here, because we don't know which one to use
 // until we've parsed the header.
 bool nsCookieService::ParseAttributes(nsCString& aCookieHeader,
                                       CookieStruct& aCookieData,
-                                      nsACString& aExpires,
-                                      nsACString& aMaxage) {
+                                      nsACString& aExpires, nsACString& aMaxage,
+                                      bool& aAcceptedByParser) {
+  aAcceptedByParser = false;
+
   static const char kPath[] = "path";
   static const char kDomain[] = "domain";
   static const char kExpires[] = "expires";
   static const char kMaxage[] = "max-age";
   static const char kSecure[] = "secure";
   static const char kHttpOnly[] = "httponly";
   static const char kSameSite[] = "samesite";
   static const char kSameSiteLax[] = "lax";
+  static const char kSameSiteNone[] = "none";
   static const char kSameSiteStrict[] = "strict";
 
   nsACString::const_char_iterator tempBegin, tempEnd;
   nsACString::const_char_iterator cookieStart, cookieEnd;
   aCookieHeader.BeginReading(cookieStart);
   aCookieHeader.EndReading(cookieEnd);
 
   aCookieData.isSecure() = false;
   aCookieData.isHttpOnly() = false;
   aCookieData.sameSite() = nsICookie::SAMESITE_NONE;
 
+  if (StaticPrefs::network_cookie_sameSite_laxByDefault()) {
+    aCookieData.sameSite() = nsICookie::SAMESITE_LAX;
+  }
+
   nsDependentCSubstring tokenString(cookieStart, cookieStart);
   nsDependentCSubstring tokenValue(cookieStart, cookieStart);
   bool newCookie, equalsFound;
 
   // extract cookie <NAME> & <VALUE> (first attribute), and copy the strings.
   // if we find multiple cookies, return for processing
   // note: if there's no '=', we assume token is <VALUE>. this is required by
   //       some sites (see bug 169091).
@@ -3788,20 +3800,34 @@ bool nsCookieService::ParseAttributes(ns
     else if (tokenString.LowerCaseEqualsLiteral(kHttpOnly))
       aCookieData.isHttpOnly() = true;
 
     else if (tokenString.LowerCaseEqualsLiteral(kSameSite)) {
       if (tokenValue.LowerCaseEqualsLiteral(kSameSiteLax)) {
         aCookieData.sameSite() = nsICookie::SAMESITE_LAX;
       } else if (tokenValue.LowerCaseEqualsLiteral(kSameSiteStrict)) {
         aCookieData.sameSite() = nsICookie::SAMESITE_STRICT;
+      } else if (tokenValue.LowerCaseEqualsLiteral(kSameSiteNone)) {
+        aCookieData.sameSite() = nsICookie::SAMESITE_NONE;
       }
     }
   }
 
+  // If same-site is set to 'none' but this is not a secure context, let's abort
+  // the parsing.
+  if (StaticPrefs::network_cookie_sameSite_laxByDefault() &&
+      StaticPrefs::network_cookie_sameSite_noneRequiresSecure() &&
+      !aCookieData.isSecure() &&
+      aCookieData.sameSite() == nsICookie::SAMESITE_NONE) {
+    return newCookie;
+  }
+
+  // Cookie accepted.
+  aAcceptedByParser = true;
+
   // re-assign aCookieHeader, in case we need to process another cookie
   aCookieHeader.Assign(Substring(cookieStart, cookieEnd));
   return newCookie;
 }
 
 /******************************************************************************
  * nsCookieService impl:
  * private domain & permission compliance enforcement functions
--- a/netwerk/cookie/nsCookieService.h
+++ b/netwerk/cookie/nsCookieService.h
@@ -291,17 +291,18 @@ class nsCookieService final : public nsI
                           mozIStorageBindingParamsArray* aParamsArray);
   static bool GetTokenValue(nsACString::const_char_iterator& aIter,
                             nsACString::const_char_iterator& aEndIter,
                             nsDependentCSubstring& aTokenString,
                             nsDependentCSubstring& aTokenValue,
                             bool& aEqualsFound);
   static bool ParseAttributes(nsCString& aCookieHeader,
                               mozilla::net::CookieStruct& aCookieData,
-                              nsACString& aExpires, nsACString& aMaxage);
+                              nsACString& aExpires, nsACString& aMaxage,
+                              bool& aAcceptedByParser);
   bool RequireThirdPartyCheck();
   static bool CheckDomain(mozilla::net::CookieStruct& aCookieData,
                           nsIURI* aHostURI, const nsCString& aBaseDomain,
                           bool aRequireHostMatch);
   static bool CheckPath(mozilla::net::CookieStruct& aCookieData,
                         nsIURI* aHostURI);
   static bool CheckPrefixes(mozilla::net::CookieStruct& aCookieData,
                             bool aSecureRequest);