SameSite: Strict/Lax behavior.

content/browser/frame_host/render_frame_message_filter.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/frame_host/render_frame_message_filter.h"
 
 #include "base/command_line.h"
 #include "base/macros.h"
 #include "base/metrics/field_trial.h"
 #include "base/strings/string_util.h"
 #include "build/build_config.h"
 #include "content/browser/bad_message.h"
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/frame_host/render_frame_host_impl.h"
 #include "content/browser/gpu/gpu_data_manager_impl.h"
 #include "content/browser/renderer_host/render_widget_helper.h"
 #include "content/common/frame_messages.h"
 #include "content/common/view_messages.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/common/content_constants.h"
 #include "content/public/common/content_switches.h"
 #include "gpu/GLES2/gl2extchromium.h"
+#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 #include "net/cookies/cookie_options.h"
 #include "net/cookies/cookie_store.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "url/gurl.h"
 
 #if !defined(OS_MACOSX)
 #include "third_party/khronos/GLES2/gl2.h"
 #include "third_party/khronos/GLES2/gl2ext.h"
 #endif
@@ skipping 333 matching lines @@
           render_process_id_, render_frame_id, options)) {
     net::URLRequestContext* context = GetRequestContextForURL(url);
     // Pass a null callback since we don't care about when the 'set' completes.
     context->cookie_store()->SetCookieWithOptionsAsync(
         url, cookie, options, net::CookieStore::SetCookiesCallback());
   }
 }
 
 void RenderFrameMessageFilter::OnGetCookies(int render_frame_id,
                                             const GURL& url,
                                             const GURL& first_party_for_cookies,

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

first_party_for_cookies is the main frame URL here?

                                             IPC::Message* reply_msg) {
   ChildProcessSecurityPolicyImpl* policy =
       ChildProcessSecurityPolicyImpl::GetInstance();
   if (!policy->CanAccessDataForOrigin(render_process_id_, url)) {
     bad_message::ReceivedBadMessage(this,
                                     bad_message::RFMF_GET_COOKIES_BAD_ORIGIN);
     delete reply_msg;
     return;
   }
 
   // If we crash here, figure out what URL the renderer was requesting.
   // http://crbug.com/99242
   char url_buf[128];
   base::strlcpy(url_buf, url.spec().c_str(), arraysize(url_buf));
   base::debug::Alias(url_buf);
 
   net::URLRequestContext* context = GetRequestContextForURL(url);
 
   net::CookieOptions options;
-  options.set_include_same_site();
+  if (net::registry_controlled_domains::SameDomainOrHost(
+          url, first_party_for_cookies,
+          net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES)) {
+    // TODO(mkwst): This check ought to further distinguish between frames
+    // initiated in a strict or lax same-site context.,[

[Mike West, 2016/03/18 14:27:17]

Honestly, I need to think about whether or not this is a bug. I think it is, but
maybe the behavior makes sense? Idunno. It's not clear that we need to reason
about the initiator after we've already opened and rendered a document.

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

My feeling is that the cookies we send while requesting the HTML file should
exactly match the cookies that the HTML file can access, modulo new cookies and
expiring cookies.  Anything else seems weird to me.

If the thinking is that the iframe knows its an iframe, and can behavior
correspondingly, seems like we could just send that information as a request
header instead of a new SameSite value - that makes the cookie security model
consistent.  If the idea is we should hide cookies based on the "initiator",
seems like should be defined the same way for HTTP and HTML.

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

nit:  Remove the ",["

+    options.set_same_site_cookie_mode(
+        net::CookieOptions::SameSiteCookieMode::INCLUDE_STRICT_AND_LAX);
+  } else {
+    options.set_same_site_cookie_mode(
+        net::CookieOptions::SameSiteCookieMode::DO_NOT_INCLUDE);
+  }
+
   context->cookie_store()->GetCookieListWithOptionsAsync(
       url, options,
       base::Bind(&RenderFrameMessageFilter::CheckPolicyForCookies, this,
                  render_frame_id, url, first_party_for_cookies, reply_msg));
 }
 
 void RenderFrameMessageFilter::OnCookiesEnabled(
     int render_frame_id,
     const GURL& url,
     const GURL& first_party_for_cookies,
@@ skipping 240 matching lines @@
   net::URLRequestContext* context =
       GetContentClient()->browser()->OverrideRequestContextForURL(
           url, resource_context_);
   if (!context)
     context = request_context_->GetURLRequestContext();
 
   return context;
 }
 
 }  // namespace content