Add UKM events to track cross-scheme cookies on requests/responses

Implement UKM metrics in storage_partition_impl to record when cookies
are sent or set across schemes.
This is the UKM version of the UMAs
Cookie.SameSiteDifferentSchemeRequest and
Cookie.SameSiteDifferentSchemeResponse.

Doing so requires that the CookieInclusionStatus warning field be
expanded to include cross-scheme warnings.

Bug: 1030938
Change-Id: Iad13d610ae39e4d250157394459297698a2879de
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2015353Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Commit-Queue: Steven Bingler <bingler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#736165}

content/browser/storage_partition_impl.cc

@@ -75,11 +75,13 @@
 #include "mojo/public/cpp/bindings/self_owned_receiver.h"
 #include "net/base/net_errors.h"
 #include "net/cookies/canonical_cookie.h"
+#include "net/cookies/cookie_options.h"
 #include "net/cookies/cookie_util.h"
 #include "net/http/http_auth_preferences.h"
 #include "net/ssl/client_cert_store.h"
 #include "net/url_request/url_request_context.h"
 #include "ppapi/buildflags/buildflags.h"
+#include "services/metrics/public/cpp/ukm_builders.h"
 #include "services/network/cookie_manager.h"
 #include "services/network/network_context.h"
 #include "services/network/network_service.h"
@@ -411,6 +413,47 @@ void DeprecateSameSiteCookies(int process_id,
   }
 }
 
+int64_t CrossSchemeWarningToContextInt64(
+    net::CanonicalCookie::CookieInclusionStatus::WarningReason reason) {
+  // Convert from the status's WarningReason enum to a SameSiteCookieContext
+  // enum and cast to a int64_t for UKM. The UKMs are using the
+  // SameSiteCookieContext in order to match up with the UMAs which are
+  // recording similar information.
+  // TODO(https://crbug.com/1046456): Remove after deprecated.
+  switch (reason) {
+    case net::CanonicalCookie::CookieInclusionStatus::
+        WARN_SAMESITE_LAX_METHOD_UNSAFE_CROSS_SCHEME_SECURE_URL:
+      return static_cast<int64_t>(
+          net::CookieOptions::SameSiteCookieContext::
+              SAME_SITE_LAX_METHOD_UNSAFE_CROSS_SCHEME_SECURE_URL);
+    case net::CanonicalCookie::CookieInclusionStatus::
+        WARN_SAMESITE_LAX_CROSS_SCHEME_SECURE_URL:
+      return static_cast<int64_t>(net::CookieOptions::SameSiteCookieContext::
+                                      SAME_SITE_LAX_CROSS_SCHEME_SECURE_URL);
+    case net::CanonicalCookie::CookieInclusionStatus::
+        WARN_SAMESITE_STRICT_CROSS_SCHEME_SECURE_URL:
+      return static_cast<int64_t>(net::CookieOptions::SameSiteCookieContext::
+                                      SAME_SITE_STRICT_CROSS_SCHEME_SECURE_URL);
+    case net::CanonicalCookie::CookieInclusionStatus::
+        WARN_SAMESITE_LAX_METHOD_UNSAFE_CROSS_SCHEME_INSECURE_URL:
+      return static_cast<int64_t>(
+          net::CookieOptions::SameSiteCookieContext::
+              SAME_SITE_LAX_METHOD_UNSAFE_CROSS_SCHEME_INSECURE_URL);
+    case net::CanonicalCookie::CookieInclusionStatus::
+        WARN_SAMESITE_LAX_CROSS_SCHEME_INSECURE_URL:
+      return static_cast<int64_t>(net::CookieOptions::SameSiteCookieContext::
+                                      SAME_SITE_LAX_CROSS_SCHEME_INSECURE_URL);
+    case net::CanonicalCookie::CookieInclusionStatus::
+        WARN_SAMESITE_STRICT_CROSS_SCHEME_INSECURE_URL:
+      return static_cast<int64_t>(
+          net::CookieOptions::SameSiteCookieContext::
+              SAME_SITE_STRICT_CROSS_SCHEME_INSECURE_URL);
+    default:
+      // Return invalid value if there is no cross-scheme warning.
+      return -1;
+  }
+}
+
 void ReportCookiesChangedOnUI(
     std::vector<GlobalFrameRoutingId> destinations,
     const GURL& url,
@@ -443,6 +486,22 @@ void ReportCookiesChangedOnUI(
         web_contents->OnCookieChange(url, site_for_cookies,
                                      cookie_and_status.cookie,
                                      /* blocked_by_policy =*/false);
+
+        // TODO(https://crbug.com/1046456): Remove after deprecated.
+        net::CanonicalCookie::CookieInclusionStatus::WarningReason
+            cross_scheme_warning;
+        if (cookie_and_status.status.HasCrossSchemeWarning(
+                &cross_scheme_warning)) {
+          ukm::SourceId source_id =
+              static_cast<WebContentsImpl*>(web_contents)
+                  ->GetUkmSourceIdForLastCommittedSource();
+
+          int64_t context =
+              CrossSchemeWarningToContextInt64(cross_scheme_warning);
+          ukm::builders::SameSiteDifferentSchemeRequest(source_id)
+              .SetSameSiteContextWithSchemes(context)
+              .Record(ukm::UkmRecorder::Get());
+        }
       }
     }
   }
@@ -460,6 +519,7 @@ void ReportCookiesReadOnUI(
   }
 
   net::CookieList accepted, blocked;
+  std::vector<net::CanonicalCookie::CookieInclusionStatus> accepted_status;
   for (auto& cookie_and_status : cookie_list) {
     if (cookie_and_status.status.HasExclusionReason(
             net::CanonicalCookie::CookieInclusionStatus::
@@ -467,6 +527,7 @@ void ReportCookiesReadOnUI(
       blocked.push_back(std::move(cookie_and_status.cookie));
     } else if (cookie_and_status.status.IsInclude()) {
       accepted.push_back(std::move(cookie_and_status.cookie));
+      accepted_status.push_back(std::move(cookie_and_status.status));
     }
   }
 
@@ -478,6 +539,23 @@ void ReportCookiesReadOnUI(
         continue;
       web_contents->OnCookiesRead(url, site_for_cookies, accepted,
                                   /* blocked_by_policy =*/false);
+
+      // TODO(https://crbug.com/1046456): Remove after deprecated.
+      for (const auto& status : accepted_status) {
+        net::CanonicalCookie::CookieInclusionStatus::WarningReason
+            cross_scheme_warning;
+        if (status.HasCrossSchemeWarning(&cross_scheme_warning)) {
+          ukm::SourceId source_id =
+              static_cast<WebContentsImpl*>(web_contents)
+                  ->GetUkmSourceIdForLastCommittedSource();
+
+          int64_t context =
+              CrossSchemeWarningToContextInt64(cross_scheme_warning);
+          ukm::builders::SameSiteDifferentSchemeResponse(source_id)
+              .SetSameSiteContextWithSchemes(context)
+              .Record(ukm::UkmRecorder::Get());
+        }
+      }
     }
   }
 

net/cookies/canonical_cookie.cc

@@ -557,6 +557,8 @@ CanonicalCookie::CookieInclusionStatus CanonicalCookie::IncludeForRequestURL(
       UMA_HISTOGRAM_ENUMERATION("Cookie.SameSiteDifferentSchemeRequest",
                                 options.same_site_cookie_context_full(),
                                 CookieOptions::SameSiteCookieContext::COUNT);
+      AddSameSiteCrossSchemeWarning(&status,
+                                    options.same_site_cookie_context_full());
     }
   }
 
@@ -656,6 +658,8 @@ void CanonicalCookie::IsSetPermittedInContext(
       UMA_HISTOGRAM_ENUMERATION("Cookie.SameSiteDifferentSchemeResponse",
                                 options.same_site_cookie_context_full(),
                                 CookieOptions::SameSiteCookieContext::COUNT);
+      AddSameSiteCrossSchemeWarning(status,
+                                    options.same_site_cookie_context_full());
     }
   }
 
@@ -748,6 +752,48 @@ std::string CanonicalCookie::BuildCookieLine(
   return cookie_line;
 }
 
+void net::CanonicalCookie::AddSameSiteCrossSchemeWarning(
+    CookieInclusionStatus* status,
+    CookieOptions::SameSiteCookieContext context) const {
+  switch (context) {
+    case CookieOptions::SameSiteCookieContext::
+        SAME_SITE_LAX_METHOD_UNSAFE_CROSS_SCHEME_SECURE_URL:
+      status->AddWarningReason(
+          CookieInclusionStatus::
+              WARN_SAMESITE_LAX_METHOD_UNSAFE_CROSS_SCHEME_SECURE_URL);
+      break;
+    case CookieOptions::SameSiteCookieContext::
+        SAME_SITE_LAX_CROSS_SCHEME_SECURE_URL:
+      status->AddWarningReason(
+          CookieInclusionStatus::WARN_SAMESITE_LAX_CROSS_SCHEME_SECURE_URL);
+      break;
+    case CookieOptions::SameSiteCookieContext::
+        SAME_SITE_STRICT_CROSS_SCHEME_SECURE_URL:
+      status->AddWarningReason(
+          CookieInclusionStatus::WARN_SAMESITE_STRICT_CROSS_SCHEME_SECURE_URL);
+      break;
+    case CookieOptions::SameSiteCookieContext::
+        SAME_SITE_LAX_METHOD_UNSAFE_CROSS_SCHEME_INSECURE_URL:
+      status->AddWarningReason(
+          CookieInclusionStatus::
+              WARN_SAMESITE_LAX_METHOD_UNSAFE_CROSS_SCHEME_INSECURE_URL);
+      break;
+    case CookieOptions::SameSiteCookieContext::
+        SAME_SITE_LAX_CROSS_SCHEME_INSECURE_URL:
+      status->AddWarningReason(
+          CookieInclusionStatus::WARN_SAMESITE_LAX_CROSS_SCHEME_INSECURE_URL);
+      break;
+    case CookieOptions::SameSiteCookieContext::
+        SAME_SITE_STRICT_CROSS_SCHEME_INSECURE_URL:
+      status->AddWarningReason(
+          CookieInclusionStatus::
+              WARN_SAMESITE_STRICT_CROSS_SCHEME_INSECURE_URL);
+      break;
+    default:
+      break;
+  }
+}
+
 // static
 CanonicalCookie::CookiePrefix CanonicalCookie::GetCookiePrefix(
     const std::string& name) {
@@ -920,6 +966,29 @@ bool CanonicalCookie::CookieInclusionStatus::HasWarningReason(
   return warning_reasons_ & GetWarningBitmask(reason);
 }
 
+bool net::CanonicalCookie::CookieInclusionStatus::HasCrossSchemeWarning(
+    CookieInclusionStatus::WarningReason* reason) const {
+  if (!ShouldWarn())
+    return false;
+
+  const CookieInclusionStatus::WarningReason cross_scheme_warnings[] = {
+      WARN_SAMESITE_LAX_METHOD_UNSAFE_CROSS_SCHEME_SECURE_URL,
+      WARN_SAMESITE_LAX_CROSS_SCHEME_SECURE_URL,
+      WARN_SAMESITE_STRICT_CROSS_SCHEME_SECURE_URL,
+      WARN_SAMESITE_LAX_METHOD_UNSAFE_CROSS_SCHEME_INSECURE_URL,
+      WARN_SAMESITE_LAX_CROSS_SCHEME_INSECURE_URL,
+      WARN_SAMESITE_STRICT_CROSS_SCHEME_INSECURE_URL};
+  for (const auto warning : cross_scheme_warnings) {
+    if (HasWarningReason(warning)) {
+      if (reason)
+        *reason = warning;
+      return true;
+    }
+  }
+
+  return false;
+}
+
 void CanonicalCookie::CookieInclusionStatus::AddWarningReason(
     WarningReason reason) {
   warning_reasons_ |= GetWarningBitmask(reason);
@@ -981,6 +1050,21 @@ std::string CanonicalCookie::CookieInclusionStatus::GetDebugString() const {
     base::StrAppend(&out, {"WARN_SAMESITE_NONE_INSECURE, "});
   if (HasWarningReason(WARN_SAMESITE_UNSPECIFIED_LAX_ALLOW_UNSAFE))
     base::StrAppend(&out, {"WARN_SAMESITE_UNSPECIFIED_LAX_ALLOW_UNSAFE, "});
+  if (HasWarningReason(WARN_SAMESITE_LAX_METHOD_UNSAFE_CROSS_SCHEME_SECURE_URL))
+    base::StrAppend(
+        &out, {"WARN_SAMESITE_LAX_METHOD_UNSAFE_CROSS_SCHEME_SECURE_URL, "});
+  if (HasWarningReason(WARN_SAMESITE_LAX_CROSS_SCHEME_SECURE_URL))
+    base::StrAppend(&out, {"WARN_SAMESITE_LAX_CROSS_SCHEME_SECURE_URL, "});
+  if (HasWarningReason(WARN_SAMESITE_STRICT_CROSS_SCHEME_SECURE_URL))
+    base::StrAppend(&out, {"WARN_SAMESITE_STRICT_CROSS_SCHEME_SECURE_URL, "});
+  if (HasWarningReason(
+          WARN_SAMESITE_LAX_METHOD_UNSAFE_CROSS_SCHEME_INSECURE_URL))
+    base::StrAppend(
+        &out, {"WARN_SAMESITE_LAX_METHOD_UNSAFE_CROSS_SCHEME_INSECURE_URL, "});
+  if (HasWarningReason(WARN_SAMESITE_LAX_CROSS_SCHEME_INSECURE_URL))
+    base::StrAppend(&out, {"WARN_SAMESITE_LAX_CROSS_SCHEME_INSECURE_URL, "});
+  if (HasWarningReason(WARN_SAMESITE_UNSPECIFIED_LAX_ALLOW_UNSAFE))
+    base::StrAppend(&out, {"WARN_SAMESITE_UNSPECIFIED_LAX_ALLOW_UNSAFE, "});
 
   // Strip trailing comma and space.
   out.erase(out.end() - 2, out.end());

net/cookies/canonical_cookie.h

@@ -265,6 +265,12 @@ class NET_EXPORT CanonicalCookie {
     COOKIE_PREFIX_LAST
   };
 
+  // Applies the appropriate warning for the given cross-scheme
+  // SameSiteCookieContext.
+  void AddSameSiteCrossSchemeWarning(
+      CookieInclusionStatus* status,
+      const CookieOptions::SameSiteCookieContext context) const;
+
   // Returns the CookiePrefix (or COOKIE_PREFIX_NONE if none) that
   // applies to the given cookie |name|.
   static CookiePrefix GetCookiePrefix(const std::string& name);
@@ -373,6 +379,16 @@ class NET_EXPORT CanonicalCookie::CookieInclusionStatus {
     // enough to activate the Lax-allow-unsafe intervention.
     WARN_SAMESITE_UNSPECIFIED_LAX_ALLOW_UNSAFE = 2,
 
+    // The following warnings indicate that a SameSite cookie is being sent/set
+    // across schemes and with what same-site context.
+    // See CookieOptions::SameSiteCookieContext.
+    WARN_SAMESITE_LAX_METHOD_UNSAFE_CROSS_SCHEME_SECURE_URL = 3,
+    WARN_SAMESITE_LAX_CROSS_SCHEME_SECURE_URL = 4,
+    WARN_SAMESITE_STRICT_CROSS_SCHEME_SECURE_URL = 5,
+    WARN_SAMESITE_LAX_METHOD_UNSAFE_CROSS_SCHEME_INSECURE_URL = 6,
+    WARN_SAMESITE_LAX_CROSS_SCHEME_INSECURE_URL = 7,
+    WARN_SAMESITE_STRICT_CROSS_SCHEME_INSECURE_URL = 8,
+
     // This should be kept last.
     NUM_WARNING_REASONS
   };
@@ -412,6 +428,12 @@ class NET_EXPORT CanonicalCookie::CookieInclusionStatus {
   // Whether the given reason for warning is present.
   bool HasWarningReason(WarningReason reason) const;
 
+  // Whether a cross-scheme warning is present.
+  // If the function returns true and |reason| is valid then |reason| will
+  // contain which warning was found.
+  bool HasCrossSchemeWarning(
+      CookieInclusionStatus::WarningReason* reason = nullptr) const;
+
   // Add an warning reason.
   void AddWarningReason(WarningReason reason);
 

net/cookies/canonical_cookie_unittest.cc

@@ -735,12 +735,20 @@ TEST(CanonicalCookieTest, IncludeForRequestURLSameSite) {
        CookieEffectiveSameSite::STRICT_MODE,
        CookieOptions::SameSiteCookieContext::
            SAME_SITE_STRICT_CROSS_SCHEME_SECURE_URL,
-       CanonicalCookie::CookieInclusionStatus()},
+       CanonicalCookie::CookieInclusionStatus::MakeFromReasonsForTesting(
+           std::vector<
+               CanonicalCookie::CookieInclusionStatus::ExclusionReason>(),
+           {CanonicalCookie::CookieInclusionStatus::
+                WARN_SAMESITE_STRICT_CROSS_SCHEME_SECURE_URL})},
       {"Common=6;SameSite=Strict", CookieSameSite::STRICT_MODE,
        CookieEffectiveSameSite::STRICT_MODE,
        CookieOptions::SameSiteCookieContext::
            SAME_SITE_STRICT_CROSS_SCHEME_INSECURE_URL,
-       CanonicalCookie::CookieInclusionStatus()},
+       CanonicalCookie::CookieInclusionStatus::MakeFromReasonsForTesting(
+           std::vector<
+               CanonicalCookie::CookieInclusionStatus::ExclusionReason>(),
+           {CanonicalCookie::CookieInclusionStatus::
+                WARN_SAMESITE_STRICT_CROSS_SCHEME_INSECURE_URL})},
       // Lax cookies:
       {"Common=7;SameSite=Lax", CookieSameSite::LAX_MODE,
        CookieEffectiveSameSite::LAX_MODE,
@@ -764,22 +772,38 @@ TEST(CanonicalCookieTest, IncludeForRequestURLSameSite) {
        CookieEffectiveSameSite::LAX_MODE,
        CookieOptions::SameSiteCookieContext::
            SAME_SITE_LAX_CROSS_SCHEME_SECURE_URL,
-       CanonicalCookie::CookieInclusionStatus()},
+       CanonicalCookie::CookieInclusionStatus::MakeFromReasonsForTesting(
+           std::vector<
+               CanonicalCookie::CookieInclusionStatus::ExclusionReason>(),
+           {CanonicalCookie::CookieInclusionStatus::
+                WARN_SAMESITE_LAX_CROSS_SCHEME_SECURE_URL})},
       {"Common=12;SameSite=Lax", CookieSameSite::LAX_MODE,
        CookieEffectiveSameSite::LAX_MODE,
        CookieOptions::SameSiteCookieContext::
            SAME_SITE_STRICT_CROSS_SCHEME_SECURE_URL,
-       CanonicalCookie::CookieInclusionStatus()},
+       CanonicalCookie::CookieInclusionStatus::MakeFromReasonsForTesting(
+           std::vector<
+               CanonicalCookie::CookieInclusionStatus::ExclusionReason>(),
+           {CanonicalCookie::CookieInclusionStatus::
+                WARN_SAMESITE_STRICT_CROSS_SCHEME_SECURE_URL})},
       {"Common=13;SameSite=Lax", CookieSameSite::LAX_MODE,
        CookieEffectiveSameSite::LAX_MODE,
        CookieOptions::SameSiteCookieContext::
            SAME_SITE_LAX_CROSS_SCHEME_INSECURE_URL,
-       CanonicalCookie::CookieInclusionStatus()},
+       CanonicalCookie::CookieInclusionStatus::MakeFromReasonsForTesting(
+           std::vector<
+               CanonicalCookie::CookieInclusionStatus::ExclusionReason>(),
+           {CanonicalCookie::CookieInclusionStatus::
+                WARN_SAMESITE_LAX_CROSS_SCHEME_INSECURE_URL})},
       {"Common=14;SameSite=Lax", CookieSameSite::LAX_MODE,
        CookieEffectiveSameSite::LAX_MODE,
        CookieOptions::SameSiteCookieContext::
            SAME_SITE_STRICT_CROSS_SCHEME_INSECURE_URL,
-       CanonicalCookie::CookieInclusionStatus()},
+       CanonicalCookie::CookieInclusionStatus::MakeFromReasonsForTesting(
+           std::vector<
+               CanonicalCookie::CookieInclusionStatus::ExclusionReason>(),
+           {CanonicalCookie::CookieInclusionStatus::
+                WARN_SAMESITE_STRICT_CROSS_SCHEME_INSECURE_URL})},
       // None and Secure cookies:
       {"Common=15;SameSite=None;Secure", CookieSameSite::NO_RESTRICTION,
        CookieEffectiveSameSite::NO_RESTRICTION,
@@ -2445,4 +2469,39 @@ TEST(CookieInclusionStatusTest, RemoveWarningReason) {
           WARN_SAMESITE_UNSPECIFIED_CROSS_SITE_CONTEXT));
 }
 
+TEST(CookieInclusionStatusTest, HasCrossSchemeWarning) {
+  std::vector<CanonicalCookie::CookieInclusionStatus::WarningReason>
+      cross_scheme_warnings = {
+          CanonicalCookie::CookieInclusionStatus::
+              WARN_SAMESITE_LAX_METHOD_UNSAFE_CROSS_SCHEME_SECURE_URL,
+          CanonicalCookie::CookieInclusionStatus::
+              WARN_SAMESITE_LAX_CROSS_SCHEME_SECURE_URL,
+          CanonicalCookie::CookieInclusionStatus::
+              WARN_SAMESITE_STRICT_CROSS_SCHEME_SECURE_URL,
+          CanonicalCookie::CookieInclusionStatus::
+              WARN_SAMESITE_LAX_METHOD_UNSAFE_CROSS_SCHEME_INSECURE_URL,
+          CanonicalCookie::CookieInclusionStatus::
+              WARN_SAMESITE_LAX_CROSS_SCHEME_INSECURE_URL,
+          CanonicalCookie::CookieInclusionStatus::
+              WARN_SAMESITE_STRICT_CROSS_SCHEME_INSECURE_URL};
+
+  CanonicalCookie::CookieInclusionStatus empty_status;
+  EXPECT_FALSE(empty_status.HasCrossSchemeWarning());
+
+  CanonicalCookie::CookieInclusionStatus not_cross_scheme;
+  not_cross_scheme.AddWarningReason(
+      CanonicalCookie::CookieInclusionStatus::
+          WARN_SAMESITE_UNSPECIFIED_CROSS_SITE_CONTEXT);
+  EXPECT_FALSE(not_cross_scheme.HasCrossSchemeWarning());
+
+  for (auto warning : cross_scheme_warnings) {
+    CanonicalCookie::CookieInclusionStatus status;
+    status.AddWarningReason(warning);
+    CanonicalCookie::CookieInclusionStatus::WarningReason reason;
+
+    EXPECT_TRUE(status.HasCrossSchemeWarning(&reason));
+    EXPECT_EQ(warning, reason);
+  }
+}
+
 }  // namespace net

tools/metrics/ukm/ukm.xml

@@ -8199,6 +8199,28 @@ be describing additional metrics about the same event.
   </metric>
 </event>
 
+<event name="SameSiteDifferentSchemeRequest">
+  <owner>bingler@chromium.org</owner>
+  <metric name="SameSiteContextWithSchemes" enum="SameSiteCookieContext">
+    <summary>
+      An enum that records the type of same-site context if a cookie marked with
+      SameSite=Lax or SameSite=Strict is sent across schemes on a request. E.x.:
+      An http site making a request to an https site.
+    </summary>
+  </metric>
+</event>
+
+<event name="SameSiteDifferentSchemeResponse">
+  <owner>bingler@chromium.org</owner>
+  <metric name="SameSiteContextWithSchemes" enum="SameSiteCookieContext">
+    <summary>
+      An enum that records the type of same-site context if a cookie marked with
+      SameSite=Lax or SameSite=Strict is set across schemes on a response. E.x.:
+      An http site returning a response to an https site.
+    </summary>
+  </metric>
+</event>
+
 <event name="ScreenBrightness">
   <owner>pdyson@chromium.org</owner>
   <summary>