New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 613501 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Feature

Blocking:
issue 506227
issue 480867



Sign in to add a comment

Certificate Transparency: Wire the STH-providing component to the tree state tracker

Project Member Reported by eranm@chromium.org, May 20 2016

Issue description

[everybody CC'ed is FYI; this is a summary of an offline discussion]

Fresh STHs are provided to Chrome clients via the component updater - particularly via the STHSetComponetInstallerTraits (https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/component_updater/sth_set_component_installer.h).
STHs are needed by the TreeStateTracker (components/certificate_transparency) for checking inclusion proofs against.

The difficulty lies in the wiring of the two:
* The STHSetComponentInstallerTraits is created in chrome_browser_main, *after* the IOThread object is created (but not, it should be noted, on the IO thread).
* The various TreeStateTrackers are created in the IOThread/ProfileIOData, some before the STHSetComponentInstallerTraits is created, some after.

The question boils down to how to share an STHDistributor instance between the IOThread and the STHSetComponentInstallerTraits.

After discussion with sorin@ and rsleevi@ we came to the following conclusion:
- The STHDistributor should be accessible via a global defined somewhere under chrome/browser/net.

Alternatives considered and disqualified:
- Threading the STHDistributor to the IOThread from chrome_browser_main via BrowserProcessImpl (the STHDistributor is passed from chrome_browser_main -> BrowserProcessImpl that posts a task to the IOTHread to set  the STHDistributor). Disqualified because it creates a tight coupling throughout the code between the component updater and net stuff.

- Creating the STHDistributor in RegisterSTHSetComponent and setting it by post via g_browser_process (g_browser_process->io_thread()). Disqualified because access to g_browser_process is strongly discouraged, particularly so if it's being introduced in a new call site that did not previously depend on it.

- Static on the SSLConfigService, similar to how the CRLSet works: Approach is conceptually OK, but the static should live under chrome/browser/net as not all Chromium embedders may choose to operate this way.

More detailed notes from the discussion about wiring:
- An agreed-upon way to share the STHDistributor object seems to be through a global singleton which would be defined in chrome/browser/net (in a new file).
- The STHSetComponentInstallerTraits will get an instance of the STHDistributor that will be obtained from the global object in RegisterSTHSetComponentInstaller.
- The IOThread will also use this global singleton when it has to register/unregister an STH Observer (which server the purpose of abstracting the presence of a singleton from the Profile implementation).

 

Comment 1 by eranm@chromium.org, May 20 2016

Blocking: -506277 506227
Project Member

Comment 2 by bugdroid1@chromium.org, May 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4da9bf69dbb6be6dbea51339edfbee17455eb111

commit 4da9bf69dbb6be6dbea51339edfbee17455eb111
Author: eranm <eranm@chromium.org>
Date: Fri May 27 10:04:56 2016

Certificate Transparency: Ensure no observers in STHDistributor d'tor.

Check, in the STHDistributor, that there are no observers registered.
This would help enforce correct usage and lifetime management of all the
observers registered with each STHDistributor instance, as there are expected
to be global instances of the STHDistributor.

BUG= 613501 

Review-Url: https://codereview.chromium.org/2015503003
Cr-Commit-Position: refs/heads/master@{#396434}

[modify] https://crrev.com/4da9bf69dbb6be6dbea51339edfbee17455eb111/net/cert/sth_distributor.h
[modify] https://crrev.com/4da9bf69dbb6be6dbea51339edfbee17455eb111/net/cert/sth_distributor_unittest.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Jun 1 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3c2d643cded7d3c68510aa69b08a19b2ac465740

commit 3c2d643cded7d3c68510aa69b08a19b2ac465740
Author: eranm <eranm@chromium.org>
Date: Wed Jun 01 10:17:27 2016

Certificate Transparency: Wire the TreeStateTracker up.

This change puts into use the TreeStateTracker class by wiring it to:
* A global STHDistributor for receiving fresh STHs via the component
  updater.
* The per-profile CTVerifier for receiving notification of verified
  SCTs that should be checked for inclusion.

As discussed with sorin and rsleevi, the wiring of the STHSetComponentInstaller
is via a globally-accessible STHDistributor, created by the IOThread.

BUG= 613501 

Review-Url: https://codereview.chromium.org/2014573002
Cr-Commit-Position: refs/heads/master@{#397096}

[modify] https://crrev.com/3c2d643cded7d3c68510aa69b08a19b2ac465740/chrome/browser/BUILD.gn
[modify] https://crrev.com/3c2d643cded7d3c68510aa69b08a19b2ac465740/chrome/browser/component_updater/sth_set_component_installer.cc
[modify] https://crrev.com/3c2d643cded7d3c68510aa69b08a19b2ac465740/chrome/browser/component_updater/sth_set_component_installer.h
[modify] https://crrev.com/3c2d643cded7d3c68510aa69b08a19b2ac465740/chrome/browser/component_updater/sth_set_component_installer_unittest.cc
[modify] https://crrev.com/3c2d643cded7d3c68510aa69b08a19b2ac465740/chrome/browser/io_thread.cc
[modify] https://crrev.com/3c2d643cded7d3c68510aa69b08a19b2ac465740/chrome/browser/io_thread.h
[add] https://crrev.com/3c2d643cded7d3c68510aa69b08a19b2ac465740/chrome/browser/net/sth_distributor_provider.cc
[add] https://crrev.com/3c2d643cded7d3c68510aa69b08a19b2ac465740/chrome/browser/net/sth_distributor_provider.h
[modify] https://crrev.com/3c2d643cded7d3c68510aa69b08a19b2ac465740/chrome/browser/profiles/profile_io_data.cc
[modify] https://crrev.com/3c2d643cded7d3c68510aa69b08a19b2ac465740/chrome/browser/profiles/profile_io_data.h
[modify] https://crrev.com/3c2d643cded7d3c68510aa69b08a19b2ac465740/chrome/chrome_browser.gypi

Comment 4 by eranm@chromium.org, Jun 29 2016

Blocking: 480867

Comment 5 by eranm@chromium.org, Jun 30 2016

Status: Fixed (was: Assigned)

Sign in to add a comment