Messages in this thread | | | From | Anjali Kulkarni <> | Subject | Re: [PATCH v7 5/6] connector/cn_proc: Allow non-root users access | Date | Thu, 6 Jul 2023 21:00:27 +0000 |
| |
> On Jul 6, 2023, at 1:16 PM, Liam Howlett <liam.howlett@oracle.com> wrote: > > * Anjali Kulkarni <anjali.k.kulkarni@oracle.com> [691231 23:00]: >> There were a couple of reasons for not allowing non-root users access >> initially - one is there was some point no proper receive buffer >> management in place for netlink multicast. But that should be long >> fixed. See link below for more context. >> >> Second is that some of the messages may contain data that is root only. But >> this should be handled with a finer granularity, which is being done at the >> protocol layer. The only problematic protocols are nf_queue and the >> firewall netlink. Hence, this restriction for non-root access was relaxed >> for NETLINK_ROUTE initially: >> https://lore.kernel.org/all/20020612013101.A22399@wotan.suse.de/ >> >> This restriction has also been removed for following protocols: >> NETLINK_KOBJECT_UEVENT, NETLINK_AUDIT, NETLINK_SOCK_DIAG, >> NETLINK_GENERIC, NETLINK_SELINUX. >> >> Since process connector messages are not sensitive (process fork, exit >> notifications etc.), and anyone can read /proc data, we can allow non-root >> access here. However, since process event notification is not the only >> consumer of NETLINK_CONNECTOR, we can make this change even more >> fine grained than the protocol level, by checking for multicast group >> within the protocol. >> >> Allow non-root access for NETLINK_CONNECTOR via NL_CFG_F_NONROOT_RECV >> but add new bind function cn_bind(), which allows non-root access only >> for CN_IDX_PROC multicast group. >> >> Signed-off-by: Anjali Kulkarni <anjali.k.kulkarni@oracle.com> >> --- >> drivers/connector/cn_proc.c | 7 ------- >> drivers/connector/connector.c | 19 +++++++++++++++++++ >> 2 files changed, 19 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c >> index dfc84d44f804..bb1fa5d66cf9 100644 >> --- a/drivers/connector/cn_proc.c >> +++ b/drivers/connector/cn_proc.c >> @@ -410,12 +410,6 @@ static void cn_proc_mcast_ctl(struct cn_msg *msg, >> !task_is_in_init_pid_ns(current)) >> return; >> >> - /* Can only change if privileged. */ >> - if (!__netlink_ns_capable(nsp, &init_user_ns, CAP_NET_ADMIN)) { >> - err = EPERM; >> - goto out; >> - } >> - >> if (msg->len == sizeof(*pinput)) { >> pinput = (struct proc_input *)msg->data; >> mc_op = pinput->mcast_op; >> @@ -467,7 +461,6 @@ static void cn_proc_mcast_ctl(struct cn_msg *msg, >> break; >> } >> >> -out: > > This label is still in use from your changes in patch 3.
Yes, will send updated patch with this fixed.
> >> cn_proc_ack(err, msg->seq, msg->ack); >> } >> >> diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c >> index d1179df2b0ba..7f7b94f616a6 100644 >> --- a/drivers/connector/connector.c >> +++ b/drivers/connector/connector.c >> @@ -166,6 +166,23 @@ static int cn_call_callback(struct sk_buff *skb) >> return err; >> } >> >> +/* >> + * Allow non-root access for NETLINK_CONNECTOR family having CN_IDX_PROC >> + * multicast group. >> + */ >> +static int cn_bind(struct net *net, int group) >> +{ >> + unsigned long groups = (unsigned long) group; >> + >> + if (ns_capable(net->user_ns, CAP_NET_ADMIN)) >> + return 0; >> + >> + if (test_bit(CN_IDX_PROC - 1, &groups)) >> + return 0; >> + >> + return -EPERM; >> +} >> + >> static void cn_release(struct sock *sk, unsigned long *groups) >> { >> if (groups && test_bit(CN_IDX_PROC - 1, groups)) { >> @@ -261,6 +278,8 @@ static int cn_init(void) >> struct netlink_kernel_cfg cfg = { >> .groups = CN_NETLINK_USERS + 0xf, >> .input = cn_rx_skb, >> + .flags = NL_CFG_F_NONROOT_RECV, >> + .bind = cn_bind, >> .release = cn_release, >> }; >> >> -- >> 2.41.0
| |