Bug 1638358 - Cookie Schemeful Same-Site - part 6 - console messages, r=mayhemer

Differential Revision: https://phabricator.services.mozilla.com/D75800
2020-06-01 15:17:31 +00:00 · 2020-06-01 15:17:31 +00:00 · bfdca98faf
--- a/netwerk/cookie/CookieCommons.cpp
+++ b/netwerk/cookie/CookieCommons.cpp
@ -5,6 +5,7 @@

 #include "Cookie.h"
 #include "CookieCommons.h"
+#include "CookieLogging.h"
 #include "CookieService.h"
 #include "mozilla/ContentBlocking.h"
 #include "mozilla/ConsoleReportCollector.h"
@ -18,6 +19,9 @@
 #include "nsIEffectiveTLDService.h"
 #include "nsScriptSecurityManager.h"

+constexpr auto CONSOLE_SCHEMEFUL_CATEGORY =
+    NS_LITERAL_CSTRING("cookieSchemeful");
+
 namespace mozilla {

 using dom::Document;
@ -456,12 +460,11 @@ bool CookieCommons::ShouldIncludeCrossSiteCookieForDocument(Cookie* aCookie) {
  return sameSiteAttr == nsICookie::SAMESITE_NONE;
 }

-// static
-bool CookieCommons::MaybeCompareScheme(Cookie* aCookie,
-                                       nsICookie::schemeType aSchemeType) {
-  if (!StaticPrefs::network_cookie_sameSite_schemeful()) {
-    return true;
-  }
+namespace {
+
+bool MaybeCompareSchemeInternal(Cookie* aCookie,
+                                nsICookie::schemeType aSchemeType) {
+  MOZ_ASSERT(aCookie);

  // This is an old cookie without a scheme yet. Let's consider it valid.
  if (aCookie->SchemeMap() == nsICookie::SCHEME_UNSET) {
@ -471,6 +474,54 @@ bool CookieCommons::MaybeCompareScheme(Cookie* aCookie,
  return !!(aCookie->SchemeMap() & aSchemeType);
 }

+}  // namespace
+
+// static
+bool CookieCommons::MaybeCompareSchemeWithLogging(
+    nsIConsoleReportCollector* aCRC, nsIURI* aHostURI, Cookie* aCookie,
+    nsICookie::schemeType aSchemeType) {
+  MOZ_ASSERT(aCookie);
+  MOZ_ASSERT(aHostURI);
+
+  if (MaybeCompareSchemeInternal(aCookie, aSchemeType)) {
+    return true;
+  }
+
+  nsAutoCString uri;
+  nsresult rv = aHostURI->GetSpec(uri);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return !StaticPrefs::network_cookie_sameSite_schemeful();
+  }
+
+  if (!StaticPrefs::network_cookie_sameSite_schemeful()) {
+    CookieLogging::LogMessageToConsole(
+        aCRC, aHostURI, nsIScriptError::warningFlag, CONSOLE_SCHEMEFUL_CATEGORY,
+        NS_LITERAL_CSTRING("CookieSchemefulRejectForBeta"),
+        AutoTArray<nsString, 2>{NS_ConvertUTF8toUTF16(aCookie->Name()),
+                                NS_ConvertUTF8toUTF16(uri)});
+    return true;
+  }
+
+  CookieLogging::LogMessageToConsole(
+      aCRC, aHostURI, nsIScriptError::warningFlag, CONSOLE_SCHEMEFUL_CATEGORY,
+      NS_LITERAL_CSTRING("CookieSchemefulReject"),
+      AutoTArray<nsString, 2>{NS_ConvertUTF8toUTF16(aCookie->Name()),
+                              NS_ConvertUTF8toUTF16(uri)});
+  return false;
+}
+
+// static
+bool CookieCommons::MaybeCompareScheme(Cookie* aCookie,
+                                       nsICookie::schemeType aSchemeType) {
+  MOZ_ASSERT(aCookie);
+
+  if (!StaticPrefs::network_cookie_sameSite_schemeful()) {
+    return true;
+  }
+
+  return MaybeCompareSchemeInternal(aCookie, aSchemeType);
+}
+
 // static
 nsICookie::schemeType CookieCommons::URIToSchemeType(nsIURI* aURI) {
  MOZ_ASSERT(aURI);
--- a/netwerk/cookie/CookieCommons.h
+++ b/netwerk/cookie/CookieCommons.h
@ -109,6 +109,10 @@ class CookieCommons final {

  static bool ShouldIncludeCrossSiteCookieForDocument(Cookie* aCookie);

+  static bool MaybeCompareSchemeWithLogging(nsIConsoleReportCollector* aCRC,
+                                            nsIURI* aHostURI, Cookie* aCookie,
+                                            nsICookie::schemeType aSchemeType);
+
  static bool MaybeCompareScheme(Cookie* aCookie,
                                 nsICookie::schemeType aSchemeType);

--- a/netwerk/cookie/CookieLogging.cpp
+++ b/netwerk/cookie/CookieLogging.cpp
@ -157,5 +157,28 @@ void CookieLogging::LogEvicted(Cookie* aCookie, const char* details) {
  MOZ_LOG(gCookieLog, LogLevel::Debug, ("\n"));
 }

+// static
+void CookieLogging::LogMessageToConsole(nsIConsoleReportCollector* aCRC,
+                                        nsIURI* aURI, uint32_t aErrorFlags,
+                                        const nsACString& aCategory,
+                                        const nsACString& aMsg,
+                                        const nsTArray<nsString>& aParams) {
+  MOZ_ASSERT(aURI);
+
+  if (!aCRC) {
+    return;
+  }
+
+  nsAutoCString uri;
+  nsresult rv = aURI->GetSpec(uri);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return;
+  }
+
+  aCRC->AddConsoleReport(aErrorFlags, aCategory,
+                         nsContentUtils::eNECKO_PROPERTIES, uri, 0, 0, aMsg,
+                         aParams);
+}
+
 }  // namespace net
 }  // namespace mozilla
--- a/netwerk/cookie/CookieLogging.h
+++ b/netwerk/cookie/CookieLogging.h
@ -9,6 +9,7 @@
 #include "mozilla/Logging.h"
 #include "nsString.h"

+class nsIConsoleReportCollector;
 class nsIURI;

 namespace mozilla {
@ -50,6 +51,12 @@ class CookieLogging final {
  static void LogCookie(Cookie* aCookie);

  static void LogEvicted(Cookie* aCookie, const char* aDetails);
+
+  static void LogMessageToConsole(nsIConsoleReportCollector* aCRC, nsIURI* aURI,
+                                  uint32_t aErrorFlags,
+                                  const nsACString& aCategory,
+                                  const nsACString& aMsg,
+                                  const nsTArray<nsString>& aParams);
 };

 }  // namespace net
--- a/netwerk/cookie/CookieService.cpp
+++ b/netwerk/cookie/CookieService.cpp
@ -910,6 +910,8 @@ void CookieService::GetCookiesForURI(
      !nsContentUtils::IsURIInPrefList(
          aHostURI, "network.cookie.sameSite.laxByDefault.disabledHosts");

+  nsCOMPtr<nsIConsoleReportCollector> crc = do_QueryInterface(aChannel);
+
  // iterate the cookies!
  for (Cookie* cookie : *cookies) {
    // check the host, since the base domain lookup is conservative.
@ -923,7 +925,8 @@ void CookieService::GetCookiesForURI(
    }

    // The scheme doesn't match.
-    if (!CookieCommons::MaybeCompareScheme(cookie, schemeType)) {
+    if (!CookieCommons::MaybeCompareSchemeWithLogging(crc, aHostURI, cookie,
+                                                      schemeType)) {
      continue;
    }

@ -1042,9 +1045,9 @@ bool CookieService::CanSetCookie(
    size.AppendInt(kMaxBytesPerCookie);
    params.AppendElement(size);

-    LogMessageToConsole(aCRC, aHostURI, nsIScriptError::warningFlag,
-                        CONSOLE_OVERSIZE_CATEGORY,
-                        NS_LITERAL_CSTRING("CookieOversize"), params);
+    CookieLogging::LogMessageToConsole(
+        aCRC, aHostURI, nsIScriptError::warningFlag, CONSOLE_OVERSIZE_CATEGORY,
+        NS_LITERAL_CSTRING("CookieOversize"), params);
    return newCookie;
  }

@ -1366,7 +1369,7 @@ bool CookieService::ParseAttributes(nsIConsoleReportCollector* aCRC,
        aCookieData.rawSameSite() = nsICookie::SAMESITE_NONE;
        sameSiteSet = true;
      } else {
-        LogMessageToConsole(
+        CookieLogging::LogMessageToConsole(
            aCRC, aHostURI, nsIScriptError::infoFlag, CONSOLE_SAMESITE_CATEGORY,
            NS_LITERAL_CSTRING("CookieSameSiteValueInvalid"),
            AutoTArray<nsString, 1>{NS_ConvertUTF8toUTF16(aCookieData.name())});
@ -1386,7 +1389,7 @@ bool CookieService::ParseAttributes(nsIConsoleReportCollector* aCRC,
      aCookieData.sameSite() == nsICookie::SAMESITE_NONE) {
    if (laxByDefault &&
        StaticPrefs::network_cookie_sameSite_noneRequiresSecure()) {
-      LogMessageToConsole(
+      CookieLogging::LogMessageToConsole(
          aCRC, aHostURI, nsIScriptError::infoFlag, CONSOLE_SAMESITE_CATEGORY,
          NS_LITERAL_CSTRING("CookieRejectedNonRequiresSecure"),
          AutoTArray<nsString, 1>{NS_ConvertUTF8toUTF16(aCookieData.name())});
@ -1394,7 +1397,7 @@ bool CookieService::ParseAttributes(nsIConsoleReportCollector* aCRC,
    }

    // if sameSite=lax by default is disabled, we want to warn the user.
-    LogMessageToConsole(
+    CookieLogging::LogMessageToConsole(
        aCRC, aHostURI, nsIScriptError::warningFlag, CONSOLE_SAMESITE_CATEGORY,
        NS_LITERAL_CSTRING("CookieRejectedNonRequiresSecureForBeta"),
        AutoTArray<nsString, 2>{NS_ConvertUTF8toUTF16(aCookieData.name()),
@ -1404,12 +1407,12 @@ bool CookieService::ParseAttributes(nsIConsoleReportCollector* aCRC,
  if (aCookieData.rawSameSite() == nsICookie::SAMESITE_NONE &&
      aCookieData.sameSite() == nsICookie::SAMESITE_LAX) {
    if (laxByDefault) {
-      LogMessageToConsole(
+      CookieLogging::LogMessageToConsole(
          aCRC, aHostURI, nsIScriptError::infoFlag, CONSOLE_SAMESITE_CATEGORY,
          NS_LITERAL_CSTRING("CookieLaxForced"),
          AutoTArray<nsString, 1>{NS_ConvertUTF8toUTF16(aCookieData.name())});
    } else {
-      LogMessageToConsole(
+      CookieLogging::LogMessageToConsole(
          aCRC, aHostURI, nsIScriptError::warningFlag,
          CONSOLE_SAMESITE_CATEGORY,
          NS_LITERAL_CSTRING("CookieLaxForcedForBeta"),
@ -1425,29 +1428,6 @@ bool CookieService::ParseAttributes(nsIConsoleReportCollector* aCRC,
  return newCookie;
 }

-// static
-void CookieService::LogMessageToConsole(nsIConsoleReportCollector* aCRC,
-                                        nsIURI* aURI, uint32_t aErrorFlags,
-                                        const nsACString& aCategory,
-                                        const nsACString& aMsg,
-                                        const nsTArray<nsString>& aParams) {
-  MOZ_ASSERT(aURI);
-
-  if (!aCRC) {
-    return;
-  }
-
-  nsAutoCString uri;
-  nsresult rv = aURI->GetSpec(uri);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return;
-  }
-
-  aCRC->AddConsoleReport(aErrorFlags, aCategory,
-                         nsContentUtils::eNECKO_PROPERTIES, uri, 0, 0, aMsg,
-                         aParams);
-}
-
 /******************************************************************************
 * CookieService impl:
 * private domain & permission compliance enforcement functions
@ -1738,9 +1718,9 @@ bool CookieService::CheckPath(CookieStruct& aCookieData,
    size.AppendInt(kMaxBytesPerPath);
    params.AppendElement(size);

-    LogMessageToConsole(aCRC, aHostURI, nsIScriptError::warningFlag,
-                        CONSOLE_OVERSIZE_CATEGORY,
-                        NS_LITERAL_CSTRING("CookiePathOversize"), params);
+    CookieLogging::LogMessageToConsole(
+        aCRC, aHostURI, nsIScriptError::warningFlag, CONSOLE_OVERSIZE_CATEGORY,
+        NS_LITERAL_CSTRING("CookiePathOversize"), params);
    return false;
  }

--- a/netwerk/cookie/CookieService.h
+++ b/netwerk/cookie/CookieService.h
@ -145,12 +145,6 @@ class CookieService final : public nsICookieService,
  nsresult RemoveCookiesFromExactHost(const nsACString& aHost,
                                      const OriginAttributesPattern& aPattern);

-  static void LogMessageToConsole(nsIConsoleReportCollector* aCRC, nsIURI* aURI,
-                                  uint32_t aErrorFlags,
-                                  const nsACString& aCategory,
-                                  const nsACString& aMsg,
-                                  const nsTArray<nsString>& aParams);
-
  // cached members.
  nsCOMPtr<mozIThirdPartyUtil> mThirdPartyUtil;
  nsCOMPtr<nsIEffectiveTLDService> mTLDService;
--- a/netwerk/cookie/test/unit/test_schemeMap.js
+++ b/netwerk/cookie/test/unit/test_schemeMap.js
@ -16,6 +16,23 @@ function inChildProcess() {
  return Services.appinfo.processType != Ci.nsIXULRuntime.PROCESS_TYPE_DEFAULT;
 }

+const { CookieXPCShellUtils } = ChromeUtils.import(
+  "resource://testing-common/CookieXPCShellUtils.jsm"
+);
+
+let cookieXPCShellUtilsInitialized = false;
+function maybeInitializeCookieXPCShellUtils() {
+  if (!cookieXPCShellUtilsInitialized) {
+    cookieXPCShellUtilsInitialized = true;
+    CookieXPCShellUtils.init(this);
+
+    CookieXPCShellUtils.createServer({ hosts: ["example.org"] });
+  }
+}
+
+// Don't pick up default permissions from profile.
+Services.prefs.setCharPref("permissions.manager.defaultsUrl", "");
+
 add_task(async _ => {
  do_get_profile();

@ -80,6 +97,8 @@ add_task(async _ => {
  add_task(async () => {
    do_get_profile();

+    maybeInitializeCookieXPCShellUtils();
+
    // Allow all cookies if the pref service is available in this process.
    if (!inChildProcess()) {
      Services.prefs.setBoolPref(
@ -97,7 +116,7 @@ add_task(async _ => {

    info("Let's set a cookie from HTTP example.org");

-    let uri = NetUtil.newURI("http://example.org/");
+    let uri = NetUtil.newURI("https://example.org/");
    let principal = Services.scriptSecurityManager.createContentPrincipal(
      uri,
      {}
@ -114,7 +133,7 @@ add_task(async _ => {
    let cookies = Services.cookies.getCookieStringFromHttp(uri, channel);
    Assert.equal(cookies, "a=b", "Cookies match");

-    uri = NetUtil.newURI("https://example.org/");
+    uri = NetUtil.newURI("http://example.org/");
    principal = Services.scriptSecurityManager.createContentPrincipal(uri, {});
    channel = NetUtil.newChannel({
      uri,
@ -130,7 +149,7 @@ add_task(async _ => {
      Assert.equal(cookies, "a=b", "Cookie even for different scheme!");
    }

-    cookies = Services.cookies.getCookieStringForPrincipal(principal);
+    cookies = await CookieXPCShellUtils.getCookieStringFromDocument(uri.spec);
    if (schemefulComparison) {
      Assert.equal(cookies, "", "No cookie for different scheme!");
    } else {
@ -191,3 +210,78 @@ add_task(async _ => {

  Services.cookies.removeAll();
 });
+
+[
+  {
+    prefValue: true,
+    consoleMessage: `Cookie “a” has been treated as cross-site against “http://example.org/” because the scheme does not match.`,
+  },
+  {
+    prefValue: false,
+    consoleMessage: `Cookie “a” will be soon treated as cross-site cookie against “http://example.org/” because the scheme does not match.`,
+  },
+].forEach(test => {
+  add_task(async () => {
+    do_get_profile();
+
+    maybeInitializeCookieXPCShellUtils();
+
+    // Allow all cookies if the pref service is available in this process.
+    if (!inChildProcess()) {
+      Services.prefs.setBoolPref(
+        "network.cookie.sameSite.schemeful",
+        test.prefValue
+      );
+      Services.prefs.setIntPref("network.cookie.cookieBehavior", 0);
+      Services.prefs.setBoolPref(
+        "network.cookieJarSettings.unblocked_for_testing",
+        true
+      );
+    }
+
+    let cs = Cc["@mozilla.org/cookieService;1"].getService(Ci.nsICookieService);
+
+    info("Let's set a cookie from HTTPS example.org");
+
+    let uri = NetUtil.newURI("https://example.org/");
+    let principal = Services.scriptSecurityManager.createContentPrincipal(
+      uri,
+      {}
+    );
+    let channel = NetUtil.newChannel({
+      uri,
+      loadingPrincipal: principal,
+      securityFlags: Ci.nsILoadInfo.SEC_ALLOW_CROSS_ORIGIN_DATA_IS_NULL,
+      contentPolicyType: Ci.nsIContentPolicy.TYPE_OTHER,
+    });
+
+    cs.setCookieStringFromHttp(uri, "a=b; sameSite=lax", channel);
+
+    // Create a console listener.
+    let consolePromise = new Promise(resolve => {
+      let listener = {
+        observe(message) {
+          // Ignore unexpected messages.
+          if (!(message instanceof Ci.nsIConsoleMessage)) {
+            return;
+          }
+
+          if (message.message.includes(test.consoleMessage)) {
+            Services.console.unregisterListener(listener);
+            resolve();
+          }
+        },
+      };
+
+      Services.console.registerListener(listener);
+    });
+
+    const contentPage = await CookieXPCShellUtils.loadContentPage(
+      "http://example.org/"
+    );
+    await contentPage.close();
+
+    await consolePromise;
+    Services.cookies.removeAll();
+  });
+});
--- a/netwerk/locales/en-US/necko.properties
+++ b/netwerk/locales/en-US/necko.properties
@ -63,5 +63,9 @@ CookieLaxForcedForBeta=Cookie “%1$S” does not have a proper “sameSite” a
 CookieSameSiteValueInvalid=Invalid “sameSite“ value for cookie “%1$S”. The supported values are: “lax“, “strict“, “none“.
 # LOCALIZATION NOTE (CookieOversize): %1$S is the cookie name. %2$S is the number of bytes. "B" means bytes.
 CookieOversize=Cookie “%1$S” is invalid because its size is too big. Max size is %2$S B.
-# LOCALIZATION NOTE (CookiePathOversiz): %1$S is the cookie name. %2$S is the number of bytes. "B" means bytes.
+# LOCALIZATION NOTE (CookiePathOversize): %1$S is the cookie name. %2$S is the number of bytes. "B" means bytes.
 CookiePathOversize=Cookie “%1$S” is invalid because its path size is too big. Max size is %2$S B.
+# LOCALIZATION NOTE (CookieSchemefulRejectForBeta): %1$S is the cookie name. %2$S is the hostname.
+CookieSchemefulRejectForBeta=Cookie “%1$S” will be soon treated as cross-site cookie against “%2$S” because the scheme does not match.
+# LOCALIZATION NOTE (CookieSchemefulReject): %1$S is the cookie name. %2$S is the hostname.
+CookieSchemefulReject=Cookie “%1$S” has been treated as cross-site against “%2$S” because the scheme does not match.