[Openvpn-devel] [PATCH] Retain CAP_NET_ADMIN when dropping privileges
---
configure.ac | 18 ++
distro/systemd/openvpn-cli...@.service.in | 2 +-
distro/systemd/openvpn-ser...@.service.in | 2 +-
src/openvpn/init.c| 25 ++-
src/openvpn/platform.c| 79 +++
src/openvpn/platform.h| 5 ++
6 files changed, 127 insertions(+), 4 deletions(-)
diff --git a/configure.ac b/configure.ac
index 7199483a..5832b62f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -794,6 +794,24 @@ dnl
esac
fi
+dnl
+dnl Depend on libcap-ng on Linux
+dnl
+case "$host" in
+ *-*-linux*)
+ PKG_CHECK_MODULES([LIBCAPNG],
+ [libcap-ng],
+ [have_libcapng="yes"],
+ [AC_MSG_ERROR([libcap-ng package not found.
Is the development package and pkg-config installed?])]
+ )
+
+ CFLAGS="${CFLAGS} ${LIBCAPNG_CFALGS}"
+ LIBS="${LIBS} ${LIBCAPNG_LIBS}"
+ AC_DEFINE(HAVE_LIBCAPNG, 1, [Enable libcap-ng support])
+ ;;
+esac
+
+
if test "${with_crypto_library}" = "openssl"; then
AC_ARG_VAR([OPENSSL_CFLAGS], [C compiler flags for OpenSSL])
AC_ARG_VAR([OPENSSL_LIBS], [linker flags for OpenSSL])
diff --git a/distro/systemd/openvpn-cli...@.service.in
b/distro/systemd/openvpn-cli...@.service.in
index cbcef653..159fb4dc 100644
--- a/distro/systemd/openvpn-cli...@.service.in
+++ b/distro/systemd/openvpn-cli...@.service.in
@@ -11,7 +11,7 @@ Type=notify
PrivateTmp=true
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
+CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_RAW CAP_SETGID
CAP_SETUID CAP_SETPCAP CAP_SYS_CHROOT CAP_DAC_OVERRIDE
LimitNPROC=10
DeviceAllow=/dev/null rw
DeviceAllow=/dev/net/tun rw
diff --git a/distro/systemd/openvpn-ser...@.service.in
b/distro/systemd/openvpn-ser...@.service.in
index d1cc72cb..6e8e7d94 100644
--- a/distro/systemd/openvpn-ser...@.service.in
+++ b/distro/systemd/openvpn-ser...@.service.in
@@ -11,7 +11,7 @@ Type=notify
PrivateTmp=true
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
CAP_AUDIT_WRITE
+CapabilityBoundingSet=CAP_IPC_LOCK CAP_NET_ADMIN CAP_NET_BIND_SERVICE
CAP_NET_RAW CAP_SETGID CAP_SETUID CAP_SETPCAP CAP_SYS_CHROOT CAP_DAC_OVERRIDE
CAP_AUDIT_WRITE
LimitNPROC=10
DeviceAllow=/dev/null rw
DeviceAllow=/dev/net/tun rw
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 8818ba6f..705eb92e 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1138,6 +1138,25 @@ possibly_become_daemon(const struct options *options)
return ret;
}
+/*
+ * Determine if we need to retain process capabilities. DCO and SITNL need it.
+ * Enforce it for DCO, but only try and soft-fail for SITNL to keep backwards
compat.
+ */
+static int
+get_need_keep_caps(struct context *c)
+{
+if (dco_enabled(>options))
+{
+return 1;
+}
+
+#ifdef ENABLE_SITNL
+return -1;
+#else
+return 0;
+#endif
+}
+
/*
* Actually do UID/GID downgrade, chroot and SELinux context switching, if
requested.
*/
@@ -1167,8 +1186,10 @@ do_uid_gid_chroot(struct context *c, bool no_delay)
{
if (no_delay)
{
-platform_group_set(>platform_state_group);
-platform_user_set(>platform_state_user);
+int keep_caps = get_need_keep_caps(c);
+platform_user_group_set(>platform_state_user,
+>platform_state_group,
+keep_caps);
}
else if (c->first_time)
{
diff --git a/src/openvpn/platform.c b/src/openvpn/platform.c
index 450f28ba..e67844ad 100644
--- a/src/openvpn/platform.c
+++ b/src/openvpn/platform.c
@@ -43,6 +43,10 @@
#include
#endif
+#ifdef HAVE_LIBCAPNG
+#include
+#endif
+
/* Redefine the top level directory of the filesystem
* to restrict access to files for security */
void
@@ -155,6 +159,81 @@ platform_group_set(const struct platform_state_group
*state)
#endif
}
+void platform_user_group_set(const struct platform_state_user *user_state,
+ const struct platform_state_group *group_state,
+ int keep_caps)
+{
+unsigned int err_flags = (keep_caps > 0) ? M_FATAL : M_NONFATAL;
+#ifdef HAVE_LIBCAPNG
+int new_gid = -1, new_uid = -1;
+int res;
+
+if (keep_caps == 0)
+{
+goto fallback;
+}
+
+/*
+ * new_uid/new_gid defaults to
Re: [Openvpn-devel] [PATCH] Retain CAP_NET_ADMIN when dropping privileges
Hi, On 30/03/2022 13:57, Gert Doering wrote: Hi, On Wed, Mar 30, 2022 at 01:31:24PM +0200, Timo Rothenpieler wrote: It is possible to argue that sitnl does low-level calls to the kernel as well. But potential libraries had an API which was making everything far more complex on the OpenVPN side. For libcap-ng at least, that is not the case; as the API it provides is pretty simple. Shouldn't caps support also be enabled when sitnl is in use? Given it also needs CAP_NET_ADMIN. That was a misunderstanding. David explained why we are not using a library but directly talk to the netlink socket for SITNL. And yes, we want CAP_NET_ADMIN for sitnl+--user as well ;-) One detail: using SITNL is a compile time decision, while using DCO is a runtime decision (assuming it was compiled in) Thanks! -- Antonio Quartulli ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Retain CAP_NET_ADMIN when dropping privileges
Hi, On Wed, Mar 30, 2022 at 01:31:24PM +0200, Timo Rothenpieler wrote: > > It is possible to argue that sitnl does low-level calls to the kernel as > > well. But potential libraries had an API which was making everything > > far more complex on the OpenVPN side. For libcap-ng at least, that is > > not the case; as the API it provides is pretty simple. > > Shouldn't caps support also be enabled when sitnl is in use? > Given it also needs CAP_NET_ADMIN. That was a misunderstanding. David explained why we are not using a library but directly talk to the netlink socket for SITNL. And yes, we want CAP_NET_ADMIN for sitnl+--user as well ;-) Thanks for your help on this, 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 g...@greenie.muc.de signature.asc Description: PGP signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Retain CAP_NET_ADMIN when dropping privileges
On 30.03.2022 11:11, David Sommerseth wrote:
On 30/03/2022 10:51, David Sommerseth wrote:
On 29/03/2022 21:29, Timo Rothenpieler wrote:
---
This patch sits on top of the current dco branch, and will not apply to
latest master.
It solves the issue of dropping root privileges breaking dco and sitnl
due to missing NET_ADMIN capabilities.
configure.ac | 3 ++
src/openvpn/init.c | 22 +-
src/openvpn/platform.c | 65 +-
src/openvpn/platform.h | 2 +-
4 files changed, 89 insertions(+), 3 deletions(-)
Thanks a lot! I've quickly looked through the code, and I have to NAK
this approach:
+#ifdef HAVE_LINUX_CAPABILITIES
+#define SET_CAP_HELPER(data, set, cap) data[(cap)>>5].set |=
1<<((cap)&31)
+
+static bool
+do_keep_caps(bool prepare)
+{
+ struct __user_cap_header_struct cap_hdr = {
_LINUX_CAPABILITY_VERSION_3 };
+ struct __user_cap_data_struct cap_data[_LINUX_CAPABILITY_U32S_3]
= {};
+
+ if (syscall(SYS_capget, _hdr, cap_data) < 0)
We should really use libcap or libcap-ng and not avoid using syscalls
directly.
Is there any preference between the two? I initially used libcap, but
wanted to avoid introducing another dependency.
But both libcap and libcap-ng seem to be widely adopted by distros, and
there isn't a huge difference in boilerplate between them for this purpose.
This did not come out well. Sorry about that.
We should really avoid using syscalls directly, as that binds us to
certain APIs and bindings.
Newer kernels may also require additional adjustments in the future, to
preserve the same behaviour. Which means we need to maintain this code
and also pay more attention to the security aspects of privilege
management, like new vulnerabilities and exploits.
The libcap or libcap-ng libraries are used by far more applications,
doing similar privilege management - and these libraries already pay
attention to the security aspects of new vulnerabilities and exploits.
The libcap-ng library is also recommended by more developers, due to its
simpler API.
It is possible to argue that sitnl does low-level calls to the kernel as
well. But potential libraries had an API which was making everything
far more complex on the OpenVPN side. For libcap-ng at least, that is
not the case; as the API it provides is pretty simple.
Shouldn't caps support also be enabled when sitnl is in use?
Given it also needs CAP_NET_ADMIN.
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Retain CAP_NET_ADMIN when dropping privileges
On 30/03/2022 10:51, David Sommerseth wrote:
On 29/03/2022 21:29, Timo Rothenpieler wrote:
---
This patch sits on top of the current dco branch, and will not apply to
latest master.
It solves the issue of dropping root privileges breaking dco and sitnl
due to missing NET_ADMIN capabilities.
configure.ac | 3 ++
src/openvpn/init.c | 22 +-
src/openvpn/platform.c | 65 +-
src/openvpn/platform.h | 2 +-
4 files changed, 89 insertions(+), 3 deletions(-)
Thanks a lot! I've quickly looked through the code, and I have to NAK
this approach:
+#ifdef HAVE_LINUX_CAPABILITIES
+#define SET_CAP_HELPER(data, set, cap) data[(cap)>>5].set |=
1<<((cap)&31)
+
+static bool
+do_keep_caps(bool prepare)
+{
+ struct __user_cap_header_struct cap_hdr = {
_LINUX_CAPABILITY_VERSION_3 };
+ struct __user_cap_data_struct cap_data[_LINUX_CAPABILITY_U32S_3]
= {};
+
+ if (syscall(SYS_capget, _hdr, cap_data) < 0)
We should really use libcap or libcap-ng and not avoid using syscalls
directly.
This did not come out well. Sorry about that.
We should really avoid using syscalls directly, as that binds us to
certain APIs and bindings.
Newer kernels may also require additional adjustments in the future, to
preserve the same behaviour. Which means we need to maintain this code
and also pay more attention to the security aspects of privilege
management, like new vulnerabilities and exploits.
The libcap or libcap-ng libraries are used by far more applications,
doing similar privilege management - and these libraries already pay
attention to the security aspects of new vulnerabilities and exploits.
The libcap-ng library is also recommended by more developers, due to its
simpler API.
It is possible to argue that sitnl does low-level calls to the kernel as
well. But potential libraries had an API which was making everything
far more complex on the OpenVPN side. For libcap-ng at least, that is
not the case; as the API it provides is pretty simple.
I have used libcap-ng in openvpn3-linux, both for preserving
capabilities and dropping root. It does all the right steps fairly easily.
The configure.ac detection, which for OpenVPN 2.x can be restricted when
DCO is going to be built into openvpn:
The code for preserving capabilities:
And the code for dropping root, ensuring the capabilities are restricted
properly:
--
kind regards,
David Sommerseth
OpenVPN Inc
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Retain CAP_NET_ADMIN when dropping privileges
On 29/03/2022 21:29, Timo Rothenpieler wrote:
---
This patch sits on top of the current dco branch, and will not apply to
latest master.
It solves the issue of dropping root privileges breaking dco and sitnl
due to missing NET_ADMIN capabilities.
configure.ac | 3 ++
src/openvpn/init.c | 22 +-
src/openvpn/platform.c | 65 +-
src/openvpn/platform.h | 2 +-
4 files changed, 89 insertions(+), 3 deletions(-)
Thanks a lot! I've quickly looked through the code, and I have to NAK
this approach:
+#ifdef HAVE_LINUX_CAPABILITIES
+#define SET_CAP_HELPER(data, set, cap) data[(cap)>>5].set |= 1<<((cap)&31)
+
+static bool
+do_keep_caps(bool prepare)
+{
+struct __user_cap_header_struct cap_hdr = { _LINUX_CAPABILITY_VERSION_3 };
+struct __user_cap_data_struct cap_data[_LINUX_CAPABILITY_U32S_3] = {};
+
+if (syscall(SYS_capget, _hdr, cap_data) < 0)
We should really use libcap or libcap-ng and not avoid using syscalls
directly.
I have used libcap-ng in openvpn3-linux, both for preserving
capabilities and dropping root. It does all the right steps fairly easily.
The configure.ac detection, which for OpenVPN 2.x can be restricted when
DCO is going to be built into openvpn:
The code for preserving capabilities:
And the code for dropping root, ensuring the capabilities are restricted
properly:
--
kind regards,
David Sommerseth
OpenVPN Inc
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Retain CAP_NET_ADMIN when dropping privileges
On 29.03.2022 21:29, Timo Rothenpieler wrote:
+static bool
+do_keep_caps(bool prepare)
+{
+struct __user_cap_header_struct cap_hdr = { _LINUX_CAPABILITY_VERSION_3 };
+struct __user_cap_data_struct cap_data[_LINUX_CAPABILITY_U32S_3] = {};
+
+if (syscall(SYS_capget, _hdr, cap_data) < 0)
+{
+msg(M_NONFATAL | M_ERRNO, "failed getting capabilities");
+return false;
+}
+
+if (prepare)
+{
+SET_CAP_HELPER(cap_data, permitted, CAP_NET_ADMIN);
+}
+else
+{
+SET_CAP_HELPER(cap_data, effective, CAP_NET_ADMIN);
This is missing something like the following:
/* Clamp permitted capabilities to effective ones.
* Without doing this, the process can give itself root-like caps at
any time. */
for (int i = 0; i < sizeof(cap_data)/sizeof(cap_data[0]); i++)
{
cap_data[i].permitted = cap_data[i].effective;
}
Without that, the permitted caps stay the full set of root caps, and the
process can make them effective at any time.
Patch on GitHub is updated with that.
+}
+
+if (syscall(SYS_capset, _hdr, cap_data) < 0)
+{
+msg(M_NONFATAL | M_ERRNO, "failed setting %s capabilities", prepare ?
"permitted" : "effective");
+return false;
+}
+
+if (prepare && prctl(PR_SET_KEEPCAPS, 1) < 0)
+{
+msg(M_NONFATAL | M_ERRNO, "failed setting keepcaps");
+return false;
+}
+
+return true;
+}
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] Retain CAP_NET_ADMIN when dropping privileges
---
This patch sits on top of the current dco branch, and will not apply to
latest master.
It solves the issue of dropping root privileges breaking dco and sitnl
due to missing NET_ADMIN capabilities.
configure.ac | 3 ++
src/openvpn/init.c | 22 +-
src/openvpn/platform.c | 65 +-
src/openvpn/platform.h | 2 +-
4 files changed, 89 insertions(+), 3 deletions(-)
diff --git a/configure.ac b/configure.ac
index 7199483a..220c63e5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -710,6 +710,9 @@ if test -z "${LIBPAM_LIBS}"; then
)
fi
+AC_CHECK_HEADERS([linux/capability.h sys/syscall.h sys/prctl.h],
+ [AC_DEFINE(HAVE_LINUX_CAPABILITIES, 1, [Linux capability
support available])])
+
case "${with_mem_check}" in
valgrind)
AC_CHECK_HEADERS(
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 8818ba6f..13c07ff0 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1138,6 +1138,25 @@ possibly_become_daemon(const struct options *options)
return ret;
}
+/*
+ * Determine if we need to retain process capabilities. DCO and SITNL need it.
+ * Enforce it for DCO, but only try and soft-fail for SITNL to keep backwards
compat.
+ */
+static int
+get_need_keep_caps(struct context *c)
+{
+if (dco_enabled(>options))
+{
+return 1;
+}
+
+#ifdef ENABLE_SITNL
+return -1;
+#else
+return 0;
+#endif
+}
+
/*
* Actually do UID/GID downgrade, chroot and SELinux context switching, if
requested.
*/
@@ -1167,8 +1186,9 @@ do_uid_gid_chroot(struct context *c, bool no_delay)
{
if (no_delay)
{
+int keep_caps = get_need_keep_caps(c);
platform_group_set(>platform_state_group);
-platform_user_set(>platform_state_user);
+platform_user_set(>platform_state_user, keep_caps);
}
else if (c->first_time)
{
diff --git a/src/openvpn/platform.c b/src/openvpn/platform.c
index 450f28ba..680ebb5f 100644
--- a/src/openvpn/platform.c
+++ b/src/openvpn/platform.c
@@ -43,6 +43,12 @@
#include
#endif
+#ifdef HAVE_LINUX_CAPABILITIES
+#include
+#include
+#include
+#endif
+
/* Redefine the top level directory of the filesystem
* to restrict access to files for security */
void
@@ -91,17 +97,74 @@ platform_user_get(const char *username, struct
platform_state_user *state)
return ret;
}
+#ifdef HAVE_LINUX_CAPABILITIES
+#define SET_CAP_HELPER(data, set, cap) data[(cap)>>5].set |= 1<<((cap)&31)
+
+static bool
+do_keep_caps(bool prepare)
+{
+struct __user_cap_header_struct cap_hdr = { _LINUX_CAPABILITY_VERSION_3 };
+struct __user_cap_data_struct cap_data[_LINUX_CAPABILITY_U32S_3] = {};
+
+if (syscall(SYS_capget, _hdr, cap_data) < 0)
+{
+msg(M_NONFATAL | M_ERRNO, "failed getting capabilities");
+return false;
+}
+
+if (prepare)
+{
+SET_CAP_HELPER(cap_data, permitted, CAP_NET_ADMIN);
+}
+else
+{
+SET_CAP_HELPER(cap_data, effective, CAP_NET_ADMIN);
+}
+
+if (syscall(SYS_capset, _hdr, cap_data) < 0)
+{
+msg(M_NONFATAL | M_ERRNO, "failed setting %s capabilities", prepare ?
"permitted" : "effective");
+return false;
+}
+
+if (prepare && prctl(PR_SET_KEEPCAPS, 1) < 0)
+{
+msg(M_NONFATAL | M_ERRNO, "failed setting keepcaps");
+return false;
+}
+
+return true;
+}
+#else
+static bool
+do_keep_caps(bool prepare)
+{
+return false;
+}
+#endif
+
void
-platform_user_set(const struct platform_state_user *state)
+platform_user_set(const struct platform_state_user *state, int keep_caps)
{
#if defined(HAVE_GETPWNAM) && defined(HAVE_SETUID)
if (state->username && state->pw)
{
+bool caps_prepared = keep_caps && do_keep_caps(true);
+
if (setuid(state->pw->pw_uid))
{
msg(M_ERR, "setuid('%s') failed", state->username);
}
msg(M_INFO, "UID set to %s", state->username);
+
+if (caps_prepared && do_keep_caps(false))
+{
+msg(M_INFO, "Capabilities retained");
+}
+else if (keep_caps > 0)
+{
+msg(M_FATAL, "Failed retaining capabilities");
+}
}
#endif
}
diff --git a/src/openvpn/platform.h b/src/openvpn/platform.h
index a3eec298..ef06da93 100644
--- a/src/openvpn/platform.h
+++ b/src/openvpn/platform.h
@@ -79,7 +79,7 @@ struct platform_state_group {
bool platform_user_get(const char *username, struct platform_state_user
*state);
-void platform_user_set(const struct platform_state_user *state);
+void platform_user_set(const struct platform_state_user *state, int keep_caps);
bool platform_group_get(const char *groupname, struct platform_state_group
*state);
--
2.25.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
