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

2018-03-12 Thread Christian Ehrhardt 
On Thu, Mar 8, 2018 at 6:09 PM, Miroslav Lichvar 
wrote:

> On Thu, Mar 08, 2018 at 05:08:16PM +0100, Christian Ehrhardt wrote:
> > 1. the option would not be default on, so "normal" users/installations
> > would not be affected
> > 2. cases that want the fallback behavior, but are unable to probe/detect
> at
> > the time:
> >- so they can not decide to use normal -x
> >- also the environment might change on them withut reconfig
> >Both of the above would be solved by them dropping the new -x=fallback
> > option for their case.
>
> Does that include an assumption that if the clock cannot be
> controlled, it's already synchronized by something else and if it can,
> it's a separate time namespace?
>
> > Our container folks will outline the CAP_SYS_TIME issue I mentioned
> before,
> > so really the best test for my suggested SYS_IsTimeAdjustable would be
> (on
> > top to what I have to check the Cap) a adjtime no-op.
> > I tried via adjtimex cmdline and thought maybe "adjtimex -s 0" (in C from
> > chrony eventually) would be a no-op check I'd think
>
> The sys_linux initialization code resets the singleshot offset, which
> could be used as an early check for adjtimex().
>
> Ok, here are some suggestions for the implementation:
> - change all SYS_*_Initialise() functions to return 1 and SYS_Initialise()
>   to check the return code (with a LOG_FATAL message if it is 0)
> - change reset_adjtime() and SYS_Linux_Initialise() to return 0 on failure
> - change SYS_Initialise() to handle the failure if clock_control is -1
>   and add (and document) -X option which sets clock_control to -1 in
>   main.c
>

For readability I used a second arg clock_fallback instead of 1/0/-1 in
clock_control.

I tried to make the known cases (e.g. lack ot CAP_SYS_TIME) being called
out explicitly.
Also I ensured that the actual issue e.g. adjtimex is reported as it would
have been before (just no more fatally)

A few tests before sending V2 ran fine after some iterations (e.g. we have
to set null-driver=1 to make later cap_set_proc not need the perm).

Other than that I realized I mostly followed you suggestion - thanks BTW!

Following are a few logs how it currently looks like now - sending that as
v2.

container: chronyd -qd
2018-03-12T16:07:14Z chronyd version 3.2 starting (+CMDMON +NTP +REFCLOCK
+RTC +PRIVDROP +SCFILTER +SECHASH +SIGND +ASYNCDNS +IPV6 -DEBUG)
2018-03-12T16:07:14Z adjtimex(0x8001) failed : Operation not permitted
2018-03-12T16:07:14Z Failed to initialize control of local system clock
2018-03-12T16:07:14Z CAP_SYS_TIME not present
2018-03-12T16:07:14Z Fatal error : No Fallback (-X) allowed, init failed

container: chronyd -qd -x
2018-03-12T16:07:30Z chronyd version 3.2 starting (+CMDMON +NTP +REFCLOCK
+RTC +PRIVDROP +SCFILTER +SECHASH +SIGND +ASYNCDNS +IPV6 -DEBUG)
2018-03-12T16:07:30Z Disabled control of system clock
2018-03-12T16:07:30Z Frequency 0.510 +/- 14.980 ppm read from
/var/lib/chrony/chrony.drift
2018-03-12T16:07:41Z System clock wrong by -0.000833 seconds (step)
2018-03-12T16:07:41Z Could not step system clock
2018-03-12T16:07:41Z chronyd exiting

container: chronyd -qd -X
2018-03-12T16:08:01Z chronyd version 3.2 starting (+CMDMON +NTP +REFCLOCK
+RTC +PRIVDROP +SCFILTER +SECHASH +SIGND +ASYNCDNS +IPV6 -DEBUG)
2018-03-12T16:08:01Z adjtimex(0x8001) failed : Operation not permitted
2018-03-12T16:08:01Z Failed to initialize control of local system clock
2018-03-12T16:08:01Z CAP_SYS_TIME not present
2018-03-12T16:08:01Z Falling back by disabling control of the system clock
2018-03-12T16:08:01Z Disabled control of system clock
2018-03-12T16:08:01Z Frequency 0.510 +/- 14.980 ppm read from
/var/lib/chrony/chrony.drift
2018-03-12T16:08:12Z System clock wrong by -0.000503 seconds (step)
2018-03-12T16:08:12Z Could not step system clock
2018-03-12T16:08:12Z chronyd exiting

Container service with -X
systemctl status chrony
● chrony.service - chrony, an NTP client/server
  Loaded: loaded (/lib/systemd/system/chrony.service; enabled; vendor
preset: enabled)
  Active: active (running) since Mon 2018-03-12 16:08:24 UTC; 3s ago
Docs: man:chronyd(8)
  man:chronyc(1)
  man:chrony.conf(5)
 Process: 23184 ExecStartPost=/usr/lib/chrony/chrony-helper update-daemon
(code=exited, status=0/SUCCESS)
 Process: 23180 ExecStart=/usr/sbin/chronyd $DAEMON_OPTS (code=exited,
status=0/SUCCESS)
Main PID: 23182 (chronyd)
   Tasks: 1 (limit: 4915)
  CGroup: /system.slice/chrony.service
  └─23182 /usr/sbin/chronyd -X

Mar 12 16:08:24 b systemd[1]: Starting chrony, an NTP client/server...
Mar 12 16:08:24 b chronyd[23182]: chronyd version 3.2 starting (+CMDMON
+NTP +REFCLOCK +RTC +PRIVDROP +SCFILTER +SECHASH +SIGND +ASYNCDNS +IPV6 -D
Mar 12 16:08:24 b chronyd[23182]: adjtimex(0x8001) failed : Operation not
permitted
Mar 12 16:08:24 b chronyd[23182]: Failed to initialize control of local
system clock
Mar 12 16:08:24 b chronyd[23182]: CAP_SYS_TIME not present
Mar 12 16:08:24 b

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

2018-03-08 Thread Christian Ehrhardt 
On Fri, Mar 9, 2018 at 8:18 AM, Miroslav Lichvar 
wrote:

> On Thu, Mar 08, 2018 at 06:09:00PM +0100, Miroslav Lichvar wrote:
> > On Thu, Mar 08, 2018 at 05:08:16PM +0100, Christian Ehrhardt wrote:
> > > 1. the option would not be default on, so "normal" users/installations
> > > would not be affected
> > > 2. cases that want the fallback behavior, but are unable to
> probe/detect at
> > > the time:
> > >- so they can not decide to use normal -x
> > >- also the environment might change on them withut reconfig
> > >Both of the above would be solved by them dropping the new
> -x=fallback
> > > option for their case.
> >
> > Does that include an assumption that if the clock cannot be
> > controlled, it's already synchronized by something else and if it can,
> > it's a separate time namespace?
>
> From the little information I found about proposed time namespaces, it
> looks like they would just have a fixed offset to the non-namespaced
> time and wouldn't have an independent frequency, so couldn't be
> controlled by chronyd anyway.
>
> I still don't see the use case for the fallback. If applications
> running in the container are ok with chronyd not controlling the
> clock, why not always use -x there? At least it will be less likely to
> hit the case where two things are trying to control the clock.
>

The use case IMHO is for anyone that wants to opt-in to a "just have ntp
serving work at any quality" behavior.
That would fulfil the needs of the reporter of my bug at least - so there
is a valid use case.

Setting -x for them breaks on two things:
1. for them it is much harder to check cap and adjtimex working on the
target
2. a system as-is might be moved, so no static "-x or no -x" config will be
correct
It moves with -x set to a place that could have good time -> wasting
accuracy
it moves without -x set -> might fail after the move
So for them the -x=fallback is just what they'd need

Thanks for the suggestions on the last post, I'm travelling today so no
patch for now.
I hope to submit a v2 to rekindle the discussion about something people can
look at next week.

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

2018-03-08 Thread Miroslav Lichvar 
On Thu, Mar 08, 2018 at 06:09:00PM +0100, Miroslav Lichvar wrote:
> On Thu, Mar 08, 2018 at 05:08:16PM +0100, Christian Ehrhardt wrote:
> > 1. the option would not be default on, so "normal" users/installations
> > would not be affected
> > 2. cases that want the fallback behavior, but are unable to probe/detect at
> > the time:
> >- so they can not decide to use normal -x
> >- also the environment might change on them withut reconfig
> >Both of the above would be solved by them dropping the new -x=fallback
> > option for their case.
> 
> Does that include an assumption that if the clock cannot be
> controlled, it's already synchronized by something else and if it can,
> it's a separate time namespace?

>From the little information I found about proposed time namespaces, it
looks like they would just have a fixed offset to the non-namespaced
time and wouldn't have an independent frequency, so couldn't be
controlled by chronyd anyway.

I still don't see the use case for the fallback. If applications
running in the container are ok with chronyd not controlling the
clock, why not always use -x there? At least it will be less likely to
hit the case where two things are trying to control the clock.

-- 
Miroslav Lichvar

-- 
To unsubscribe email chrony-dev-requ...@chrony.tuxfamily.org with "unsubscribe" 
in the subject.
For help email chrony-dev-requ...@chrony.tuxfamily.org with "help" in the 
subject.
Trouble?  Email listmas...@chrony.tuxfamily.org.

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

2018-03-08 Thread Miroslav Lichvar 
On Thu, Mar 08, 2018 at 05:08:16PM +0100, Christian Ehrhardt wrote:
> 1. the option would not be default on, so "normal" users/installations
> would not be affected
> 2. cases that want the fallback behavior, but are unable to probe/detect at
> the time:
>- so they can not decide to use normal -x
>- also the environment might change on them withut reconfig
>Both of the above would be solved by them dropping the new -x=fallback
> option for their case.

Does that include an assumption that if the clock cannot be
controlled, it's already synchronized by something else and if it can,
it's a separate time namespace?

> Our container folks will outline the CAP_SYS_TIME issue I mentioned before,
> so really the best test for my suggested SYS_IsTimeAdjustable would be (on
> top to what I have to check the Cap) a adjtime no-op.
> I tried via adjtimex cmdline and thought maybe "adjtimex -s 0" (in C from
> chrony eventually) would be a no-op check I'd think

The sys_linux initialization code resets the singleshot offset, which
could be used as an early check for adjtimex().

Ok, here are some suggestions for the implementation:
- change all SYS_*_Initialise() functions to return 1 and SYS_Initialise()
  to check the return code (with a LOG_FATAL message if it is 0)
- change reset_adjtime() and SYS_Linux_Initialise() to return 0 on failure
- change SYS_Initialise() to handle the failure if clock_control is -1
  and add (and document) -X option which sets clock_control to -1 in
  main.c

-- 
Miroslav Lichvar

-- 
To unsubscribe email chrony-dev-requ...@chrony.tuxfamily.org with "unsubscribe" 
in the subject.
For help email chrony-dev-requ...@chrony.tuxfamily.org with "help" in the 
subject.
Trouble?  Email listmas...@chrony.tuxfamily.org.

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

2018-03-08 Thread Bill Unruh 

...

concept of a time namespace in the future. For most non-namespaced
resources applications that are expected to run in user namespaces
(systemd, lxc, etc.) follow the concept of "seek forgiveness, not
permission" meaning one should usually check whether an operation is



That, for a program, sounds really really dangerous. "Opps, sorry I just
destroyed 20 years or work. You do have a backup don't you?"



possible and if it is not move on or fallback to another more
appropriate mode. Now, this is of course an application-by-application
decision. Now, I think it would make sense for chrony to be a "seek
forgiveness, not permission" application since a) it is expected to be
run in user namespaces and b) there'll be a time-namespace. Actually a
good friend of mine from Red Hat is looking into this. :)


I think planning on a kernel  implimentation that has just been thought of is 
also a
pretty dangerous proceedure. I must say I like "If it cannot do what it is
supposed to do, stop and issue an error message" philosophy much better.
Having that linked to an option to force some behaviour so that the decision
is entirely up to the user is better if really needed.

It sounds like this is all an effort to get around bad design for the
container or for some program. The effort should go there, rather than adding
features which could come back to bite you into chrony.
Every new feature introduces bugs. 


Christian





Btw - for the reason above (haivng the cap, but not able to set time) is
there any way to adjtimex, but


Check if adjtimex() works before chronyd is started? The adjtimex(8)
utility could be used for that.


An additional default-off option to then in turn disable syncing the

clock

would be my perfect solution.
Can we disable it so late, as I thought we detect the inability to do so
rather late?
That way people could opt-in to a "instead of the (now better) failure, I
want it to run on less accuracy in pure server mode"

How about that?


I'd rather keep it simple and avoid implementing an automatic fallback
in chrony.

--
Miroslav Lichvar

--
To unsubscribe email chrony-dev-requ...@chrony.tuxfamily.org with
"unsubscribe" in the subject.
For help email chrony-dev-requ...@chrony.tuxfamily.org with "help" in the
subject.
Trouble?  Email listmas...@chrony.tuxfamily.org.





--
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd


--
To unsubscribe email chrony-dev-requ...@chrony.tuxfamily.org with "unsubscribe" 
in the subject.
For help email chrony-dev-requ...@chrony.tuxfamily.org with "help" in the 
subject.
Trouble?  Email listmas...@chrony.tuxfamily.org.



--
To unsubscribe email chrony-dev-requ...@chrony.tuxfamily.org with "unsubscribe" 
in the subject.
For help email chrony-dev-requ...@chrony.tuxfamily.org with "help" in the 
subject.
Trouble?  Email listmas...@chrony.tuxfamily.org.

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

2018-03-08 Thread Miroslav Lichvar 
On Thu, Mar 08, 2018 at 05:15:44PM +0100, Christian Brauner wrote:
> Now, I think it would make sense for chrony to be a "seek
> forgiveness, not permission" application since a) it is expected to be
> run in user namespaces and

I think chronyd is already doing that, except there is no other way to
control the clock, i.e. its primary job, so it gives up.

> b) there'll be a time-namespace. Actually a
> good friend of mine from Red Hat is looking into this. :)

This is very interesting and I'm curious how it will be implemented.
Is there an upstream discussion? Some time ago, when I suggested that
the CLOCK_MONOTONIC should be independent from CLOCK_REALTIME, the
idea was rejected for performance reasons. Now, if I understand it
correctly, each namespace would need to have its own clock running at
a different frequency, which would separate the timers in the
namespaces.

-- 
Miroslav Lichvar

-- 
To unsubscribe email chrony-dev-requ...@chrony.tuxfamily.org with "unsubscribe" 
in the subject.
For help email chrony-dev-requ...@chrony.tuxfamily.org with "help" in the 
subject.
Trouble?  Email listmas...@chrony.tuxfamily.org.

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

2018-03-08 Thread Christian Brauner 
On Thu, Mar 08, 2018 at 03:59:30PM +0100, Christian Ehrhardt wrote:
> On Thu, Mar 8, 2018 at 3:52 PM, Miroslav Lichvar 
> wrote:
> 
> > On Thu, Mar 08, 2018 at 03:15:36PM +0100, Christian Ehrhardt wrote:
> > > On Thu, Mar 8, 2018 at 1:49 PM, Miroslav Lichvar 
> > > > But this is what users expect in most cases, right? If an NTP
> > > > client/server is installed and enabled in a container, it's usually
> > > > not intended, e.g. it was installed as a dependency of another
> > > > package.
> > > >
> > >
> > > The people concerned on the link Launchpad bug for example want to serve
> > > time.
> >
> > If they have no other option how to do that, then it's ok. The number
> > of such users is (hopefully) very small.
> >
> > > Hmm if it wouldn't sound as awkward I'd actually say you'd want to split
> > > the client & server services.
> >
> > The client and server are tightly coupled. Some information need to be
> > exchanged between them for each response of the server.
> >
> > > I agree that you'd want to reach time-sync target only after you sync -
> > btw
> > > what does systemd-timesyncd in that case?
> >
> > With timesyncd the time-sync target has almost no meaning. It's
> > reached as soon as the system time is compared with (and possibly
> > restored from) a timestamp saved in a file from previous boot. It does
> > not wait for any responses from NTP servers and the clock will be
> > stepped after the target is reached. As I understand it, it was done
> > this way to avoid delaying the boot.
> >
> > > You mean as the container is not able to steer the clock it's
> > > own time-sync.target should actually be that of its host?
> >
> > Yes, I think that would be nice if it was possible.
> >
> > > Just  as a heads up, due to the crazy world of capabilities some
> > containers
> > > will soon expose CAP_SYS_TIME since it is correct to have the cap for
> > their
> > > space.
> > > But when you actually adjtimex it will fail as it will eventually apply
> > the
> > > hosts caps to it and that you don't have.
> > > This comes at the additional WTF (for me) that checking for CAP_SYS_TIME
> > > has effectively lost almost all its meaning :-/
> >
> > Interesting. Is this described anywhere?
> >
> 
> For this answer I'm reaching out to a few of said "they can explain better"
> people :-)
> @Stephane / @ Christian - could one of you outline what is going on in the
> container world in better words that I'm able to.
> Mind you - the list requires subscription.

Right, container that use user namespaces **always** start off with a full
capability set including CAP_SYS_TIME unless they are explicitly
dropped. The kernel will enforce the correct permissions automatically so
dropping a given capability is not needed. The problem is that
capabilites are sometimes checked against the initial user namespace and
sometimes against the current user namespace. The reason for this is
that while some parts of the kernel are already namespaced (mount
tables, networks etc.) others are not. Allowing access to non-namespaced
kernel resources from a non-initial user namespace is almost always a
security vulnerability. Since time is non-namespaced resource it cannot
be allowed to change it from a non-initial user namespace since it would
affect the whole system. However, it is expected that there will be a
concept of a time namespace in the future. For most non-namespaced
resources applications that are expected to run in user namespaces
(systemd, lxc, etc.) follow the concept of "seek forgiveness, not
permission" meaning one should usually check whether an operation is
possible and if it is not move on or fallback to another more
appropriate mode. Now, this is of course an application-by-application
decision. Now, I think it would make sense for chrony to be a "seek
forgiveness, not permission" application since a) it is expected to be
run in user namespaces and b) there'll be a time-namespace. Actually a
good friend of mine from Red Hat is looking into this. :)

Christian

> 
> 
> > > Btw - for the reason above (haivng the cap, but not able to set time) is
> > > there any way to adjtimex, but
> >
> > Check if adjtimex() works before chronyd is started? The adjtimex(8)
> > utility could be used for that.
> >
> > > An additional default-off option to then in turn disable syncing the
> > clock
> > > would be my perfect solution.
> > > Can we disable it so late, as I thought we detect the inability to do so
> > > rather late?
> > > That way people could opt-in to a "instead of the (now better) failure, I
> > > want it to run on less accuracy in pure server mode"
> > >
> > > How about that?
> >
> > I'd rather keep it simple and avoid implementing an automatic fallback
> > in chrony.
> >
> > --
> > Miroslav Lichvar
> >
> > --
> > To unsubscribe email chrony-dev-requ...@chrony.tuxfamily.org with
> > "unsubscribe" in the subject.
> > For help email chrony-dev-requ...@chrony.tuxfamily.org with "help"

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

2018-03-08 Thread Christian Ehrhardt 
On Thu, Mar 8, 2018 at 4:34 PM, Miroslav Lichvar 
wrote:

> On Thu, Mar 08, 2018 at 04:20:48PM +0100, Christian Ehrhardt wrote:
> > On Thu, Mar 8, 2018 at 3:52 PM, Miroslav Lichvar 
> > wrote:
> > > I'd rather keep it simple and avoid implementing an automatic fallback
> > > in chrony.
> > >
> >
> > I understand the POV, but without even a default-off function for it I'm
> > kind of forced to wrap a round it to be able to support the use-case in
> > some way at least.
>
> I still don't see a good use case for the option.
>
> Is your intention to use in the default configuration? If not, what
> difference will it make for the user to add -x or some other option
> which enables -x?
>

No, by adding a new option like -x=fallback (or whatever) to chrony we
could have the best of both worlds.

This is based on the following proposed changes:
- add an option to not set clock if not able to adjust it
- drop the condition check on CAP_SYS_TIME from the service file
- If unable to set time, but the new option is not set call out more
clearly why chrony doesn't work in this environment

1. the option would not be default on, so "normal" users/installations
would not be affected
2. cases that want the fallback behavior, but are unable to probe/detect at
the time:
   - so they can not decide to use normal -x
   - also the environment might change on them withut reconfig
   Both of the above would be solved by them dropping the new -x=fallback
option for their case.

Our container folks will outline the CAP_SYS_TIME issue I mentioned before,
so really the best test for my suggested SYS_IsTimeAdjustable would be (on
top to what I have to check the Cap) a adjtime no-op.
I tried via adjtimex cmdline and thought maybe "adjtimex -s 0" (in C from
chrony eventually) would be a no-op check I'd think
If the above is not ok as a probe in your opinion, if you happen to know
how a timex struct should look like to essentially change nothing (as it is
only a probe) but trigger EPERM if forbidden that would be great to know
about.

If yes, will be the users that don't want chronyd to be started in
> containers (but have it installed for some reason they cannot change)
> happy with that?
>

The default install would not enable it, so users would not be affected in
general.


> > In general such a wrapper is inherently always slightly behind as not all
> > downstreams share it, and I'd much more prefer coding something up that
> we
> > share..
>
> A script like that could be included the contrib directory if you
> think others will find it useful.
>
> > If that is a pre-nack to a patch adding a "-Please-fall-back-if-failing"
> > then I don't have to waste the time writing it.
> > So is there a chance at all to add such a switch in your opinion?
>
> Not until I better understand the use case :).


Ok, that stance is totally fair - thanks.
Going on to discuss and at some point sending a new patch then.

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

2018-03-08 Thread Christian Ehrhardt 
On Thu, Mar 8, 2018 at 3:52 PM, Miroslav Lichvar 
wrote:

> On Thu, Mar 08, 2018 at 03:15:36PM +0100, Christian Ehrhardt wrote:
> > On Thu, Mar 8, 2018 at 1:49 PM, Miroslav Lichvar 
> > > But this is what users expect in most cases, right? If an NTP
> > > client/server is installed and enabled in a container, it's usually
> > > not intended, e.g. it was installed as a dependency of another
> > > package.
> > >
> >
> > The people concerned on the link Launchpad bug for example want to serve
> > time.
>
> If they have no other option how to do that, then it's ok. The number
> of such users is (hopefully) very small.
>
> > Hmm if it wouldn't sound as awkward I'd actually say you'd want to split
> > the client & server services.
>
> The client and server are tightly coupled. Some information need to be
> exchanged between them for each response of the server.
>
> > I agree that you'd want to reach time-sync target only after you sync -
> btw
> > what does systemd-timesyncd in that case?
>
> With timesyncd the time-sync target has almost no meaning. It's
> reached as soon as the system time is compared with (and possibly
> restored from) a timestamp saved in a file from previous boot. It does
> not wait for any responses from NTP servers and the clock will be
> stepped after the target is reached. As I understand it, it was done
> this way to avoid delaying the boot.
>
> > You mean as the container is not able to steer the clock it's
> > own time-sync.target should actually be that of its host?
>
> Yes, I think that would be nice if it was possible.
>
> > Just  as a heads up, due to the crazy world of capabilities some
> containers
> > will soon expose CAP_SYS_TIME since it is correct to have the cap for
> their
> > space.
> > But when you actually adjtimex it will fail as it will eventually apply
> the
> > hosts caps to it and that you don't have.
> > This comes at the additional WTF (for me) that checking for CAP_SYS_TIME
> > has effectively lost almost all its meaning :-/
>
> Interesting. Is this described anywhere?
>

For this answer I'm reaching out to a few of said "they can explain better"
people :-)
@Stephane / @ Christian - could one of you outline what is going on in the
container world in better words that I'm able to.
Mind you - the list requires subscription.


> > Btw - for the reason above (haivng the cap, but not able to set time) is
> > there any way to adjtimex, but
>
> Check if adjtimex() works before chronyd is started? The adjtimex(8)
> utility could be used for that.
>
> > An additional default-off option to then in turn disable syncing the
> clock
> > would be my perfect solution.
> > Can we disable it so late, as I thought we detect the inability to do so
> > rather late?
> > That way people could opt-in to a "instead of the (now better) failure, I
> > want it to run on less accuracy in pure server mode"
> >
> > How about that?
>
> I'd rather keep it simple and avoid implementing an automatic fallback
> in chrony.
>
> --
> Miroslav Lichvar
>
> --
> To unsubscribe email chrony-dev-requ...@chrony.tuxfamily.org with
> "unsubscribe" in the subject.
> For help email chrony-dev-requ...@chrony.tuxfamily.org with "help" in the
> subject.
> Trouble?  Email listmas...@chrony.tuxfamily.org.
>
>


-- 
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd

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

2018-03-08 Thread Miroslav Lichvar 
On Thu, Mar 08, 2018 at 03:15:36PM +0100, Christian Ehrhardt wrote:
> On Thu, Mar 8, 2018 at 1:49 PM, Miroslav Lichvar 
> > But this is what users expect in most cases, right? If an NTP
> > client/server is installed and enabled in a container, it's usually
> > not intended, e.g. it was installed as a dependency of another
> > package.
> >
> 
> The people concerned on the link Launchpad bug for example want to serve
> time.

If they have no other option how to do that, then it's ok. The number
of such users is (hopefully) very small.

> Hmm if it wouldn't sound as awkward I'd actually say you'd want to split
> the client & server services.

The client and server are tightly coupled. Some information need to be
exchanged between them for each response of the server.

> I agree that you'd want to reach time-sync target only after you sync - btw
> what does systemd-timesyncd in that case?

With timesyncd the time-sync target has almost no meaning. It's
reached as soon as the system time is compared with (and possibly
restored from) a timestamp saved in a file from previous boot. It does
not wait for any responses from NTP servers and the clock will be
stepped after the target is reached. As I understand it, it was done
this way to avoid delaying the boot.

> You mean as the container is not able to steer the clock it's
> own time-sync.target should actually be that of its host?

Yes, I think that would be nice if it was possible.

> Just  as a heads up, due to the crazy world of capabilities some containers
> will soon expose CAP_SYS_TIME since it is correct to have the cap for their
> space.
> But when you actually adjtimex it will fail as it will eventually apply the
> hosts caps to it and that you don't have.
> This comes at the additional WTF (for me) that checking for CAP_SYS_TIME
> has effectively lost almost all its meaning :-/

Interesting. Is this described anywhere?

> Btw - for the reason above (haivng the cap, but not able to set time) is
> there any way to adjtimex, but

Check if adjtimex() works before chronyd is started? The adjtimex(8)
utility could be used for that.

> An additional default-off option to then in turn disable syncing the clock
> would be my perfect solution.
> Can we disable it so late, as I thought we detect the inability to do so
> rather late?
> That way people could opt-in to a "instead of the (now better) failure, I
> want it to run on less accuracy in pure server mode"
> 
> How about that?

I'd rather keep it simple and avoid implementing an automatic fallback
in chrony.

-- 
Miroslav Lichvar

-- 
To unsubscribe email chrony-dev-requ...@chrony.tuxfamily.org with "unsubscribe" 
in the subject.
For help email chrony-dev-requ...@chrony.tuxfamily.org with "help" in the 
subject.
Trouble?  Email listmas...@chrony.tuxfamily.org.

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

2018-03-08 Thread Christian Ehrhardt 
On Thu, Mar 8, 2018 at 1:49 PM, Miroslav Lichvar 
wrote:

> On Thu, Mar 08, 2018 at 12:21:16PM +0100, Christian Ehrhardt wrote:
> > 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.
>
> The systemctl status command seems to print in bold letters that
> "start condition failed".
>
> But this is what users expect in most cases, right? If an NTP
> client/server is installed and enabled in a container, it's usually
> not intended, e.g. it was installed as a dependency of another
> package.
>

The people concerned on the link Launchpad bug for example want to serve
time.
They would like to have good time, but if for test purpose deployed in a
container it is a requirement to them that it at least serves the time as
good as it can (not perfect, but better than not).
So they install chrony and wonder that nothing works.
Today they have to change the systemd unit (drop the Cap check) and set -x.

There are additional constraints to them that make it appealing to have a
"make it as good as possible but just work" option for containers.

> - If there are services depending on the chrony service they would not
> > start either
>
> Depending on chronyd specifically or the time-sync target? In either
> case it probably means they require a synchronized clock. Is it ok to
> satisfy that with chronyd -x, which doesn't touch the system clock at
> all?
>

Good thought Miroslav,
Hmm if it wouldn't sound as awkward I'd actually say you'd want to split
the client & server services.
I think we can't, but that way the "client" portion would provide
time-sync.target and things depending on it can not start.
But at the same time another service can run for the "server" portion and
work, it might be at reduced accuracy, but it would work.

I agree that you'd want to reach time-sync target only after you sync - btw
what does systemd-timesyncd in that case?
Not sure what that implies, it feels it is wrong whatever way I turn.


> It would be nice if there was some mechanism to pass this information
> to containers.


You mean as the container is not able to steer the clock it's
own time-sync.target should actually be that of its host?
I agree that would be correct for the container world as of today, not sure
how one would implement that thou.


> > 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.
>
> That's a good point. Maybe chronyd should print a warning when it
> detects something else is messing with the clock. Such check would
> probably come at a cost.
>
> > For #1 you want to default to -x if you are in a container
>
> I'd say that applies only to a minority of cases when people actually
> want to run an NTP server and chrony wasn't just installed as a
> dependency.
>
> > 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
>
> I'm not sure about this either. If a container does have CAP_SYS_TIME,
> it was probably intended to run an NTP client/server.
>

Just  as a heads up, due to the crazy world of capabilities some containers
will soon expose CAP_SYS_TIME since it is correct to have the cap for their
space.
But when you actually adjtimex it will fail as it will eventually apply the
hosts caps to it and that you don't have.
This comes at the additional WTF (for me) that checking for CAP_SYS_TIME
has effectively lost almost all its meaning :-/
(other people might be more elaborate to explain if you need details)
Which is another reason to at least provide the "fallback if unable" IMHO -
we can bikeshed at the default of that and decide either way, but I'd still
like the actual feature.

Btw - for the reason above (haivng the cap, but not able to set time) is
there any way to adjtimex, but

> 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
>
> It looks complicated and fragile to me.
>
> Would it make sense to simply remove the CAP_SYS_TIME condition from
> the unit file and let chronyd fail if

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

2018-03-08 Thread Miroslav Lichvar 
On Thu, Mar 08, 2018 at 12:21:16PM +0100, Christian Ehrhardt wrote:
> 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.

The systemctl status command seems to print in bold letters that
"start condition failed". 

But this is what users expect in most cases, right? If an NTP
client/server is installed and enabled in a container, it's usually
not intended, e.g. it was installed as a dependency of another
package.

> - If there are services depending on the chrony service they would not
> start either

Depending on chronyd specifically or the time-sync target? In either
case it probably means they require a synchronized clock. Is it ok to
satisfy that with chronyd -x, which doesn't touch the system clock at
all?

It would be nice if there was some mechanism to pass this information
to containers.

> 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.

That's a good point. Maybe chronyd should print a warning when it
detects something else is messing with the clock. Such check would
probably come at a cost.

> For #1 you want to default to -x if you are in a container

I'd say that applies only to a minority of cases when people actually
want to run an NTP server and chrony wasn't just installed as a
dependency.

> 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

I'm not sure about this either. If a container does have CAP_SYS_TIME,
it was probably intended to run an NTP client/server.

> 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

It looks complicated and fragile to me.

Would it make sense to simply remove the CAP_SYS_TIME condition from
the unit file and let chronyd fail if it doesn't have it (possibly
with a better error message)?

-- 
Miroslav Lichvar

-- 
To unsubscribe email chrony-dev-requ...@chrony.tuxfamily.org with "unsubscribe" 
in the subject.
For help email chrony-dev-requ...@chrony.tuxfamily.org with "help" in the 
subject.
Trouble?  Email listmas...@chrony.tuxfamily.org.

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

2018-03-08 Thread Christian Ehrhardt 
On Thu, Mar 8, 2018 at 10:14 AM, Miroslav Lichvar 
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

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

2018-03-08 Thread Miroslav Lichvar 
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?

> 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.

I'd say running chronyd with -x as a server while the clock is
synchronized by another process is better than two processes
controlling the clock at the same time, but I'd still not recommend
it.

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?

FWIW, I personally use -x only for testing.

-- 
Miroslav Lichvar

-- 
To unsubscribe email chrony-dev-requ...@chrony.tuxfamily.org with "unsubscribe" 
in the subject.
For help email chrony-dev-requ...@chrony.tuxfamily.org with "help" in the 
subject.
Trouble?  Email listmas...@chrony.tuxfamily.org.

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

2018-03-07 Thread Miroslav Lichvar 
On Wed, Mar 07, 2018 at 01:51:31PM +0100, Christian Ehrhardt wrote:
> In unprivileged containers even after e8096330 "sys_linux: don't keep
> CAP_SYS_TIME with -x option" default installations
> will still run without an explicit -x being set and therefore fail by
> missing CAP_SYS_TIME.
> 
> Usually users want services to "just work" which in a non-CAP_SYS_TIME
> environment means that chrony will fulfil just the NTP serving task but
> not the local time control.
> 
> Therefore imply -x in those environments.

I'm not sure about this.

I can see that it would be convenient in some cases to have a service
that automatically falls back to -x, but in the vast majority of cases
I think chronyd is expected to control the clock and if that is not
possible, it should fail.

For example, if I had a docker image with chronyd configured as an NTP
client and I forgot to give it the CAP_SYS_TIME capability, it would
be much likely for me to miss the problem if it fell back to -x.

There are some considerations that should be made before enabling -x.
In order for chronyd to be a good NTP server, the system clock either
needs to be free running (and all applications running on the system
happy with an unsynchronized system clock), or it needs to be
synchronized by another process and chronyd with -x is just serving
the local time (chrony.conf includes the local directive, but no time
sources are specified).

There are other issues with running an NTP client/server in a
container, e.g. network interfaces don't support SW/HW timestamping,
which has an impact on accuracy.

What do others think?

-- 
Miroslav Lichvar

-- 
To unsubscribe email chrony-dev-requ...@chrony.tuxfamily.org with "unsubscribe" 
in the subject.
For help email chrony-dev-requ...@chrony.tuxfamily.org with "help" in the 
subject.
Trouble?  Email listmas...@chrony.tuxfamily.org.