Re: [Openvpn-devel] [PATCH 1/1] Refuse to daemonize when running from systemd

[Christian H. <li...@ew...>]

David Sommerseth <op...@sf...> on Wed, 2016/11/30 12:52:
> On 30/11/16 09:59, Christian Hesse wrote:
> > From: Christian Hesse <ma...@ew...>
> > 
> > We start with systemd Type=notify, so refuse to daemonize.
> > 
> > Signed-off-by: Christian Hesse <ma...@ew...>
> > ---
> >  distro/systemd/openvpn-client@.service | 1 -
> >  distro/systemd/openvpn-server@.service | 1 -
> >  src/openvpn/init.c                     | 7 +++++++
> >  3 files changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/distro/systemd/openvpn-client@.service
> > b/distro/systemd/openvpn-client@.service index f64a239..5618af3 100644
> > --- a/distro/systemd/openvpn-client@.service
> > +++ b/distro/systemd/openvpn-client@.service
> > @@ -12,7 +12,6 @@ PrivateTmp=true
> >  RuntimeDirectory=openvpn-client
> >  RuntimeDirectoryMode=0710
> >  WorkingDirectory=/etc/openvpn/client
> > -ExecStartPre=/bin/sh -c 'grep -q -E ^daemon %i.conf || exit 0
> > && /usr/bin/echo "OpenVPN configuration cannot contain --daemon when
> > being managed by systemd" ; exit 1' ExecStart=/usr/sbin/openvpn
> > --suppress-timestamps --nobind --config %i.conf
> > CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_RAW CAP_SETGID
> > CAP_SETUID CAP_SYS_CHROOT CAP_DAC_OVERRIDE LimitNPROC=10 diff --git
> > a/distro/systemd/openvpn-server@.service
> > b/distro/systemd/openvpn-server@.service index 890e6a9..b9b4dba 100644
> > --- a/distro/systemd/openvpn-server@.service +++
> > b/distro/systemd/openvpn-server@.service @@ -12,7 +12,6 @@
> > PrivateTmp=true RuntimeDirectory=openvpn-server RuntimeDirectoryMode=0710
> >  WorkingDirectory=/etc/openvpn/server
> > -ExecStartPre=/bin/sh -c 'grep -q -E ^daemon %i.conf || exit 0
> > && /usr/bin/echo "OpenVPN configuration cannot contain --daemon when
> > being managed by systemd" ; exit 1' ExecStart=/usr/sbin/openvpn --status
> > %t/openvpn-server/status-%i.log --status-version 2 --suppress-timestamps
> > --config %i.conf CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN
> > CAP_NET_BIND_SERVICE CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SYS_CHROOT
> > CAP_DAC_OVERRIDE LimitNPROC=10 diff --git a/src/openvpn/init.c
> > b/src/openvpn/init.c index 551e579..7ab5c52 100644 ---
> > a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -926,6 +926,13 @@ bool
> >  possibly_become_daemon (const struct options *options)
> >  {
> >    bool ret = false;
> > +
> > +#ifdef ENABLE_SYSTEMD
> > +  /* return without forking if we are running from systemd */
> > +  if (sd_notify(0, "READY=0") > 0)
> > +    return ret;
> > +#endif
> > +
> >    if (options->daemon)
> >      {
> >        ASSERT (!options->inetd);
> >   
> 
> NAK on this approach.  We cannot dictate that users _must_ start OpenVPN
> as a daemon via systemd if it has been built with systemd support.
> 
> I understand the sentiment for this change, but we need to ensure users
> may use their own scripts and hand-crafted configs to start OpenVPN,
> also if systemd is present.

We do not dictate anything. Starting openvpn from scripts or command line
this is a no-op. From man sd_notify(3):

> On failure, these calls return a negative errno-style error code. If
> $NOTIFY_SOCKET was not set and hence no status data could be sent, 0 is
> returned. If the status was sent, these functions return with a positive
> return value. In order to support both, init systems that implement this
> scheme and those which do not, it is generally recommended to ignore the
> return value of this call.

That is what we check for here: If (and only if) openvpn is started from
systemd (read: from a systemd unit/service) it denies to fork. The openvpn
service starts properly even with "daemon" in config file.

This is unrelated to systemd being installed or the system being booted with
systemd.
-- 
main(a){char*c=/*    Schoene Gruesse                         */"B?IJj;MEH"
"CX:;",b;for(a/*    Best regards             my address:    */=0;b=c[a++];)
putchar(b-1/(/*    Chris            cc -ox -xc - && ./x    */b/42*2-3)*42);}