[PATCH] net: allow CAP_NET_RAW to setsockopt SO_PRIORITY

[PATCH] net: allow CAP_NET_RAW to setsockopt SO_PRIORITY

[Maciej Żenczykowski]

From: Maciej Żenczykowski <maze@google.com>

CAP_NET_ADMIN is and should continue to be about configuring the
system as a whole, not about configuring per-socket or per-packet
parameters.
Sending and receiving raw packets is what CAP_NET_RAW is all about.

It can already send packets with any VLAN tag, and any IPv4 TOS
mark, and any IPv6 TCLASS mark, simply by virtue of building
such a raw packet.  Not to mention using any protocol and source/
/destination ip address/port tuple.

These are the fields that networking gear uses to prioritize packets.

Hence, a CAP_NET_RAW process is already capable of affecting traffic
prioritization after it hits the wire.  This change makes it capable
of affecting traffic prioritization even in the host at the nic and
before that in the queueing disciplines (provided skb->priority is
actually being used for prioritization, and not the TOS/TCLASS field)

Hence it makes sense to allow a CAP_NET_RAW process to set the
priority of sockets and thus packets it sends.

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 net/core/sock.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/core/sock.c b/net/core/sock.c
index 31a2b79c9b38..1e49a127adef 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1135,6 +1135,7 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
 
 	case SO_PRIORITY:
 		if ((val >= 0 && val <= 6) ||
+		    ns_capable(sock_net(sk)->user_ns, CAP_NET_RAW) ||
 		    ns_capable(sock_net(sk)->user_ns, CAP_NET_ADMIN))
 			sk->sk_priority = val;
 		else
-- 
2.34.0.rc2.393.gf8c9666880-goog

Re: [PATCH] net: allow CAP_NET_RAW to setsockopt SO_PRIORITY

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 23 Nov 2021 12:37:02 -0800 you wrote:
> From: Maciej Żenczykowski <maze@google.com>
> 
> CAP_NET_ADMIN is and should continue to be about configuring the
> system as a whole, not about configuring per-socket or per-packet
> parameters.
> Sending and receiving raw packets is what CAP_NET_RAW is all about.
> 
> [...]

Here is the summary with links:
  - net: allow CAP_NET_RAW to setsockopt SO_PRIORITY
    https://git.kernel.org/netdev/net-next/c/a1b519b74548

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

Re: [PATCH] net: allow CAP_NET_RAW to setsockopt SO_PRIORITY

[Maciej Żenczykowski]

> SO_PRIORITY can control prioritizations that are not influenced
> by packet contents.
>
> There is a huge difference.

I do agree that it can do that, but I'm not sure the difference is
huge as you so put it.
In the vast majority of cases prioritization is desired based on some
meaningful aspect
of a packet that can also be observed later on in the network - hence
vlan tags, tos/tclass marks,
protocol, port numbers and the like.

I guess to me, in the real world (or at least the portion of it I'm
aware of), the difference is minor at best.

Being able to select xmit queues, is kind of like being able to select
output device.
You give a CAP_NET_RAW process raw access to the wire - prioritization seems
just one more aspect of this.  Especially since this isn't permanently
reconfiguring anything.

Anyway, enough discussion ;-)

Cheers,
Maciej

Re: [PATCH] net: allow CAP_NET_RAW to setsockopt SO_PRIORITY

[David Miller]

From: Maciej Żenczykowski <zenczykowski@gmail.com>
Date: Mon, 24 Oct 2011 22:07:20 -0700

> A raw socket can already spew traffic, ie. packets, with any content.
> This includes VLAN tags and IPv4 TOS field, and IPv6 TCLASS field.
> These are the fields used for packet prioritization at switches and in
> the rest of network fabric.

SO_PRIORITY can control prioritizations that are not influenced
by packet contents.

There is a huge difference.

Re: [PATCH] net: allow CAP_NET_RAW to setsockopt SO_PRIORITY

[Maciej Żenczykowski]

> SO_PRIORITY influences the packet scheduler and internal
> queueing policies, not just the packet contents that hit
> the wire.
>
> Thus CAP_NET_ADMIN is the appropriate capability check.

I can certainly see why you would think that, but:

A raw socket can already spew traffic, ie. packets, with any content.
This includes VLAN tags and IPv4 TOS field, and IPv6 TCLASS field.
These are the fields used for packet prioritization at switches and in
the rest of network fabric.

AFAICT, it makes no sense to allow a raw socket to prioritize traffic
outside of the host, but
not allow it to prioritize traffic in the host.

- Maciej

Re: [PATCH] net: allow CAP_NET_RAW to setsockopt SO_PRIORITY

[David Miller]

From: Maciej Żenczykowski <zenczykowski@gmail.com>
Date: Mon, 24 Oct 2011 21:47:18 -0700

> It is my belief that CAP_NET_ADMIN is and should continue to be about
> configuring the system as a whole, not about configuring per-socket
> or per-packet parameters.
> 
> Sending and receiving raw packets is what CAP_NET_RAW is all about.
> Hence it makes sense to allow a CAP_NET_RAW process to set the
> priority of sockets and thus packets it sends.

SO_PRIORITY influences the packet scheduler and internal
queueing policies, not just the packet contents that hit
the wire.

Thus CAP_NET_ADMIN is the appropriate capability check.

[PATCH] net: allow CAP_NET_RAW to setsockopt SO_PRIORITY

[Maciej Żenczykowski]

From: Maciej Żenczykowski <maze@google.com>

It is my belief that CAP_NET_ADMIN is and should continue to be about
configuring the system as a whole, not about configuring per-socket
or per-packet parameters.

Sending and receiving raw packets is what CAP_NET_RAW is all about.
Hence it makes sense to allow a CAP_NET_RAW process to set the
priority of sockets and thus packets it sends.

Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 net/core/sock.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/core/sock.c b/net/core/sock.c
index 5a08762..3d163b6 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -612,7 +612,8 @@ set_rcvbuf:
 		break;
 
 	case SO_PRIORITY:
-		if ((val >= 0 && val <= 6) || capable(CAP_NET_ADMIN))
+		if ((val >= 0 && val <= 6)
+		    || capable(CAP_NET_ADMIN) || capable(CAP_NET_RAW))
 			sk->sk_priority = val;
 		else
 			ret = -EPERM;
-- 
1.7.3.1