Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 10 Jul 2023 15:21:50 -0400
From:   "Liam R. Howlett" <Liam.Howlett@Oracle.com>
To:     Anjali Kulkarni <anjali.k.kulkarni@oracle.com>
Cc:     davem@davemloft.net, akpm@linux-foundation.org, david@fries.net,
        edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
        zbr@ioremap.net, brauner@kernel.org, johannes@sipsolutions.net,
        ecree.xilinx@gmail.com, leon@kernel.org, keescook@chromium.org,
        socketcan@hartkopp.net, petrm@nvidia.com,
        linux-kernel@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH v8 5/6] connector/cn_proc: Allow non-root users access
Message-ID: <20230710192150.vzb66clppgvifj6v@revolver>
References: <20230708021512.3929237-1-anjali.k.kulkarni@oracle.com>
 <20230708021512.3929237-6-anjali.k.kulkarni@oracle.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230708021512.3929237-6-anjali.k.kulkarni@oracle.com>
User-Agent: NeoMutt/20220429
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?qq+WgGt1H/aW0s21vgQsiz/CJxnBNbfBnLjuddsvuDm34S3/Vmc5FYS6XGVx?=
 =?us-ascii?Q?35BPSl48E06M+l8v/Mv8i5JAR6tXKQFc2SWIxptjPdcYbxGVPip75Tv0l/kh?=
 =?us-ascii?Q?99CXJvoyZpX+1jdlRb3YZwhmT8/4qRH8uOkXnjzA8GzHh06r+KqtaD2NX/0r?=
 =?us-ascii?Q?hAR0DzhRoXEOSOrXYf+qjcaLVAUStdopC46CNGG6NQpz0bXtP1ScJ6fRcrfV?=
 =?us-ascii?Q?gW/BSm/U1542chtmmTA+YAnDCW/juBVAHK3bBSwJ0B7pw+KepGDlpiTQbehD?=
 =?us-ascii?Q?XOi5mcp6XXPMcgOCvTCwvOQmFlbqHKLmftf0//emtAgxrAiRr+9niUUGdfys?=
 =?us-ascii?Q?mHq3caS+AEQ01kCkc5d+ZupBJoCxtfP1rwo8dPvcccB1TmNpgq5UXe6HLFpl?=
 =?us-ascii?Q?tao/fklax6hPR7EFJf/H0jjTrCw5Z3ubnwWmBAzkj1zqJPMPUakhj6zJlxXf?=
 =?us-ascii?Q?0gascw+PZUeZ7luVYorNVgftRAi773AxceDUf8zrabG/1tlylQk4d7TTKxh9?=
 =?us-ascii?Q?jYjqzX9jBR7jgpvqgO4UeO3iKkLZ2Z5f9r8oWpQQ9Pu394zzujZ3U8saB6iS?=
 =?us-ascii?Q?ABpTn5Lb7ftTy8HQdXVA57i6TkAa7TEyVLlYqvaIfMkaGAtEpZzOO6jXrazM?=
 =?us-ascii?Q?LUE6XvSnaUn57BsOn04sIBWjvowC5izq8z9lFdUoQX7RFJoGeBsfOzNwSUG0?=
 =?us-ascii?Q?LO783h7+JlxLH4I321Yi94DkJKo++Vo7iNgtxK6C+6tnw9LvJo49IYnSDXES?=
 =?us-ascii?Q?qpGtfr5vgdiSqFs5wO3GKwrlsZY8U4z9hvAWPDQVyVZ2HxQHaJ8ykfjTdGM2?=
 =?us-ascii?Q?LXfUm2dy+2TeNblJN75GJjBhp0k+/t9N00AdXB2Q9an2o8tuRU4e/Xjv7sGU?=
 =?us-ascii?Q?LvGqGXXM6z4lkgXMUtgH0+S8P1QjOce4OGjSPW5jfUs/P9iOCguDKN5auoV4?=
 =?us-ascii?Q?6t7IubEBoR6R/R6jdg/PVOAIpAPMLr9EUxXCkJu6LGGZOhyixrvm9BnYS/HK?=
 =?us-ascii?Q?Lz7QoKEBV+J1TBe3jhuVtdB431epxUEQ5MAdMFvuDpFyTxRJ39E00q04z3/a?=
 =?us-ascii?Q?pbIzNpnXHmTsUPVqs2avc6EUZWPx79uhdkxCyTBA/DzNQf3bpmM8y5f8kqfo?=
 =?us-ascii?Q?4VfnJaJ3z968uGio/5+3I5zbD7gAGZ7kQHCnLJuQOXFa160Mi23MtZIgeiis?=
 =?us-ascii?Q?BtiC5oRVlm742UkQVZ29lGo4bBMv2MwqTnPlZiduiia8F73//rkEmp3v6hRY?=
 =?us-ascii?Q?mG9eeNQKToBAYASi2oFco5OcGYpXsVlL26aHIlo+zDQEseWnT3bvpFxZBXYG?=
 =?us-ascii?Q?t3wXfvE1ZpnaApyOGkXJ6oXWACyGYTdlBJ4OLXmoOJ/G0s/bbaSil+nvG/LA?=
 =?us-ascii?Q?TcDYSphU761GSx9o40zGDAe+DDzaeUmGpdwVan2lhMibaXQfuKLwXvqQGaGs?=
 =?us-ascii?Q?XLYgOGFQvE35LYsQyKbMuVJHBKo3PEVf0mJnc+w6VOsuWDLDSR2sIXEZV1Ib?=
 =?us-ascii?Q?ofBdlX8fl3FA3O81tQM9jdlKPUZmJW5MqC7aBY2wSRPZEvgQNUmN/5qngbSp?=
 =?us-ascii?Q?xCIQXU6H+nZX6vNj/6Ik+IqXzye20jqolMQnahk4Plp1Beo/BIuykeI1RFlu?=
 =?us-ascii?Q?gA=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?us-ascii?Q?16OWR/1An+AxwksKAoYycZgRUhLW47Jtytr6k6QEuLk/wDmHJwCVh30s4imH?=
 =?us-ascii?Q?/WSlHoWfrAF6YSDPZ+as/+LgEZy7CPRM91L3+HFgQCtWZSVojqm7etWVv/Kd?=
 =?us-ascii?Q?xH94xpZZyce2fqPE5GGMjkdp1QLW0wTAHMCo06W0NWnmWsI1ajeSs/LDiZfb?=
 =?us-ascii?Q?FQnj7CyyfBUA7x2kRVTQWoP86CrhHbx/U8rS6xlXMniBCM9fA6rFh+C1dT9v?=
 =?us-ascii?Q?pevAW5uKle/IkiYs3ut3hlnqD3f+daBLijW9hK3vxKkD5bkM1NLXT/Onqix6?=
 =?us-ascii?Q?csGWD4iKKz1RPwl7HuhLeqqfLgRh3ueGNIXjln2uI0G2Y2W64AcIDEWuearm?=
 =?us-ascii?Q?jSCgvb8Rm1z/KZUluAvFJDcLhOHH1uX5bV8IIecl91BUMh6iE39w8SzpYna1?=
 =?us-ascii?Q?1m/GyX4wC714B1zcsuKVL9TH6LGgCUnLSZfgo0q1ony7iHfJtFU9J5lGneIn?=
 =?us-ascii?Q?ljjpF/VWgd7bKT6wLdTKPy646I8DIU3iODDxef6DUYqylw+7gXwQzDST1gWT?=
 =?us-ascii?Q?WZg7dlEKfx5Gioi6E32e4UMgv9RYraJe3RR22DOKPsktiDhdUqHAJlJ6KMyi?=
 =?us-ascii?Q?fMxa4jhGHoQ0iF9SBy//UVvum0IscZB3ScRkPbJCzv7YxjWhxTGQpeel++OS?=
 =?us-ascii?Q?dHcYIU19zTzKQMCW6ZJXTGj7rUIOiPT7ozDrCKSf9esGuQtIUCFwhO6vk0+d?=
 =?us-ascii?Q?3C3T51wrnhqxhiOPO0KCLCzJZp99EUeXx5rL7e/34mI+znU9poFGgJAL7yFf?=
 =?us-ascii?Q?RnVqvspXIiIZPXXYGASoTdN06pQODpx/M37PYSnCRsa/Zitd29bJb1o7eK38?=
 =?us-ascii?Q?i+LauibefdfAy23yHnJP5GMadxf7bmKg4htYhng22VsVnAgI73HqsyakjCgI?=
 =?us-ascii?Q?BzpIXZ9kUvk4WCt0C3U+9nEeQWVe8ZFCSNN8LVSBXUJk4LLKgw53u8AmVJ/+?=
 =?us-ascii?Q?OB4NTXltrkS06YrVWhQFfXnqAILI8B6Z0lu47rlZwXnWHqP4sgA/A/zbVdJM?=
 =?us-ascii?Q?Vvp9cySYXVToNQcighE6VSR3HWnRIQ6UVdwP3QcKKQSDnD2yDfTbT3mqoZ7W?=
 =?us-ascii?Q?qiPf6cwCnV70tCOtV0r6dNtcDUIM1x1q+v4LTpgq4Z3plAxRU5pu1Hw6IGJ0?=
 =?us-ascii?Q?xXWvsdc+fSMu1KzNVdtC+hRpV3y2H1Uf9NA0RnUzUuVZgeN8fRHpcOE=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 6a28c8c7-86c4-4639-01eb-08db817aebe2
X-MS-Exchange-CrossTenant-AuthSource: SN6PR10MB3022.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Jul 2023 19:21:54.9053
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 00wmHOeTC0b/J+g5zkexfNfsh17eQAeIq0KAf8tUVUxFEb9frRvzZMcWoSXqx2doZ+U3LntmjkBcbaXfOWhndg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR10MB7332
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.591,FMLib:17.11.176.26
 definitions=2023-07-10_14,2023-07-06_02,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 mlxlogscore=999
 adultscore=0 mlxscore=0 suspectscore=0 phishscore=0 bulkscore=0
 malwarescore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2305260000 definitions=main-2307100175
X-Proofpoint-GUID: x0n1mThuQwLdRCufg0HJaqbxwJUdiRH0
X-Proofpoint-ORIG-GUID: x0n1mThuQwLdRCufg0HJaqbxwJUdiRH0
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

* Anjali Kulkarni <anjali.k.kulkarni@oracle.com> [230707 22:15]:
> 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>

Thanks, looks good!

Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>

> ---
>  drivers/connector/cn_proc.c   |  6 ------
>  drivers/connector/connector.c | 19 +++++++++++++++++++
>  2 files changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
> index dfc84d44f804..05d562e9c8b1 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;
> 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
>