All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH iproute2] ip: do not drop capabilities if net_admin=i is set
@ 2018-05-11 12:39 Luca Boccassi
  2018-05-15  4:10 ` Stephen Hemminger
  0 siblings, 1 reply; 2+ messages in thread
From: Luca Boccassi @ 2018-05-11 12:39 UTC (permalink / raw)
  To: netdev; +Cc: dsahern, luto, stephen, Luca Boccassi

Users have reported a regression due to ip now dropping capabilities
unconditionally.
zerotier-one VPN and VirtualBox use ambient capabilities in their
binary and then fork out to ip to set routes and links, and this
does not work anymore.

As a workaround, do not drop caps if CAP_NET_ADMIN (the most common
capability used by ip) is set with the INHERITABLE flag.
Users that want ip vrf exec to work do not need to set INHERITABLE,
which will then only set when the calling program had privileges to
give itself the ambient capability.

Fixes: ba2fc55b99f8 ("Drop capabilities if not running ip exec vrf with libcap")

Signed-off-by: Luca Boccassi <bluca@debian.org>
---

Reported on Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=898015
The reporter tested this patch and verified it solves the issue.

 lib/utils.c       | 15 ++++++++++++---
 man/man8/ip-vrf.8 |  4 ++++
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/lib/utils.c b/lib/utils.c
index 8a0bff0b..7b2c6dd1 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -1612,14 +1612,23 @@ void drop_cap(void)
 	/* don't harmstring root/sudo */
 	if (getuid() != 0 && geteuid() != 0) {
 		cap_t capabilities;
+		cap_value_t net_admin = CAP_NET_ADMIN;
+		cap_flag_t inheritable = CAP_INHERITABLE;
+		cap_flag_value_t is_set;
 
 		capabilities = cap_get_proc();
 		if (!capabilities)
 			exit(EXIT_FAILURE);
-		if (cap_clear(capabilities) != 0)
-			exit(EXIT_FAILURE);
-		if (cap_set_proc(capabilities) != 0)
+		if (cap_get_flag(capabilities, net_admin, inheritable,
+		    &is_set) != 0)
 			exit(EXIT_FAILURE);
+		/* apps with ambient caps can fork and call ip */
+		if (is_set == CAP_CLEAR) {
+			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 1a42cebe..c1c9b958 100644
--- a/man/man8/ip-vrf.8
+++ b/man/man8/ip-vrf.8
@@ -70,6 +70,10 @@ 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.
+.br
+NOTE: capabilities will NOT be dropped if CAP_NET_ADMIN is set to INHERITABLE
+to avoid breaking programs with ambient capabilities that call ip.
+Do not set the INHERITABLE flag on the ip binary itself.
 
 .TP
 .B ip vrf identify [PID] - Report VRF association for process
-- 
2.14.2

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

* Re: [PATCH iproute2] ip: do not drop capabilities if net_admin=i is set
  2018-05-11 12:39 [PATCH iproute2] ip: do not drop capabilities if net_admin=i is set Luca Boccassi
@ 2018-05-15  4:10 ` Stephen Hemminger
  0 siblings, 0 replies; 2+ messages in thread
From: Stephen Hemminger @ 2018-05-15  4:10 UTC (permalink / raw)
  To: Luca Boccassi; +Cc: netdev, dsahern, luto

On Fri, 11 May 2018 13:39:56 +0100
Luca Boccassi <bluca@debian.org> wrote:

> Users have reported a regression due to ip now dropping capabilities
> unconditionally.
> zerotier-one VPN and VirtualBox use ambient capabilities in their
> binary and then fork out to ip to set routes and links, and this
> does not work anymore.
> 
> As a workaround, do not drop caps if CAP_NET_ADMIN (the most common
> capability used by ip) is set with the INHERITABLE flag.
> Users that want ip vrf exec to work do not need to set INHERITABLE,
> which will then only set when the calling program had privileges to
> give itself the ambient capability.
> 
> Fixes: ba2fc55b99f8 ("Drop capabilities if not running ip exec vrf with libcap")
> 
> Signed-off-by: Luca Boccassi <bluca@debian.org>

Applied

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

end of thread, other threads:[~2018-05-15  4:10 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-11 12:39 [PATCH iproute2] ip: do not drop capabilities if net_admin=i is set Luca Boccassi
2018-05-15  4:10 ` 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.