Page MenuHomeFreeBSD

Automatically run ntpd as non-root when possible.
ClosedPublic

Authored by ian on Jun 28 2018, 6:18 PM.

Details

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.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 17697

Event Timeline

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 requested changes to this revision.Jun 28 2018, 7:46 PM

Thanks for doing this.

etc/rc.d/ntpd
185

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

221

Why the name change?

This revision now requires changes to proceed.Jun 28 2018, 7:46 PM
etc/rc.d/ntpd
185

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

221

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 added a subscriber: bcr.

Thanks indeed for the work. OK from manpages.

etc/rc.d/ntpd
185

Thanks.

221

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

Thanks again.

ian retitled this revision from Automatically run ntpd in a chroot with reduced privileges, when possible. to Automatically run ntpd as non-root when possible..
ian edited the summary of this revision. (Show Details)

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 marked 6 inline comments as done.Jul 15 2018, 9:18 PM
ian marked an inline comment as done.

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

cy requested changes to this revision.Jul 16 2018, 5:08 AM

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

etc/rc.d/ntpd
105

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.

110

Similarly, inform the sysadmin that files are being moved.

This revision now requires changes to proceed.Jul 16 2018, 5:08 AM

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.

ian marked 2 inline comments as done.Jul 16 2018, 5:58 PM
This revision is now accepted and ready to land.Jul 17 2018, 4:06 AM
delphij added inline comments.
etc/rc.d/ntpd
122

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

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.

This revision now requires review to proceed.Jul 18 2018, 2:29 PM
ian marked an inline comment as done.Jul 18 2018, 2:31 PM
ian added inline comments.
etc/rc.d/ntpd
122

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

This revision is now accepted and ready to land.Jul 19 2018, 7:10 AM
This revision was automatically updated to reflect the committed changes.
ian marked an inline comment as done.
bdrewery added inline comments.
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.