Automatically run ntpd as non-root when possible.

[• ian]

Summary

Ntpd needs only a subset of full root privileges to do its job. Specifically it needs the ability to manipulate system time, and to re-bind to a privileged UDP port after interface changes. The mac_ntpd(4) policy module (see D16281) can grant these privs.

These changes detect the availability of mac_ntpd(4). If enabled, and if the ntpd configuration is fairly vanilla, it automatically runs ntpd as the non-root user 'ntpd' (uid 123). "Vanilla" means the config doesn't include command line or ntp.conf options changing the location of files or usling any files/dirs likely to be inaccessible to user ntpd.

Note that these changes also address PR 199127 by using the command_args technique suggested in the patch. They also tangentially address PR 113552, which is primarily about inconsistent filenames in documentation, but some of the inconsistancy was caused by old code in rc.d/ntpd which is leftover from the intial import from netbsd. There was code to do chroot setup which required the use of the netbsd clockctl(4) device; that code never had any effect on freebsd, because we lack that device and don't build ntpd with the options that would allow using it.

[• ian]

BTW, here's some information that took a while to figure out, so I want to capture it for future reference, somewhere other than a .txt file I'm sure to lose over time.

ntpd file accesses before and after chroot'ing

  ntpd's files...

    pidfile    - open/write/close before chroot
    configfile - open/read/close before chroot
    logfile    - open before chroot, write after, never closed/reopened
    driftfile  - open/read/close before, open/write/close after
    leapfile   - open/read/close before and after
    statsdir   - open/write/close files in this dir after chroot

  /etc files...

    /etc/spwd.db         - open/read/close before chroot
    /etc/nsswitch.conf   - open/read/close before chroot
    /etc/group           - open/read/close before and after
    /etc/services        - open/read/close before and after
    /etc/hosts           - open/read/close before and after
    /etc/resolv.conf     - open/read/close after chroot

etc/rc.d/ntpd

61

Oops, typo here, already fixed locally; it'll be nullfs on the next rev.

[cy]

Thanks for doing this.

etc/rc.d/ntpd

156

This is not related to this review. I have no objection but it needs to be committed in a separate commit.

192

Why the name change?

[• ian]

etc/rc.d/ntpd

156

Ooops, yeah, leftover debugging unrelated to these changes, but I do think I'll commit it on it's own since it's useful.

192

I was getting lost in the maze of similar-named ntp-related rcvars, so I decided to name all the ones that are local to this script (rather than coming from rc.conf) to start with an underbar. I might just commit this alone too, so that it doesn't clutter up the next rev of the main diff.

[bcr]

Thanks indeed for the work. OK from manpages.

[cy]

etc/rc.d/ntpd

156

Thanks.

192

Yes, alone so it doesn't confuse this change with unrelated changes.

Thanks again.

[• ian]

While testing and redeveloping the previous version of the rc.d/ntpd changes, I came to the conclusion that automatically setting up a chroot in an rc.d script is complicated and fraught with potential peril. When I got the point of having a embedded awk program that parsed the ntp.conf file so I could figure out whether we needed to mount a devfs inside the chroot for PPS device access, I realized I was way down a too-complicated path.

So now the change is split into two separate reviews. The new D16281 contains everything related to granting ntpd the limited privs it needs to run as non-root, and this part of the change is just the rc.d changes.

It turns out a chroot isn't really necessary to get ntpd running as non-root. In fact, all that's needed to make it do so is to remove a check in ntpd that makes it exit with an error if you run it as non-root. It's much less complicated to automatically set up to run as non-root in rc.d/ntpd if populating a chroot is no longer part of the problem, so that's the approach taken in this latest revision.

The ntpd precmd function now checks whether the admin used any command line or ntp.conf options that affect files and directories, because if these point to non-standard locations, they may not be accessible if running as non-root. If the setup is fairly vanilla, it automatically runs the daemon as user ntpd, otherwise it runs as it always has.

Even non-vanilla setups can easily run as non-root, but the admin will need to make sure that user ntpd has access to the files and directories. The admin can do so, then set ntpd_user=ntpd in rc.conf to run as non-root. I'll look into putting together an update for the ntp chapter of the handbook to document this (as a separate change/review).

[• ian]

Fix a typo (s/driftopt-/driftopt=/) that made it through my initial testing.

[cy]

Except for a couple of logger -s statements, looks good.

etc/rc.d/ntpd

66

I think a message to syslog to inform the sysadmin that directory permissions have changed should address any POLA concerns. What do you think of daemon.notice? Also to stderr, logger -s.

75

Similarly, inform the sysadmin that files are being moved.

[• ian]

Add requested logging when moving/changing files and dirs.

Add a missing 'then' in a series of if/elif.

Fix a || conditional that should have been && when checking for file keywords in ntp.conf.

[delphij]

etc/rc.d/ntpd

56

If MAC is not present, should it be treated as a fail (e.g. else return 1;;)?

[• ian]

Fix the logic for handling mac(4)-not-in-kernel and mac_ntpd policy not-enabled. The last round of refactoring changed all this stuff to 'early return to run as root' and the logic for these checks got pasted from the old code without change.

[• ian]

etc/rc.d/ntpd

56

Oh, good catch. This got screwed up on the latest refactoring. Fixed now, and this time tested as well. :)

[delphij]

LGTM, thanks!

[bdrewery]

head/etc/rc.d/ntpd

17–24 ↗

(On Diff #45603)

Why is the pidfile not in /var/run? It persisting through reboot in /var/db means there is a chance it won't start if the value in there is picked up by a new process. Plus removing the -p flag from the rc vars (without something like ntpd_pidfile) means users can't fix this mistake.