netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Maciej Żenczykowski" <maze@google.com>
To: "Maciej Żenczykowski" <zenczykowski@gmail.com>
Cc: "Linux Network Development Mailing List" <netdev@vger.kernel.org>,
	"Maciej Żenczykowski" <maze@google.com>,
	"Larysa Zaremba" <larysa.zaremba@intel.com>,
	"Simon Horman" <simon.horman@corigine.com>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Eyal Birger" <eyal.birger@gmail.com>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Eric Dumazet" <edumazet@google.com>,
	"Patrick Rohr" <prohr@google.com>
Subject: [PATCH net v2] revert "net: align SO_RCVMARK required privileges with SO_MARK"
Date: Sun, 18 Jun 2023 03:31:30 -0700	[thread overview]
Message-ID: <20230618103130.51628-1-maze@google.com> (raw)
In-Reply-To: <7915b31f96108bee8dd92a229df6823ebe9c55b0.camel@redhat.com>

This reverts commit 1f86123b9749 ("net: align SO_RCVMARK required
privileges with SO_MARK") because the reasoning in the commit message
is not really correct:
  SO_RCVMARK is used for 'reading' incoming skb mark (via cmsg), as such
  it is more equivalent to 'getsockopt(SO_MARK)' which has no priv check
  and retrieves the socket mark, rather than 'setsockopt(SO_MARK) which
  sets the socket mark and does require privs.

  Additionally incoming skb->mark may already be visible if
  sysctl_fwmark_reflect and/or sysctl_tcp_fwmark_accept are enabled.

  Furthermore, it is easier to block the getsockopt via bpf
  (either cgroup setsockopt hook, or via syscall filters)
  then to unblock it if it requires CAP_NET_RAW/ADMIN.

On Android the socket mark is (among other things) used to store
the network identifier a socket is bound to.  Setting it is privileged,
but retrieving it is not.  We'd like unprivileged userspace to be able
to read the network id of incoming packets (where mark is set via
iptables [to be moved to bpf])...

An alternative would be to add another sysctl to control whether
setting SO_RCVMARK is privilged or not.
(or even a MASK of which bits in the mark can be exposed)
But this seems like over-engineering...

Note: This is a non-trivial revert, due to later merged commit e42c7beee71d
("bpf: net: Consider has_current_bpf_ctx() when testing capable() in sk_setsockopt()")
which changed both 'ns_capable' into 'sockopt_ns_capable' calls.

Fixes: 1f86123b9749 ("net: align SO_RCVMARK required privileges with SO_MARK")
Cc: Larysa Zaremba <larysa.zaremba@intel.com>
Cc: Simon Horman <simon.horman@corigine.com>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Eyal Birger <eyal.birger@gmail.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Patrick Rohr <prohr@google.com>
Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
 net/core/sock.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/net/core/sock.c b/net/core/sock.c
index 24f2761bdb1d..6e5662ca00fe 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1362,12 +1362,6 @@ int sk_setsockopt(struct sock *sk, int level, int optname,
 		__sock_set_mark(sk, val);
 		break;
 	case SO_RCVMARK:
-		if (!sockopt_ns_capable(sock_net(sk)->user_ns, CAP_NET_RAW) &&
-		    !sockopt_ns_capable(sock_net(sk)->user_ns, CAP_NET_ADMIN)) {
-			ret = -EPERM;
-			break;
-		}
-
 		sock_valbool_flag(sk, SOCK_RCVMARK, valbool);
 		break;
 
-- 
2.41.0.162.gfafddb0af9-goog


  reply	other threads:[~2023-06-18 10:31 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-05  8:12 [PATCH] net: revert "align SO_RCVMARK required privileges with SO_MARK" Maciej Żenczykowski
2023-06-05 15:27 ` Larysa Zaremba
2023-06-05 17:30   ` Simon Horman
2023-06-06 11:37 ` Paolo Abeni
2023-06-18 10:31   ` Maciej Żenczykowski [this message]
2023-06-19 14:12     ` [PATCH net v2] revert "net: align " Simon Horman
2023-06-20  0:17     ` Kuniyuki Iwashima
2023-06-22 10:00     ` patchwork-bot+netdevbpf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230618103130.51628-1-maze@google.com \
    --to=maze@google.com \
    --cc=edumazet@google.com \
    --cc=eyal.birger@gmail.com \
    --cc=kuba@kernel.org \
    --cc=larysa.zaremba@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=prohr@google.com \
    --cc=simon.horman@corigine.com \
    --cc=zenczykowski@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).