Network Service: Enable encrypted cookies

Cookies will now be stored encrypted by default in the network service.
Tested that crbug.com/848361 is fixed with this change.

Bug: 789632, 848361, 789644
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Ic627501ddf1c5030bbf2a203f005f5ebca92dfd8
Reviewed-on: https://chromium-review.googlesource.com/1104791
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Maks Orlovich <morlovich@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Christos Froussios <cfroussios@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572444}

chrome/browser/chrome_browser_main_linux.cc

@@ -64,6 +64,8 @@ void ChromeBrowserMainPartsLinux::PreProfileInit() {
       l10n_util::GetStringUTF8(IDS_SHORT_PRODUCT_NAME));
 
 #if !defined(OS_CHROMEOS)
+  // Set up crypt config. This should be kept in sync with the OSCrypt parts of
+  // SystemNetworkContextManager::OnNetworkServiceCreated.
   std::unique_ptr<os_crypt::Config> config(new os_crypt::Config());
   // Forward to os_crypt the flag to use a specific password store.
   config->store =

chrome/browser/chrome_network_service_browsertest.cc

+// Copyright 2018 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 "base/test/bind_test_util.h"
+#include "base/test/scoped_feature_list.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "components/cookie_config/cookie_store_util.h"
+#include "content/public/browser/browser_context.h"
+#include "content/public/browser/network_service_instance.h"
+#include "content/public/test/browser_test.h"
+#include "net/extras/sqlite/cookie_crypto_delegate.h"
+#include "services/network/public/cpp/features.h"
+
+namespace content {
+namespace {
+
+constexpr char kCookieName[] = "Name";
+constexpr char kCookieValue[] = "Value";
+
+net::CookieList GetCookies(
+    const network::mojom::CookieManagerPtr& cookie_manager) {
+  base::RunLoop run_loop;
+  net::CookieList cookies_out;
+  cookie_manager->GetAllCookies(
+      base::BindLambdaForTesting([&](const net::CookieList& cookies) {
+        cookies_out = cookies;
+        run_loop.Quit();
+      }));
+  run_loop.Run();
+  return cookies_out;
+}
+
+void SetCookie(const network::mojom::CookieManagerPtr& cookie_manager) {
+  base::Time t = base::Time::Now();
+  net::CanonicalCookie cookie(kCookieName, kCookieValue, "www.test.com", "/", t,
+                              t + base::TimeDelta::FromDays(1), base::Time(),
+                              false, false, net::CookieSameSite::DEFAULT_MODE,
+                              net::COOKIE_PRIORITY_DEFAULT);
+  base::RunLoop run_loop;
+  cookie_manager->SetCanonicalCookie(
+      cookie, false, false,
+      base::BindLambdaForTesting([&](bool success) { run_loop.Quit(); }));
+  run_loop.Run();
+}
+
+// See |NetworkServiceBrowserTest| for content's version of tests.
+class ChromeNetworkServiceBrowserTest : public InProcessBrowserTest {
+ public:
+  ChromeNetworkServiceBrowserTest() {
+    scoped_feature_list_.InitAndEnableFeature(
+        network::features::kNetworkService);
+  }
+
+ protected:
+  network::mojom::NetworkContextPtr CreateNetworkContext(
+      bool enable_encrypted_cookies) {
+    network::mojom::NetworkContextPtr network_context;
+    network::mojom::NetworkContextParamsPtr context_params =
+        network::mojom::NetworkContextParams::New();
+    context_params->enable_encrypted_cookies = enable_encrypted_cookies;
+    context_params->cookie_path =
+        browser()->profile()->GetPath().Append(FILE_PATH_LITERAL("cookies"));
+    GetNetworkService()->CreateNetworkContext(
+        mojo::MakeRequest(&network_context), std::move(context_params));
+    return network_context;
+  }
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+
+  DISALLOW_COPY_AND_ASSIGN(ChromeNetworkServiceBrowserTest);
+};
+
+IN_PROC_BROWSER_TEST_F(ChromeNetworkServiceBrowserTest, PRE_EncryptedCookies) {
+  // First set a cookie with cookie encryption enabled.
+  network::mojom::NetworkContextPtr context =
+      CreateNetworkContext(/*enable_encrypted_cookies=*/true);
+  network::mojom::CookieManagerPtr cookie_manager;
+  context->GetCookieManager(mojo::MakeRequest(&cookie_manager));
+
+  SetCookie(cookie_manager);
+
+  net::CookieList cookies = GetCookies(cookie_manager);
+  ASSERT_EQ(1u, cookies.size());
+  EXPECT_EQ(kCookieName, cookies[0].Name());
+  EXPECT_EQ(kCookieValue, cookies[0].Value());
+}
+
+IN_PROC_BROWSER_TEST_F(ChromeNetworkServiceBrowserTest, EncryptedCookies) {
+  net::CookieCryptoDelegate* crypto_delegate =
+      cookie_config::GetCookieCryptoDelegate();
+  std::string ciphertext;
+  crypto_delegate->EncryptString(kCookieValue, &ciphertext);
+  // These checks are only valid if crypto is enabled on the platform.
+  if (!crypto_delegate->ShouldEncrypt() || ciphertext == kCookieValue)
+    return;
+
+  // Now attempt to read the cookie with encryption disabled.
+  network::mojom::NetworkContextPtr context =
+      CreateNetworkContext(/*enable_encrypted_cookies=*/false);
+  network::mojom::CookieManagerPtr cookie_manager;
+  context->GetCookieManager(mojo::MakeRequest(&cookie_manager));
+
+  net::CookieList cookies = GetCookies(cookie_manager);
+  ASSERT_EQ(1u, cookies.size());
+  EXPECT_EQ(kCookieName, cookies[0].Name());
+  EXPECT_EQ("", cookies[0].Value());
+}
+
+}  // namespace
+}  // namespace content

chrome/browser/net/system_network_context_manager.cc

@@ -49,6 +49,13 @@
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
 #endif  // defined(OS_CHROMEOS)
 
+#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
+#include "chrome/common/chrome_paths_internal.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/grit/chromium_strings.h"
+#include "ui/base/l10n/l10n_util.h"
+#endif  // defined(OS_LINUX) && !defined(OS_CHROMEOS)
+
 namespace {
 
 // Called on IOThread to disable QUIC for HttpNetworkSessions not using the
@@ -431,6 +438,21 @@ void SystemNetworkContextManager::OnNetworkServiceCreated(
   GetStubResolverConfig(&stub_resolver_enabled, &dns_over_https_servers);
   content::GetNetworkService()->ConfigureStubHostResolver(
       stub_resolver_enabled, std::move(dns_over_https_servers));
+
+#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
+  const base::CommandLine& command_line =
+      *base::CommandLine::ForCurrentProcess();
+
+  // Set up crypt config. This should be kept in sync with the OSCrypt parts of
+  // ChromeBrowserMainPartsLinux::PreProfileInit.
+  network::mojom::CryptConfigPtr config = network::mojom::CryptConfig::New();
+  config->store = command_line.GetSwitchValueASCII(switches::kPasswordStore);
+  config->product_name = l10n_util::GetStringUTF8(IDS_PRODUCT_NAME);
+  config->should_use_preference =
+      command_line.HasSwitch(switches::kEnableEncryptionSelection);
+  chrome::GetDefaultUserDataDirectory(&config->user_data_path);
+  content::GetNetworkService()->SetCryptConfig(std::move(config));
+#endif
 }
 
 void SystemNetworkContextManager::DisableQuic() {

chrome/test/BUILD.gn

@@ -503,6 +503,7 @@ test("browser_tests") {
       "../browser/chrome_find_request_manager_browsertest.cc",
       "../browser/chrome_main_browsertest.cc",
       "../browser/chrome_navigation_browsertest.cc",
+      "../browser/chrome_network_service_browsertest.cc",
       "../browser/chrome_network_service_restart_browsertest.cc",
       "../browser/chrome_origin_trials_browsertest.cc",
       "../browser/chrome_plugin_browsertest.cc",

components/safe_browsing/browser/safe_browsing_network_context.cc

@@ -166,6 +166,7 @@ class SafeBrowsingNetworkContext::SharedURLLoaderFactory
     base::FilePath cookie_path = user_data_dir_.Append(
         base::FilePath::StringType(kSafeBrowsingBaseFilename) + kCookiesFile);
     network_context_params->cookie_path = cookie_path;
+    network_context_params->enable_encrypted_cookies = false;
 
     base::FilePath channel_id_path = user_data_dir_.Append(
         base::FilePath::StringType(kSafeBrowsingBaseFilename) + kChannelIDFile);

services/network/BUILD.gn

@@ -142,6 +142,7 @@ component("network_service") {
     "//components/cookie_config",
     "//components/network_session_configurator/browser",
     "//components/network_session_configurator/common",
+    "//components/os_crypt",
     "//components/prefs",
     "//mojo/public/cpp/bindings",
     "//mojo/public/cpp/system",
@@ -186,6 +187,10 @@ component("network_service") {
   }
 
   defines = [ "IS_NETWORK_SERVICE_IMPL" ]
+
+  if (is_chromecast) {
+    defines += [ "IS_CHROMECAST" ]
+  }
 }
 
 source_set("tests") {

services/network/DEPS

@@ -3,6 +3,7 @@ include_rules = [
   "+components/content_settings/core/common",
   "+components/cookie_config",
   "+components/network_session_configurator",
+  "+components/os_crypt",
   # Prefs are used to create an independent file with a persisted key:value
   # store for networking-related data (Like which servers support QUIC), rather
   # than to store user preferences.

services/network/network_context.cc

@@ -1093,8 +1093,6 @@ URLRequestContextOwner NetworkContext::MakeURLRequestContext(
   // have to figure out which of the latter needs to move to the network
   // process). TODO: http://crbug.com/789644
   if (params_->cookie_path) {
-    net::CookieCryptoDelegate* crypto_delegate = nullptr;
-
     scoped_refptr<base::SequencedTaskRunner> client_task_runner =
         base::MessageLoopCurrent::Get()->task_runner();
     scoped_refptr<base::SequencedTaskRunner> background_task_runner =
@@ -1111,6 +1109,15 @@ URLRequestContextOwner NetworkContext::MakeURLRequestContext(
           new net::DefaultChannelIDStore(channel_id_db.get()));
     }
 
+    net::CookieCryptoDelegate* crypto_delegate = nullptr;
+    if (params_->enable_encrypted_cookies) {
+#if defined(OS_LINUX) && !defined(OS_CHROMEOS) && !defined(IS_CHROMECAST)
+      DCHECK(network_service_->os_crypt_config_set())
+          << "NetworkService::SetCryptConfig must be called before creating a "
+             "NetworkContext with encrypted cookies.";
+#endif
+      crypto_delegate = cookie_config::GetCookieCryptoDelegate();
+    }
     scoped_refptr<net::SQLitePersistentCookieStore> sqlite_store(
         new net::SQLitePersistentCookieStore(
             params_->cookie_path.value(), client_task_runner,

services/network/network_service.cc

@@ -14,7 +14,6 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/task_scheduler/post_task.h"
 #include "base/values.h"
-#include "build/build_config.h"
 #include "components/certificate_transparency/sth_distributor.h"
 #include "components/certificate_transparency/sth_observer.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
@@ -39,6 +38,11 @@
 #include "third_party/boringssl/src/include/openssl/cpu.h"
 #endif
 
+#if defined(OS_LINUX) && !defined(OS_CHROMEOS) && !defined(IS_CHROMECAST)
+#include "components/os_crypt/key_storage_config_linux.h"
+#include "components/os_crypt/os_crypt.h"
+#endif
+
 namespace network {
 
 namespace {
@@ -363,6 +367,21 @@ void NetworkService::UpdateSignedTreeHead(const net::ct::SignedTreeHead& sth) {
   sth_distributor_->NewSTHObserved(sth);
 }
 
+#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
+void NetworkService::SetCryptConfig(mojom::CryptConfigPtr crypt_config) {
+#if !defined(IS_CHROMECAST)
+  auto config = std::make_unique<os_crypt::Config>();
+  config->store = crypt_config->store;
+  config->product_name = crypt_config->product_name;
+  config->main_thread_runner = base::ThreadTaskRunnerHandle::Get();
+  config->should_use_preference = crypt_config->should_use_preference;
+  config->user_data_path = crypt_config->user_data_path;
+  OSCrypt::SetConfig(std::move(config));
+  os_crypt_config_set_ = true;
+#endif
+}
+#endif
+
 net::HttpAuthHandlerFactory* NetworkService::GetHttpAuthHandlerFactory() {
   if (!http_auth_handler_factory_) {
     http_auth_handler_factory_ = net::HttpAuthHandlerFactory::CreateDefault(

services/network/network_service.h

@@ -14,6 +14,7 @@
 #include "base/macros.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/optional.h"
+#include "build/build_config.h"
 #include "mojo/public/cpp/bindings/binding.h"
 #include "net/http/http_auth_preferences.h"
 #include "net/log/net_log.h"
@@ -132,6 +133,9 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkService
   void GetTotalNetworkUsages(
       mojom::NetworkService::GetTotalNetworkUsagesCallback callback) override;
   void UpdateSignedTreeHead(const net::ct::SignedTreeHead& sth) override;
+#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
+  void SetCryptConfig(mojom::CryptConfigPtr crypt_config) override;
+#endif
 
   // Returns the shared HttpAuthHandlerFactory for the NetworkService, creating
   // one if needed.
@@ -155,6 +159,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkService
 
   certificate_transparency::STHReporter* sth_reporter();
 
+  bool os_crypt_config_set() const { return os_crypt_config_set_; }
+
   static NetworkService* GetNetworkServiceForTesting();
 
  private:
@@ -221,6 +227,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkService
 
   bool quic_disabled_ = false;
 
+  bool os_crypt_config_set_ = false;
+
   std::unique_ptr<certificate_transparency::STHDistributor> sth_distributor_;
 
   DISALLOW_COPY_AND_ASSIGN(NetworkService);

services/network/network_service_unittest.cc

@@ -97,6 +97,7 @@ TEST_F(NetworkServiceTest, DestroyingServiceDestroysContext) {
 TEST_F(NetworkServiceTest, CreateContextWithoutChannelID) {
   mojom::NetworkContextParamsPtr params = CreateContextParams();
   params->cookie_path = base::FilePath();
+  params->enable_encrypted_cookies = false;
   mojom::NetworkContextPtr network_context;
   service()->CreateNetworkContext(mojo::MakeRequest(&network_context),
                                   std::move(params));

services/network/public/mojom/BUILD.gn

@@ -121,4 +121,10 @@ mojom("mojom") {
     export_define_blink = "BLINK_PLATFORM_IMPLEMENTATION=1"
     export_header_blink = "third_party/blink/public/platform/web_common.h"
   }
+
+  # This is only needed on desktop linux, but the defines make this difficult
+  # because IS_CHROMECAST is not available in build/build_config.h.
+  if (is_linux && !is_chromeos) {
+    enabled_features = [ "needs_crypt_config" ]
+  }
 }

services/network/public/mojom/network_context.mojom

@@ -62,6 +62,9 @@ struct NetworkContextParams {
   // Points to the cookie file. An in-memory cookie store is used if it's empty.
   mojo_base.mojom.FilePath? cookie_path;
 
+  // If true, cookies will be stored encrypted.
+  bool enable_encrypted_cookies = true;
+
   // If the cookie file is given, this controls whether previously written
   // session cookies are restored. Otherwise it should be false.
   bool restore_old_session_cookies = false;

services/network/public/mojom/network_service.mojom

@@ -163,6 +163,22 @@ struct HttpAuthDynamicParams {
   string android_negotiate_account_type;
 };
 
+// Values for configuring OSCrypt.
+[EnableIf=needs_crypt_config]
+struct CryptConfig {
+  // Force OSCrypt to use a specific linux password store.
+  string store;
+
+  // The product name to use for permission prompts.
+  string product_name;
+
+  // Controls whether preference on using or ignoring backends is used.
+  bool should_use_preference;
+
+  // Preferences are stored in a separate file in the user data directory.
+  mojo_base.mojom.FilePath user_data_path;
+};
+
 // Browser interface to the network service.
 interface NetworkService {
   SetClient(NetworkServiceClient client);
@@ -231,4 +247,9 @@ interface NetworkService {
   // Transparency. Broadcast to each NetworkContext using the NetworkService.
   // NetworkContextes ignore STHs from unrecognized logs.
   UpdateSignedTreeHead(SignedTreeHead signed_tree_head);
+
+  // Sets up OSCrypt for the network service process. Must be called before
+  // encrypted cookies can be read or set.
+  [EnableIf=needs_crypt_config]
+  SetCryptConfig(CryptConfig crypt_config);
 };