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]

Date:   Tue, 27 Mar 2018 10:40:36 -0600
From:   David Ahern <dsahern@...il.com>
To:     Luca Boccassi <bluca@...ian.org>, 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 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).

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