Repopulate Signed Tree Heads in the network process after it crashes.

Bug: 840444
Change-Id: Idb614469052bbb9bff0232513e88879e1dbd8b2c
Reviewed-on: https://chromium-review.googlesource.com/1252502Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595304}

chrome/browser/component_updater/sth_set_component_installer.cc

@@ -13,6 +13,7 @@
 #include "base/json/json_reader.h"
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
+#include "base/no_destructor.h"
 #include "base/path_service.h"
 #include "base/sequenced_task_runner.h"
 #include "base/strings/string_number_conversions.h"
@@ -37,6 +38,13 @@ using component_updater::ComponentUpdateService;
 namespace {
 const base::FilePath::CharType kSTHsDirName[] = FILE_PATH_LITERAL("sths");
 
+network::mojom::NetworkService* g_network_service_for_testing = nullptr;
+
+base::FilePath& GetInstallDir() {
+  static base::NoDestructor<base::FilePath> install_dir;
+  return *install_dir;
+}
+
 base::FilePath GetInstalledPath(const base::FilePath& base) {
   return base.Append(FILE_PATH_LITERAL("_platform_specific"))
       .Append(FILE_PATH_LITERAL("all"))
@@ -117,6 +125,14 @@ void LoadSTHsFromDisk(
   }
 }
 
+// Indicates that a new STH has been loaded.
+void OnSTHLoaded(const net::ct::SignedTreeHead& sth) {
+  network::mojom::NetworkService* network_service =
+      g_network_service_for_testing ? g_network_service_for_testing
+                                    : content::GetNetworkService();
+  network_service->UpdateSignedTreeHead(sth);
+}
+
 }  // namespace
 
 namespace component_updater {
@@ -130,15 +146,20 @@ const uint8_t kSthSetPublicKeySHA256[32] = {
 
 const char kSTHSetFetcherManifestName[] = "Signed Tree Heads";
 
-STHSetComponentInstallerPolicy::STHSetComponentInstallerPolicy()
-    : weak_ptr_factory_(this) {}
+STHSetComponentInstallerPolicy::STHSetComponentInstallerPolicy() {}
 
 STHSetComponentInstallerPolicy::~STHSetComponentInstallerPolicy() = default;
 
+// static
+void STHSetComponentInstallerPolicy::ReconfigureAfterNetworkRestart() {
+  if (!GetInstallDir().empty())
+    ConfigureNetworkService();
+}
+
 void STHSetComponentInstallerPolicy::SetNetworkServiceForTesting(
     network::mojom::NetworkService* network_service) {
   DCHECK(network_service);
-  network_service_for_testing_ = network_service;
+  g_network_service_for_testing = network_service;
 }
 
 bool STHSetComponentInstallerPolicy::
@@ -146,16 +167,6 @@ bool STHSetComponentInstallerPolicy::
   return false;
 }
 
-void STHSetComponentInstallerPolicy::OnSTHLoaded(
-    const net::ct::SignedTreeHead& sth) {
-  // TODO(rsleevi): https://crbug.com/840444 - Ensure the network service
-  // is notified of the STHs if it crashes/restarts.
-  network::mojom::NetworkService* network_service =
-      network_service_for_testing_ ? network_service_for_testing_
-                                   : content::GetNetworkService();
-  network_service->UpdateSignedTreeHead(sth);
-}
-
 // Public data is delivered via this component, no need for encryption.
 bool STHSetComponentInstallerPolicy::RequiresNetworkEncryption() const {
   return false;
@@ -174,19 +185,8 @@ void STHSetComponentInstallerPolicy::ComponentReady(
     const base::Version& version,
     const base::FilePath& install_dir,
     std::unique_ptr<base::DictionaryValue> manifest) {
-  // Load and parse the STH JSON on a background task runner, then
-  // dispatch back to the current task runner with all of the successfully
-  // parsed results.
-  auto background_runner = base::MakeRefCounted<AfterStartupTaskUtils::Runner>(
-      base::CreateTaskRunnerWithTraits(
-          {base::TaskPriority::BEST_EFFORT, base::MayBlock()}));
-  background_runner->PostTask(
-      FROM_HERE,
-      base::BindOnce(
-          &LoadSTHsFromDisk, GetInstalledPath(install_dir),
-          base::SequencedTaskRunnerHandle::Get(),
-          base::BindRepeating(&STHSetComponentInstallerPolicy::OnSTHLoaded,
-                              weak_ptr_factory_.GetWeakPtr())));
+  GetInstallDir() = install_dir;
+  ConfigureNetworkService();
 }
 
 // Called during startup and installation before ComponentReady().
@@ -218,6 +218,21 @@ std::vector<std::string> STHSetComponentInstallerPolicy::GetMimeTypes() const {
   return std::vector<std::string>();
 }
 
+// static
+void STHSetComponentInstallerPolicy::ConfigureNetworkService() {
+  // Load and parse the STH JSON on a background task runner, then
+  // dispatch back to the current task runner with all of the successfully
+  // parsed results.
+  auto background_runner = base::MakeRefCounted<AfterStartupTaskUtils::Runner>(
+      base::CreateTaskRunnerWithTraits(
+          {base::TaskPriority::BEST_EFFORT, base::MayBlock()}));
+  background_runner->PostTask(
+      FROM_HERE,
+      base::BindOnce(&LoadSTHsFromDisk, GetInstalledPath(GetInstallDir()),
+                     base::SequencedTaskRunnerHandle::Get(),
+                     base::BindRepeating(OnSTHLoaded)));
+}
+
 void RegisterSTHSetComponent(ComponentUpdateService* cus,
                              const base::FilePath& user_data_dir) {
   DVLOG(1) << "Registering STH Set fetcher component.";

chrome/browser/component_updater/sth_set_component_installer.h

@@ -15,12 +15,6 @@
 #include "base/memory/weak_ptr.h"
 #include "components/component_updater/component_installer.h"
 
-namespace net {
-namespace ct {
-struct SignedTreeHead;
-}  // namespace ct
-}  // namespace net
-
 namespace network {
 namespace mojom {
 class NetworkService;
@@ -46,14 +40,14 @@ class STHSetComponentInstallerPolicy : public ComponentInstallerPolicy {
   STHSetComponentInstallerPolicy();
   ~STHSetComponentInstallerPolicy() override;
 
+  // Update the STHs after a network process crash.
+  static void ReconfigureAfterNetworkRestart();
+
  private:
   friend class STHSetComponentInstallerTest;
   void SetNetworkServiceForTesting(
       network::mojom::NetworkService* network_service);
 
-  // Indicates that a new STH has been loaded.
-  void OnSTHLoaded(const net::ct::SignedTreeHead& sth);
-
   // ComponentInstallerPolicy implementation.
   bool SupportsGroupPolicyEnabledComponentUpdates() const override;
   bool RequiresNetworkEncryption() const override;
@@ -72,9 +66,7 @@ class STHSetComponentInstallerPolicy : public ComponentInstallerPolicy {
   update_client::InstallerAttributes GetInstallerAttributes() const override;
   std::vector<std::string> GetMimeTypes() const override;
 
-  network::mojom::NetworkService* network_service_for_testing_ = nullptr;
-
-  base::WeakPtrFactory<STHSetComponentInstallerPolicy> weak_ptr_factory_;
+  static void ConfigureNetworkService();
 
   DISALLOW_COPY_AND_ASSIGN(STHSetComponentInstallerPolicy);
 };

chrome/browser/component_updater/sth_set_component_installer_unittest.cc

@@ -41,14 +41,15 @@ class StoringSTHObserver : public certificate_transparency::STHObserver {
 
 class STHSetComponentInstallerTest : public PlatformTest {
  public:
-  STHSetComponentInstallerTest() : network_service_(nullptr) {
+  STHSetComponentInstallerTest()
+      : network_service_(std::make_unique<network::NetworkService>(nullptr)) {
     AfterStartupTaskUtils::SetBrowserStartupIsCompleteForTesting();
 
-    network_service_.sth_reporter()->RegisterObserver(&observer_);
+    network_service_->sth_reporter()->RegisterObserver(&observer_);
   }
 
   ~STHSetComponentInstallerTest() override {
-    network_service_.sth_reporter()->UnregisterObserver(&observer_);
+    network_service_->sth_reporter()->UnregisterObserver(&observer_);
   }
 
   void SetUp() override {
@@ -57,7 +58,16 @@ class STHSetComponentInstallerTest : public PlatformTest {
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
 
     policy_ = std::make_unique<STHSetComponentInstallerPolicy>();
-    policy_->SetNetworkServiceForTesting(&network_service_);
+    policy_->SetNetworkServiceForTesting(network_service_.get());
+  }
+
+  void SimulateCrash() {
+    network_service_->sth_reporter()->UnregisterObserver(&observer_);
+    observer_.sths.clear();
+    network_service_.reset();
+    network_service_ = std::make_unique<network::NetworkService>(nullptr);
+    network_service_->sth_reporter()->RegisterObserver(&observer_);
+    policy_->SetNetworkServiceForTesting(network_service_.get());
   }
 
   void WriteSTHToFile(const std::string& sth_json,
@@ -92,7 +102,7 @@ class STHSetComponentInstallerTest : public PlatformTest {
  protected:
   content::TestBrowserThreadBundle thread_bundle_;
 
-  network::NetworkService network_service_;
+  std::unique_ptr<network::NetworkService> network_service_;
   StoringSTHObserver observer_;
 
   base::ScopedTempDir temp_dir_;
@@ -160,4 +170,30 @@ TEST_F(STHSetComponentInstallerTest,
   EXPECT_EQ(0u, observer_.sths.size());
 }
 
+// Tests recovery after network process crashes.
+TEST_F(STHSetComponentInstallerTest, ReconfiguresAfterRestart) {
+  const base::DictionaryValue manifest;
+  const base::FilePath sths_dir(GetSTHsDir());
+  CreateSTHsDir(manifest, sths_dir);
+
+  const std::string good_sth_json = net::ct::GetSampleSTHAsJson();
+  const base::FilePath sth_file =
+      sths_dir.Append(FILE_PATH_LITERAL("616263.sth"));
+  WriteSTHToFile(good_sth_json, sth_file);
+
+  LoadSTHs(manifest, sths_dir);
+
+  const std::string log_id("abc");
+  ASSERT_TRUE(observer_.sths.find(log_id) != observer_.sths.end());
+  const net::ct::SignedTreeHead& sth(observer_.sths[log_id]);
+  EXPECT_EQ(21u, sth.tree_size);
+
+  // Simulate a Network Service crash
+  SimulateCrash();
+  STHSetComponentInstallerPolicy::ReconfigureAfterNetworkRestart();
+  thread_bundle_.RunUntilIdle();
+
+  ASSERT_TRUE(observer_.sths.find(log_id) != observer_.sths.end());
+}
+
 }  // namespace component_updater

chrome/browser/net/system_network_context_manager.cc

@@ -20,6 +20,7 @@
 #include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/component_updater/crl_set_component_installer.h"
+#include "chrome/browser/component_updater/sth_set_component_installer.h"
 #include "chrome/browser/io_thread.h"
 #include "chrome/browser/net/chrome_mojo_proxy_resolver_factory.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
@@ -511,6 +512,10 @@ void SystemNetworkContextManager::OnNetworkServiceCreated(
 
   // Asynchronously reapply the most recently received CRLSet (if any).
   component_updater::CRLSetPolicy::ReconfigureAfterNetworkRestart();
+
+  // Asynchronously reapply the most recently received STHs (if any).
+  component_updater::STHSetComponentInstallerPolicy::
+      ReconfigureAfterNetworkRestart();
 }
 
 void SystemNetworkContextManager::DisableQuic() {