netdev - Re: [RFC PATCH iproute2] Drop capabilities if not running ip exec vrf with libcap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1522170345.14111.110.camel@debian.org>
Date:   Tue, 27 Mar 2018 18:05:45 +0100
From:   Luca Boccassi <bluca@...ian.org>
To:     David Ahern <dsahern@...il.com>, netdev@...r.kernel.org
Cc:     luto@...capital.net, stephen@...workplumber.org
Subject: Re: [RFC PATCH iproute2] Drop capabilities if not running ip exec
 vrf with libcap

On Tue, 2018-03-27 at 10:40 -0600, David Ahern wrote:
> On 3/27/18 10:24 AM, Luca Boccassi wrote:
> > ip vrf exec requires root or CAP_NET_ADMIN, CAP_SYS_ADMIN and
> > CAP_DAC_OVERRIDE. It is not possible to run unprivileged commands
> > like
> > ping as non-root or non-cap-enabled due to this requirement.
> > To allow users and administrators to safely add the required
> > capabilities to the binary, drop all capabilities on start if not
> > invoked with "vrf exec".
> > Update the manpage with the requirements.
> > 
> > Signed-off-by: Luca Boccassi <bluca@...ian.org>
> > ---
> > 
> > I'd like to be able to run ip vrf exec as a normal user, does this
> > approach
> > sound sensible? Any concerns? Are there any other alternatives?
> > It would be up to each user/admin/whatever to make sure the program
> > is
> > built with libcap and to add the capabilities manually, so nothing
> > will be
> > there by default.
> 
> This is very similar to a change I recently made for our
> distribution. I
> created a separate 'runvrf' command so as to not contaminate 'ip'
> (the
> runvrf command has the limitation it can not configure the VRF cgroup
> so
> that needs to be done before runvrf).

Great, thanks for the feedback!

> > 
> >  configure         | 17 +++++++++++++++++
> >  ip/ip.c           | 34 ++++++++++++++++++++++++++++++++++
> >  man/man8/ip-vrf.8 |  8 ++++++++
> >  3 files changed, 59 insertions(+)
> > 
> > diff --git a/configure b/configure
> > index f7c2d7a7..5ef5cd4c 100755
> > --- a/configure
> > +++ b/configure
> > @@ -336,6 +336,20 @@ EOF
> >      rm -f $TMPDIR/strtest.c $TMPDIR/strtest
> >  }
> >  
> > +check_cap()
> > +{
> > +	if ${PKG_CONFIG} libcap --exists
> > +	then
> > +		echo "HAVE_CAP:=y" >>$CONFIG
> > +		echo "yes"
> > +
> > +		echo 'CFLAGS += -DHAVE_LIBCAP' `${PKG_CONFIG}
> > libcap --cflags` >>$CONFIG
> > +		echo 'LDLIBS +=' `${PKG_CONFIG} libcap --libs` >>
> > $CONFIG
> > +	else
> > +		echo "no"
> > +	fi
> > +}
> > +
> >  quiet_config()
> >  {
> >  	cat <<EOF
> > @@ -410,6 +424,9 @@ check_berkeley_db
> >  echo -n "need for strlcpy: "
> >  check_strlcpy
> >  
> > +echo -n "libcap support: "
> > +check_cap
> > +
> >  echo >> $CONFIG
> >  echo "%.o: %.c" >> $CONFIG
> >  echo '	$(QUIET_CC)$(CC) $(CFLAGS) $(EXTRA_CFLAGS) -c -o $@
> > $<' >> $CONFIG
> > diff --git a/ip/ip.c b/ip/ip.c
> > index e0cd96cb..49739571 100644
> > --- a/ip/ip.c
> > +++ b/ip/ip.c
> > @@ -17,6 +17,10 @@
> >  #include <netinet/in.h>
> >  #include <string.h>
> >  #include <errno.h>
> > +#include <sys/types.h>
> > +#ifdef HAVE_LIBCAP
> > +#include <sys/capability.h>
> > +#endif
> >  
> >  #include "SNAPSHOT.h"
> >  #include "utils.h"
> > @@ -68,6 +72,24 @@ static int do_help(int argc, char **argv)
> >  	return 0;
> >  }
> >  
> > +static void drop_cap(void)
> > +{
> > +#ifdef HAVE_LIBCAP
> > +	/* don't harmstring root/sudo */
> > +	if (getuid() != 0 && geteuid() != 0) {
> > +		cap_t capabilities;
> > +		capabilities = cap_get_proc();
> > +		if (!capabilities)
> > +			exit(EXIT_FAILURE);
> > +		if (cap_clear(capabilities) != 0)
> > +			exit(EXIT_FAILURE);
> > +		if (cap_set_proc(capabilities) != 0)
> > +			exit(EXIT_FAILURE);
> > +		cap_free(capabilities);
> > +	}
> > +#endif
> > +}
> 
> You don't need the capabilities after the cgroup has been changed, so
> you can add a call to drop_cap at the end of vrf_switch.

Ok, if Stephen finds the approach correct I'll send a v1 with that
change (it shouldn't be strictly necessary as execvp which is ran just
after vrf_switch drops caps, but it won't hurt either).

-- 
Kind regards,
Luca Boccassi
Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)