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

[David S. <op...@sf...>]

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.


-- 
kind regards,

David Sommerseth
OpenVPN Technologies, Inc