This is a note to let you know that I've just added the patch titled
drop_monitor: Require 'CAP_SYS_ADMIN' when joining "events" group
to the 5.10-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
drop_monitor-require-cap_sys_admin-when-joining-events-group.patch
and it can be found in the queue-5.10 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.
>From stable+bounces-5296-greg=kroah.com@xxxxxxxxxxxxxxx Mon Dec 11 13:39:14 2023
From: Ido Schimmel <idosch@xxxxxxxxxx>
Date: Mon, 11 Dec 2023 14:37:40 +0200
Subject: drop_monitor: Require 'CAP_SYS_ADMIN' when joining "events" group
To: <stable@xxxxxxxxxxxxxxx>
Cc: <davem@xxxxxxxxxxxxx>, <kuba@xxxxxxxxxx>, <pabeni@xxxxxxxxxx>, <edumazet@xxxxxxxxxx>, <nhorman@xxxxxxxxxxxxx>, <yotam.gi@xxxxxxxxx>, <sashal@xxxxxxxxxx>, <fw@xxxxxxxxx>, <jacob.e.keller@xxxxxxxxx>, <jiri@xxxxxxxxxx>
Message-ID: <20231211123740.822802-5-idosch@xxxxxxxxxx>
From: Ido Schimmel <idosch@xxxxxxxxxx>
commit e03781879a0d524ce3126678d50a80484a513c4b upstream.
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.
Tested using [1].
Before:
# capsh -- -c ./dm_repo
# capsh --drop=cap_sys_admin -- -c ./dm_repo
After:
# capsh -- -c ./dm_repo
# capsh --drop=cap_sys_admin -- -c ./dm_repo
Failed to join "events" multicast group
[1]
$ cat dm.c
#include <stdio.h>
#include <netlink/genl/ctrl.h>
#include <netlink/genl/genl.h>
#include <netlink/socket.h>
int main(int argc, char **argv)
{
struct nl_sock *sk;
int grp, err;
sk = nl_socket_alloc();
if (!sk) {
fprintf(stderr, "Failed to allocate socket\n");
return -1;
}
err = genl_connect(sk);
if (err) {
fprintf(stderr, "Failed to connect socket\n");
return err;
}
grp = genl_ctrl_resolve_grp(sk, "NET_DM", "events");
if (grp < 0) {
fprintf(stderr,
"Failed to resolve \"events\" multicast group\n");
return grp;
}
err = nl_socket_add_memberships(sk, grp, NFNLGRP_NONE);
if (err) {
fprintf(stderr, "Failed to join \"events\" multicast group\n");
return err;
}
return 0;
}
$ gcc -I/usr/include/libnl3 -lnl-3 -lnl-genl-3 -o dm_repo dm.c
Fixes: 9a8afc8d3962 ("Network Drop Monitor: Adding drop monitor implementation & Netlink protocol")
Reported-by: "The UK's National Cyber Security Centre (NCSC)" <security@xxxxxxxxxxx>
Signed-off-by: Ido Schimmel <idosch@xxxxxxxxxx>
Reviewed-by: Jacob Keller <jacob.e.keller@xxxxxxxxx>
Reviewed-by: Jiri Pirko <jiri@xxxxxxxxxx>
Link: https://lore.kernel.org/r/20231206213102.1824398-3-idosch@xxxxxxxxxx
Signed-off-by: Jakub Kicinski <kuba@xxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
include/net/genetlink.h | 2 ++
net/core/drop_monitor.c | 4 +++-
net/netlink/genetlink.c | 3 +++
3 files changed, 8 insertions(+), 1 deletion(-)
--- a/include/net/genetlink.h
+++ b/include/net/genetlink.h
@@ -11,10 +11,12 @@
/**
* struct genl_multicast_group - generic netlink multicast group
* @name: name of the multicast group, names are per-family
+ * @cap_sys_admin: whether %CAP_SYS_ADMIN is required for binding
*/
struct genl_multicast_group {
char name[GENL_NAMSIZ];
u8 flags;
+ u8 cap_sys_admin:1;
};
struct genl_ops;
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -183,7 +183,7 @@ out:
}
static const struct genl_multicast_group dropmon_mcgrps[] = {
- { .name = "events", },
+ { .name = "events", .cap_sys_admin = 1 },
};
static void send_dm_alert(struct work_struct *work)
@@ -1616,11 +1616,13 @@ static const struct genl_small_ops dropm
.cmd = NET_DM_CMD_START,
.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
.doit = net_dm_cmd_trace,
+ .flags = GENL_ADMIN_PERM,
},
{
.cmd = NET_DM_CMD_STOP,
.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
.doit = net_dm_cmd_trace,
+ .flags = GENL_ADMIN_PERM,
},
{
.cmd = NET_DM_CMD_CONFIG_GET,
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -1387,6 +1387,9 @@ static int genl_bind(struct net *net, in
if ((grp->flags & GENL_UNS_ADMIN_PERM) &&
!ns_capable(net->user_ns, CAP_NET_ADMIN))
ret = -EPERM;
+ if (grp->cap_sys_admin &&
+ !ns_capable(net->user_ns, CAP_SYS_ADMIN))
+ ret = -EPERM;
break;
}
Patches currently in stable-queue which might be from kroah.com@xxxxxxxxxxxxxxx are
queue-5.10/psample-require-cap_net_admin-when-joining-packets-group.patch
queue-5.10/drop_monitor-require-cap_sys_admin-when-joining-events-group.patch
queue-5.10/netlink-don-t-call-netlink_bind-with-table-lock-held.patch
queue-5.10/genetlink-add-cap_net_admin-test-for-multicast-bind.patch
