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

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

On Tue, Apr 22, 2025 at 01:11:27PM -0300, Jason Gunthorpe wrote:
> On Tue, Apr 22, 2025 at 08:14:33AM -0500, Serge E. Hallyn wrote:
> > 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?
> 
> Yes, in that case you'd check something stored in the uobject.

Perfect, that's exactly the kind of thing I was looking for.  Thanks.

> This happens sort of indirectly, for instance an object may become
> associated with a netdevice and the netdevice is linked to a net
> namespace. Eg we should do route lookups relative to that associated
> net devices's namespaces.
> 
> I'm not sure we have a capable like check like that though.
> 
> > > 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? 
> 
> I don't quite know what you mean here?
> 
> read/write has a security problem in that you can pass a FD to a
> setuid program as its stdout and have that setuid program issue a
> write() to trigger a kernel operation using it's elevated
> privilege. This is not possible with ioctl.
> 
> When this bug was discovered the read/write path started calling
> ib_safe_file_access() which blanket disallows *any* credential change
> from open() to write().
> 
> ioctl removes this excessive restriction and we are back to
> per-process checks.
> 
> > 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?
> 
> Yes. The permissions affiliate with the object contained inside the
> FD, not the FD itself. The FD is just a container and a way to route
> system calls.
> 
> > 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.
> 
> There are very few permission checks. Most boil down to implicit
> things, like we have a netdevice relative to current's net namespace
> and we need to find a gid table index for that netdevice. We don't
> actually need to do anything special here as the ifindex code
> automatically validates the namespaces and struct net_device * are
> globally unique.
> 
> Similarly with route lookups and things, once we validated the net
> device objects are supposed to remain bound to it.
> 
> The cases like cap_net_raw are one time checks at creation time that
> modify the devices' rules for processing the queues. The devices check
> the creation property of the queue when processing the queue.

Thank you for the detailed explanation.

-serge