From mboxrd@z Thu Jan 1 00:00:00 1970 From: Luca Boccassi Subject: Re: [RFC PATCH iproute2] Drop capabilities if not running ip exec vrf with libcap Date: Tue, 27 Mar 2018 18:05:45 +0100 Message-ID: <1522170345.14111.110.camel@debian.org> References: <20180327162419.8962-1-bluca@debian.org> <4f03183a-3373-62b7-06ea-b1198299c8d5@gmail.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg="pgp-sha512"; protocol="application/pgp-signature"; boundary="=-RKjPkY5ZEC894XqniWxV" Cc: luto@amacapital.net, stephen@networkplumber.org To: David Ahern , netdev@vger.kernel.org Return-path: Received: from mail-wr0-f195.google.com ([209.85.128.195]:40627 "EHLO mail-wr0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751765AbeC0RFt (ORCPT ); Tue, 27 Mar 2018 13:05:49 -0400 Received: by mail-wr0-f195.google.com with SMTP id z8so23130039wrh.7 for ; Tue, 27 Mar 2018 10:05:48 -0700 (PDT) In-Reply-To: <4f03183a-3373-62b7-06ea-b1198299c8d5@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: --=-RKjPkY5ZEC894XqniWxV Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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. > >=20 > > Signed-off-by: Luca Boccassi > > --- > >=20 > > 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. >=20 > 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! > >=20 > > =C2=A0configure=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0| = 17 +++++++++++++++++ > > =C2=A0ip/ip.c=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0| 34 ++++++++++++++++++++++++++++++++++ > > =C2=A0man/man8/ip-vrf.8 |=C2=A0=C2=A08 ++++++++ > > =C2=A03 files changed, 59 insertions(+) > >=20 > > diff --git a/configure b/configure > > index f7c2d7a7..5ef5cd4c 100755 > > --- a/configure > > +++ b/configure > > @@ -336,6 +336,20 @@ EOF > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0rm -f $TMPDIR/strtest.c $TMPDIR/strtest > > =C2=A0} > > =C2=A0 > > +check_cap() > > +{ > > + if ${PKG_CONFIG} libcap --exists > > + then > > + echo "HAVE_CAP:=3Dy" >>$CONFIG > > + echo "yes" > > + > > + echo 'CFLAGS +=3D -DHAVE_LIBCAP' `${PKG_CONFIG} > > libcap --cflags` >>$CONFIG > > + echo 'LDLIBS +=3D' `${PKG_CONFIG} libcap --libs` >> > > $CONFIG > > + else > > + echo "no" > > + fi > > +} > > + > > =C2=A0quiet_config() > > =C2=A0{ > > =C2=A0 cat < > @@ -410,6 +424,9 @@ check_berkeley_db > > =C2=A0echo -n "need for strlcpy: " > > =C2=A0check_strlcpy > > =C2=A0 > > +echo -n "libcap support: " > > +check_cap > > + > > =C2=A0echo >> $CONFIG > > =C2=A0echo "%.o: %.c" >> $CONFIG > > =C2=A0echo ' $(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 @@ > > =C2=A0#include > > =C2=A0#include > > =C2=A0#include > > +#include > > +#ifdef HAVE_LIBCAP > > +#include > > +#endif > > =C2=A0 > > =C2=A0#include "SNAPSHOT.h" > > =C2=A0#include "utils.h" > > @@ -68,6 +72,24 @@ static int do_help(int argc, char **argv) > > =C2=A0 return 0; > > =C2=A0} > > =C2=A0 > > +static void drop_cap(void) > > +{ > > +#ifdef HAVE_LIBCAP > > + /* don't harmstring root/sudo */ > > + if (getuid() !=3D 0 && geteuid() !=3D 0) { > > + cap_t capabilities; > > + capabilities =3D cap_get_proc(); > > + if (!capabilities) > > + exit(EXIT_FAILURE); > > + if (cap_clear(capabilities) !=3D 0) > > + exit(EXIT_FAILURE); > > + if (cap_set_proc(capabilities) !=3D 0) > > + exit(EXIT_FAILURE); > > + cap_free(capabilities); > > + } > > +#endif > > +} >=20 > 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). --=20 Kind regards, Luca Boccassi --=-RKjPkY5ZEC894XqniWxV Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part -----BEGIN PGP SIGNATURE----- iQEzBAABCgAdFiEE6g0RLAGYhL9yp9G8SylmgFB4UWIFAlq6eekACgkQSylmgFB4 UWLfdgf/QCYbaBWJ+F6KBrwoVAWuiSPx4nBRxGNTRZrcyOWD1aqsEG3b3oCH/wQA IZRFnI+4unOxBPDQRIUMV1GDWsfssTDYlfJutMU1V/wRPOUv4y+KWtw5TZHASm7a wqUomKAL959XC70JDPwMB2kt/A+/03Vubh6tovDaqqyyxYgMm/Lw6YbpCyXTRT8N 11WNpnmVSqPbK5OpJUjrITMKfWaXwSMsw7wrG/zlec92E1qPAHf/MkveZgBVjFgT R7HvCzPiud/G/LwQ6XzntKd2ig22n1UIwXL1Bqoj8S9303WAmsBmGOfAZioTuu5H pd136pB2Wvc6V7j48kLezxg0fE6sDQ== =wajJ -----END PGP SIGNATURE----- --=-RKjPkY5ZEC894XqniWxV--