netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ido Schimmel <idosch@idosch.org>
To: "Johannes Wiesböck" <johannes.wiesboeck@aisec.fraunhofer.de>
Cc: gyroidos@aisec.fraunhofer.de, sw@simonwunderlich.de,
	"Michael Weiß" <michael.weiss@aisec.fraunhofer.de>,
	"Harshal Gohel" <hg@simonwunderlich.de>,
	"David S. Miller" <davem@davemloft.net>,
	"Eric Dumazet" <edumazet@google.com>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Simon Horman" <horms@kernel.org>,
	"Kuniyuki Iwashima" <kuniyu@google.com>,
	"Stanislav Fomichev" <sdf@fomichev.me>,
	"Xiao Liang" <shaw.leon@gmail.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] rtnetlink: Allow deleting FDB entries in user namespace
Date: Wed, 24 Sep 2025 15:52:52 +0300	[thread overview]
Message-ID: <aNPppN7crz-n7bej@shredder> (raw)
In-Reply-To: <20250923082153.60030-1-johannes.wiesboeck@aisec.fraunhofer.de>

On Tue, Sep 23, 2025 at 10:21:40AM +0200, Johannes Wiesböck wrote:
> Deletion of FDB entries requires CAP_NET_ADMIN, yet, processes in a
> non-initial user namespace receive an EPERM because the capability is
> always checked against the initial user namespace. This restricts the
> FDB management from unprivileged containers.

It's worth mentioning that unprivileged containers can add FDB entries,
but not delete them:

$ id
uid=1000(idosch) gid=1000(idosch) groups=1000(idosch),10(wheel)
$ unshare -Urn
$ id
uid=0(root) gid=0(root) groups=0(root),65534(nobody)
$ ip link add name br0 up type bridge
$ bridge fdb add 00:11:22:33:44:55 dev br0 self permanent
$ bridge fdb del 00:11:22:33:44:55 dev br0 self permanent
RTNETLINK answers: Operation not permitted

After (not exactly your patch, see below):

$ id
uid=1000(idosch) gid=1000(idosch) groups=1000(idosch),10(wheel)
$ unshare -Urn
$ id
uid=0(root) gid=0(root) groups=0(root),65534(nobody)
$ ip link add name br0 up type bridge
$ bridge fdb add 00:11:22:33:44:55 dev br0 self permanent
$ bridge fdb del 00:11:22:33:44:55 dev br0 self permanent
$ echo $?
0

> 
> Replace netlink_capable with netlink_net_capable that performs the
> capability check on the user namespace the netlink socket was opened in.
> 
> This patch was tested using a container on GyroidOS, where it was
> possible to delete FDB entries from an unprivileged user namespace and
> private network namespace.
> 
> Reviewed-by: Michael Weiß <michael.weiss@aisec.fraunhofer.de>
> Tested-by: Harshal Gohel <hg@simonwunderlich.de>
> Signed-off-by: Johannes Wiesböck <johannes.wiesboeck@aisec.fraunhofer.de>
> ---
>  net/core/rtnetlink.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 094b085cff206..2f96258bd4fd7 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -4707,7 +4707,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
>  	int err;
>  	u16 vid;
>  
> -	if (!netlink_capable(skb, CAP_NET_ADMIN))
> +	if (!netlink_net_capable(skb, CAP_NET_ADMIN))
>  		return -EPERM;

AFAICT, before commit 1690be63a27b ("bridge: Add vlan support to static
neighbors") it was possible for unprivileged containers to delete FDB
entries and the commit message does not say anything about why this
check was added. So, unless I'm missing something, I think your patch
should be treated as a fix and targeted at "net". See:

https://docs.kernel.org/process/maintainer-netdev.html

It might be a rebase issue. Commit c5c351088ae7 ("netns: fdb: allow
unprivileged users to add/del fdb entries") removed the CAP_NET_ADMIN
check from both rtnl_fdb_add() and rtnl_fdb_del() about two weeks before
v11 of 1690be63a27b was applied.

Also, there's already a netlink_net_capable(skb, CAP_NET_ADMIN) check in
rtnetlink_rcv_msg(), so I would just remove the capability check from
rtnl_fdb_del().

      reply	other threads:[~2025-09-24 12:52 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-23  8:21 [PATCH] rtnetlink: Allow deleting FDB entries in user namespace Johannes Wiesböck
2025-09-24 12:52 ` Ido Schimmel [this message]

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=aNPppN7crz-n7bej@shredder \
    --to=idosch@idosch.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=gyroidos@aisec.fraunhofer.de \
    --cc=hg@simonwunderlich.de \
    --cc=horms@kernel.org \
    --cc=johannes.wiesboeck@aisec.fraunhofer.de \
    --cc=kuba@kernel.org \
    --cc=kuniyu@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michael.weiss@aisec.fraunhofer.de \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sdf@fomichev.me \
    --cc=shaw.leon@gmail.com \
    --cc=sw@simonwunderlich.de \
    /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).