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.

>
> 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.

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



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux