The "chrony" package is currently shipping a file at "/etc/chrony.keys" which is owned by the "chrony" group: ``` $ stat /etc/chrony.keys File: /etc/chrony.keys Size: 540 Blocks: 8 IO Block: 4096 regular file Access: (0640/-rw-r-----) Uid: ( 0/ root) Gid: ( 992/ chrony) ``` However the specfile does create the "chrony" user/group with a dynamic ID at install time: ``` #Type Name ID GECOS Home directory Shell u chrony - "chrony system user" /var/lib/chrony /sbin/nologin ``` This means that across different installs/composes the numeric GID of the file may vary arbitrarily (in the example above it got "992"). This poses a problem from the point of view of OS content reproducibility. It also causes issues to systems doing out-of-band composes (e.g. ostree or other image-based technologies). For these reasons, it would be better to adopt a different arrangement for that file/group. Two options that come to mind are: * ship the example file under /usr and owned by root:root, then use a systemd-tmpfiles fragment to copy into /etc with the relevant group ownership * get a static ID allocated for the "chrony" user/group in Fedora
Given that the file is basically empty and acts as an example file, I think we should either remove it entirely, move it to an example folder or do the tmpfiles logic.
To me it would make sense to remove /etc/chrony.keys from the package. With the NTS support it should be less important than before. However, there are at least two issues that I think need to be considered. One is that if /etc/chrony.conf was modified, upgrading to a newer package which doesn't contain the file will cause the modified file to be renamed to /etc/chrony.conf.rpmsave, which will break the configuration. I think a workaround could be implemented in an rpm scriptlet. The other issue might be scripts which add keys to the file assuming it already exists. If it doesn't exist, they will create a new file with wrong owner and permissions, possibly leading to a security issue. I'm not sure if this is a real concern and if it means we would have to carry the file forever. Thoughts?
/etc/chrony.conf in the previous post was supposed to be /etc/chrony.keys.
I'm definitely not a packaging expert, but leaving aside the "static ID" approach (which is always feasible anyway), the other approach I mentioned would have consisted of: * having the package install the sample-keys-file into "/usr/share/factory/etc/chrony.keys" with "root:root" ownership * tweaking the existing "chrony.keys" entry in the specfile into a %ghost line * having the package install a "/usr/lib/tmpfiles.d/chrony.conf" file with content similar to the following: ``` C /etc/chrony.keys - - - - - z /etc/chrony.keys 0640 root chrony - - ``` Admittedly I didn't try this in practice in an upgrade flow, but the idea would be that the %ghost line would take care of the .rpmsave problem, while the C+z combo in the tmpfiles.d fragment would take care of creating the file on boot (if missing).
We might be able to make a post scriptlet that just moves the .rpmsave file back into place for upgrades.
If I understand it correctly, with the tmpfiles appoach the file would be created only on boot, and not after the package is installed and the service started, which would cause an error due to missing file. Static UID/GIDs seem to be strongly discouraged for packages that don't have files shared over network. There are multiple options, but they all seem to have some issue.
> If I understand it correctly, with the tmpfiles appoach the file would be created only on boot I'd need to double-check the existing RPM macros, but I think it can be run in %pre or %post to the same effect. > There are multiple options, but they all seem to have some issue. I agree with this summary :( Let's leave this BZ open for now, as we aren't in a hurry and we can brainstorm/experiment a bit more. As this is a file in /etc, I think there are a few workaround paths that we can explore directly in ostree.
If the tmpfiles conf was applied immediately in %pre or %post, wouldn't that functionally be equivalent to the package carrying /etc/chrony.key itself? That would be non-reproducible OS content, right?
Thinking about this more, I don't think we have to setup a default file that basically has no config value in it. Administrators can setup the file themselves and make sure it has the right permissions. Thus I don't think we need a tmpfiles.d config at all. So I would suggest removing it entirely from the package (or shipping it root:root as /usr/lib/chrony/chrony.keys or another example dir) and adding a scriptlet that moves the config back into place only if we have an rpmsave file.
One issue with removing the file is that existing scripts may assume the file exists and don't check/set the permissions and ownership when adding a new key. Another issue is that with a modified chrony.conf and an unmodified chrony.keys, there will be an error message from chronyd that it could not open the file. For the first one, I think we could modify chronyd to refuse to load the file if it has world-readable permissions, like sshd does for instance. For the second one we could try to remove the keyfile directive from the config in a scriptlet on upgrade.
The keyfile line in default chrony.conf is commented out and the chrony.keys config is no longer present. It's now marked as %ghost %config to avoid rpm removing the file on package upgrade and uninstallation. chronyd was patched to log a warning message if the file has wrong permissions to catch (at least some of) the cases where it's created incorrectly by a script or admin.
Thanks!