Revert "Network Service: Enable encrypted cookies"

This reverts commit 87135a31.

Reason for revert: ChromeNetworkServiceBrowserTest.EncryptedCookies
keeps failing on Mac10.11 bot.
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.11%20Tests/27835

Original change's description:
> 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}

TBR=dcheng@chromium.org,jam@chromium.org,mmenke@chromium.org,cfroussios@chromium.org,morlovich@chromium.org,cduvall@chromium.org

Change-Id: I192a10c6d28a102dcf5ae247cdbda4c602872eb9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 789632, 848361, 789644
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/1125460Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Commit-Queue: Makoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572490}

chrome/browser/chrome_browser_main_linux.cc

@@ -64,8 +64,6 @@ 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,13 +49,6 @@
 #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
@@ -438,21 +431,6 @@ 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,7 +503,6 @@ 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,7 +166,6 @@ 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,7 +142,6 @@ 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",
@@ -187,10 +186,6 @@ component("network_service") {
   }
 
   defines = [ "IS_NETWORK_SERVICE_IMPL" ]
-
-  if (is_chromecast) {
-    defines += [ "IS_CHROMECAST" ]
-  }
 }
 
 source_set("tests") {

services/network/DEPS

@@ -3,7 +3,6 @@ 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,6 +1093,8 @@ 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 =
@@ -1109,15 +1111,6 @@ 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,6 +14,7 @@
 #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"
@@ -38,11 +39,6 @@
 #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 {
@@ -367,21 +363,6 @@ 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,7 +14,6 @@
 #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"
@@ -133,9 +132,6 @@ 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.
@@ -159,8 +155,6 @@ 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:
@@ -227,8 +221,6 @@ 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,7 +97,6 @@ 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,10 +121,4 @@ 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,9 +62,6 @@ 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,22 +163,6 @@ 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);
@@ -247,9 +231,4 @@ 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);
 };