GNOME Bugzilla – Bug 555518
NM invokes openvpn dæmon as root
Last modified: 2015-08-27 14:30:36 UTC
In most cases, there should be no need to invoke vpnc/openvpn/openconnect/etc. as root. Typically, the only thing they actually need root privs for is the TUNSETIFF ioctl to create a tun interface. But that can be done as an unprivileged user, if root first creates the device and uses the TUNSETPERSIST and TUNSETOWNER ioctls to make it persistent and owned by the user in question -- then the user can bind to it at will. We could make NetworkManager do that, then pass the --ifname or --interface options (for vpnc/openconnect resp.) to get the vpn dæmon to use the specified interface. And then we could run the dæmons as non-root.
This is fixed for N-M-openconnect, but should be fixed for other VPN types too. It may be hard for vpnc to do certain things (like IKE) without root privs; we may need a cunning solution to that, like passing it a socket that we've opened for it. Or maybe just accept that we can only run vpnc as non-root if it's using NAT-T. But openvpn shouldn't be a problem, surely? That's as simple as openconnect.
Since openconnect is done, I'm reassinging to openvpn. Please open similar bugreports to other VPNs if it makes sense.
The following patch was provided downstream for this: https://513202.bugs.gentoo.org/attachment.cgi?id=378862 https://bugs.gentoo.org/show_bug.cgi?id=513202
I have pushed branch jk/openvpn-unprivilege-user allowing to run openvpn as non-root (based on the gentoo patch). Basic testing shows it works. However, I am not sure if this approach works for all cases like reconnect. etc. Another approach might be creating tun/tap device beforehand as in openconnect. This is probably what is described in hhtps://community.openvpn.net/openvpn/wiki/UnprivilegedUser. But I am a bit confused with the description in regards with the scripts.
http://man7.org/linux/man-pages/man3/getgrouplist.3.html If the number of groups of which user is a member is less than or equal to *ngroups, then the value *ngroups is returned. If the user is a member of more than *ngroups groups, then getgrouplist() returns -1. In this case, the value returned in *ngroups can be used to resize the buffer passed to a further call getgrouplist(). I think that must be done. Otherwise, this leads to buffer over-read. if you chroot the binary, don't you have to give it access to the NM_OPENVPN_HELPER_PATH binary? + nm_openvpn_user = getenv ("NM_OPENVPN_USER"); + nm_openvpn_group = getenv ("NM_OPENVPN_GROUP"); + nm_openvpn_chroot = getenv ("NM_OPENVPN_CHROOT"); + if (!nm_openvpn_user) + nm_openvpn_user = NM_OPENVPN_USER; + if (!nm_openvpn_group) + nm_openvpn_group = NM_OPENVPN_GROUP; + if (!nm_openvpn_chroot) + nm_openvpn_chroot = NM_OPENVPN_CHROOT; how about accepting environment variables "" to mean: don't jail/chuser?
(In reply to Thomas Haller from comment #5) > http://man7.org/linux/man-pages/man3/getgrouplist.3.html > > If the number of groups of which user is a member is less than or > equal to *ngroups, then the value *ngroups is returned. > > If the user is a member of more than *ngroups groups, then > getgrouplist() returns -1. In this case, the value returned in > *ngroups can be used to resize the buffer passed to a further call > getgrouplist(). > > > I think that must be done. Otherwise, this leads to buffer over-read. > I use static array and check maximum of 128 groups, which seems to me enough. Do you feel we should allocate a dynamic array and call getgrouplist() again if the first array was too small? > > if you chroot the binary, don't you have to give it access to the > NM_OPENVPN_HELPER_PATH binary? > Hmm, maybe. Did you actually test? Any idea how to do that? > > + nm_openvpn_user = getenv ("NM_OPENVPN_USER"); > + nm_openvpn_group = getenv ("NM_OPENVPN_GROUP"); > + nm_openvpn_chroot = getenv ("NM_OPENVPN_CHROOT"); > + if (!nm_openvpn_user) > + nm_openvpn_user = NM_OPENVPN_USER; > + if (!nm_openvpn_group) > + nm_openvpn_group = NM_OPENVPN_GROUP; > + if (!nm_openvpn_chroot) > + nm_openvpn_chroot = NM_OPENVPN_CHROOT; > > how about accepting environment variables "" to mean: don't jail/chuser? It was using the root/root and no-chroot when failing to verify environment data. So it worked for "" too. But I have made it explicit now, so that there is no warning about bad value for "". The branch has been re-based to master.
+/* User name and group to run nm-openvpn-service under */ +#define NM_OPENVPN_USER "nm-openvpn" +#define NM_OPENVPN_GROUP "nm-openvpn" +#define NM_OPENVPN_CHROOT "/var/lib/openvpn/chroot"; Extra ';' at the end. And how about: #define NM_OPENVPN_CHROOT LOCALSTATEDIR "/lib/openvpn/chroot" + if (sb.st_mode & 0002) + [...] + else if (sb.st_mode & 0020) { + [...] + } else if (sb.st_mode & 0200) { Maybe use the S_IWOTH, S_IWGRP, S_IWUSR macros?
(In reply to Beniamino Galvani from comment #7) > +/* User name and group to run nm-openvpn-service under */ > +#define NM_OPENVPN_USER "nm-openvpn" > +#define NM_OPENVPN_GROUP "nm-openvpn" > +#define NM_OPENVPN_CHROOT "/var/lib/openvpn/chroot"; > > Extra ';' at the end. And how about: > > #define NM_OPENVPN_CHROOT LOCALSTATEDIR "/lib/openvpn/chroot" > Done. > + if (sb.st_mode & 0002) > + [...] > + else if (sb.st_mode & 0020) { > + [...] > + } else if (sb.st_mode & 0200) { > > Maybe use the S_IWOTH, S_IWGRP, S_IWUSR macros? Done.
(In reply to Jiri Klimes from comment #4) > I have pushed branch jk/openvpn-unprivilege-user allowing to run openvpn as > non-root (based on the gentoo patch). > > Basic testing shows it works. However, I am not sure if this approach works > for all cases like reconnect. etc. > Another approach might be creating tun/tap device beforehand as in > openconnect. This is probably what is described in > hhtps://community.openvpn.net/openvpn/wiki/UnprivilegedUser. But I am a bit > confused with the description in regards with the scripts. openvpn will close and recreate the tun device at various points, unless you pass --persist-tun which we already do. So as long as the tunnel/tap is created before privs are dropped then I think restart will be fine. Creating it before spawning openvpn and then passing it with --dev X --devtype [tun/tap] would probably work fine, like openconnect does?
Also, for user/group checks, I think the VPN service should bail out if the user/group isn't found. Just using 'root' could make the user think it is dropping privileges, but instead nm-openvpn would run as root.
(In reply to Dan Williams from comment #9) > (In reply to Jiri Klimes from comment #4) > > I have pushed branch jk/openvpn-unprivilege-user allowing to run openvpn as > > non-root (based on the gentoo patch). > > > > Basic testing shows it works. However, I am not sure if this approach works > > for all cases like reconnect. etc. > > Another approach might be creating tun/tap device beforehand as in > > openconnect. This is probably what is described in > > hhtps://community.openvpn.net/openvpn/wiki/UnprivilegedUser. But I am a bit > > confused with the description in regards with the scripts. > > openvpn will close and recreate the tun device at various points, unless you > pass --persist-tun which we already do. So as long as the tunnel/tap is > created before privs are dropped then I think restart will be fine. > openvpn drop privileges after initializing, so we are ok. Thanks for clarifying. (In reply to Dan Williams from comment #10) > Also, for user/group checks, I think the VPN service should bail out if the > user/group isn't found. Just using 'root' could make the user think it is > dropping privileges, but instead nm-openvpn would run as root. Done.
Patch looks good; just one more thing to double-check... does it open the management socket before dropping privs? We now use a unix socket on the filesystem for management instead of the old TCP socket, so we should make sure that works; but that should be as easy as just running it and making sure that entering the password + token code works...
(In reply to Dan Williams from comment #12) > Patch looks good; just one more thing to double-check... does it open the > management socket before dropping privs? We now use a unix socket on the > filesystem for management instead of the old TCP socket, so we should make > sure that works; but that should be as easy as just running it and making > sure that entering the password + token code works... I can successfully connect. $ ss | grep openvpn u_str ESTAB 0 0 /var/run/NetworkManager/nm-openvpn-69b00f64-8b5d-4fb5-8a02-dbc300f0431a 202389 * 204922 $ ps aux | grep sbin/openvpn jklimes 12341 0.3 0.0 65616 7044 pts/4 S+ 09:40 0:00 /usr/sbin/openvpn --remote 209.132.183.3 443 udp --nobind --dev tun --cipher AES-256-CBC --auth-nocache --tls-remote ovpn-phx2.redhat.com --reneg-sec 0 --verb 10 --script-security 2 --up /usr/libexec/nm-openvpn-service-openvpn-helper --helper-debug --tun -- --up-restart --persist-key --persist-tun --management /var/run/NetworkManager/nm-openvpn-69b00f64-8b5d-4fb5-8a02-dbc300f0431a unix --management-client-user root --management-client-group root --management-query-passwords --auth-retry interact --route-noexec --ifconfig-noexec --client --auth-user-pass --ca /home/jklimes/.cert/newca.crt --user jklimes --group jklimes Committed as master: 4bb9236 service: allow running openvpn as an unprivilege user (bgo #555518) nm-1-0: 87631c2 service: allow running openvpn as an unprivilege user (bgo #555518)