Add enterprise policy for network time service

Add enterprise policy for network time service

The NetworkTimeQueriesEnabled policy, if set to false, will disable
NetworkTimeTracker's queries to Google to fetch an accurate timestamp.

NetworkTimeTracker uses these to decide when to prompt the user to fix their
clock if they encounter a certificate date error.

BUG=725232
Review-Url: https://codereview.chromium.org/2902603002
Cr-Commit-Position: refs/heads/master@{#474137}
Committed: https://chromium.googlesource.com/chromium/src/+/80618c346d3f78823fe6061349319a98a867b6cf

[estark, 2017-05-22 21:17:38]

The CQ bit was checked by estark@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-22 21:18:05]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[estark, 2017-05-22 21:23:37]

The CQ bit was checked by estark@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-22 21:24:02]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[estark, 2017-05-22 21:25:14]

estark@chromium.org changed reviewers:
+ jwd@chromium.org, pastarmovj@chromium.org, zea@chromium.org

[estark, 2017-05-22 21:25:15]

Can the following OWNERS please review:
- zea: components/network_time
- jwd: histograms.xml
- pastarmovj: everything else
Thanks!

[commit-bot: I haz the power, 2017-05-22 22:09:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-22 22:09:30]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[estark, 2017-05-22 22:36:38]

The CQ bit was checked by estark@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-22 22:37:10]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-22 23:50:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-22 23:50:30]

Dry run: This issue passed the CQ dry run.

[pastarmovj, 2017-05-23 07:20:41]

lgtm with one nit.

https://codereview.chromium.org/2902603002/diff/40001/components/policy/resou...
File components/policy/resources/policy_templates.json (right):

https://codereview.chromium.org/2902603002/diff/40001/components/policy/resou...
components/policy/resources/policy_templates.json:9562: 'caption': '''Enables
queries to a Google time service to obtain an accurate timestamp''',
Can you please consider shortening this caption. Also I think allow is better
than enable because the default without the policy is anyway enabled. Maybe just
"Allow queries to a Google time service"

[Nicolas Zea, 2017-05-23 17:03:10]

lgtm

[jwd, 2017-05-23 19:02:24]

LGTM

And fyi, since it's just enums.xml, you don't need metrics owners.

[estark, 2017-05-23 21:24:21]

On 2017/05/23 19:02:24, jwd wrote:
> LGTM
> 
> And fyi, since it's just enums.xml, you don't need metrics owners.

Oops -- TIL. Thanks!

[estark, 2017-05-23 21:26:09]

Thanks, all.

https://codereview.chromium.org/2902603002/diff/40001/components/policy/resou...
File components/policy/resources/policy_templates.json (right):

https://codereview.chromium.org/2902603002/diff/40001/components/policy/resou...
components/policy/resources/policy_templates.json:9562: 'caption': '''Enables
queries to a Google time service to obtain an accurate timestamp''',
On 2017/05/23 07:20:41, pastarmovj wrote:
> Can you please consider shortening this caption. Also I think allow is better
> than enable because the default without the policy is anyway enabled. Maybe
just
> "Allow queries to a Google time service"

Done.

[estark, 2017-05-23 21:26:12]

The CQ bit was checked by estark@chromium.org

[estark, 2017-05-23 21:26:13]

The patchset sent to the CQ was uploaded after l-g-t-m from
pastarmovj@chromium.org, zea@chromium.org, jwd@chromium.org
Link to the patchset: https://codereview.chromium.org/2902603002/#ps60001
(title: "shorten caption")

[commit-bot: I haz the power, 2017-05-23 21:27:13]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-24 02:48:35]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1495574772707020,
"parent_rev": "2eef24ccf673d5374278b61aebc29afa75d40134", "commit_rev":
"80618c346d3f78823fe6061349319a98a867b6cf"}

[commit-bot: I haz the power, 2017-05-24 02:48:47]

Description was changed from

==========
Add enterprise policy for network time service

The NetworkTimeQueriesEnabled policy, if set to false, will disable
NetworkTimeTracker's queries to Google to fetch an accurate timestamp.

NetworkTimeTracker uses these to decide when to prompt the user to fix their
clock if they encounter a certificate date error.

BUG=725232
==========

to

==========
Add enterprise policy for network time service

The NetworkTimeQueriesEnabled policy, if set to false, will disable
NetworkTimeTracker's queries to Google to fetch an accurate timestamp.

NetworkTimeTracker uses these to decide when to prompt the user to fix their
clock if they encounter a certificate date error.

BUG=725232

Review-Url: https://codereview.chromium.org/2902603002
Cr-Commit-Position: refs/heads/master@{#474137}
Committed:
https://chromium.googlesource.com/chromium/src/+/80618c346d3f78823fe606134931...
==========

[commit-bot: I haz the power, 2017-05-24 02:48:48]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/80618c346d3f78823fe606134931...

[Nico, 2017-05-24 15:38:16]

thakis@chromium.org changed reviewers:
+ thakis@chromium.org

[Nico, 2017-05-24 15:38:17]

The test added in this Cl fails on bots doing official builds, e.g.
https://build.chromium.org/p/chromium.fyi/builders/CrWinClang%20tester/builds...

Can you take a look?

[estark, 2017-05-24 16:02:50]

On 2017/05/24 15:38:17, Nico wrote:
> The test added in this Cl fails on bots doing official builds, e.g.
>
https://build.chromium.org/p/chromium.fyi/builders/CrWinClang%20tester/builds...
> 
> Can you take a look?

Uh oh. I suspect this is because I forgot to enable the Finch flag in the test.
I'll send a fix in a follow-up.

[Thiemo Nagel, 2017-05-26 11:06:51]

tnagel@chromium.org changed reviewers:
+ tnagel@chromium.org

[Thiemo Nagel, 2017-05-26 11:06:52]

https://codereview.chromium.org/2902603002/diff/60001/components/policy/resou...
File components/policy/resources/policy_templates.json (right):

https://codereview.chromium.org/2902603002/diff/60001/components/policy/resou...
components/policy/resources/policy_templates.json:9552: 'name':
'NetworkTimeQueriesEnabled',
Potentially rename this to clarify that it doesn't apply to CrOS system daemons,
e.g. BrowserNetworkTimeQueriesEnabled?

https://codereview.chromium.org/2902603002/diff/60001/components/policy/resou...
components/policy/resources/policy_templates.json:9564: 'desc': '''Setting this
policy to false stops <ph name="PRODUCT_NAME">$1<ex>Google Chrome</ex></ph> from
occasionally sending queries to a Google server to retrieve an accurate
timestamp. These queries will be enabled if this policy is set to True or is not
set.''',
Emily, could you please clarify that this does only affect Chrome the browser? 
On Chrome OS, we use tlsdated to set system time and I'm not aware of any plans
to disable that.

[estark, 2017-05-30 16:45:22]

https://codereview.chromium.org/2902603002/diff/60001/components/policy/resou...
File components/policy/resources/policy_templates.json (right):

https://codereview.chromium.org/2902603002/diff/60001/components/policy/resou...
components/policy/resources/policy_templates.json:9552: 'name':
'NetworkTimeQueriesEnabled',
On 2017/05/26 11:06:51, Thiemo Nagel wrote:
> Potentially rename this to clarify that it doesn't apply to CrOS system
daemons,
> e.g. BrowserNetworkTimeQueriesEnabled?

Will do in a follow-up.

https://codereview.chromium.org/2902603002/diff/60001/components/policy/resou...
components/policy/resources/policy_templates.json:9564: 'desc': '''Setting this
policy to false stops <ph name="PRODUCT_NAME">$1<ex>Google Chrome</ex></ph> from
occasionally sending queries to a Google server to retrieve an accurate
timestamp. These queries will be enabled if this policy is set to True or is not
set.''',
On 2017/05/26 11:06:52, Thiemo Nagel wrote:
> Emily, could you please clarify that this does only affect Chrome the browser?

> On Chrome OS, we use tlsdated to set system time and I'm not aware of any
plans
> to disable that.

Hmm. Just to clarify, this policy would not affect the use of tlsdated to set
the system time. If this policy were enabled on ChromeOS, if the system time did
somehow get off and the user encountered a certificate date error as a result,
the policy would use the Google time service to decide to show them a prompt to
fix the clock -- it wouldn't actually affect the system time. However, digging
through some old email, it looks like it is possible that the prompt to fix the
clock is useless/confusing on ChromeOS because it does not allow the user to
actually fix the clock. So I'll change this to non-ChromeOS.

[Thiemo Nagel, 2017-05-31 13:39:49]

> Hmm. Just to clarify, this policy would not affect the use of tlsdated to set
> the system time. If this policy were enabled on ChromeOS, if the system time
did
> somehow get off and the user encountered a certificate date error as a result,
> the policy would use the Google time service to decide to show them a prompt
to
> fix the clock -- it wouldn't actually affect the system time. However, digging
> through some old email, it looks like it is possible that the prompt to fix
the
> clock is useless/confusing on ChromeOS because it does not allow the user to
> actually fix the clock. So I'll change this to non-ChromeOS.

Thank you for looking into this!

On Chrome OS, the system clock can only be changed by the user when since bootup
tlsdated did not (yet) manage to obtain the time from
https://clients3.google.com/.  There's a D-Bus call to determine whether time
has been sync'ed yet or not, thus in theory, Chrome could find out whether the
"fix your clock" prompt is actionable.  However, imho the more interesting
question would be why tlsdated didn't manage to sync the clock (do we need to
tweak the retry logic?).  Since we probably have stats for that: Does the
incidence of clock-out-of-sync on Chrome OS even warrant any further work?