systemd: run openvpn with dedicated user

From: Christian Hesse <[email protected]...>

Now that we have a native netlink interface run the process with dedicated user 'openvpn'. This is possible by granting ambient capabilities, see systemd.exec(5).

---
 .gitignore                                |  1 +
 configure.ac                              |  9 +++++++++
 distro/systemd/Makefile.am                | 24 ++++++++++++++++++++++-
 distro/systemd/[email protected]... |  4 +++-
 distro/systemd/[email protected]... |  4 +++-
 distro/systemd/sysusers-openvpn.conf      |  1 +
 distro/systemd/tmpfiles-openvpn.conf      |  2 --
 distro/systemd/tmpfiles-openvpn.conf.in   |  4 ++++
 src/openvpn/init.c                        |  8 ++++++++
 9 files changed, 52 insertions(+), 5 deletions(-)
 create mode 100644 distro/systemd/sysusers-openvpn.conf
 delete mode 100644 distro/systemd/tmpfiles-openvpn.conf
 create mode 100644 distro/systemd/tmpfiles-openvpn.conf.in

index 25009d81..00abdd5a 100644
@@ -55,6 +55,7 @@ doc/openvpn.8.html
 /doc/doxygen/openvpn.doxyfile
 distro/rpm/openvpn.spec
 distro/systemd/*.service
+distro/systemd/tmpfiles-openvpn.conf
 sample/sample-keys/sample-ca/
 vendor/.build
 vendor/dist

index 251cb9a2..ef8f5864 100644
@@ -367,6 +367,7 @@ AC_ARG_VAR([GIT], [path to git utility])
 AC_ARG_VAR([SYSTEMD_ASK_PASSWORD], [path to systemd-ask-password utility])
 AC_ARG_VAR([SYSTEMD_UNIT_DIR], [Path of systemd unit directory @<:@default=LIBDIR/systemd/system@:>@])
 AC_ARG_VAR([TMPFILES_DIR], [Path of tmpfiles directory @<:@default=LIBDIR/tmpfiles.d@:>@])
+AC_ARG_VAR([SYSUSERS_DIR], [Path of sysusers directory @<:@default=LIBDIR/sysusers.d@:>@])
 AC_PATH_PROGS([IFCONFIG], [ifconfig],, [$PATH:/usr/local/sbin:/usr/sbin:/sbin])
 AC_PATH_PROGS([ROUTE], [route],, [$PATH:/usr/local/sbin:/usr/sbin:/sbin])
 AC_PATH_PROGS([IPROUTE], [ip],, [$PATH:/usr/local/sbin:/usr/sbin:/sbin])

@@ -1200,6 +1201,12 @@ if test "$enable_systemd" = "yes" ; then
     else
         tmpfilesdir="\${libdir}/tmpfiles.d"
     fi
+
+    if test -n "${SYSUSERS_DIR}"; then
+        sysusersdir="${SYSUSERS_DIR}"
+    else
+        sysusersdir="\${libdir}/sysusers.d"
+    fi
 fi
 
 

@@ -1375,6 +1382,7 @@ AM_CONDITIONAL([GIT_CHECKOUT], [test "${GIT_CHECKOUT}" = "yes"])
 AM_CONDITIONAL([ENABLE_PLUGIN_AUTH_PAM], [test "${enable_plugin_auth_pam}" = "yes"])
 AM_CONDITIONAL([ENABLE_PLUGIN_DOWN_ROOT], [test "${enable_plugin_down_root}" = "yes"])
 AM_CONDITIONAL([HAVE_LD_WRAP_SUPPORT], [test "${have_ld_wrap_support}" = "yes"])
+AM_CONDITIONAL([ENABLE_IPROUTE], [test "${enable_iproute2}" = "yes"])
 
 sampledir="\$(docdir)/sample"
 AC_SUBST([plugindir])

@@ -1382,6 +1390,7 @@ AC_SUBST([sampledir])
 
 AC_SUBST([systemdunitdir])
 AC_SUBST([tmpfilesdir])
+AC_SUBST([sysusersdir])
 
 VENDOR_SRC_ROOT="\$(abs_top_srcdir)/vendor/"
 VENDOR_DIST_ROOT="\$(abs_top_builddir)/vendor/dist"

index 69e12699..1b7ce5f9 100644
@@ -10,14 +10,35 @@
 
 %.service: %.service.in Makefile
 	$(AM_V_GEN)sed -e 's|\@sbindir\@|$(sbindir)|' \
+		-e 's|\@SYSTEMD_USER\@|$(SYSTEMD_USER)|' \
+		-e 's|\@SYSTEMD_CAPS_OPTION\@|$(SYSTEMD_CAPS_OPTION)|' \
+		-e 's|\@SYSTEMD_CAPS_VALUES\@|$(SYSTEMD_CAPS_VALUES)|' \
+		$< > [email protected] && mv [email protected] $@
+
+%.conf: %.conf.in Makefile
+	$(AM_V_GEN)sed -e 's|\@SYSTEMD_USER\@|$(SYSTEMD_USER)|g' \
 		$< > [email protected] && mv [email protected] $@
 
 EXTRA_DIST = \
-	tmpfiles-openvpn.conf \
+	sysusers-openvpn.conf \
+	tmpfiles-openvpn.conf.in \
 	[email protected]... \
 	[email protected]...
 
 if ENABLE_SYSTEMD
+if ENABLE_IPROUTE
+SYSTEMD_USER=root
+SYSTEMD_CAPS_OPTION=CapabilityBoundingSet
+SYSTEMD_CAPS_VALUES=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_BIND_SERVICE CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SYS_CHROOT CAP_DAC_OVERRIDE
+else
+SYSTEMD_USER=openvpn
+SYSTEMD_CAPS_OPTION=AmbientCapabilities
+SYSTEMD_CAPS_VALUES=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_BIND_SERVICE CAP_NET_RAW CAP_SYS_CHROOT CAP_DAC_OVERRIDE
+
+sysusers_DATA = \
+	sysusers-openvpn.conf
+endif
+
 systemdunit_DATA = \
 	[email protected]... \
 	[email protected]...

@@ -28,6 +49,7 @@ dist_doc_DATA = \
 
 install-data-hook:
 	mv $(DESTDIR)$(tmpfilesdir)/tmpfiles-openvpn.conf $(DESTDIR)$(tmpfilesdir)/openvpn.conf
+	mv $(DESTDIR)$(sysusersdir)/sysusers-openvpn.conf $(DESTDIR)$(sysusersdir)/openvpn.conf || true
 endif
 
 MAINTAINERCLEANFILES = \

index cbcef653..96cbf68e 100644
@@ -9,9 +9,11 @@ Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO
 [Service]
 Type=notify
 PrivateTmp=true
+User=@SYSTEMD_USER@
+Group=@SYSTEMD_USER@
 WorkingDirectory=/etc/openvpn/client
 ExecStart=@sbindir@/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
+@SYSTEMD_CAPS_OPTION@=@SYSTEMD_CAPS_VALUES@
 LimitNPROC=10
 DeviceAllow=/dev/null rw
 DeviceAllow=/dev/net/tun rw

index a8366a04..3f00642e 100644
@@ -9,9 +9,11 @@ Documentation=https://community.openvpn.net/openvpn/wiki/HOWTO
 [Service]
 Type=notify
 PrivateTmp=true
+User=@SYSTEMD_USER@
+Group=@SYSTEMD_USER@
 WorkingDirectory=/etc/openvpn/server
 ExecStart=@sbindir@/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
+@SYSTEMD_CAPS_OPTION@=@SYSTEMD_CAPS_VALUES@
 LimitNPROC=10
 DeviceAllow=/dev/null rw
 DeviceAllow=/dev/net/tun rw

new file mode 100644
index 00000000..d200852b
@@ -0,0 +1 @@
+u openvpn - "OpenVPN user" /

deleted file mode 100644
index bb79671e..00000000
@@ -1,2 +0,0 @@
-d /run/openvpn-client 0710 root root -
-d /run/openvpn-server 0710 root root -

new file mode 100644
index 00000000..f58d2967
@@ -0,0 +1,4 @@
+d /run/openvpn-client 0750 @SYSTEMD_USER@ @SYSTEMD_USER@ -
+d /run/openvpn-server 0750 @SYSTEMD_USER@ @SYSTEMD_USER@ -
+d /etc/openvpn/client 0750 @SYSTEMD_USER@ @SYSTEMD_USER@ -
+d /etc/openvpn/server 0750 @SYSTEMD_USER@ @SYSTEMD_USER@ -

index 36c1a4c4..0fc60d62 100644
@@ -1151,6 +1151,14 @@ do_uid_gid_chroot(struct context *c, bool no_delay)
         /* set user and/or group if we want to setuid/setgid */
         if (c0->uid_gid_specified)
         {
+#ifdef ENABLE_SYSTEMD
+            if (sd_notify(0, "READY=0") > 0 && getuid() != 0)
+            {
+                msg(M_INFO, "NOTE: Running from systemd with non-root uid, skipping downgrade");
+                return;
+            }
+#endif
+
             if (no_delay)
             {
                 platform_group_set(&c0->platform_state_group);

Hello,

I haven't followed the netlink conversion in detail, so please tell me if the following was already discussed and I've just missed it.

+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9

Simon Ruderich <[email protected]...> on Tue, 2018/04/24 10:38:

No, it has not been discussed and needs a review.

The above snippet holds code for both, netlink and iproute2 versions.

The iproute2 version (that is what is used currently) uses systemd option "CapabilityBoundingSet" to limit the capabilities to the given set. If configured openvpn will drop privileges after setup.

With netlink and my patch on top we go the other way: The process runs (and is started) with user "openvpn". To grant required privileges we use systemd option "AmbientCapabilities" and give capabilities to the process. The process keeps these capabilities, but that's a benefit: The process survives a reconnect that requires configuration changes and shuts down cleanly (takes down routes and addresses).

I do not agree that the process is running with root privileges. It has some extra capabilities, but it can not kill processes, fork away and change cgroups, etc.
IMHO that is what we want to achieve.

For this patch I took the current set of capabilities and stripped CAP_SETGID and CAP_SETUID for the netlink version. Whether or not the other capabilities are required should be discussed independently. Wondering why we have CAP_DAC_OVERRIDE in our capability capability set... That looks suspicious indeed.

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);}

Hello Christian,

Thanks for the confirmation, that's what I assumed.

I disagree. A process with CAP_DAC_OVERRIDE can read/write _every_ file on the system (man 7 capabilities)! This equals root privileges. Even CAP_NET_ADMIN is very powerful as it allows modifying the firewall which might give access to sensitive services which are normally not exposed.

Even with CAP_DAC_OVERRIDE stripped this change keeps openvpn running with (much) more privileges than before. Is this desirable?

Regards
Simon

+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9

Hi,

I think it depends on your perspective.

What Christian says is that with this patch:

1) you can start openvpn as non-root directly (impossible right now)

2) you have full support for tunnel reconfiguration even when running as non-root (people willing to achieve this now must start and keep openvpn running as root)

I consider both points above steps forward towards a better security model for OpenVPN.

OTOH I understand that there are people that don't care about having a working tunnel reconfiguration and are fine with starting openvpn as root (and then dropping privileges).

For these people, adding the above capabilities results in giving the openvpn process more power than before.

Maybe users willing to adopt this stricter behaviour should have a knob somewhere that will enable the usual run-as-root-and-then-drop-priv-with-no-caps?

Generally speaking I believe that openvpn, as a VPN and partly routing daemon, should be allowed to run with CAP_NET_ADMIN set as it enables more features (tunnel reconfiguration to start with).

Maybe clients should run with the caps by default while servers should be launched with the original behaviour? Not sure, honestly.

Cheers,

Antonio Quartulli

Antonio Quartulli <[email protected]...> on Tue, 2018/04/24 23:08:

NAK. :-p

I think the solution for this dilemma is pretty easy: I should strip the part from my patch that disables user switching when started from systemd. We can start as user "openvpn" any way - as long as the process has capabilities CAP_SETGID and CAP_SETUID it still can switch user context and drop privileges.

So users have two options: keep the process running as user "openvpn" with capabilities for more flexibility or switch to unprivileged user for more security.

No need to have root involved. Sounds good?

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);}

Hi,

This is not our traditional approach of "give people rope to hang themselves if they want so". So I'll NAK any patch that *requires* use of systemd, capabilities and non-root users on Linux.

As an option, welcome.

(I'm not finding a really good reason right now why I would need to run openvpn as root which couldn't be done with CAP_NET_ADMIN, so the recommendation should certainly be "use non-root" - but then, we're not enforcing --setuid today either)

gert

"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
                             Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany                             [email protected]...

Hi,

I'm not yet up to speed with capabilities in recent versions of linux, but some of the proposed ideas look unsafe to me. In particular:

Be careful setting CAP_SETUID on the file without code changes. Once that is set, starting as root and dropping privileges is not the same as starting as non-root and dropping privileges. In the former case all capabilities will get cleared (which is good) but not so in the latter case. The process can elevate back to root in the second case which is bad. We'll need to add some code to clear all caps before the setuid() call to do this safely.

Also, if the server is using pam for user-auth, the forked copy needs to run as root which is currently handled by forking before the setuid call. Need to consider how that will work with capabilities without handing out cap_setuid to all.

As Gert said, looks like we are jumping the gun here. First thoroughly test and evaluate the nuances of the use of capabilities in context before deciding on how to make use of it.

Selva

Hi,

If we go there, we might consider dropping all non-default capabilities before running external programs (--up, --client-connect, ...)

This patch is a bit "gallopping ahead", though - it needs a somewhat more solid agreement on our privilege model before modifying systemd unit files...

gert

"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
                             Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany                             [email protected]...

Hi,

"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
                             Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany                             [email protected]...