Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2621083004:
Disable Certificate Transparency enforcement for Chromecast. (Closed)
DescriptionDisable Certificate Transparency enforcement for Chromecast.
A dummy CTVerifier and CTPolicyEnforcer is used to turn off parsing,
validation, and evaluation of results.
BUG= internal b/34104027
Review-Url: https://codereview.chromium.org/2621083004
Cr-Commit-Position: refs/heads/master@{#443030}
Committed: https://chromium.googlesource.com/chromium/src/+/1962aeba7680080d0da7b482ac1d82da74db74a3
Patch Set 1 #Patch Set 2 : Disable CT enforcement #Messages
Total messages: 19 (9 generated)
Description was changed from ========== Initialize CT state state for Chromecast URLRequestContextFactory. URLRequestContextFactory needs to be initialized with Certificate Transparency logs. Without the logs, CT checks will fail for certificates that enforce CT. BUG= internal b/34104027 ========== to ========== Initialize CT state state for Chromecast URLRequestContextFactory. URLRequestContextFactory needs to be initialized with Certificate Transparency logs. Without the logs, CT checks will fail for certificates that enforce CT. BUG= internal b/34104027 ==========
ryanchung@chromium.org changed reviewers: + alokp@chromium.org, dougsteed@chromium.org, rsleevi@chromium.org
Description was changed from ========== Initialize CT state state for Chromecast URLRequestContextFactory. URLRequestContextFactory needs to be initialized with Certificate Transparency logs. Without the logs, CT checks will fail for certificates that enforce CT. BUG= internal b/34104027 ========== to ========== Initialize CT state for Chromecast URLRequestContextFactory. URLRequestContextFactory needs to be initialized with Certificate Transparency logs. Without the logs, CT checks will fail for certificates that enforce CT. BUG= internal b/34104027 ==========
rsleevi@chromium.org changed reviewers: + eranm@chromium.org
It'd be easier to have a quick VC on this if you have time today, otherwise, we can discuss the tradeoffs on this CL. High-level questions: 1) Do you know the upgrade rate of Chromecast devices? 2) Do you keep to 6-week releases? 3) Is Chromecast ever intended to access 'enterprise' scenarios?
OK, to recap the discussion, we'll go ahead with not enforcing CT for Chromecast for now, and we'll revisit around Chrome 59/60 as the implementation matures for more 'longer-term' devices. The optimal (e.g. causes no undue CPU work) solution is: - Using a CTVerifier like https://cs.chromium.org/chromium/src/net/cert/do_nothing_ct_verifier.h?rcl=0&... (Turns off all parsing/signature validation) - Changing a CTPolicyEnforcer to a subclass like: https://cs.chromium.org/chromium/src/remoting/protocol/ssl_hmac_channel_authe... (Turns off all counting/evaluation of the parsed/validated results) I mentioned that we interact with Enterprise Policies for disabling CT exceptions, so 'another' route is giving a TransportSecurityState::RequireCTDelegate (see https://cs.chromium.org/chromium/src/net/http/transport_security_state.h?rcl=... ) which just says "Not required" for everything. However, with that, you still have to pay the parse/verify cost
Description was changed from
==========
Initialize CT state for Chromecast URLRequestContextFactory.
URLRequestContextFactory needs to be initialized with Certificate Transparency
logs. Without the logs, CT checks will fail for certificates that enforce CT.
BUG= internal b/34104027
==========
to
==========
Disable Certificate Transparency enforcement for Chromecast.
A dummy CTVerifier and CTPolicyEnforcer is used to turn off parsing,
validation, and evaluation of results.
BUG= internal b/34104027
==========
On 2017/01/10 23:31:45, Ryan Sleevi wrote: > OK, to recap the discussion, we'll go ahead with not enforcing CT for Chromecast > for now, and we'll revisit around Chrome 59/60 as the implementation matures for > more 'longer-term' devices. > > The optimal (e.g. causes no undue CPU work) solution is: > - Using a CTVerifier like > https://cs.chromium.org/chromium/src/net/cert/do_nothing_ct_verifier.h?rcl=0&... > (Turns off all parsing/signature validation) > - Changing a CTPolicyEnforcer to a subclass like: > https://cs.chromium.org/chromium/src/remoting/protocol/ssl_hmac_channel_authe... > (Turns off all counting/evaluation of the parsed/validated results) > > > I mentioned that we interact with Enterprise Policies for disabling CT > exceptions, so 'another' route is giving a > TransportSecurityState::RequireCTDelegate (see > https://cs.chromium.org/chromium/src/net/http/transport_security_state.h?rcl=... > ) which just says "Not required" for everything. However, with that, you still > have to pay the parse/verify cost Thanks. I've updated the code. The IgnoresCTPolicyEnforcer is directly from the example you provided. Should we move that to a common place to prevent duplicate code? or is the duplication preferred in order to discourage disabling of CT?
On 2017/01/11 02:18:00, ryanchung wrote: > Thanks. I've updated the code. The IgnoresCTPolicyEnforcer is directly from the > example you provided. Should we move that to a common place to prevent duplicate > code? or is the duplication preferred in order to discourage disabling of CT? I think the duplication is fine (for now). So far we've only had one case (the place you're copying from) outside of tests. That said, we're keeping an eye on it (like the DoNothingCTVerifier). It hasn't been a high priority right now mostly because we're trying to figure how to simplify these interfaces related to CT to make it less error prone and easier to set :)
lgtm
ryanchung@chromium.org changed reviewers: + halliwell@chromium.org
+halliwell for /chromecast owner review Thanks.
On 2017/01/11 21:11:40, ryanchung wrote: > +halliwell for /chromecast owner review > > Thanks. lgtm
The CQ bit was checked by ryanchung@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484170630881350,
"parent_rev": "2d4b91cca2e8d886ca9fe64f58987448fe72289c", "commit_rev":
"1962aeba7680080d0da7b482ac1d82da74db74a3"}
Message was sent while issue was closed.
Description was changed from
==========
Disable Certificate Transparency enforcement for Chromecast.
A dummy CTVerifier and CTPolicyEnforcer is used to turn off parsing,
validation, and evaluation of results.
BUG= internal b/34104027
==========
to
==========
Disable Certificate Transparency enforcement for Chromecast.
A dummy CTVerifier and CTPolicyEnforcer is used to turn off parsing,
validation, and evaluation of results.
BUG= internal b/34104027
Review-Url: https://codereview.chromium.org/2621083004
Cr-Commit-Position: refs/heads/master@{#443030}
Committed:
https://chromium.googlesource.com/chromium/src/+/1962aeba7680080d0da7b482ac1d...
==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/1962aeba7680080d0da7b482ac1d... |
