New issue
Advanced search Search tips

Issue 633614 link

Starred by 4 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

URLRequestContext users should indicate if Certificate Transparency support is required

Project Member Reported by eranm@chromium.org, Aug 2 2016

Issue description

Currently, all classes that use net::URLRequestContext have to provide a valid CTVerifier pointer. Since the only concrete implementation is MultiLogCTVerifier, that class is instantiated every where a CTVerifier instance is used, even if the callers do not care about the result of checking Signed Certificate Timestamps (SCTs). This is bad because the MultiLogCTVerifier does a fair amount of work even if it's not initialized with any CT logs (i.e. with the AddLogs call): Extracting and de-serializing SCTs from the certificate, stapled OCSP response and TLS handshake, resulting in memory allocations that are unnecessary because these SCTs cannot be verified.

Once that's done we can remove the empty constructor from MultiLogCTVerifire, then remove AddLogs in favour of initialization in the constructor and DCHECK that the list of logs passed into the c'tor is non-empty.
 

Comment 1 by eranm@chromium.org, Aug 2 2016

Trying to capture some more notes from the discussion with Ryan:
* We should find out, for each user of the URLRequestContext, if they explicitly care about SCT validation or not.
* There are obvious places we could clean up immediately, for some callers we'll have to explicitly ask the team that owns it.
* The question is also related to whether the CT requirement for Symantec certs should be enforced, for example, since the behaviour of all default components right now would lead to marking Symantec certs as non-valid.

Comment 2 by eranm@chromium.org, Aug 2 2016

A note about the implication of this clean up on 6962-bis:

Right now it's unclear how implementation of 6962-bis will affect the codebase. SCTs are different structures (the Log ID is different), in 6962-bis there's reliance on TransItem being passed around to preserve some properties (for example, the distinction between an SCT for a precert and an SCT for an X.509 cert is by having two different types, both kept in a TransItem). Also, 6962-bis is not finalized.

For this reason we can't take into account how this cleanup may affect 6962-bis implementation, so 6962-bis should not block it.
Components: Internals>Network
1) We need to know whether a component should care about CT validation. The default answer should be "yes", but there may be exceptional cases
2) When they do care about CT verification, we need to also see if they care about enterprise policies. If someone is creating a MultiLogCTVerifier, then it means they're generally creating a new (URLRequestContext/HTTPNetworkSession), and as a result, aren't piping in things like policy controls from the SystemURLRequestContext/profile's URLRequestContext. So there's a broader question of: where places are creating a MultiLogCTVerifier, should they even be creating a URLRequestContext to begin with.

MultiLogCTVerifier() is a symptom of a deeper cause, which is a lot of places creating URLRequestContexts, and we want to carefully evaluate whether they should, whether they expect things like enterprise-policy based CT opt-outs to work, and if they do, how best to thread that dependency down.

Comment 4 by eranm@chromium.org, Aug 24 2016

Summary of the discussion from a soon-to-be-closed issue:
AddLogs can be removed  from MultiLogCTVerifier in favour of a c'tor with a log list, but first Ryan suggested we move off the use of the MultiLogCTVerifier with the default c'tor where no initialization takes place, so I'll send a CL to that effect.

Comment 5 by eranm@chromium.org, Sep 13 2016

Document of call sites and the reason for/against supporting CT in each one:
https://docs.google.com/document/d/15Z8-rz5uCuxuYqKwuRK5dVyD0AkPsqBbP11GBA5YJ-Y/edit

Comment 6 by eranm@chromium.org, Jun 19 2017

Cc: -rsleevi@chromium.org eranm@chromium.org
Owner: rsleevi@chromium.org
Ryan, you've followed up on that a bit, so assigning to you.
I'll be happy to take on some of the work (since it's a clean-up) if there's a clear path forward.
Status: Started (was: Assigned)

Sign in to add a comment