Simon Ruderich <si...@ruderich.org> on Tue, 2018/04/24 10:38:
> 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.

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

> On Mon, Apr 23, 2018 at 11:28:13AM +0200, Christian Hesse wrote:
> >  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  
> 
> Are those capabilities dropped after initialization? If they are
> not this sounds like a serious issue as the process is basically
> running as root even if it's using another user (CAP_NET_ADMIN
> and CAP_DAC_OVERRIDE). Or am I missing something here?
>
> Regarding the netlink change in general: From what I understand
> it means that openvpn will always run with CAP_NET_ADMIN
> capabilities. Is this correct? If so, this sounds like it
> requires much more privileges than before for the normal
> operation (unless I misunderstand the current setup - to my
> knowledge it only requires a normal user after setup and no
> further capabilities or privileges once setup/connected).

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

Attachment: pgpOWT9rvHVUt.pgp
Description: OpenPGP digital signature

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to