netdev - Re: [PATCH net 2/2] drop_monitor: Require 'CAP_SYS

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <cc284e70-23c9-463a-8ac2-9a4df8e03374@intel.com>
Date: Wed, 6 Dec 2023 15:19:37 -0800
From: Jacob Keller <jacob.e.keller@...el.com>
To: Ido Schimmel <idosch@...dia.com>, <netdev@...r.kernel.org>
CC: <davem@...emloft.net>, <kuba@...nel.org>, <pabeni@...hat.com>,
	<edumazet@...gle.com>, <nhorman@...driver.com>, <yotam.gi@...il.com>,
	<jiri@...nulli.us>, <johannes@...solutions.net>, <horms@...nel.org>,
	<andriy.shevchenko@...ux.intel.com>, <jhs@...atatu.com>
Subject: Re: [PATCH net 2/2] drop_monitor: Require 'CAP_SYS_ADMIN' when
 joining "events" group



On 12/6/2023 1:31 PM, Ido Schimmel wrote:
> The "NET_DM" generic netlink family notifies drop locations over the
> "events" multicast group. This is problematic since by default generic
> netlink allows non-root users to listen to these notifications.
> 
> Fix by adding a new field to the generic netlink multicast group
> structure that when set prevents non-root users or root without the
> 'CAP_SYS_ADMIN' capability (in the user namespace owning the network
> namespace) from joining the group. Set this field for the "events"
> group. Use 'CAP_SYS_ADMIN' rather than 'CAP_NET_ADMIN' because of the
> nature of the information that is shared over this group.
> 
> Note that the capability check in this case will always be performed
> against the initial user namespace since the family is not netns aware
> and only operates in the initial network namespace.
> 
> A new field is added to the structure rather than using the "flags"
> field because the existing field uses uAPI flags and it is inappropriate
> to add a new uAPI flag for an internal kernel check. In net-next we can
> rework the "flags" field to use internal flags and fold the new field
> into it. But for now, in order to reduce the amount of changes, add a
> new field.
> 
> Since the information can only be consumed by root, mark the control
> plane operations that start and stop the tracing as root-only using the
> 'GENL_ADMIN_PERM' flag.
> 

Makes sense.

Reviewed-by: Jacob Keller <jacob.e.keller@...el.com>