All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH iproute2] Drop capabilities if not running ip exec vrf with libcap
@ 2018-03-27 16:24 Luca Boccassi
  2018-03-27 16:40 ` David Ahern
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Luca Boccassi @ 2018-03-27 16:24 UTC (permalink / raw)
  To: netdev; +Cc: dsahern, luto, stephen, Luca Boccassi

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@debian.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.

 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
+}
+
 static const struct cmd {
 	const char *cmd;
 	int (*func)(int argc, char **argv);
@@ -174,6 +196,18 @@ int main(int argc, char **argv)
 	char *batch_file = NULL;
 	int color = 0;
 
+	/* to run vrf exec without root, capabilities might be set, drop them
+	 * if not needed as the first thing.
+	 * execv will drop them for the child command.
+	 * vrf exec requires:
+	 * - cap_dac_override to create the cgroup subdir in /sys
+	 * - cap_sys_admin to load the BPF program
+	 * - cap_net_admin to set the socket into the cgroup
+	 */
+	if (argc < 3 || strcmp(argv[1], "vrf") != 0 ||
+			strcmp(argv[2], "exec") != 0)
+		drop_cap();
+
 	basename = strrchr(argv[0], '/');
 	if (basename == NULL)
 		basename = argv[0];
diff --git a/man/man8/ip-vrf.8 b/man/man8/ip-vrf.8
index 18789339..1a42cebe 100644
--- a/man/man8/ip-vrf.8
+++ b/man/man8/ip-vrf.8
@@ -63,6 +63,14 @@ a VRF other than the default VRF (main table). A command can be run against
 the default VRF by passing the "default" as the VRF name. This is useful if
 the current shell is associated with another VRF (e.g, Management VRF).
 
+This command requires the system to be booted with cgroup v2 (e.g. with systemd,
+add systemd.unified_cgroup_hierarchy=1 to the kernel command line).
+
+This command also requires to be ran as root or with the CAP_SYS_ADMIN,
+CAP_NET_ADMIN and CAP_DAC_OVERRIDE capabilities. If built with libcap and if
+capabilities are added to the ip binary program via setcap, the program will
+drop them as the first thing when invoked, unless the command is vrf exec.
+
 .TP
 .B ip vrf identify [PID] - Report VRF association for process
 .sp
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH iproute2] Drop capabilities if not running ip exec vrf with libcap
  2018-03-27 16:24 [RFC PATCH iproute2] Drop capabilities if not running ip exec vrf with libcap Luca Boccassi
@ 2018-03-27 16:40 ` David Ahern
  2018-03-27 17:05   ` Luca Boccassi
  2018-03-27 17:15 ` Stephen Hemminger
  2018-03-27 17:48 ` [PATCH iproute2 v1] " Luca Boccassi
  2 siblings, 1 reply; 7+ messages in thread
From: David Ahern @ 2018-03-27 16:40 UTC (permalink / raw)
  To: Luca Boccassi, netdev; +Cc: luto, stephen

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@debian.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).

> 
>  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.

> +
>  static const struct cmd {
>  	const char *cmd;
>  	int (*func)(int argc, char **argv);
> @@ -174,6 +196,18 @@ int main(int argc, char **argv)
>  	char *batch_file = NULL;
>  	int color = 0;
>  
> +	/* to run vrf exec without root, capabilities might be set, drop them
> +	 * if not needed as the first thing.
> +	 * execv will drop them for the child command.
> +	 * vrf exec requires:
> +	 * - cap_dac_override to create the cgroup subdir in /sys
> +	 * - cap_sys_admin to load the BPF program
> +	 * - cap_net_admin to set the socket into the cgroup
> +	 */
> +	if (argc < 3 || strcmp(argv[1], "vrf") != 0 ||
> +			strcmp(argv[2], "exec") != 0)
> +		drop_cap();
> +
>  	basename = strrchr(argv[0], '/');
>  	if (basename == NULL)
>  		basename = argv[0];
> diff --git a/man/man8/ip-vrf.8 b/man/man8/ip-vrf.8
> index 18789339..1a42cebe 100644
> --- a/man/man8/ip-vrf.8
> +++ b/man/man8/ip-vrf.8
> @@ -63,6 +63,14 @@ a VRF other than the default VRF (main table). A command can be run against
>  the default VRF by passing the "default" as the VRF name. This is useful if
>  the current shell is associated with another VRF (e.g, Management VRF).
>  
> +This command requires the system to be booted with cgroup v2 (e.g. with systemd,
> +add systemd.unified_cgroup_hierarchy=1 to the kernel command line).
> +
> +This command also requires to be ran as root or with the CAP_SYS_ADMIN,
> +CAP_NET_ADMIN and CAP_DAC_OVERRIDE capabilities. If built with libcap and if
> +capabilities are added to the ip binary program via setcap, the program will
> +drop them as the first thing when invoked, unless the command is vrf exec.
> +
>  .TP
>  .B ip vrf identify [PID] - Report VRF association for process
>  .sp
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH iproute2] Drop capabilities if not running ip exec vrf with libcap
  2018-03-27 16:40 ` David Ahern
@ 2018-03-27 17:05   ` Luca Boccassi
  0 siblings, 0 replies; 7+ messages in thread
From: Luca Boccassi @ 2018-03-27 17:05 UTC (permalink / raw)
  To: David Ahern, netdev; +Cc: luto, stephen

[-- Attachment #1: Type: text/plain, Size: 3757 bytes --]

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@debian.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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH iproute2] Drop capabilities if not running ip exec vrf with libcap
  2018-03-27 16:24 [RFC PATCH iproute2] Drop capabilities if not running ip exec vrf with libcap Luca Boccassi
  2018-03-27 16:40 ` David Ahern
@ 2018-03-27 17:15 ` Stephen Hemminger
  2018-03-27 17:43   ` Luca Boccassi
  2018-03-27 17:48 ` [PATCH iproute2 v1] " Luca Boccassi
  2 siblings, 1 reply; 7+ messages in thread
From: Stephen Hemminger @ 2018-03-27 17:15 UTC (permalink / raw)
  To: Luca Boccassi; +Cc: netdev, dsahern, luto

On Tue, 27 Mar 2018 17:24:19 +0100
Luca Boccassi <bluca@debian.org> 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@debian.org>

Gets a little messy, but don't have a better answer.
When a command like iproute gets involved in security policy things
I become concerned that it may have unexpected consequences.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH iproute2] Drop capabilities if not running ip exec vrf with libcap
  2018-03-27 17:15 ` Stephen Hemminger
@ 2018-03-27 17:43   ` Luca Boccassi
  0 siblings, 0 replies; 7+ messages in thread
From: Luca Boccassi @ 2018-03-27 17:43 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, dsahern, luto

[-- Attachment #1: Type: text/plain, Size: 1317 bytes --]

On Tue, 2018-03-27 at 10:15 -0700, Stephen Hemminger wrote:
> On Tue, 27 Mar 2018 17:24:19 +0100
> Luca Boccassi <bluca@debian.org> 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@debian.org>
> 
> Gets a little messy, but don't have a better answer.
> When a command like iproute gets involved in security policy things
> I become concerned that it may have unexpected consequences.

Yeah I understand. It requires an explicit action by the sysadmin, to
give you plausible deniability :-)

I've seen changes to let BPF permissions be managed via an LSM (I think
SELinux support is already merged in 4.15), so perhaps one day we'll be
able to do the whole shebang (subdir in /sys + load bpf + manipulate
cgroup) in a more fine-grained way, but for now I think this will do.

I'll send v1 shortly with the change asked by David.

-- 
Kind regards,
Luca Boccassi

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH iproute2 v1] Drop capabilities if not running ip exec vrf with libcap
  2018-03-27 16:24 [RFC PATCH iproute2] Drop capabilities if not running ip exec vrf with libcap Luca Boccassi
  2018-03-27 16:40 ` David Ahern
  2018-03-27 17:15 ` Stephen Hemminger
@ 2018-03-27 17:48 ` Luca Boccassi
  2018-03-27 18:52   ` Stephen Hemminger
  2 siblings, 1 reply; 7+ messages in thread
From: Luca Boccassi @ 2018-03-27 17:48 UTC (permalink / raw)
  To: netdev; +Cc: dsahern, luto, stephen, Luca Boccassi

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@debian.org>
---

Changes since RFC: moved drop_cap to lib/util.c to call it from
ipvrf.c after vrf_switch, which is the function that requires the
additional permissions, has finished.

 configure         | 17 +++++++++++++++++
 include/utils.h   |  2 ++
 ip/ip.c           | 12 ++++++++++++
 ip/ipvrf.c        |  2 ++
 lib/utils.c       | 21 +++++++++++++++++++++
 man/man8/ip-vrf.8 |  8 ++++++++
 6 files changed, 62 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/include/utils.h b/include/utils.h
index 0394268e..e7bffe8a 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -299,4 +299,6 @@ size_t strlcpy(char *dst, const char *src, size_t size);
 size_t strlcat(char *dst, const char *src, size_t size);
 #endif
 
+void drop_cap(void);
+
 #endif /* __UTILS_H__ */
diff --git a/ip/ip.c b/ip/ip.c
index e0cd96cb..e716fed8 100644
--- a/ip/ip.c
+++ b/ip/ip.c
@@ -174,6 +174,18 @@ int main(int argc, char **argv)
 	char *batch_file = NULL;
 	int color = 0;
 
+	/* to run vrf exec without root, capabilities might be set, drop them
+	 * if not needed as the first thing.
+	 * execv will drop them for the child command.
+	 * vrf exec requires:
+	 * - cap_dac_override to create the cgroup subdir in /sys
+	 * - cap_sys_admin to load the BPF program
+	 * - cap_net_admin to set the socket into the cgroup
+	 */
+	if (argc < 3 || strcmp(argv[1], "vrf") != 0 ||
+			strcmp(argv[2], "exec") != 0)
+		drop_cap();
+
 	basename = strrchr(argv[0], '/');
 	if (basename == NULL)
 		basename = argv[0];
diff --git a/ip/ipvrf.c b/ip/ipvrf.c
index f9277e1e..8a6b7f97 100644
--- a/ip/ipvrf.c
+++ b/ip/ipvrf.c
@@ -436,6 +436,8 @@ out2:
 out:
 	free(mnt);
 
+	drop_cap();
+
 	return rc;
 }
 
diff --git a/lib/utils.c b/lib/utils.c
index eba4fa74..d20cc594 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -30,6 +30,9 @@
 #include <time.h>
 #include <sys/time.h>
 #include <errno.h>
+#ifdef HAVE_LIBCAP
+#include <sys/capability.h>
+#endif
 
 #include "rt_names.h"
 #include "utils.h"
@@ -1482,3 +1485,21 @@ size_t strlcat(char *dst, const char *src, size_t size)
 	return dlen + strlcpy(dst + dlen, src, size - dlen);
 }
 #endif
+
+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
+}
diff --git a/man/man8/ip-vrf.8 b/man/man8/ip-vrf.8
index 18789339..1a42cebe 100644
--- a/man/man8/ip-vrf.8
+++ b/man/man8/ip-vrf.8
@@ -63,6 +63,14 @@ a VRF other than the default VRF (main table). A command can be run against
 the default VRF by passing the "default" as the VRF name. This is useful if
 the current shell is associated with another VRF (e.g, Management VRF).
 
+This command requires the system to be booted with cgroup v2 (e.g. with systemd,
+add systemd.unified_cgroup_hierarchy=1 to the kernel command line).
+
+This command also requires to be ran as root or with the CAP_SYS_ADMIN,
+CAP_NET_ADMIN and CAP_DAC_OVERRIDE capabilities. If built with libcap and if
+capabilities are added to the ip binary program via setcap, the program will
+drop them as the first thing when invoked, unless the command is vrf exec.
+
 .TP
 .B ip vrf identify [PID] - Report VRF association for process
 .sp
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH iproute2 v1] Drop capabilities if not running ip exec vrf with libcap
  2018-03-27 17:48 ` [PATCH iproute2 v1] " Luca Boccassi
@ 2018-03-27 18:52   ` Stephen Hemminger
  0 siblings, 0 replies; 7+ messages in thread
From: Stephen Hemminger @ 2018-03-27 18:52 UTC (permalink / raw)
  To: Luca Boccassi; +Cc: netdev, dsahern, luto

On Tue, 27 Mar 2018 18:48:55 +0100
Luca Boccassi <bluca@debian.org> 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@debian.org>

Applied

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-03-27 18:52 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-27 16:24 [RFC PATCH iproute2] Drop capabilities if not running ip exec vrf with libcap Luca Boccassi
2018-03-27 16:40 ` David Ahern
2018-03-27 17:05   ` Luca Boccassi
2018-03-27 17:15 ` Stephen Hemminger
2018-03-27 17:43   ` Luca Boccassi
2018-03-27 17:48 ` [PATCH iproute2 v1] " Luca Boccassi
2018-03-27 18:52   ` Stephen Hemminger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.