Certificate Transparency: New component for obtaining fresh STHs.

chrome/browser/chrome_browser_main.cc

Index: chrome/browser/chrome_browser_main.cc
diff --git a/chrome/browser/chrome_browser_main.cc b/chrome/browser/chrome_browser_main.cc
index 61723fabe97a7bec9423c992cbc780a2d318ec0b..34447f4f7ca737dbea2e122cf8c6b0076bf34c25 100644
--- a/chrome/browser/chrome_browser_main.cc
+++ b/chrome/browser/chrome_browser_main.cc
@@ -50,6 +50,7 @@
 #include "chrome/browser/component_updater/ev_whitelist_component_installer.h"
 #include "chrome/browser/component_updater/flash_component_installer.h"
 #include "chrome/browser/component_updater/recovery_component_installer.h"
+#include "chrome/browser/component_updater/sth_set_component_installer.h"
 #include "chrome/browser/component_updater/supervised_user_whitelist_installer.h"
 #include "chrome/browser/component_updater/swiftshader_component_installer.h"
 #include "chrome/browser/component_updater/widevine_cdm_component_installer.h"
@@ -150,6 +151,7 @@
 #include "grit/platform_locale_settings.h"
 #include "media/base/media_resources.h"
 #include "net/base/net_module.h"
+#include "net/cert/sth_distributor.h"
 #include "net/cookies/cookie_monster.h"
 #include "net/http/http_network_layer.h"
 #include "net/http/http_stream_factory.h"
@@ -442,7 +444,8 @@ OSStatus KeychainCallback(SecKeychainEvent keychain_event,
 }
 #endif  // defined(OS_MACOSX)
 
-void RegisterComponentsForUpdate() {
+void RegisterComponentsForUpdate(
+    scoped_ptr<net::ct::STHDistributor> sth_distributor) {

[Sorin Jianu, 2016/04/04 22:06:12]

Small issue.

I wonder if we could avoid passing this parameter to the function and have
RegisterSTHSetComponent implementation get the value of the parameter instead.

This is a recurring pattern here. Every component needs some kind of run time
parameter for registration. We wanted to avoid the case where we'd end up with a
bunch of unrelated function parameters, which are used only for one RegisterFoo
call below.

[Eran Messeri, 2016/04/05 15:34:06]

This CL contains a bigger picture of the intended wiring:
https://codereview.chromium.org/1845113003

Briefly, the sth_distributor instance passed into the component has to be shared
with the IOThread. The component will invoke a callback on the sth_distributor
notifying of new data and all objects that care about it (there's one per
profile plus one in the IOThread) will receive it from the sth_distributor.

The bigger question is how do components, particularly ones implementing
ComponentInstallerTraits export the data they receive. The common pattern I've
seen (cld_component_installer, ev_whitelist_component_installer) is by setting
global static variable.

I wanted to avoid it here because (1) globals and (2) a notification-based
mechanism is more suitable for the data pushed to this component.

Any advice on how to wire things differently?

   component_updater::ComponentUpdateService* cus =
       g_browser_process->component_updater();
 
@@ -489,6 +492,11 @@ void RegisterComponentsForUpdate() {
     // 1. Android: Because it currently does not have the EV indicator.
     // 2. Chrome OS: On Chrome OS this registration is delayed until user login.
     RegisterEVWhitelistComponent(cus, path);
+
+    // Registration of the STH set fetcher here is not done for:
+    // Android: Because the story around CT on Mobile is not finalized yet.
+    // Chrome OS: On Chrome OS this registration is delayed until user login.
+    RegisterSTHSetComponent(cus, path, std::move(sth_distributor));
 #endif  // defined(OS_ANDROID)
   }
 
@@ -1707,8 +1715,13 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() {
   // data source is based on the Component Updater.
   translate::BrowserCldUtils::ConfigureDefaultDataProvider();
 
-  if (!parsed_command_line().HasSwitch(switches::kDisableComponentUpdate))
-    RegisterComponentsForUpdate();
+  if (!parsed_command_line().HasSwitch(switches::kDisableComponentUpdate)) {
+    scoped_ptr<net::ct::STHDistributor> distributor(
+        new net::ct::STHDistributor());
+    // TODO(eranm): Pass the distributor to the IOThread so CT TreeStateTracker
+    // instances can register for STH updates.
+    RegisterComponentsForUpdate(std::move(distributor));

[Sorin Jianu, 2016/04/04 22:06:13]

Would it be possible to handle this argument inside the STH registration? Please
see the comments above.

[Eran Messeri, 2016/04/05 15:34:06]

See my reply above; in
https://codereview.chromium.org/1845113003/diff/40001/chrome/browser/chrome_b...
you can see how I intend to use the distributor.

+  }
 
 #if defined(OS_ANDROID)
   variations::VariationsService* variations_service =