netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* CAP_NET_ADMIN check in tc_ctl_action() makes it not allowed for user ns root
@ 2021-06-01  8:17 tianyu zhou
  2021-06-08  4:22 ` Cong Wang
  0 siblings, 1 reply; 2+ messages in thread
From: tianyu zhou @ 2021-06-01  8:17 UTC (permalink / raw)
  To: Jamal Hadi Salim, Cong Wang, Jiri Pirko
  Cc: David S. Miller, Jakub Kicinski, netdev

Hi, from commit "net: Allow tc changes in user
namespaces"(SHA:4e8bbb819d1594a01f91b1de83321f68d3e6e245) I learned
that "root in a user namespace may set tc rules inside that
namespace".

I do see the CAP_NET_ADMIN check in tc_* functions has changed from
capable() to ns_capable() (which is now in term of
netlink_ns_capable())

However, in function tc_ctl_action(), the check for CAP_NET_ADMIN is
still netlink_capable which does not allow user ns root to pass this
check.

static int tc_ctl_action(struct sk_buff *skb, struct nlmsghdr *n,
             struct netlink_ext_ack *extack)
{
    ...
    if ((n->nlmsg_type != RTM_GETACTION) &&
        !netlink_capable(skb, CAP_NET_ADMIN))
        return -EPERM;
    ...
}

So is this a check missing changing for user ns?

Thanks!

Best regards,
Tianyu

^ permalink raw reply	[flat|nested] 2+ messages in thread
* Re: CAP_NET_ADMIN check in tc_ctl_action() makes it not allowed for user ns root
  2021-06-01  8:17 CAP_NET_ADMIN check in tc_ctl_action() makes it not allowed for user ns root tianyu zhou
@ 2021-06-08  4:22 ` Cong Wang
  0 siblings, 0 replies; 2+ messages in thread
From: Cong Wang @ 2021-06-08  4:22 UTC (permalink / raw)
  To: tianyu zhou
  Cc: Jamal Hadi Salim, Jiri Pirko, David S. Miller, Jakub Kicinski,
	Linux Kernel Network Developers

On Tue, Jun 1, 2021 at 1:17 AM tianyu zhou <tyjoe.linux@gmail.com> wrote:
>
> Hi, from commit "net: Allow tc changes in user
> namespaces"(SHA:4e8bbb819d1594a01f91b1de83321f68d3e6e245) I learned
> that "root in a user namespace may set tc rules inside that
> namespace".
>
> I do see the CAP_NET_ADMIN check in tc_* functions has changed from
> capable() to ns_capable() (which is now in term of
> netlink_ns_capable())
>
> However, in function tc_ctl_action(), the check for CAP_NET_ADMIN is
> still netlink_capable which does not allow user ns root to pass this
> check.
>
> static int tc_ctl_action(struct sk_buff *skb, struct nlmsghdr *n,
>              struct netlink_ext_ack *extack)
> {
>     ...
>     if ((n->nlmsg_type != RTM_GETACTION) &&
>         !netlink_capable(skb, CAP_NET_ADMIN))
>         return -EPERM;
>     ...
> }
>
> So is this a check missing changing for user ns?

It seems so, I do not see TC action is any different with other
TC objects here. So feel free to send a patch.

Thanks.

^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-06-08  4:23 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-06-01  8:17 CAP_NET_ADMIN check in tc_ctl_action() makes it not allowed for user ns root tianyu zhou
2021-06-08  4:22 ` Cong Wang

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).