Message ID | 20220817131817.467-1-timo@rothenpieler.org |
---|---|
State | Accepted |
Headers | show |
Series | [Openvpn-devel] dco: disable DCO if --user specified but unable to retain capabilities | expand |
Acked-by: Gert Doering <gert@greenie.muc.de> Thanks. Bernhard has already tested this on his Debian/NM testing environment and confirms it fixes the issues seen in https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1017379 I've added a bit of explanatory text to the commit message (quite a bit of blurb, actually, but the issue was confusing us enough that this might help future readers). Your patch has been applied to the master branch. commit da31c1654c8534658157cfe9c9de5750ee752608 Author: Timo Rothenpieler Date: Wed Aug 17 15:18:17 2022 +0200 dco: disable DCO if --user specified but unable to retain capabilities Acked-by: Gert Doering <gert@greenie.muc.de> Message-Id: <20220817131817.467-1-timo@rothenpieler.org> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24952.html Signed-off-by: Gert Doering <gert@greenie.muc.de> -- kind regards, Gert Doering
Hi, On Wed, Aug 17, 2022 at 04:18:39PM +0200, Gert Doering wrote: > Acked-by: Gert Doering <gert@greenie.muc.de> > > Thanks. Bernhard has already tested this on his Debian/NM testing > environment and confirms it fixes the issues seen in > > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1017379 > > I've added a bit of explanatory text to the commit message (quite a > bit of blurb, actually, but the issue was confusing us enough that > this might help future readers). > > Your patch has been applied to the master branch. > > commit da31c1654c8534658157cfe9c9de5750ee752608 > Author: Timo Rothenpieler > Date: Wed Aug 17 15:18:17 2022 +0200 > > dco: disable DCO if --user specified but unable to retain capabilities I really hate to say this... but... we seem to be calling this function for incoming MULTI connects: I have an UDP p2mp server with "--user nobody + DCO", which starts up fine (because it has root and can set CAP_NET_ADMIN), but on incoming client connects, it goes there *again*, and then fails: Aug 17 21:00:18 ubuntu2004 tun-udp-p2mp[2167001]: freebsd-14-amd64/194.97.140.5:23821 --user specified but lacking CAP_SETPCAP. Cannot retain CAP_NET_ADMIN. Disabling data channel offload Aug 17 21:00:18 ubuntu2004 tun-udp-p2mp[2167001]: freebsd-14-amd64/194.97.140.5:23821 MULTI: client has been rejected due to incompatible DCO options Aug 17 21:00:19 ubuntu2004 tun-udp-p2mp[2167001]: freebsd-14-amd64/194.97.140.5:23821 PUSH: Received control message: 'PUSH_REQUEST' Aug 17 21:00:19 ubuntu2004 tun-udp-p2mp[2167001]: freebsd-14-amd64/194.97.140.5:23821 Delayed exit in 5 seconds Aug 17 21:00:19 ubuntu2004 tun-udp-p2mp[2167001]: freebsd-14-amd64/194.97.140.5:23821 SENT CONTROL [freebsd-14-amd64]: 'AUTH_FAILED' (status=1) This is hairy mess of checks, double checks, and too many checks... (a quick fix would be to add "if (getuid() == 0 && o->user && ...)" to ensure this is only called if we haven't already changed user id, but it still feels wrong) Better suggestions where to move "this is something we only care at startup, not at per-instance config" checks? gert
diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c index caa4ce32..b7db23f4 100644 --- a/src/openvpn/dco.c +++ b/src/openvpn/dco.c @@ -44,6 +44,10 @@ #include "ssl_ncp.h" #include "tun.h" +#ifdef HAVE_LIBCAPNG +#include <cap-ng.h> +#endif + static int dco_install_key(struct tls_multi *multi, struct key_state *ks, const uint8_t *encrypt_key, const uint8_t *encrypt_iv, @@ -247,6 +251,28 @@ dco_check_option_conflict_platform(int msglevel, const struct options *o) } } #endif /* if defined(TARGET_LINUX) */ + +#if defined(HAVE_LIBCAPNG) + /* DCO can't operate without CAP_NET_ADMIN. To retain it when switching user + * we need CAP_SETPCAP. CAP_NET_ADMIN also needs to be part of the permitted set + * of capabilities in order to retain it. + */ + if (o->username) + { + if (!capng_have_capability(CAPNG_EFFECTIVE, CAP_SETPCAP)) + { + msg(msglevel, "--user specified but lacking CAP_SETPCAP. " + "Cannot retain CAP_NET_ADMIN. Disabling data channel offload"); + return false; + } + if (!capng_have_capability(CAPNG_PERMITTED, CAP_NET_ADMIN)) + { + msg(msglevel, "--user specified but not permitted to retain CAP_NET_ADMIN. " + "Disabling data channel offload"); + return false; + } + } +#endif /* if defined(HAVE_LIBCAPNG) */ return true; }