[PATCH iproute2-next] ip-vrf: recommend using CAP_BPF rather than CAP_SYS_ADMIN

[PATCH iproute2-next] ip-vrf: recommend using CAP_BPF rather than CAP_SYS_ADMIN

[Maximilian Bosch]

The CAP_SYS_ADMIN capability allows far too much, to quote
`capabilities(7)`:

    Note: this capability is overloaded; see Notes to kernel developers, below.

In the case of `ip-vrf(8)` this is needed to load a BPF program.
According to the same section of the same man-page, using `CAP_BPF` is
preferred if that's the reason for `CAP_SYS_ADMIN`;

    perform  the  same BPF operations as are governed by CAP_BPF (but the latter, weaker capability is preferred for accessing
    that functionality).

Local testing revealed that `ip vrf exec` for an unprivileged user is
sufficient if the `CAP_BPF` capability is given rather than
`CAP_SYS_ADMIN`.

Since this was introduced in Linux 5.8, a note is left that on older
kernels `CAP_SYS_ADMIN` must be used instead.

Signed-off-by: Maximilian Bosch <maximilian@mbosch.me>
---
 ip/ip.c           | 2 +-
 man/man8/ip-vrf.8 | 9 +++++----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/ip/ip.c b/ip/ip.c
index 8424736f..8c046ef1 100644
--- a/ip/ip.c
+++ b/ip/ip.c
@@ -175,7 +175,7 @@ int main(int argc, char **argv)
 	 * 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_bpf to load the BPF program
 	 * - cap_net_admin to set the socket into the cgroup
 	 */
 	if (argc < 3 || strcmp(argv[1], "vrf") != 0 ||
diff --git a/man/man8/ip-vrf.8 b/man/man8/ip-vrf.8
index c1c9b958..798a6808 100644
--- a/man/man8/ip-vrf.8
+++ b/man/man8/ip-vrf.8
@@ -66,10 +66,11 @@ 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.
+This command also requires to be ran as root or with the CAP_BPF (or
+CAP_SYS_ADMIN on Linux <5.8), 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.
-- 
2.40.1

Re: [PATCH iproute2-next] ip-vrf: recommend using CAP_BPF rather than CAP_SYS_ADMIN

[Stephen Hemminger]

On Wed, 9 Aug 2023 11:26:36 +0200
Maximilian Bosch <maximilian@mbosch.me> wrote:

> -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.
> +This command also requires to be ran as root or with the CAP_BPF (or
> +CAP_SYS_ADMIN on Linux <5.8), 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.

I don't like it when documentation becomes kernel version dependent.
And distro kernels backport all the time. Documentation should cover why
instead of hiding it in comments.

This paragraph is almost unreadable even before the patch. The verb tenses
and wording are not those that would be used by a native English speaker.

[PATCH iproute2-next v2] ip-vrf: recommend using CAP_BPF rather than CAP_SYS_ADMIN

[Maximilian Bosch]

The CAP_SYS_ADMIN capability allows far too much, to quote
`capabilities(7)`:

    Note: this capability is overloaded; see Notes to kernel developers, below.

In the case of `ip-vrf(8)` this is needed to load a BPF program.
According to the same section of the same man-page, using `CAP_BPF` is
preferred if that's the reason for `CAP_SYS_ADMIN`;

    perform  the  same BPF operations as are governed by CAP_BPF (but the latter, weaker capability is preferred for accessing
    that functionality).

Local testing revealed that `ip vrf exec` for an unprivileged user is
sufficient if the `CAP_BPF` capability is given rather than
`CAP_SYS_ADMIN`.

In a previous version of the patch[1] it was mentioned that
CAP_SYS_ADMIN was still required for Linux <5.8, however it was
suggested to not make man-pages dependent on the kernel version. Also,
it was suggested to improve the wording and the formatting of the entire
paragraph mentioning capabilities which was also done.

Signed-off-by: Maximilian Bosch <maximilian@mbosch.me>

[1] https://lore.kernel.org/netdev/e6t4ucjdrcitzneh2imygsaxyb2aasxfn2q2a4zh5yqdx3vold@kutwh5kwixva/T/#m628a1900a7e5012bb87e6cb3c94af6c7281cf2bf
---
 ip/ip.c           |  2 +-
 man/man8/ip-vrf.8 | 40 ++++++++++++++++++++++++++++++++++------
 2 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/ip/ip.c b/ip/ip.c
index 8424736f..8c046ef1 100644
--- a/ip/ip.c
+++ b/ip/ip.c
@@ -175,7 +175,7 @@ int main(int argc, char **argv)
 	 * 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_bpf to load the BPF program
 	 * - cap_net_admin to set the socket into the cgroup
 	 */
 	if (argc < 3 || strcmp(argv[1], "vrf") != 0 ||
diff --git a/man/man8/ip-vrf.8 b/man/man8/ip-vrf.8
index c1c9b958..946e8f8a 100644
--- a/man/man8/ip-vrf.8
+++ b/man/man8/ip-vrf.8
@@ -66,14 +66,42 @@ 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.
+This command also requires to be run as root. Alternatively it
+can be run by an unprivileged user if the following
+.BR capabilities (7)
+are given:
+
+.RS
+.IP \fBCAP_BPF\fP
+To load the BPF program.
+.IP \fBCAP_NET_ADMIN\fP
+To set the socket into the cgroup.
+.IP \fBCAP_DAC_OVERRIDE\fP
+To create the cgroup subdir in /sys.
+.RE
+
+.IP
+If these capabilities are added and if
+.BR ip (8)
+is built with
+.BR libcap (3)
+then these capabilities will be dropped before
+.BR cmd
+is executed by
+.B ip vrf exec.
+For every other unprivileged invocation of
+.BR ip (8)
+all capabilities will be dropped.
+
 .br
-NOTE: capabilities will NOT be dropped if CAP_NET_ADMIN is set to INHERITABLE
+.B NOTE:
+capabilities will
+.B NOT
+be dropped if
+.B CAP_NET_ADMIN
+is set to
+.B 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.40.1

Re: [PATCH iproute2-next v2] ip-vrf: recommend using CAP_BPF rather than CAP_SYS_ADMIN

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to iproute2/iproute2.git (main)
by Stephen Hemminger <stephen@networkplumber.org>:

On Tue, 22 Aug 2023 14:33:07 +0200 you wrote:
> The CAP_SYS_ADMIN capability allows far too much, to quote
> `capabilities(7)`:
> 
>     Note: this capability is overloaded; see Notes to kernel developers, below.
> 
> In the case of `ip-vrf(8)` this is needed to load a BPF program.
> According to the same section of the same man-page, using `CAP_BPF` is
> preferred if that's the reason for `CAP_SYS_ADMIN`;
> 
> [...]

Here is the summary with links:
  - [iproute2-next,v2] ip-vrf: recommend using CAP_BPF rather than CAP_SYS_ADMIN
    https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=df210e83e0fa

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html