Re: [PATCH] RDMA/uverbs: Fix CAP_NET_RAW check for flow create in user namespace

["Serge E. Hallyn" <serge@hallyn.com>]

On Sat, Mar 08, 2025 at 08:06:02PM +0200, Parav Pandit wrote:
> A process running in a non-init user namespace possesses the
> CAP_NET_RAW capability. However, the patch cited in the fixes
> tag checks the capability in the default init user namespace.
> Because of this, when the process was started by Podman in a
> non-default user namespace, the flow creation failed.
> 
> Fix this issue by checking the CAP_NET_RAW networking capability
> in the owner user namespace that created the network namespace.

Hi,

you say

 > Fix this issue by checking the CAP_NET_RAW networking capability
 > in the owner user namespace that created the network namespace.

But in fact you are checking the CAP_NET_RAW against the user's
network namespace.  That is usually not the same thing, although
it is possible that in this case it is.

What is cmd.flow_id?  Is that guaranteed to represent something in
the current process' network namespace?  Or is it possible that a
user without privilege in his user namespace could unshare userns+netns
but then cause this fn to be called against a flow in the original
network namespace?

> 
> This change is similar to the following cited patches.
> 
> commit 5e1fccc0bfac ("net: Allow userns root control of the core of the network stack.")
> commit 52e804c6dfaa ("net: Allow userns root to control ipv4")
> commit 59cd7377660a ("net: openvswitch: allow conntrack in non-initial user namespace")
> commit 0a3deb11858a ("fs: Allow listmount() in foreign mount namespace")
> commit dd7cb142f467 ("fs: relax permissions for listmount()")
> 
> Fixes: c938a616aadb ("IB/core: Add raw packet QP type")
> Signed-off-by: Parav Pandit <parav@nvidia.com>
> 
> ---
> I would like to have feedback from the LSM experts to make sure this
> fix is correct. Given the widespread usage of the capable() call,
> it makes me wonder if the patch right.
> 
> Secondly, I wasn't able to determine which primary namespace (such as
> mount or IPC, etc.) to consider for the CAP_IPC_LOCK capability.
> (not directly related to this patch, but as concept)
> ---
>  drivers/infiniband/core/uverbs_cmd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
> index 5ad14c39d48c..8d6615f390f5 100644
> --- a/drivers/infiniband/core/uverbs_cmd.c
> +++ b/drivers/infiniband/core/uverbs_cmd.c
> @@ -3198,7 +3198,7 @@ static int ib_uverbs_ex_create_flow(struct uverbs_attr_bundle *attrs)
>  	if (cmd.comp_mask)
>  		return -EINVAL;
>  
> -	if (!capable(CAP_NET_RAW))
> +	if (!ns_capable(current->nsproxy->net_ns->user_ns, CAP_NET_RAW))
>  		return -EPERM;
>  
>  	if (cmd.flow_attr.flags >= IB_FLOW_ATTR_FLAGS_RESERVED)
> -- 
> 2.26.2
>