CertTransparencyVerifier created but never used for the OTR Profile |
|||
Issue descriptionCurrently, we create a CTVerifier for OffTheRecordProfileIOData, but never actually use it. Instead, we use the system URLRequestContext's CTVerifier, which is set up using the exact same parameters as the one created by ProfileIOData.
,
Jun 12 2017
Oh, making sure I understand: We create one in ProfileIOData ( https://cs.chromium.org/chromium/src/chrome/browser/profiles/profile_io_data.h?rcl=57cbe4e5fd1f7d7c24b404b783e0587aa97cbfd3&l=565 ) but since OTRProfileIOData is derived from Profile, we don't use it in OTR The ProfileIOData attached one has 'user state' hanging off it (c.f. https://cs.chromium.org/chromium/src/chrome/browser/profiles/profile_io_data.cc?rcl=ad604940edf862c4379cf916eaa81d3fc8b7ee36&l=1148 ) that is used to wire up the inclusion proof fetching (which stores which certs you've visited in memory, and we were exploring persisting to disk), so we didn't want to use that same OTR mechanism. Does OTRProfileIOData use ProfileIOData::Init()? It wasn't readily apparent from https://cs.chromium.org/chromium/src/chrome/browser/profiles/off_the_record_profile_io_data.cc?rcl=57cbe4e5fd1f7d7c24b404b783e0587aa97cbfd3&l=194 Using the SystemURLRequestContext's CTVerifier is (less) problematic, because we don't wire up the system context to any SCT state, so it's just a memory optimization, but I'd rather be inclined to think we should try to separate out the OTR properly. Hopefully that made sense :)
,
Jun 12 2017
ProfileIOData is completely shared by the OTR profile and ProfileImplIOData. So what's happening is we create one got OTR in ProfileIOData, but then the the OTR profile overwrites the pointer to its own (Which it doesn't delete, so it just sits there in memory, unused) with a pointer to the System URLRequestContext's verifier. I'm not seeing any user state hanging off of ProfileIOData's CTVerifier - looks like we only hook up IOThread's (global) CTLogs and a fresh TreeStateTracker.
,
Jun 12 2017
I think we should only go with things that are globally shared (CTEnforcer, NetLog, HostResolverImpl, currently), or things that are per-URLRequestContext. From a NetworkService perspective, things that are only shared between some URLRequestContexts are weird.
,
Jun 12 2017
The TreeStateTracker is the user-state (of sorts). It doesn't persist to disk, and is thus outside the bounds of OTR's guarantees, but does note when/what certs a user is seeing and tries to fetch details over DNS (or will). This would be fine to be per-URC
,
Jun 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1a6910dc3ffb95581db2b63a7ccd9b089585d323 commit 1a6910dc3ffb95581db2b63a7ccd9b089585d323 Author: mmenke <mmenke@chromium.org> Date: Mon Jun 12 22:29:02 2017 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} [modify] https://crrev.com/1a6910dc3ffb95581db2b63a7ccd9b089585d323/chrome/browser/profiles/off_the_record_profile_io_data.cc
,
Jun 13 2017
,
Jun 13 2017
|
|||
►
Sign in to add a comment |
|||
Comment 1 by rsleevi@chromium.org
, Jun 12 2017