Skip to content

Commit e037818

Browse files
idoschkuba-moo
authored andcommitted
drop_monitor: Require 'CAP_SYS_ADMIN' when joining "events" group
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: 9a8afc8 ("Network Drop Monitor: Adding drop monitor implementation & Netlink protocol") Reported-by: "The UK's National Cyber Security Centre (NCSC)" <[email protected]> Signed-off-by: Ido Schimmel <[email protected]> Reviewed-by: Jacob Keller <[email protected]> Reviewed-by: Jiri Pirko <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent 44ec98e commit e037818

File tree

3 files changed

+8
-1
lines changed

3 files changed

+8
-1
lines changed

include/net/genetlink.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,12 @@
1212
* struct genl_multicast_group - generic netlink multicast group
1313
* @name: name of the multicast group, names are per-family
1414
* @flags: GENL_* flags (%GENL_ADMIN_PERM or %GENL_UNS_ADMIN_PERM)
15+
* @cap_sys_admin: whether %CAP_SYS_ADMIN is required for binding
1516
*/
1617
struct genl_multicast_group {
1718
char name[GENL_NAMSIZ];
1819
u8 flags;
20+
u8 cap_sys_admin:1;
1921
};
2022

2123
struct genl_split_ops;

net/core/drop_monitor.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ static struct sk_buff *reset_per_cpu_data(struct per_cpu_dm_data *data)
183183
}
184184

185185
static const struct genl_multicast_group dropmon_mcgrps[] = {
186-
{ .name = "events", },
186+
{ .name = "events", .cap_sys_admin = 1 },
187187
};
188188

189189
static void send_dm_alert(struct work_struct *work)
@@ -1619,11 +1619,13 @@ static const struct genl_small_ops dropmon_ops[] = {
16191619
.cmd = NET_DM_CMD_START,
16201620
.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
16211621
.doit = net_dm_cmd_trace,
1622+
.flags = GENL_ADMIN_PERM,
16221623
},
16231624
{
16241625
.cmd = NET_DM_CMD_STOP,
16251626
.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
16261627
.doit = net_dm_cmd_trace,
1628+
.flags = GENL_ADMIN_PERM,
16271629
},
16281630
{
16291631
.cmd = NET_DM_CMD_CONFIG_GET,

net/netlink/genetlink.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1691,6 +1691,9 @@ static int genl_bind(struct net *net, int group)
16911691
if ((grp->flags & GENL_UNS_ADMIN_PERM) &&
16921692
!ns_capable(net->user_ns, CAP_NET_ADMIN))
16931693
ret = -EPERM;
1694+
if (grp->cap_sys_admin &&
1695+
!ns_capable(net->user_ns, CAP_SYS_ADMIN))
1696+
ret = -EPERM;
16941697

16951698
break;
16961699
}

0 commit comments

Comments
 (0)