Re: [chrony-dev] [PATCH] main: imply -x if time can't be set

On Thu, Mar 8, 2018 at 10:14 AM, Miroslav Lichvar <mlichvar@xxxxxxxxxx> wrote:

On Wed, Mar 07, 2018 at 04:43:20PM +0100, Christian Ehrhardt wrote:
> I thought that having the server portion work by default would be very nice
> to user.

Are there any users that want to run an NTP server knowing that the
clock is not synchronized by something else, but not care if it will
be synchronized or not when the server is running?

I think I did a bad decision to link the suggestion to "the ability to set time" like CAP_SYS_TIME, I'll outline an alternative below at the end of this mail.

I think in containers where (mostly) you can't "set" time you still might want to serve time even being low-quality time.

See below for more of my (hopefully now better outlined) concerns and reasons.

> But I agree that if time there is like "really bad" then it has to be an
> explicit opt in by user/tool that sets it up.

[...]

If there is no other way to run an NTP server (e.g. on a VPS), ok,
there is an option. But I think that disabling the control is such an
important aspect of the operation that it should always be explicitly
configured for that.

Does that make sense?

Yeah I can follow the argument.

I had a few more discussions with the initial reports that made me work on this and more people considering alternatives.

But even following the former arguments we had so far, we found several cases that I think could need improvements.

Note: All of this is triggered by [1] and I try to keep bug and mails here a bit in sync :-)

1. if you are in a container you very likely can't set the time.

Installing chrony there would silently not start the chrony service for lacking CAP_SYS_TIME.
- You now installed chrony got no error/warning, but it does nothing.
- If there are services depending on the chrony service they would not start either

2. if you are in busted Host system (or VM) that grants no CAP_SYS_TIME the same as above will silently happen.
And on a Metal machine or even a VM you'd really want to know it is failing, because you'd expect it to work.

3. If you are in a container with special privileges that allow CAP_SYS_TIME (rare) it is very dangerous to do so.

There are no time namespaces for containers to use. Therefore multiple containers on that system would start fighting with time adjustments which would be the worst IMHO.

4. If run in a container a log message should make it clear that expectations should not be to sync the local time as it is impossible (in 99.9+% of the cases)

For #1 you want to default to -x if you are in a container
For #2 you want to drop the condition on CAP_SYS_TIME
For #3 you want to default to -x if you are in a container
For #4 you want to log a message if the case is detected

These should be defaults, and admins would be given a way to override this behavior.

This could either be done either:
- in a Chrony patch to provide this behavior and a cmdline option to override.
- in a wrapper script to the ExecStart of the service file doing the checks/messaging and adding -x as needed

The former would be nicer, but requires to re-implement a lot of systemd-detect-virt in chrony which feels wrong a bit.

The latter seems easier as one could re-use systemd-detect-virt which might be worth to do it outside the binary.

Thanks already for the great discussions so far, I wonder what opinions are about the revised approach.

[1]: https://bugs.launchpad.net/ubuntu/+source/chrony/+bug/1589780