Bug 2168863 - chrony should have "sourcedir /etc/chrony/sources.d" in chrony.conf
Summary: chrony should have "sourcedir /etc/chrony/sources.d" in chrony.conf
Keywords:
Status: NEW
Alias: None
Product: Fedora
Classification: Fedora
Component: chrony
Version: 39
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Miroslav Lichvar
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2023-02-10 10:11 UTC by Marius Vollmer
Modified: 2023-08-16 07:06 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed:
Type: Bug
Embargoed:


Attachments (Terms of Use)

Description Marius Vollmer 2023-02-10 10:11:35 UTC
Debian/Ubuntu have the convenient "sourcedir /etc/chrony/sources.d" directory in their default chrony.conf.  I think Fedora and RHEL should follow suit.

Cockpit would drop a file there, see bug 1330465.

Comment 1 Miroslav Lichvar 2023-02-13 08:16:16 UTC
This is similar to bug #2080056.

Splitting the configuration would cause some tools to not be aware of the separate sources.

Would cockpit display correctly sources in the main chrony.conf and other files in some sourcedirs? Sometimes admins want to add new sources, sometimes they want to use a completely new set of sources.

Comment 2 Marius Vollmer 2023-02-13 09:27:08 UTC
> Splitting the configuration would cause some tools to not be aware of the separate sources.

Given that the sourcedir directive exists, those tools need to support it, no?  Augeas in particular is also in Debian, which seems to have "sourcedir /etc/chrony/sources.d" by default.  Maybe we can find a patch there to support sourcedir.

> Would cockpit display correctly sources in the main chrony.conf and other files in some sourcedirs?

No, Cockpit would not try to show all sources.  It would only allow adding additional ones via its own drop-in file in /etc/chrony/sources.d.  This is a compromise to at least get some amount of configurability.

Comment 3 Miroslav Lichvar 2023-02-13 09:52:33 UTC
Hm, I suspect that might lead to unexpected configurations using invisible sources. Only being able to add new sources doesn't seem very useful to me.

I think from the user's point of view it's better to modify the chrony.conf file and show all server/pool/peer sources with their options (as a single string, no need to parse them individually).

The Fedora/RHEL default config has "sourcedir /run/chrony-dhcp". It would be great if the user could see that and disable it.

Comment 4 Marius Vollmer 2023-02-13 10:10:42 UTC
(In reply to Miroslav Lichvar from comment #3)
 
> I think from the user's point of view it's better to modify the chrony.conf
> file and show all server/pool/peer sources with their options (as a single
> string, no need to parse them individually).
> 
> The Fedora/RHEL default config has "sourcedir /run/chrony-dhcp". It would be
> great if the user could see that and disable it.

This is curiously similar to what we first did in https://github.com/cockpit-project/cockpit/pull/17992, before deciding on a simpler approach.  (You might want to watch the demo: https://www.youtube.com/watch?v=7DTgSjuw1Uo)

Martin, what are your thoughts?

Comment 5 Martin Pitt 2023-02-13 10:31:01 UTC
(In reply to Miroslav Lichvar from comment #3)
> I think from the user's point of view it's better to modify the chrony.conf
> file and show all server/pool/peer sources with their options (as a single
> string, no need to parse them individually).

With my Debian packager and also server admin hats on, this is always strongly discouraged. chrony.conf is nontrivial, and the minute you edit such a config file, you make it un-update-able by the distro package. That's why conf.d/ directories are so widespread and convenient, as in most cases they avoid conflicts between evolving default config files and local additions.

> The Fedora/RHEL default config has "sourcedir /run/chrony-dhcp". It would be
> great if the user could see that and disable it.

My personal opinion is a very strong "no" to that. This is distro integration, nothing that users should fiddle with (in particular not Cockpit).

In fact, if admins have the choice, sending custom NTP servers via DHCP is the right solution IMO. Configuring all client machines that way is just a hack.

Comment 6 Miroslav Lichvar 2023-02-13 11:16:24 UTC
IMHO it's not worth implementing if it cannot display and remove the default servers. If all sources were specified in a single directive (like with timesyncd), the conf.d approach would make more sense to me.

I like how it's done in anaconda.

> https://www.youtube.com/watch?v=7DTgSjuw1Uo

This looks good to me, except it doesn't show the source type (pool vs server) and the options.

> In fact, if admins have the choice, sending custom NTP servers via DHCP is the right solution IMO. Configuring all client machines that way is just a hack.

If you care about security and want to authenticate NTP with NTS for instance, you cannot use DHCP for configuring the NTP sources. Even if you add nts to NTPSERVERARGS in /etc/sysconfig/network, MITM attacks on NTP via DHCP will still be possible.

Comment 7 Martin Pitt 2023-02-13 13:34:46 UTC
> I like how it's done in anaconda.

Anaconda has the luxury to assume an unmodified system, it doesn't have to parse and understand "any possible complex and customized configuration", but that's what cockpit would have to do. chrony's config files are too complex and unstructured, and unstandardized across distros to be able to do this -- Marius had a huge PR, and it was full of contradictions and corner cases.

However, this bugzilla isn't primarily for cockpit's sake, but for administrators. Having a standardized .d/ directory where additional time servers can be dropped in is hugely beneficial for e.g. Ansible, container volumes, kubernetes secrets etc. as well -- having to modify the primary config file is brittle with Ansible, and completely breaks the latter two use cases.

> MITM attacks on NTP via DHCP will still be possible.

Yes, of course -- if you have a malicious router and DHCP server in your network, that can also change default routes, do IP address rewriting, lots of other fun things. But this isn't the case in most environments, and guarding against that kind of inside attacker requires full end-to-end auth anyway (i.e. certificates and ntp-over-TLS or something such) -- I don't see how it matters whether you get NTP servers via a local config file or DHCP then. I can probably be convinced about disabling NTP-via-DHCP in the UI if there is a good use case, but this is a side discussion. I'd like to focus this bz about making chrony's config more standardized.

Thanks!

Comment 8 Miroslav Lichvar 2023-02-13 14:00:42 UTC
(In reply to Martin Pitt from comment #7)
> > I like how it's done in anaconda.
> 
> Anaconda has the luxury to assume an unmodified system, it doesn't have to
> parse and understand "any possible complex and customized configuration",
> but that's what cockpit would have to do. chrony's config files are too
> complex and unstructured, and unstandardized across distros to be able to do
> this -- Marius had a huge PR, and it was full of contradictions and corner
> cases.

I don't agree here. I think the chrony configuration syntax is very simple. There are no sections, no context. It's just lines with directives. You can safely ignore everything except the few directives specifying sources. You can move them at the end of the file an it will work exactly as before.

Adding new configuration files to a specific location is not very useful with chrony. You need to be able to disable the default servers. Switching to a fragmented configuration would not help with that. You would still need to be able to identify and override files specifying sources.

> However, this bugzilla isn't primarily for cockpit's sake, but for
> administrators. Having a standardized .d/ directory where additional time
> servers can be dropped in is hugely beneficial for e.g. Ansible, container
> volumes, kubernetes secrets etc. as well -- having to modify the primary
> config file is brittle with Ansible, and completely breaks the latter two
> use cases.

All ansible roles for configuring chrony I have seen recreate the config from scratch.

> I don't see
> how it matters whether you get NTP servers via a local config file or DHCP
> then.

With DHCP the attacker can point the client to their own server which has a valid certificate and serve them bogus time. The local config can be trusted. DHCP cannot.

Comment 9 Martin Pitt 2023-02-13 16:09:45 UTC
> I think the chrony configuration syntax is very simple.

Well, cockpit would have to parse all sourcedir, include files, and reason about how to present and edit them. Apparently it was also not trivial to parse the "server" entries with its options, etc.

> With DHCP the attacker can point the client to their own server which has a valid certificate and serve them bogus time. The local config can be trusted. DHCP cannot.

Err.. no. But let's not expand on that, as it's unrelated to this bz.

The main point to discuss here is: Are you open to adding a standard sources.d/ like Debian has?

Thanks!

Martin

Comment 10 Miroslav Lichvar 2023-02-14 07:50:54 UTC
(In reply to Martin Pitt from comment #9)
> The main point to discuss here is: Are you open to adding a standard
> sources.d/ like Debian has?

If there is a good reason, sure. So far, I'm not convinced.

Comment 11 Martin Pitt 2023-02-16 10:54:10 UTC
> All ansible roles for configuring chrony I have seen recreate the config from scratch.

Right, but right now there's no other option really. This is much easier on Debian where you can rely on sources.d/. Replacing the config entirely destroys all distro integration, and should better be avoided. I.e. that's the point why we are asking for this, so that things like Ansible or Cockpit *don't* have to much around with the main config file any more.

Comment 12 Miroslav Lichvar 2023-02-16 12:40:40 UTC
If you wanted to modify chrony.conf to use a new set of sources with ansible, you would simply remove all source lines and add new sources to end of the file, e.g. using the lineinfile and blockinfile modules.

If you wanted to keep reused sources in their original place in the config, you might need to write your own module, but it would still be fairly simple. The old chrony helper script did this in about 20 lines of shell code:

https://src.fedoraproject.org/rpms/chrony/blob/304dad1ba3e72da305a7b746c29472565798fa15/f/chrony.helper#_173

If you move the sources to separate files, tools and scripts like this will no longer work as expected.

If the configuration tool managed only its own set of sources in a separate file ignoring the rest, that would be pointless. Relying on the distribution default to contain a specific sourcedir, that looks like a distro integration to me anyway.

Comment 13 Fedora Release Engineering 2023-08-16 07:06:43 UTC
This bug appears to have been reported against 'rawhide' during the Fedora Linux 39 development cycle.
Changing version to 39.


Note You need to log in before you can comment on or make changes to this bug.