[PATCH] RDMA/uverbs: Consider capability of the process that opens the file

[Serge E. Hallyn]

Hi Jason,

On Tue, Apr 22, 2025 at 09:46:40AM -0300, Jason Gunthorpe wrote:
> On Mon, Apr 21, 2025 at 12:22:36PM -0500, Serge E. Hallyn wrote:
> > > > 1. the create should check ns_capable(current->nsproxy->net->user_ns,
> > > > CAP_NET_RAW) 
> > > I believe this is sufficient as this create call happens through the ioctl().
> > > But more question on #3.
> 
> I think this is the right one to use everywhere.

It's the right one to use when creating resources, but when later using
them, since below you say that the resource should in fact be tied to
the creator's network namespace, that means that checking
current->nsproxy->net->user_ns would have nothing to do with the
resource being used, right?

> > > > 3. the ioctl should check file_ns_capable(attrs->ufile->filp->f_cred->user_ns,
> > > > CAP_NET_RAW)
> > > > 
> > > > Two notes about (3).  First, note that it's different from what you had.
> > > > It explicitly checks that the caller has CAP_NET_RAW against the net
> > > > namespace that was used to open the file.  
> > > How is the net namespace linked in #3?
> > > Is it because when file was opened, the rdma device was accessible in a given net ns?
> > > But again the net ns explicitly not accessed in #3.
> > 
> > I'll have to look around and see if we can deduce the netns from elsewhere,
> > the device perhaps.  But IIUC the file's user_ns should be the one for
> > which we checked that it has CAP_NET_RAW over the actual net->user_ns,
> > so if you have CAP_NET_RAW in that user_ns, then you're good.  Where it
> > *could* get wonky is if the opener was in a parent userns of the net->userns.
> > In that case the file's userns will be sufficient to access the net, but
> > we could end up denying access from a privileged process in its child
> > user_ns, that is, potentially, the net->userns.
> 
> We should never be taking any security check from the struct file.
> 
> All security checks are only done on current in rdma, the context of
> the file opener must contribute nothing. The file opener could have
> had more privilege than the child process somehow, and that extra
> privilege should not leak into the child.
> 
> Even in goofy cases like passing a FD between processes with different
> net namespaces, the expectation is that objects can be created
> relative to net namespace of the process calling the ioctl, and then
> accessed by the other process in the other namespace.

So when earlier it was said that uverbs was switching from read/write
to ioctl so that permissions could be checked, that is not actually
the case?  The intent is for a privileged task to create the
resource and be able to pass it to any task in any namespace with any
or no privilege and have that task be able to use it with the
opener's original privilege, just as with read/write?

> Objects should and do become bound to the net namespaces that created
> them, just not the FD.

I was trying last night to track down where the uverb ioctls are doing 
permission checks, but failing to find it.  I see where the
pbundle->method_elm->handler gets dereferenced, but not where those
are defined.