https://codereview.chromium.org/2723783002/

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

commit 39f9eb3f5301f73d385c6d65601c60dba4fae63e
Author: rsleevi <rsleevi@chromium.org>
Date: Wed Mar 01 00:31:53 2017

Make SSLConfigManager notify clients if the SHA-1 policy changes

An oversight in the comparator for the SSLConfigManager that handles
the logic for determining when an SSLConfig is "new" (and thus
should invalidate all existing SSLConfigs) meant that changes to the
SHA-1 policy for local anchors wouldn't be immediately propagated.

BUG= 697154 

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

[modify] https://crrev.com/39f9eb3f5301f73d385c6d65601c60dba4fae63e/net/ssl/ssl_config_service.cc
[modify] https://crrev.com/39f9eb3f5301f73d385c6d65601c60dba4fae63e/net/ssl/ssl_config_service_unittest.cc

Hello TPMs, both Release and Enterprise:

This was bug I noticed when working on some M58 code that potentially means that changes to the EnableSha1ForLocalAnchors policy won't propagate.

Because we introduced that policy because M-57 bears with it some risk for Enterprise (notably, in the Edu market, where MITM is prevalent and insecure), it seems like this can/should be a candidate for merging. We've been telling enterprises to enable that policy (in https://security.googleblog.com/2016/11/sha-1-certificates-in-chrome.html ) and I would love to make sure this gets in a release so we can make sure, sure, sure that all our bases are covered to avoid disruption.

However, I'm not sure how we handle "group policy changes while Chrome is running" - whether restarting Chrome is an acceptable workaround until M58 (e.g. denying the merge) or whether we should consider this as a ReleaseBlocker.

Please advise on whether this should be RB-S

This bug requires manual review: We are only 12 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

+ awhalley@ for M57 merge review

+ blumberg@ (Enterprise PM)

  It is not common for customers (IT-Admins) to make changes to GPO on a regular basis or deploy new GPOS 'on the fly' when managing a large number of systems. Typically changes are reviewed, deployed and scheduled reboots happen when updates are deployed (for the whole system, not just Chrome as typically other SW updates are also applied). So if the only risk here is that companies who deploy this will not have policy applied until the Chrome  process restarts, I think it is low risk.  

Don't forget that if they set the policy today and then AU or deploy M57 that will be the 'restart' needed, right?

Having said that, if the change is very small (low risk) it would be great to have this in m57.

It would also affect clients if we delay initializing enterprise policies after profile startup. Unless/until they changed one of the _other_ policies, so as to tickle a "Hey, there's a new config" notification, then they'd stick with the old config from prefs.

It's very much a small and low-risk change; The effect of the change is just "Consider this policy different if the old policy and the new policy don't share the same value for SHA-1" - like they already examine every other policy-or-command-line configured option.

Makes sense, so if this is a small/low-risk change, is there any objection to a merge?

Thank you  blumberg@ and rsleevi@.

rsleevi@, please update this bug once change is baked/verified in Canary. If things look good in Canary, I will approve the merge to M57.

rsleevi@, how is this change looking in Canary?

It's looking fine, no issues reported and works with manual testing :)

Thank you rsleevi@. Approving merge to M57 branch 2987 based on comment #8, #9 and #12. 

Please merge your change to M57 branch 2987 latest by 5:00 PM PT Friday, 03/03 (sooner the better) so we can take it in for M57 Desktop Stable cut. Thank you.

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

commit da57f51fe86456e1e909011e8495d089f21516cb
Author: Ryan Sleevi <rsleevi@chromium.org>
Date: Fri Mar 03 02:30:39 2017

Make SSLConfigManager notify clients if the SHA-1 policy changes

An oversight in the comparator for the SSLConfigManager that handles
the logic for determining when an SSLConfig is "new" (and thus
should invalidate all existing SSLConfigs) meant that changes to the
SHA-1 policy for local anchors wouldn't be immediately propagated.

BUG= 697154 

Review-Url: https://codereview.chromium.org/2723783002
Cr-Commit-Position: refs/heads/master@{#453772}
(cherry picked from commit 39f9eb3f5301f73d385c6d65601c60dba4fae63e)

Review-Url: https://codereview.chromium.org/2730623005 .
Cr-Commit-Position: refs/branch-heads/2987@{#746}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/da57f51fe86456e1e909011e8495d089f21516cb/net/ssl/ssl_config_service.cc
[modify] https://crrev.com/da57f51fe86456e1e909011e8495d089f21516cb/net/ssl/ssl_config_service_unittest.cc