On Fri, Jun 22, 2018 at 09:39:01PM +0200, Ludovic Courtès wrote:
> Hello Efraim,
> 
> Efraim Flashner <efraim@flashner.co.il> skribis:
> 
> > I tested this patch with the included vm image, using the following
> > script. After logging in, 'ntpctl -s all' shows openntpd connecting to
> > the ntp servers and updating the time.
> >
> > /.$(./pre-inst-env guix environment guix -- ./pre-inst-env guix system vm ~/vm-image.scm) -m 768 -device e1000,netdev=net0 -netdev user,id=net0,hostfwd=tcp::5555-:53
> 
> [...]
> 
> > From 064903c5a976280b95cd9bba17e958e662be605d Mon Sep 17 00:00:00 2001
> > From: Efraim Flashner <efraim@flashner.co.il>
> > Date: Tue, 19 Jun 2018 12:24:47 +0300
> > Subject: [PATCH] services: openntpd: Containerize openntpd service.
> >
> > * gnu/packages/ntp.scm (openntpd)[arguments]: Add 'privsep-path' to
> > 'configure-flags and adjust the 'localstatedir' flag.
> > * gnu/services/networking.scm (openntpd-shepherd-service): Change the
> > start-service command to run in a container, expose '/var/log/openntpd'
> > and '/var/lib/openntpd' to the container.
> > (openntpd-service-activation): Adjust directories for the changes above.
> 
> Neat!  The patch LGTM, especially since you’ve confirmed that it still
> works as expected.  :-)
> 
> One thing though: could you make sure containerization isn’t redundant
> with what OpenNTPD already does?  Namely, could you grep the source for
> calls to “chroot”, “unshare”, or “seccomp”?  If it happens to be already
> doing one of these things, it may be that using a container brings
> little or nothing.
> 
> If it’s OK, please push!

From grepping the source:

./INSTALL-OpenNTPD always uses Privilege Separation (ie the majority of the
./INSTALL:processing is done as a chroot'ed, unprivileged user).

The code also supports the assertion.

it defaults to /var/empty, unless the --with-privsep-path=path flag is
set, so it looks like my patch is unnecessary after all. :)

> 
> While I’m at it, one question about this comment (which was already there):
> 
> > +                           ;; When ntpd is daemonized it repeatedly tries to respawn
> > +                           ;; while running, leading shepherd to disable it.  To
> > +                           ;; prevent spamming stderr, redirect output to logfile.
> > +                           #:log-file "/var/log/ntpd"))
> 
> What’s described here is expected: when it daemonizes, the initial
> process that shepherd spawned terminates immediately, which is why
> shepherd tries to respawn it (it cannot guess that there’s in fact a
> child process that keeps running.)
> 
> The right thing to do for things that daemonize is to use the #:pid-file
> option, which instructs shepherd to poll that file.  Should we do this
> here?  There are many examples of that, including bitlbee, which is
> containerized.
> 

I'll take a look at that and see if I can fix that.

> Thanks,
> Ludo’.

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted