Implement $Secure- cookie prefix

This CL implements the rule that cookies whose names start with
$Secure- can only be set if the Secure attribute is enabled. The
implementation is a runtime-enabled web platform feature for now.

Intent to Implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/IU5t6eLuS2Y/Uq-7Kat9BwAJ

BUG=541511
TBR=droger@chromium.org

Committed: https://crrev.com/467a098174e062d4380eb0ad8f7b2f0b0b7ed5fa
Cr-Commit-Position: refs/heads/master@{#354676}

Review URL: https://codereview.chromium.org/1393193005

Cr-Commit-Position: refs/heads/master@{#354832}

8 files changed, 268 insertions, 6 deletions

diff --git a/net/cookies/cookie_monster.cc b/net/cookies/cookie_monster.cc
index 2292f8c..0f8ba79 100644
--- a/net/cookies/cookie_monster.cc
+++ b/net/cookies/cookie_monster.cc
@@ -331,6 +331,13 @@ void RunAsync(scoped_refptr<base::TaskRunner> proxy,
   proxy->PostTask(FROM_HERE, base::Bind(callback, cookie, removed));
 }
 
+bool CheckCookiePrefix(CanonicalCookie* cc, const CookieOptions& options) {
+  const char kSecurePrefix[] = "$Secure-";
+  if (cc->Name().find(kSecurePrefix) == 0)
+    return cc->IsSecure() && cc->Source().SchemeIsCryptographic();
+  return true;
+}
+
 }  // namespace
 
 CookieMonster::CookieMonster(PersistentCookieStore* store,
@@ -439,6 +446,7 @@ class CookieMonster::SetCookieWithDetailsTask : public CookieMonsterTask {
                            bool secure,
                            bool http_only,
                            bool first_party_only,
+                           bool enforce_prefixes,
                            CookiePriority priority,
                            const SetCookiesCallback& callback)
       : CookieMonsterTask(cookie_monster),
@@ -451,6 +459,7 @@ class CookieMonster::SetCookieWithDetailsTask : public CookieMonsterTask {
         secure_(secure),
         http_only_(http_only),
         first_party_only_(first_party_only),
+        enforce_prefixes_(enforce_prefixes),
         priority_(priority),
         callback_(callback) {}
 
@@ -470,6 +479,7 @@ class CookieMonster::SetCookieWithDetailsTask : public CookieMonsterTask {
   bool secure_;
   bool http_only_;
   bool first_party_only_;
+  bool enforce_prefixes_;
   CookiePriority priority_;
   SetCookiesCallback callback_;
 
@@ -479,7 +489,7 @@ class CookieMonster::SetCookieWithDetailsTask : public CookieMonsterTask {
 void CookieMonster::SetCookieWithDetailsTask::Run() {
   bool success = this->cookie_monster()->SetCookieWithDetails(
       url_, name_, value_, domain_, path_, expiration_time_, secure_,
-      http_only_, first_party_only_, priority_);
+      http_only_, first_party_only_, enforce_prefixes_, priority_);
   if (!callback_.is_null()) {
     this->InvokeCallback(base::Bind(&SetCookiesCallback::Run,
                                     base::Unretained(&callback_), success));
@@ -928,11 +938,12 @@ void CookieMonster::SetCookieWithDetailsAsync(
     bool secure,
     bool http_only,
     bool first_party_only,
+    bool enforce_prefixes,
     CookiePriority priority,
     const SetCookiesCallback& callback) {
   scoped_refptr<SetCookieWithDetailsTask> task = new SetCookieWithDetailsTask(
       this, url, name, value, domain, path, expiration_time, secure, http_only,
-      first_party_only, priority, callback);
+      first_party_only, enforce_prefixes, priority, callback);
   DoCookieTaskForURL(task, url);
 }
 
@@ -1112,6 +1123,7 @@ bool CookieMonster::SetCookieWithDetails(const GURL& url,
                                          bool secure,
                                          bool http_only,
                                          bool first_party_only,
+                                         bool enforce_prefixes,
                                          CookiePriority priority) {
   base::AutoLock autolock(lock_);
 
@@ -1132,6 +1144,8 @@ bool CookieMonster::SetCookieWithDetails(const GURL& url,
   CookieOptions options;
   options.set_include_httponly();
   options.set_include_first_party_only();
+  if (enforce_prefixes)
+    options.set_enforce_prefixes();
   return SetCanonicalCookie(&cc, creation_time, options);
 }
 
@@ -1888,6 +1902,12 @@ bool CookieMonster::SetCanonicalCookie(scoped_ptr<CanonicalCookie>* cc,
   const std::string key(GetKey((*cc)->Domain()));
   bool already_expired = (*cc)->IsExpired(creation_time);
 
+  if (options.enforce_prefixes() && !CheckCookiePrefix(cc->get(), options)) {
+    VLOG(kVlogSetCookies) << "SetCookie() not storing cookie '" << (*cc)->Name()
+                          << "' that violates prefix rules.";
+    return false;
+  }
+
   if (DeleteAnyEquivalentCookie(key, **cc, options.exclude_httponly(),
                                 already_expired)) {
     VLOG(kVlogSetCookies) << "SetCookie() not clobbering httponly cookie";
diff --git a/net/cookies/cookie_monster.h b/net/cookies/cookie_monster.h
index 72d09d0..8426d72 100644
--- a/net/cookies/cookie_monster.h
+++ b/net/cookies/cookie_monster.h
@@ -167,6 +167,7 @@ class NET_EXPORT CookieMonster : public CookieStore {
                                  bool secure,
                                  bool http_only,
                                  bool first_party,
+                                 bool enforce_prefixes,
                                  CookiePriority priority,
                                  const SetCookiesCallback& callback);
 
@@ -464,6 +465,7 @@ class NET_EXPORT CookieMonster : public CookieStore {
                             bool secure,
                             bool http_only,
                             bool first_party,
+                            bool enforce_prefixes,
                             CookiePriority priority);
 
   CookieList GetAllCookies();
diff --git a/net/cookies/cookie_monster_unittest.cc b/net/cookies/cookie_monster_unittest.cc
index 839add8..82ae3a2 100644
--- a/net/cookies/cookie_monster_unittest.cc
+++ b/net/cookies/cookie_monster_unittest.cc
@@ -100,6 +100,7 @@ struct CookieMonsterTestTraits {
   static const bool filters_schemes = true;
   static const bool has_path_prefix_bug = false;
   static const int creation_time_granularity_in_ms = 0;
+  static const bool enforces_prefixes = true;
 };
 
 INSTANTIATE_TYPED_TEST_CASE_P(CookieMonster,
@@ -160,7 +161,7 @@ class CookieMonsterTest : public CookieStoreTest<CookieMonsterTestTraits> {
     ResultSavingCookieCallback<bool> callback;
     cm->SetCookieWithDetailsAsync(
         url, name, value, domain, path, expiration_time, secure, http_only,
-        first_party_only, priority,
+        first_party_only, false /* enforce prefixes */, priority,
         base::Bind(&ResultSavingCookieCallback<bool>::Run,
                    base::Unretained(&callback)));
     RunFor(kTimeout);
@@ -670,8 +671,8 @@ ACTION_P4(DeleteAllCreatedBetweenAction,
 ACTION_P3(SetCookieWithDetailsAction, cookie_monster, cc, callback) {
   cookie_monster->SetCookieWithDetailsAsync(
       cc.url, cc.name, cc.value, cc.domain, cc.path, cc.expiration_time,
-      cc.secure, cc.http_only, cc.first_party_only, cc.priority,
-      callback->AsCallback());
+      cc.secure, cc.http_only, cc.first_party_only,
+      false /* enforce prefixes */, cc.priority, callback->AsCallback());
 }
 
 ACTION_P2(GetAllCookiesAction, cookie_monster, callback) {
@@ -2475,7 +2476,7 @@ class MultiThreadedCookieMonsterTest : public CookieMonsterTest {
     CookiePriority priority = COOKIE_PRIORITY_DEFAULT;
     cm->SetCookieWithDetailsAsync(
         url, name, value, domain, path, expiration_time, secure, http_only,
-        first_party_only, priority,
+        first_party_only, false /* enforce prefixes */, priority,
         base::Bind(&ResultSavingCookieCallback<bool>::Run,
                    base::Unretained(callback)));
   }
@@ -2966,6 +2967,29 @@ TEST_F(CookieMonsterTest, CookieSourceHistogram) {
       CookieMonster::COOKIE_SOURCE_NONSECURE_COOKIE_NONCRYPTOGRAPHIC_SCHEME, 1);
 }
 
+TEST_F(CookieMonsterTest, SecureCookiePrefix) {
+  scoped_refptr<CookieMonster> cm(new CookieMonster(NULL, NULL));
+  // A $Secure- cookie must be Secure.
+  EXPECT_FALSE(SetCookie(cm.get(), https_www_google_.url(), "$Secure-A=B"));
+  EXPECT_FALSE(
+      SetCookie(cm.get(), https_www_google_.url(), "$Secure-A=B; httponly"));
+
+  // A typoed prefix does not have to be Secure.
+  EXPECT_TRUE(
+      SetCookie(cm.get(), https_www_google_.url(), "$secure-A=B; Secure"));
+  EXPECT_TRUE(SetCookie(cm.get(), https_www_google_.url(), "$secure-A=C;"));
+  EXPECT_TRUE(
+      SetCookie(cm.get(), https_www_google_.url(), "$SecureA=B; Secure"));
+  EXPECT_TRUE(SetCookie(cm.get(), https_www_google_.url(), "$SecureA=C;"));
+
+  EXPECT_TRUE(
+      SetCookie(cm.get(), https_www_google_.url(), "$Secure-A=B; Secure"));
+
+  // A $Secure- cookie can't be set on a non-secure origin.
+  EXPECT_FALSE(
+      SetCookie(cm.get(), http_www_google_.url(), "$Secure-A=B; Secure"));
+}
+
 class CookieMonsterNotificationTest : public CookieMonsterTest {
  public:
   CookieMonsterNotificationTest()
diff --git a/net/cookies/cookie_options.cc b/net/cookies/cookie_options.cc
index 5cacb6b..73b7641 100644
--- a/net/cookies/cookie_options.cc
+++ b/net/cookies/cookie_options.cc
@@ -11,6 +11,7 @@ namespace net {
 CookieOptions::CookieOptions()
     : exclude_httponly_(true),
       include_first_party_only_(false),
+      enforce_prefixes_(false),
       server_time_() {}
 
 }  // namespace net
diff --git a/net/cookies/cookie_options.h b/net/cookies/cookie_options.h
index d3d7fd0..9b40886 100644
--- a/net/cookies/cookie_options.h
+++ b/net/cookies/cookie_options.h
@@ -35,6 +35,11 @@ class NET_EXPORT CookieOptions {
   void set_first_party_url(const GURL& url) { first_party_url_ = url; }
   GURL first_party_url() const { return first_party_url_; }
 
+  // TODO(estark): Remove once we decide whether to ship cookie
+  // prefixes. https://crbug.com/541511
+  void set_enforce_prefixes() { enforce_prefixes_ = true; }
+  bool enforce_prefixes() const { return enforce_prefixes_; }
+
   // |server_time| indicates what the server sending us the Cookie thought the
   // current time was when the cookie was produced.  This is used to adjust for
   // clock skew between server and host.
@@ -48,6 +53,7 @@ class NET_EXPORT CookieOptions {
   bool exclude_httponly_;
   bool include_first_party_only_;
   GURL first_party_url_;
+  bool enforce_prefixes_;
   base::Time server_time_;
 };
 
diff --git a/net/cookies/cookie_store_unittest.h b/net/cookies/cookie_store_unittest.h
index b380fbc..f62f7ac 100644
--- a/net/cookies/cookie_store_unittest.h
+++ b/net/cookies/cookie_store_unittest.h
@@ -69,6 +69,9 @@ const char kValidCookieLine[] = "A=B; path=/";
 //   // Time to wait between two cookie insertions to ensure that cookies have
 //   // different creation times.
 //   static const int creation_time_granularity_in_ms;
+//
+//   // The cookie store enforces cookie prefixes.
+//   static const bool enforces_prefixes;
 // };
 
 template <class CookieStoreTestTraits>
@@ -155,6 +158,8 @@ class CookieStoreTest : public testing::Test {
     CookieOptions options;
     if (!CookieStoreTestTraits::supports_http_only)
       options.set_include_httponly();
+    if (CookieStoreTestTraits::enforces_prefixes)
+      options.set_enforce_prefixes();
     return SetCookieWithOptions(cs, url, cookie_line, options);
   }
 
diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc
index 3cace90..263f502 100644
--- a/net/url_request/url_request_http_job.cc
+++ b/net/url_request/url_request_http_job.cc
@@ -749,6 +749,8 @@ void URLRequestHttpJob::SaveNextCookie() {
     CookieOptions options;
     options.set_include_httponly();
     options.set_server_time(response_date_);
+    if (network_delegate()->AreExperimentalCookieFeaturesEnabled())
+      options.set_enforce_prefixes();
 
     CookieStore::SetCookiesCallback callback(base::Bind(
         &URLRequestHttpJob::OnCookieSaved, weak_factory_.GetWeakPtr(),
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index f91a283..530c3e6 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -78,6 +78,9 @@
 #include "net/ssl/ssl_cipher_suite_names.h"
 #include "net/ssl/ssl_connection_status_flags.h"
 #include "net/test/cert_test_util.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
+#include "net/test/embedded_test_server/http_request.h"
+#include "net/test/embedded_test_server/http_response.h"
 #include "net/test/spawned_test_server/spawned_test_server.h"
 #include "net/test/url_request/url_request_failed_job.h"
 #include "net/url_request/data_protocol_handler.h"
@@ -662,6 +665,11 @@ class MockCertificateReportSender
   std::string latest_report_;
 };
 
+class TestExperimentalFeaturesNetworkDelegate : public TestNetworkDelegate {
+ public:
+  bool OnAreExperimentalCookieFeaturesEnabled() const override { return true; }
+};
+
 }  // namespace
 
 // Inherit PlatformTest since we require the autorelease pool on Mac OS X.
@@ -2202,6 +2210,24 @@ class LocalHttpTestServer : public SpawnedTestServer {
                           base::FilePath()) {}
 };
 
+scoped_ptr<net::test_server::HttpResponse> HandleSetCookieRequest(
+    const test_server::HttpRequest& request) {
+  scoped_ptr<test_server::BasicHttpResponse> http_response(
+      new test_server::BasicHttpResponse());
+  if (request.relative_url.find("/set-cookie?") != 0) {
+    http_response->set_code(net::HTTP_NOT_FOUND);
+    http_response->set_content("hello");
+    return http_response.Pass();
+  }
+  http_response->set_code(net::HTTP_OK);
+  http_response->set_content("hello");
+  http_response->set_content_type("text/plain");
+  http_response->AddCustomHeader(
+      "Set-Cookie",
+      request.relative_url.substr(request.relative_url.find("?") + 1));
+  return http_response.Pass();
+}
+
 }  // namespace
 
 TEST_F(URLRequestTest, DelayedCookieCallback) {
@@ -2722,6 +2748,182 @@ TEST_F(URLRequestTest, FirstPartyOnlyCookiesDisabled) {
   }
 }
 
+// Tests that $Secure- cookies can't be set on non-secure origins.
+TEST_F(URLRequestTest, SecureCookiePrefixOnNonsecureOrigin) {
+  test_server::EmbeddedTestServer test_server;
+  test_server.RegisterRequestHandler(base::Bind(&HandleSetCookieRequest));
+  ASSERT_TRUE(test_server.InitializeAndWaitUntilReady());
+  SpawnedTestServer test_server_https(
+      SpawnedTestServer::TYPE_HTTPS, SpawnedTestServer::kLocalhost,
+      base::FilePath(FILE_PATH_LITERAL("net/data/ssl")));
+  ASSERT_TRUE(test_server_https.Start());
+
+  TestExperimentalFeaturesNetworkDelegate network_delegate;
+  TestURLRequestContext context(true);
+  context.set_network_delegate(&network_delegate);
+  context.Init();
+
+  // Try to set a Secure $Secure- cookie, with experimental features
+  // enabled.
+  {
+    TestDelegate d;
+    scoped_ptr<URLRequest> req(context.CreateRequest(
+        test_server.GetURL("/set-cookie?$Secure-nonsecure-origin=1;Secure"),
+        DEFAULT_PRIORITY, &d));
+    req->Start();
+    base::RunLoop().Run();
+    EXPECT_EQ(0, network_delegate.blocked_get_cookies_count());
+    EXPECT_EQ(0, network_delegate.blocked_set_cookie_count());
+  }
+
+  // Verify that the cookie is not set.
+  {
+    TestDelegate d;
+    scoped_ptr<URLRequest> req(context.CreateRequest(
+        test_server_https.GetURL("echoheader?Cookie"), DEFAULT_PRIORITY, &d));
+    req->Start();
+    base::RunLoop().Run();
+
+    EXPECT_TRUE(d.data_received().find("$Secure-nonsecure-origin=1") ==
+                std::string::npos);
+    EXPECT_EQ(0, network_delegate.blocked_get_cookies_count());
+    EXPECT_EQ(0, network_delegate.blocked_set_cookie_count());
+  }
+}
+
+TEST_F(URLRequestTest, SecureCookiePrefixNonexperimental) {
+  SpawnedTestServer test_server(
+      SpawnedTestServer::TYPE_HTTPS, SpawnedTestServer::kLocalhost,
+      base::FilePath(FILE_PATH_LITERAL("net/data/ssl")));
+  ASSERT_TRUE(test_server.Start());
+
+  TestNetworkDelegate network_delegate;
+  TestURLRequestContext context(true);
+  context.set_network_delegate(&network_delegate);
+  context.Init();
+
+  // Without experimental features, there should be no restrictions on
+  // $Secure- cookies.
+
+  // Set a non-Secure cookie with the $Secure- prefix.
+  {
+    TestDelegate d;
+    scoped_ptr<URLRequest> req(context.CreateRequest(
+        test_server.GetURL("set-cookie?$Secure-nonsecure-not-experimental=1"),
+        DEFAULT_PRIORITY, &d));
+    req->Start();
+    base::RunLoop().Run();
+    EXPECT_EQ(0, network_delegate.blocked_get_cookies_count());
+    EXPECT_EQ(0, network_delegate.blocked_set_cookie_count());
+  }
+
+  // Set a Secure cookie with the $Secure- prefix.
+  {
+    TestDelegate d;
+    scoped_ptr<URLRequest> req(context.CreateRequest(
+        test_server.GetURL(
+            "set-cookie?$Secure-secure-not-experimental=1;Secure"),
+        DEFAULT_PRIORITY, &d));
+    req->Start();
+    base::RunLoop().Run();
+    EXPECT_EQ(0, network_delegate.blocked_get_cookies_count());
+    EXPECT_EQ(0, network_delegate.blocked_set_cookie_count());
+  }
+
+  // Verify that the cookies are set. Neither should have any
+  // restrictions because the experimental flag is off.
+  {
+    TestDelegate d;
+    scoped_ptr<URLRequest> req(context.CreateRequest(
+        test_server.GetURL("echoheader?Cookie"), DEFAULT_PRIORITY, &d));
+    req->Start();
+    base::RunLoop().Run();
+
+    EXPECT_TRUE(d.data_received().find("$Secure-secure-not-experimental=1") !=
+                std::string::npos);
+    EXPECT_TRUE(
+        d.data_received().find("$Secure-nonsecure-not-experimental=1") !=
+        std::string::npos);
+    EXPECT_EQ(0, network_delegate.blocked_get_cookies_count());
+    EXPECT_EQ(0, network_delegate.blocked_set_cookie_count());
+  }
+}
+
+TEST_F(URLRequestTest, SecureCookiePrefixExperimentalNonsecure) {
+  SpawnedTestServer test_server(
+      SpawnedTestServer::TYPE_HTTPS, SpawnedTestServer::kLocalhost,
+      base::FilePath(FILE_PATH_LITERAL("net/data/ssl")));
+  ASSERT_TRUE(test_server.Start());
+
+  TestExperimentalFeaturesNetworkDelegate network_delegate;
+  TestURLRequestContext context(true);
+  context.set_network_delegate(&network_delegate);
+  context.Init();
+
+  // Try to set a non-Secure $Secure- cookie, with experimental features
+  // enabled.
+  {
+    TestDelegate d;
+    scoped_ptr<URLRequest> req(context.CreateRequest(
+        test_server.GetURL("set-cookie?$Secure-foo=1"), DEFAULT_PRIORITY, &d));
+    req->Start();
+    base::RunLoop().Run();
+    EXPECT_EQ(0, network_delegate.blocked_get_cookies_count());
+    EXPECT_EQ(0, network_delegate.blocked_set_cookie_count());
+  }
+
+  // Verify that the cookie is not set.
+  {
+    TestDelegate d;
+    scoped_ptr<URLRequest> req(context.CreateRequest(
+        test_server.GetURL("echoheader?Cookie"), DEFAULT_PRIORITY, &d));
+    req->Start();
+    base::RunLoop().Run();
+
+    EXPECT_TRUE(d.data_received().find("$Secure-foo=1") == std::string::npos);
+    EXPECT_EQ(0, network_delegate.blocked_get_cookies_count());
+    EXPECT_EQ(0, network_delegate.blocked_set_cookie_count());
+  }
+}
+
+TEST_F(URLRequestTest, SecureCookiePrefixExperimentalSecure) {
+  SpawnedTestServer test_server(
+      SpawnedTestServer::TYPE_HTTPS, SpawnedTestServer::kLocalhost,
+      base::FilePath(FILE_PATH_LITERAL("net/data/ssl")));
+  ASSERT_TRUE(test_server.Start());
+
+  TestExperimentalFeaturesNetworkDelegate network_delegate;
+  TestURLRequestContext context(true);
+  context.set_network_delegate(&network_delegate);
+  context.Init();
+
+  // Try to set a Secure $Secure- cookie, with experimental features
+  // enabled.
+  {
+    TestDelegate d;
+    scoped_ptr<URLRequest> req(context.CreateRequest(
+        test_server.GetURL("set-cookie?$Secure-bar=1;Secure"), DEFAULT_PRIORITY,
+        &d));
+    req->Start();
+    base::RunLoop().Run();
+    EXPECT_EQ(0, network_delegate.blocked_get_cookies_count());
+    EXPECT_EQ(0, network_delegate.blocked_set_cookie_count());
+  }
+
+  // Verify that the cookie is set.
+  {
+    TestDelegate d;
+    scoped_ptr<URLRequest> req(context.CreateRequest(
+        test_server.GetURL("echoheader?Cookie"), DEFAULT_PRIORITY, &d));
+    req->Start();
+    base::RunLoop().Run();
+
+    EXPECT_TRUE(d.data_received().find("$Secure-bar=1") != std::string::npos);
+    EXPECT_EQ(0, network_delegate.blocked_get_cookies_count());
+    EXPECT_EQ(0, network_delegate.blocked_set_cookie_count());
+  }
+}
+
 // Tests that a request is cancelled while entering suspend mode. Uses mocks
 // rather than a spawned test server because the connection used to talk to
 // the test server is affected by entering suspend mode on Android.