RE: [PATCH 1/2] fs: Check f_cred instead of current's creds in should_remove_suid()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> On Wed, Jan 25, 2017 at 1:43 PM, Ben Hutchings <ben@xxxxxxxxxxxxxxx>
> wrote:
> > On Wed, 2017-01-25 at 13:06 -0800, Andy Lutomirski wrote:
> >> If an unprivileged program opens a setgid file for write and passes
> >> the fd to a privileged program and the privileged program writes to
> >> it, we currently fail to clear the setgid bit.  Fix it by checking
> >> f_cred instead of current's creds whenever a struct file is involved.
> > [...]
> >
> > What if, instead, a privileged program passes the fd to an un
> > unprivileged program?  It sounds like a bad idea to start with, but at
> > least currently the unprivileged program is going to clear the setgid
> > bit when it writes.  This change would make that behaviour more
> > dangerous.
> 
> Hmm.  Although, if a privileged program does something like:
> 
> (sudo -u nobody echo blah) >setuid_program
> 
> presumably it wanted to make the change.

I'm not following all the intricacies here, though I need to...

What about a privileged program that drops privilege for certain operations?

Specifically the Ganesha user space NFS server runs as root, but sets fsuid/fsgid for specific threads performing I/O operations on behalf of NFS clients.

I want to make sure setgid bit handling is proper for these cases.

Ganesha does some permission checking, but this is one area I want to defer to the underlying  filesystem because it's not easy for Ganesha to get it right.

> > Perhaps there should be a capability check on both the current
> > credentials and file credentials?  (I realise that we've considered
> > file credential checks to be sufficient elsewhere, but those cases
> > involved virtual files with special semantics, where it's clearer that
> > a privileged process should not pass them to an unprivileged process.)
> >
> 
> I could go either way.
> 
> What I really want to do is to write a third patch that isn't for -stable that just
> removes the capable() check entirely.  I'm reasonably confident it won't
> break things for a silly reason: because it's capable() and not ns_capable(),
> anything it would break would also be broken in an unprivileged container,
> and I haven't seen any reports of package managers or similar breaking for
> this reason.

Frank


---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]