Re: [RFC:PATCH] How best to handle implicit clearing of setuid/setgid bits on NFS?

[Jeff Layton <jlayton@redhat.com>]

On Thu, 28 Jun 2007 09:38:22 -0400
Trond Myklebust <trond.myklebust@fys.uio.no> wrote:

> On Wed, 2007-06-27 at 22:13 -0400, Jeff Layton wrote:
> > Ok. This is a bit more complex now since we remove suid bits on
> > truncate, but don't set ATTR_FORCE.
> > 
> > Here's a patch that should do this. I know there's a general
> > aversion to adding new flags to vfs structures, but I couldn't think of
> > a way to cleanly do this without adding one.
> > 
> > Note that I've not tested this patch at all so this is just a RFC.
> > 
> > CC'ing Al here since he's expressed interest in this problem as well.
> > 
> > Thoughts?
> 
> We don't really need to do this with extra VFS flags. Here is my
> preferred approach for dealing with this problem.
> 
>         http://article.gmane.org/gmane.linux.nfs/8511/match=attr%5fkill%5fsuid
> 
> As you can see, that still allows the filesystem to determine how it
> should deal with the ATTR_KILL_SUID/ATTR_KILL_SGID flags. The default
> behaviour is provided by inode_setattr(), and is the same as today. Only
> filesystems that don't use inode_setattr() will need to be audited for
> whether or not they need a fix.
> 
> Cheers,
>   Trond


I had a look at this today, and I have some reservations about this
approach. Quite a few filesystems define a .setattr inode op, but don't
call inode_setattr. As a first pass through the current git tree, the
following setattr ops never seem to call inode_setattr:

adfs_notify_change
afs_setattr
coda_setattr
ecryptfs_setattr
fuse_setattr
jffs2_setattr
nfs_setattr (expected)
ntfs_setattr
smb_notify_change
xfs_vn_setattr

...some of these won't matter, but some will need to be fixed. There
may be other situations we'll need to fix as well.

My concern here is that we'll be moving from a "default safe" model for
filesystems that define a .setattr op to one where those filesystems
are expected to make sure that they clear setuid/gid bits. They can do
this with a simple call to inode_setattr, or on their own, but they
must do it. Is this really the right thing to do here?

It seems like an "opt-out" approach when clearing these bits might be
safer. i.e., make it so that filesystems have to explicitly disable
clearing of setuid/setgid bits.

Thoughts?

-- 
Jeff Layton <jlayton@redhat.com>

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs