Add a preference for enabling the TLS origin-bound certificates extension.

Add a preference for enabling the TLS origin-bound certificates extension.

The preference is controlled by the command line and enterprise policy.

R=mattm@chromium.org,mnissler@chromium.org
BUG=106077
TEST=Existing policy unit tests are updated.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=112848

[wtc, 2011-12-01 21:39:34]

mattm: please review the entire CL.

mnissler: I'd appreciate your review of how I add a new
preference that can be controlled by the command line and
enterprise policy.  I found the files to modify by searching
for an existing boolean preference,
"EnableAuthNegotiatePort", in the source tree.  I may have
missed something or modified more than I need.  You may
skip the following files in your review:
  chrome/browser/net/ssl_config_service_manager_pref.cc (maybe)
  content/browser/browser_main_loop.cc
  net/base/ssl_config_service.h
  net/base/ssl_config_service.cc

mmoss: just FYI.

http://codereview.chromium.org/8772014/diff/1/chrome/app/policy/policy_templa...
File chrome/app/policy/policy_templates.json (right):

http://codereview.chromium.org/8772014/diff/1/chrome/app/policy/policy_templa...
chrome/app/policy/policy_templates.json:2137: 'future': False,

mnissler: I assume the 'main' type means it is a boolean.
What does 'future' mean?

[mattm, 2011-12-01 23:22:52]

lgtm

http://codereview.chromium.org/8772014/diff/1/chrome/app/policy/policy_templa...
File chrome/app/policy/policy_templates.json (right):

http://codereview.chromium.org/8772014/diff/1/chrome/app/policy/policy_templa...
chrome/app/policy/policy_templates.json:2142: 
Is the extra whitespace and repeating the caption intentional?  I don't know
where this text is used, so maybe it is.

[wtc, 2011-12-02 01:48:11]

mattm: thanks for the review.  I addressed your comment in
Patch Set 2.

http://codereview.chromium.org/8772014/diff/1/chrome/app/policy/policy_templa...
File chrome/app/policy/policy_templates.json (right):

http://codereview.chromium.org/8772014/diff/1/chrome/app/policy/policy_templa...
chrome/app/policy/policy_templates.json:2142: 

The extra whitespace in the 'desc' was copied from another
boolean setting that I modeled this after.  I removed the
extra whitespace from all of them.

I reworded the 'desc' so that it doesn't repeat the caption.

[Mattias Nissler (ping if slow), 2011-12-02 12:40:08]

LGTM with nits.

http://codereview.chromium.org/8772014/diff/1004/chrome/app/policy/policy_tem...
File chrome/app/policy/policy_templates.json (right):

http://codereview.chromium.org/8772014/diff/1004/chrome/app/policy/policy_tem...
chrome/app/policy/policy_templates.json:2143: This setting is used to enable the
TLS origin-bound certificates extension for a group of Google employees for
testing.  This setting will be deleted in the future.''',
Oh, so this is Google only? You can then put 'future': True. Doing so, the
policy is still enabled and can be configured manually, but it won't show up in
documentation and we won't generate group policy templates and example files.
Also, I'd just mention here that this is experimental and may be removed in the
future, the reference to Google is not necessary.

http://codereview.chromium.org/8772014/diff/1004/chrome/browser/net/ssl_confi...
File chrome/browser/net/ssl_config_service_manager_pref.cc (right):

http://codereview.chromium.org/8772014/diff/1004/chrome/browser/net/ssl_confi...
chrome/browser/net/ssl_config_service_manager_pref.cc:229:
config->origin_bound_certs_enabled = origin_bound_certs_enabled_.GetValue();
This is also invoked on PREF_CHANGED notifications. If the SetSSLConfigFlags
call below activates the new configuration, you can set the dynamic_refresh flag
in the policy template. To test whether it works, you can write the policy file
in Linux, wait 5 seconds, and watch the PREF_CHANGED event to fire and call this
function.

[wtc, 2011-12-02 18:31:54]

mnissler: thanks a lot for your review.  I made your suggested
changes to the policy_templates.json in Patch Set 3.

http://codereview.chromium.org/8772014/diff/1004/chrome/browser/net/ssl_confi...
File chrome/browser/net/ssl_config_service_manager_pref.cc (right):

http://codereview.chromium.org/8772014/diff/1004/chrome/browser/net/ssl_confi...
chrome/browser/net/ssl_config_service_manager_pref.cc:229:
config->origin_bound_certs_enabled = origin_bound_certs_enabled_.GetValue();
On 2011/12/02 12:40:08, Mattias Nissler wrote:
> This is also invoked on PREF_CHANGED notifications. If the SetSSLConfigFlags
> call below activates the new configuration, you can set the dynamic_refresh
flag
> in the policy template.

The SetSSLConfigFlags call below does not activate the new
configuration, but the SetNewSSLConfig task posted on line
217 above does.  So I changed the dynamic_refresh flag in
the policy template to 1.

[commit-bot: I haz the power, 2011-12-02 18:32:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wtc@chromium.org/8772014/31

[commit-bot: I haz the power, 2011-12-02 18:32:21]

Presubmit check for 8772014-31 failed and returned exit status 1.

Running presubmit commit checks ...
Finished checking
/mnt/data/b/commit-queue/workdir/chromium/chrome/app/policy/policy_templates.json.
0 errors, 0 warnings.

** Presubmit ERRORS **
Missing LGTM from an OWNER for: content/browser/browser_main_loop.cc

Presubmit checks took 2.5s to calculate.

[wtc, 2011-12-02 18:48:31]

brettw: please review the change to
    content/browser/browser_main_loop.cc
in this CL.

I moved the handling of the command-line switch
switches::kEnableOriginBoundCerts from that file to
chrome/browser/prefs/command_line_pref_store.cc.

Thanks.

[Mattias Nissler (ping if slow), 2011-12-02 18:52:02]

Thanks, looks good.

[brettw, 2011-12-02 19:09:58]

browser_main_loop LGTM

[commit-bot: I haz the power, 2011-12-02 19:14:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wtc@chromium.org/8772014/31

[commit-bot: I haz the power, 2011-12-02 23:16:47]

Try job failure for 8772014-31 (retry) on mac_rel for steps "browser_tests,
ui_tests".
It's a second try, previously, steps "sync_unit_tests, browser_tests, ui_tests"
failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2011-12-02 23:18:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wtc@chromium.org/8772014/31

[commit-bot: I haz the power, 2011-12-03 02:02:24]

Change committed as 112848