Make OffTheRecordProfileIOData use its own CTVerifier.

Make OffTheRecordProfileIOData use its own CTVerifier.

It subclasses ProfileIOData, which always makes and sets a CTVerifier
for the MainURLRequestContext, but the OffTheRecordProfileIOData was
replacing the pointer with on to the system context's CTVerifier. Both
verifiers are created with the same arguments, so this doesn't seem to
server any useful purpose.

The CTVerifier created by ProfileIOData contains no reference to
outside data, other than CTLogs and as an STH Observers, so it should
be fine to use for Incognito mode (Also note that it is not shared
between Incognito and non-Incognito).

BUG=732537
Review-Url: https://codereview.chromium.org/2936643004
Cr-Commit-Position: refs/heads/master@{#478792}
Committed: https://chromium.googlesource.com/chromium/src/+/1a6910dc3ffb95581db2b63a7ccd9b089585d323

[mmenke, 2017-06-12 21:07:33]

The CQ bit was checked by mmenke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-12 21:08:03]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mmenke, 2017-06-12 21:12:41]

mmenke@chromium.org changed reviewers:
+ rsleevi@chromium.org

[mmenke, 2017-06-12 21:12:42]

The code to create the CertVerifier is
https://cs.chromium.org/chromium/src/chrome/browser/profiles/profile_io_data....
(Creates them independently for both the OTR and non-OTR profiles).  If that
creation code isn't appropriate for the OTR's verifier, someone who knows what
they're doing should take this over, though it does look to me like it's created
just like the System URLRequestContext's, so I assume it's fine.

[Ryan Sleevi, 2017-06-12 21:33:00]

Explanation for the context:
- Sets up a verifier (using the global logs)
- Makes the verifier notify the (per-profile) ct_tree_tracker_
- Wires the ct_tree_tracker_ to the IO thread's component updater (to be
notified when new STHs come in from component updater)

the ct_tree_tracker_ does consistency proof fetching (via DNS), which discloses
the SCTs seen via the verifier via DNS.

From the POV of an OTR profile, my understanding is that this fits within the
scope/remit of OTR, and we're not cloning any state when initializing an OTR
profile, and we don't (yet) have any persistence layer persisting these lookups
(but was planned, just a matter of restaffing).

If that matches your understanding of OTR, then LGTM

[mmenke, 2017-06-12 21:39:26]

Description was changed from

==========
Make OffTheRecordProfileIOData use its own CTVerifier.

It subclasses ProfileIOData, which always makes and sets a CTVerifier
for the MainURLRequestContext, but the OffTheRecordProfileIOData was
replacing the pointer with on to the system context's CTVerifier. Both
verifiers are created with the same arguments, so this doesn't seem to
server any useful purpose.

BUG=732537
==========

to

==========
Make OffTheRecordProfileIOData use its own CTVerifier.

It subclasses ProfileIOData, which always makes and sets a CTVerifier
for the MainURLRequestContext, but the OffTheRecordProfileIOData was
replacing the pointer with on to the system context's CTVerifier. Both
verifiers are created with the same arguments, so this doesn't seem to
server any useful purpose.

The CTVerifier created by ProfileIOData contains no reference to
outside data, other than CTLogs and as an STH Observers, so it should
be fine to use for Incognito mode (Also note that it is not shared
between Incognito and non-Incognito).

BUG=732537
==========

[mmenke, 2017-06-12 21:41:10]

On 2017/06/12 21:33:00, Ryan Sleevi wrote:
> Explanation for the context:
> - Sets up a verifier (using the global logs)
> - Makes the verifier notify the (per-profile) ct_tree_tracker_
> - Wires the ct_tree_tracker_ to the IO thread's component updater (to be
> notified when new STHs come in from component updater)
> 
> the ct_tree_tracker_ does consistency proof fetching (via DNS), which
discloses
> the SCTs seen via the verifier via DNS.
> 
> From the POV of an OTR profile, my understanding is that this fits within the
> scope/remit of OTR, and we're not cloning any state when initializing an OTR
> profile, and we don't (yet) have any persistence layer persisting these
lookups
> (but was planned, just a matter of restaffing).
> 
> If that matches your understanding of OTR, then LGTM

Thanks!  That matches my understanding.

[mmenke, 2017-06-12 21:41:35]

The CQ bit was unchecked by mmenke@chromium.org

[mmenke, 2017-06-12 21:41:40]

The CQ bit was checked by mmenke@chromium.org

[commit-bot: I haz the power, 2017-06-12 21:42:01]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-12 22:29:04]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1497303700616680, "parent_rev":
"15b2b0b0e96b31574fd1a975a9acef92a71a7842", "commit_rev":
"1a6910dc3ffb95581db2b63a7ccd9b089585d323"}

[commit-bot: I haz the power, 2017-06-12 22:29:18]

Description was changed from

==========
Make OffTheRecordProfileIOData use its own CTVerifier.

It subclasses ProfileIOData, which always makes and sets a CTVerifier
for the MainURLRequestContext, but the OffTheRecordProfileIOData was
replacing the pointer with on to the system context's CTVerifier. Both
verifiers are created with the same arguments, so this doesn't seem to
server any useful purpose.

The CTVerifier created by ProfileIOData contains no reference to
outside data, other than CTLogs and as an STH Observers, so it should
be fine to use for Incognito mode (Also note that it is not shared
between Incognito and non-Incognito).

BUG=732537
==========

to

==========
Make OffTheRecordProfileIOData use its own CTVerifier.

It subclasses ProfileIOData, which always makes and sets a CTVerifier
for the MainURLRequestContext, but the OffTheRecordProfileIOData was
replacing the pointer with on to the system context's CTVerifier. Both
verifiers are created with the same arguments, so this doesn't seem to
server any useful purpose.

The CTVerifier created by ProfileIOData contains no reference to
outside data, other than CTLogs and as an STH Observers, so it should
be fine to use for Incognito mode (Also note that it is not shared
between Incognito and non-Incognito).

BUG=732537

Review-Url: https://codereview.chromium.org/2936643004
Cr-Commit-Position: refs/heads/master@{#478792}
Committed:
https://chromium.googlesource.com/chromium/src/+/1a6910dc3ffb95581db2b63a7ccd...
==========

[commit-bot: I haz the power, 2017-06-12 22:29:19]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/1a6910dc3ffb95581db2b63a7ccd...