Bug 1526214 - Get rid of network.cookie.leave-secure-alone, r=Ehsan

Differential Revision: https://phabricator.services.mozilla.com/D19125 --HG-- extra : moz-landing-system : lando
2019-02-20 09:42:22 +00:00 · 2019-02-20 09:42:22 +00:00 · 9e8594dfcb
--- a/gfx/skia/skia/src/ports/SkFontHost_cairo.cpp
+++ b/gfx/skia/skia/src/ports/SkFontHost_cairo.cpp
@ -267,7 +267,7 @@ public:

    cairo_font_face_t* GetCairoFontFace() const { return fFontFace; }

-private:
+   private:
    ~SkCairoFTTypeface()
    {
        cairo_font_face_destroy(fFontFace);
@ -283,7 +283,8 @@ private:
 };

 static bool FindByCairoFontFace(SkTypeface* typeface, void* context) {
-    return static_cast<SkCairoFTTypeface*>(typeface)->GetCairoFontFace() == static_cast<cairo_font_face_t*>(context);
+  return static_cast<SkCairoFTTypeface*>(typeface)->GetCairoFontFace() ==
+         static_cast<cairo_font_face_t*>(context);
 }

 SkTypeface* SkCreateTypefaceFromCairoFTFontWithFontconfig(cairo_scaled_font_t* scaledFont, FcPattern* pattern)
@ -292,10 +293,11 @@ SkTypeface* SkCreateTypefaceFromCairoFTFontWithFontconfig(cairo_scaled_font_t* s
    SkASSERT(cairo_font_face_status(fontFace) == CAIRO_STATUS_SUCCESS);
    SkASSERT(cairo_font_face_get_type(fontFace) == CAIRO_FONT_TYPE_FT);

-    SkTypeface* typeface = SkTypefaceCache::FindByProcAndRef(FindByCairoFontFace, fontFace);
+    SkTypeface* typeface =
+        SkTypefaceCache::FindByProcAndRef(FindByCairoFontFace, fontFace);
    if (!typeface) {
-        typeface = new SkCairoFTTypeface(fontFace, pattern);
-        SkTypefaceCache::Add(typeface);
+      typeface = new SkCairoFTTypeface(fontFace, pattern);
+      SkTypefaceCache::Add(typeface);
    }

    return typeface;
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@ -2372,7 +2372,6 @@ pref("network.proxy.failover_timeout",      1800); // 30 minutes
 pref("network.online",                      true); //online/offline
 pref("network.cookie.thirdparty.sessionOnly", false);
 pref("network.cookie.thirdparty.nonsecureSessionOnly", false);
-pref("network.cookie.leave-secure-alone",   true);
 pref("network.cookie.same-site.enabled",    true); // Honor the SameSite cookie attribute

 // Cookie lifetime policy. Possible values:
--- a/netwerk/cookie/CookieServiceChild.cpp
+++ b/netwerk/cookie/CookieServiceChild.cpp
@ -40,8 +40,6 @@ static const char kPrefThirdPartySession[] =
    "network.cookie.thirdparty.sessionOnly";
 static const char kPrefThirdPartyNonsecureSession[] =
    "network.cookie.thirdparty.nonsecureSessionOnly";
-static const char kCookieLeaveSecurityAlone[] =
-    "network.cookie.leave-secure-alone";
 static const char kCookieMoveIntervalSecs[] =
    "network.cookie.move.interval_sec";

@ -64,7 +62,6 @@ CookieServiceChild::CookieServiceChild()
    : mCookieBehavior(nsICookieService::BEHAVIOR_ACCEPT),
      mThirdPartySession(false),
      mThirdPartyNonsecureSession(false),
-      mLeaveSecureAlone(true),
      mIPCOpen(false) {
  NS_ASSERTION(IsNeckoChild(), "not a child process");

@ -94,7 +91,6 @@ CookieServiceChild::CookieServiceChild()
    prefBranch->AddObserver(kPrefCookieBehavior, this, true);
    prefBranch->AddObserver(kPrefThirdPartySession, this, true);
    prefBranch->AddObserver(kPrefThirdPartyNonsecureSession, this, true);
-    prefBranch->AddObserver(kCookieLeaveSecurityAlone, this, true);
    prefBranch->AddObserver(kCookieMoveIntervalSecs, this, true);
    PrefChanged(prefBranch);
  }
@ -271,10 +267,6 @@ void CookieServiceChild::PrefChanged(nsIPrefBranch *aPrefBranch) {
          aPrefBranch->GetBoolPref(kPrefThirdPartyNonsecureSession, &boolval)))
    mThirdPartyNonsecureSession = boolval;

-  if (NS_SUCCEEDED(
-          aPrefBranch->GetBoolPref(kCookieLeaveSecurityAlone, &boolval)))
-    mLeaveSecureAlone = !!boolval;
-
  if (!mThirdPartyUtil && RequireThirdPartyCheck()) {
    mThirdPartyUtil = do_GetService(THIRDPARTYUTIL_CONTRACTID);
    NS_ASSERTION(mThirdPartyUtil, "require ThirdPartyUtil service");
@ -607,8 +599,8 @@ nsresult CookieServiceChild::SetCookieStringInternal(nsIURI *aHostURI,
    bool canSetCookie = false;
    moreCookies = nsCookieService::CanSetCookie(
        aHostURI, key, cookieAttributes, requireHostMatch, cookieStatus,
-        cookieString, serverTime, aFromHttp, aChannel, mLeaveSecureAlone,
-        canSetCookie, mThirdPartyUtil);
+        cookieString, serverTime, aFromHttp, aChannel, canSetCookie,
+        mThirdPartyUtil);

    // We need to see if the cookie we're setting would overwrite an httponly
    // one. This would not affect anything we send over the net (those come from
--- a/netwerk/cookie/CookieServiceChild.h
+++ b/netwerk/cookie/CookieServiceChild.h
@ -110,7 +110,6 @@ class CookieServiceChild : public PCookieServiceChild,
  uint8_t mCookieBehavior;
  bool mThirdPartySession;
  bool mThirdPartyNonsecureSession;
-  bool mLeaveSecureAlone;
  bool mIPCOpen;
 };

--- a/netwerk/cookie/nsCookieService.cpp
+++ b/netwerk/cookie/nsCookieService.cpp
@ -134,8 +134,6 @@ static const char kPrefThirdPartySession[] =
    "network.cookie.thirdparty.sessionOnly";
 static const char kPrefThirdPartyNonsecureSession[] =
    "network.cookie.thirdparty.nonsecureSessionOnly";
-static const char kCookieLeaveSecurityAlone[] =
-    "network.cookie.leave-secure-alone";

 // For telemetry COOKIE_LEAVE_SECURE_ALONE
 #define BLOCKED_SECURE_SET_FROM_HTTP 0
@ -601,7 +599,6 @@ nsCookieService::nsCookieService()
      mCookieBehavior(nsICookieService::BEHAVIOR_ACCEPT),
      mThirdPartySession(false),
      mThirdPartyNonsecureSession(false),
-      mLeaveSecureAlone(true),
      mMaxNumberOfCookies(kMaxNumberOfCookies),
      mMaxCookiesPerHost(kMaxCookiesPerHost),
      mCookieQuotaPerHost(kCookieQuotaPerHost),
@ -631,7 +628,6 @@ nsresult nsCookieService::Init() {
    prefBranch->AddObserver(kPrefCookiePurgeAge, this, true);
    prefBranch->AddObserver(kPrefThirdPartySession, this, true);
    prefBranch->AddObserver(kPrefThirdPartyNonsecureSession, this, true);
-    prefBranch->AddObserver(kCookieLeaveSecurityAlone, this, true);
    PrefChanged(prefBranch);
  }

@ -2403,10 +2399,6 @@ void nsCookieService::PrefChanged(nsIPrefBranch *aPrefBranch) {
  if (NS_SUCCEEDED(
          aPrefBranch->GetBoolPref(kPrefThirdPartyNonsecureSession, &boolval)))
    mThirdPartyNonsecureSession = boolval;
-
-  if (NS_SUCCEEDED(
-          aPrefBranch->GetBoolPref(kCookieLeaveSecurityAlone, &boolval)))
-    mLeaveSecureAlone = boolval;
 }

 /******************************************************************************
@ -3239,8 +3231,7 @@ bool nsCookieService::CanSetCookie(nsIURI *aHostURI, const nsCookieKey &aKey,
                                   bool aRequireHostMatch, CookieStatus aStatus,
                                   nsDependentCString &aCookieHeader,
                                   int64_t aServerTime, bool aFromHttp,
-                                   nsIChannel *aChannel, bool aLeaveSecureAlone,
-                                   bool &aSetCookie,
+                                   nsIChannel *aChannel, bool &aSetCookie,
                                   mozIThirdPartyUtil *aThirdPartyUtil) {
  NS_ASSERTION(aHostURI, "null host!");

@ -3373,7 +3364,7 @@ bool nsCookieService::CanSetCookie(nsIURI *aHostURI, const nsCookieKey &aKey,
  // If the new cookie is non-https and wants to set secure flag,
  // browser have to ignore this new cookie.
  // (draft-ietf-httpbis-cookie-alone section 3.1)
-  if (aLeaveSecureAlone && aCookieAttributes.isSecure && !isSecure) {
+  if (aCookieAttributes.isSecure && !isSecure) {
    COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, aCookieHeader,
                      "non-https cookie can't set secure flag");
    Telemetry::Accumulate(Telemetry::COOKIE_LEAVE_SECURE_ALONE,
@ -3427,7 +3418,7 @@ bool nsCookieService::SetCookieInternal(nsIURI *aHostURI,
  bool newCookie =
      CanSetCookie(aHostURI, aKey, cookieAttributes, aRequireHostMatch, aStatus,
                   aCookieHeader, aServerTime, aFromHttp, aChannel,
-                   mLeaveSecureAlone, canSetCookie, mThirdPartyUtil);
+                   canSetCookie, mThirdPartyUtil);

  if (!canSetCookie) {
    return newCookie;
@ -3498,41 +3489,39 @@ void nsCookieService::AddInternal(const nsCookieKey &aKey, nsCookie *aCookie,
    isSecure = false;
  }
  bool oldCookieIsSession = false;
-  if (mLeaveSecureAlone) {
-    // Step1, call FindSecureCookie(). FindSecureCookie() would
-    // find the existing cookie with the security flag and has
-    // the same name, host and path of the new cookie, if there is any.
-    // Step2, Confirm new cookie's security setting. If any targeted
-    // cookie had been found in Step1, then confirm whether the
-    // new cookie could modify it. If the new created cookie’s
-    // "secure-only-flag" is not set, and the "scheme" component
-    // of the "request-uri" does not denote a "secure" protocol,
-    // then ignore the new cookie.
-    // (draft-ietf-httpbis-cookie-alone section 3.2)
-    if (!aCookie->IsSecure() &&
-        (foundSecureExact || FindSecureCookie(aKey, aCookie))) {
-      if (!isSecure) {
-        COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, aCookieHeader,
-                          "cookie can't save because older cookie is secure "
-                          "cookie but newer cookie is non-secure cookie");
-        if (foundSecureExact) {
-          Telemetry::Accumulate(Telemetry::COOKIE_LEAVE_SECURE_ALONE,
-                                BLOCKED_DOWNGRADE_SECURE_EXACT);
-        } else {
-          Telemetry::Accumulate(Telemetry::COOKIE_LEAVE_SECURE_ALONE,
-                                BLOCKED_DOWNGRADE_SECURE_INEXACT);
-        }
-        return;
-      }
-      // A secure site is allowed to downgrade a secure cookie
-      // but we want to measure anyway.
+  // Step1, call FindSecureCookie(). FindSecureCookie() would
+  // find the existing cookie with the security flag and has
+  // the same name, host and path of the new cookie, if there is any.
+  // Step2, Confirm new cookie's security setting. If any targeted
+  // cookie had been found in Step1, then confirm whether the
+  // new cookie could modify it. If the new created cookie’s
+  // "secure-only-flag" is not set, and the "scheme" component
+  // of the "request-uri" does not denote a "secure" protocol,
+  // then ignore the new cookie.
+  // (draft-ietf-httpbis-cookie-alone section 3.2)
+  if (!aCookie->IsSecure() &&
+      (foundSecureExact || FindSecureCookie(aKey, aCookie))) {
+    if (!isSecure) {
+      COOKIE_LOGFAILURE(SET_COOKIE, aHostURI, aCookieHeader,
+                        "cookie can't save because older cookie is secure "
+                        "cookie but newer cookie is non-secure cookie");
      if (foundSecureExact) {
        Telemetry::Accumulate(Telemetry::COOKIE_LEAVE_SECURE_ALONE,
-                              DOWNGRADE_SECURE_FROM_SECURE_EXACT);
+                              BLOCKED_DOWNGRADE_SECURE_EXACT);
      } else {
        Telemetry::Accumulate(Telemetry::COOKIE_LEAVE_SECURE_ALONE,
-                              DOWNGRADE_SECURE_FROM_SECURE_INEXACT);
+                              BLOCKED_DOWNGRADE_SECURE_INEXACT);
      }
+      return;
+    }
+    // A secure site is allowed to downgrade a secure cookie
+    // but we want to measure anyway.
+    if (foundSecureExact) {
+      Telemetry::Accumulate(Telemetry::COOKIE_LEAVE_SECURE_ALONE,
+                            DOWNGRADE_SECURE_FROM_SECURE_EXACT);
+    } else {
+      Telemetry::Accumulate(Telemetry::COOKIE_LEAVE_SECURE_ALONE,
+                            DOWNGRADE_SECURE_FROM_SECURE_INEXACT);
    }
  }

@ -3624,16 +3613,12 @@ void nsCookieService::AddInternal(const nsCookieKey &aKey, nsCookie *aCookie,
      nsTArray<nsListIter> removedIterList;
      // Prioritize evicting insecure cookies.
      // (draft-ietf-httpbis-cookie-alone section 3.3)
-      mozilla::Maybe<bool> optionalSecurity =
-          mLeaveSecureAlone ? Some(false) : Nothing();
      uint32_t limit = mMaxCookiesPerHost - mCookieQuotaPerHost;
-      FindStaleCookies(entry, currentTime, optionalSecurity, removedIterList,
-                       limit);
+      FindStaleCookies(entry, currentTime, false, removedIterList, limit);
      if (removedIterList.Length() == 0) {
        if (aCookie->IsSecure()) {
          // It's valid to evict a secure cookie for another secure cookie.
-          FindStaleCookies(entry, currentTime, Some(true), removedIterList,
-                           limit);
+          FindStaleCookies(entry, currentTime, true, removedIterList, limit);
        } else {
          Telemetry::Accumulate(Telemetry::COOKIE_LEAVE_SECURE_ALONE,
                                EVICTING_SECURE_BLOCKED);
@ -3651,7 +3636,7 @@ void nsCookieService::AddInternal(const nsCookieKey &aKey, nsCookie *aCookie,
      for (auto it = removedIterList.rbegin(); it != removedIterList.rend();
           it++) {
        RefPtr<nsCookie> evictedCookie = (*it).Cookie();
-        if (mLeaveSecureAlone && evictedCookie->Expiry() <= currentTime) {
+        if (evictedCookie->Expiry() <= currentTime) {
          TelemetryForEvictingStaleCookie(evictedCookie,
                                          evictedCookie->LastAccessed());
        }
@ -4571,8 +4556,7 @@ class CookieIterComparator {
 // Given the output iter array and the count limit, find cookies
 // sort by expiry and lastAccessed time.
 void nsCookieService::FindStaleCookies(nsCookieEntry *aEntry,
-                                       int64_t aCurrentTime,
-                                       const mozilla::Maybe<bool> &aIsSecure,
+                                       int64_t aCurrentTime, bool aIsSecure,
                                       nsTArray<nsListIter> &aOutput,
                                       uint32_t aLimit) {
  MOZ_ASSERT(aLimit);
@ -4591,7 +4575,7 @@ void nsCookieService::FindStaleCookies(nsCookieEntry *aEntry,
      continue;
    }

-    if (aIsSecure.isSome() && !aIsSecure.value()) {
+    if (!aIsSecure) {
      // We want to look for the non-secure cookie first time through,
      // then find the secure cookie the second time this function is called.
      if (cookie->IsSecure()) {
--- a/netwerk/cookie/nsCookieService.h
+++ b/netwerk/cookie/nsCookieService.h
@ -255,8 +255,7 @@ class nsCookieService final : public nsICookieService,
                           bool aRequireHostMatch, CookieStatus aStatus,
                           nsDependentCString &aCookieHeader,
                           int64_t aServerTime, bool aFromHttp,
-                           nsIChannel *aChannel, bool aLeaveSercureAlone,
-                           bool &aSetCookie,
+                           nsIChannel *aChannel, bool &aSetCookie,
                           mozIThirdPartyUtil *aThirdPartyUtil);
  static CookieStatus CheckPrefs(
      nsICookiePermission *aPermissionServices, uint8_t aCookieBehavior,
@ -353,8 +352,8 @@ class nsCookieService final : public nsICookieService,
                  nsListIter &aIter);
  bool FindSecureCookie(const nsCookieKey &aKey, nsCookie *aCookie);
  void FindStaleCookies(nsCookieEntry *aEntry, int64_t aCurrentTime,
-                        const mozilla::Maybe<bool> &aIsSecure,
-                        nsTArray<nsListIter> &aOutput, uint32_t aLimit);
+                        bool aIsSecure, nsTArray<nsListIter> &aOutput,
+                        uint32_t aLimit);
  void TelemetryForEvictingStaleCookie(nsCookie *aEvicted,
                                       int64_t oldestCookieTime);
  void NotifyAccepted(nsIChannel *aChannel);
@ -406,7 +405,6 @@ class nsCookieService final : public nsICookieService,
                            // LIMITFOREIGN}
  bool mThirdPartySession;
  bool mThirdPartyNonsecureSession;
-  bool mLeaveSecureAlone;
  uint16_t mMaxNumberOfCookies;
  uint16_t mMaxCookiesPerHost;
  uint16_t mCookieQuotaPerHost;
--- a/netwerk/test/TestCookie.cpp
+++ b/netwerk/test/TestCookie.cpp
@ -32,8 +32,6 @@ static NS_DEFINE_CID(kPrefServiceCID, NS_PREFSERVICE_CID);
 static const char kCookiesPermissions[] = "network.cookie.cookieBehavior";
 static const char kPrefCookieQuotaPerHost[] = "network.cookie.quotaPerHost";
 static const char kCookiesMaxPerHost[] = "network.cookie.maxPerHost";
-static const char kCookieLeaveSecurityAlone[] =
-    "network.cookie.leave-secure-alone";

 #define OFFSET_ONE_WEEK int64_t(604800) * PR_USEC_PER_SEC
 #define OFFSET_ONE_DAY int64_t(86400) * PR_USEC_PER_SEC
@ -176,7 +174,6 @@ void InitPrefs(nsIPrefBranch *aPrefBranch) {
  // we use the most restrictive set of prefs we can;
  // however, we don't test third party blocking here.
  aPrefBranch->SetIntPref(kCookiesPermissions, 0);  // accept all
-  aPrefBranch->SetBoolPref(kCookieLeaveSecurityAlone, true);
  // Set quotaPerHost to maxPerHost - 1, so there is only one cookie
  // will be evicted everytime.
  aPrefBranch->SetIntPref(kPrefCookieQuotaPerHost, 49);
@ -980,8 +977,6 @@ TEST(TestCookie, TestCookieMain) {
  GetACookie(cookieService, "http://creation.ordering.tests/", nullptr, cookie);
  EXPECT_TRUE(CheckResult(cookie.get(), MUST_EQUAL, expected.get()));

-  // *** eviction and creation ordering tests after enable
-  // network.cookie.leave-secure-alone reset cookie
  cookieMgr->RemoveAll();

  for (int32_t i = 0; i < 60; ++i) {