SameSite: Strict/Lax behavior.

net/url_request/url_request_http_job.cc

Index: net/url_request/url_request_http_job.cc
diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc
index ccc38d544ead9009417fdda7e4b7595c53defc7e..067011ccb6c4481ae2375a5a1d4e74e1e070c8c2 100644
--- a/net/url_request/url_request_http_job.cc
+++ b/net/url_request/url_request_http_job.cc
@@ -26,6 +26,7 @@
 #include "net/base/net_errors.h"
 #include "net/base/network_delegate.h"
 #include "net/base/network_quality_estimator.h"
+#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 #include "net/base/sdch_manager.h"
 #include "net/base/sdch_net_log_params.h"
 #include "net/base/url_util.h"
@@ -725,19 +726,36 @@ void URLRequestHttpJob::AddCookieHeaderAndStart() {
     CookieOptions options;
     options.set_include_httponly();
 
-    // TODO(mkwst): If same-site cookies aren't enabled, pretend the request is
-    // same-site regardless, in order to include all cookies. Drop this check
-    // once we decide whether or not we're shipping this feature:
-    // https://crbug.com/459154
+    // Set |options|' SameSiteCookieMode according to the rules laid out in

[mmenke, 2016/03/18 15:58:13]

As-is, this comment really confused me as I tried to parse it.  |options|'s? 
|options| is a variable name, so it's singular.  Or maybe just remove
|options|'?

+    // https://tools.ietf.org/html/draft-west-first-party-cookies. In short,
+    // include both "strict" and "lax" same-site cookies if the request's |URL|,
+    // |initiator|, and |first_party_for_cookies| all have the same registrable
+    // domain. Include "lax" same-site cookies if the request's |URL| and
+    // |first_party_for_cookies| have the same registrable domain, _and_ the
+    // request's |method| is "safe" ("GET" or "HEAD"). Otherwise, do not include
+    // same-site cookies.
     url::Origin requested_origin(request_->url());
+    url::Origin site_for_cookies(request_->first_party_for_cookies());
+
     if (!network_delegate() ||
         !network_delegate()->AreExperimentalCookieFeaturesEnabled()) {
-      options.set_include_same_site();
-    } else if (requested_origin.IsSameOriginWith(
-                   url::Origin(request_->first_party_for_cookies())) &&
-               (IsMethodSafe(request_->method()) ||
-                requested_origin.IsSameOriginWith(request_->initiator()))) {
-      options.set_include_same_site();
+      // TODO(mkwst): If same-site cookies aren't enabled, then tag the request
+      // as including both strict and lax same-site cookies. Drop this check
+      // once the feature is no longer behind a flag: https://crbug.com/459154.

[mmenke, 2016/03/18 15:58:13]

Just a comment on the design of this stuff:  Letting an iframe access a lax
cookie, but then hiding it from POSTs generated by that iframe seems like a
behavior that's going to cause developer confusion.

[Mike West, 2016/03/21 14:41:01]

I don't think there's a scenario in which an request in an iframe would be
same-site enough to get a lax cookie but not a strict cookie. The document is
either same-site with its embedder, in which case it gets both, or it's not
same-site with its embedder, in which case it gets neither.

Strict/lax only has effect on things like redirects and popups, which create a
top-level browsing context whose "first-party for cookies" matches the URL
that's being loaded (modulo the `URLRequest::FirstPartyURLPolicy`). I agree that
this might cause confusion, but my hope is that the majority of the confusion
will actually be on the part of attackers trying to execute CSRF attacks. :)

I've updated the comment in an effort to make that more clear.

+      options.set_same_site_cookie_mode(
+          CookieOptions::SameSiteCookieMode::INCLUDE_STRICT_AND_LAX);
+    } else if (registry_controlled_domains::SameDomainOrHost(
+                   requested_origin, site_for_cookies,
+                   registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES)) {
+      if (registry_controlled_domains::SameDomainOrHost(
+              requested_origin, request_->initiator(),
+              registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES)) {
+        options.set_same_site_cookie_mode(
+            CookieOptions::SameSiteCookieMode::INCLUDE_STRICT_AND_LAX);
+      } else if (IsMethodSafe(request_->method())) {
+        options.set_same_site_cookie_mode(
+            CookieOptions::SameSiteCookieMode::INCLUDE_LAX);
+      }
     }
 
     cookie_store->GetCookieListWithOptionsAsync(