[linux-cifs-client] review 5, was Re: projected date for mount.cifs to support DFS junction points

[Jeff Layton]

On Mon, 10 Mar 2008 22:34:35 -0500
"Steve French" <smfrench at gmail.com> wrote:

> On Sat, Mar 8, 2008 at 1:43 PM, Christoph Hellwig <hch at infradead.org> wrote:
> > On Mon, Feb 25, 2008 at 02:25:50PM -0600, Steve French wrote:
> >  > On Fri, Feb 15, 2008 at 4:11 PM, Christoph Hellwig <hch at infradead.org> wrote:
> >  Okay, now that we have the basic consolidation in can I get you to
> >  review force_uid_gid paramter to cifs_unix_info_to_inode?  It seems
> >  more than fishy to me to ignore the CIFS_MOUNT_OVERR_{UID,GID} options
> >  in cifs_get_inode_info_unix but not in posix_fill_in_inode.  This
> >  seems more like a missed propagation of changes for a newly added
> >  feature to me.  If not it should at least get some documentation.
> 
> Jeff Layton and I have been discussing the issue of which uid to use
> to fill in the inode->i_uid and I am leaning toward changing the
> default behavior (for the mount to Windows server case) and adding
> another mount parm to allow users to get the older behavior if they
> want.
> 
> Unfortunately there are many cases (and the uid/gid owner fields of
> inodes can get filled in potentially in various places
> mkdir/create/mknod and lookup and readdir and of course setattr).  The
> mount could be to:
>
> 1) server which support returning uid/gid owner fields for an inode
> (e.g. Samba) on QueryPathInfo
> 2) servers which would support returning a uid, but for which the uids
> on the server don't match the client
> 3) servers like Windows which don't support returning the inodes uid
> 
> and for the latter we also have the case of files/directories for
> which the user has chowned it so we know what uid the app thinks it
> should be (or newly created files/directories)
> 
> Some of this is documented, and I have started a table which needs to
> be added to the CIFS User's guide - but the case I am most worried
> about at the moment is the behavior when the server would support the
> Unix extensions - but the user has overridden the uid or gid
> (specified on mount, perhaps because the server and client's uids
> differ).   For this case should we always use the default uid - or
> should an app be allowed to do a chmod and should we honor the uid/gid
> passed in on the mkdir/create/mknod (as we do for the equivalent
> windows case).
> 

Here's what I'd like to see...

The best option here is to have a new upcall that does idmapping to map
windows RIDs to unix UIDs. It wouldn't be too hard to do and I have it
on my to-do list, but it's pretty far down and it'll be a while before
I can get started on it. Even with that though, we'll need sensible
defaults for when the upcall doesn't work for some reason.

In the meantime we have some pretty messy inconsistencies, particularly
when POSIX extensions aren't supported. CIFS sets the in-memory inode's
mode to be consistent with the mkdir/open call, but the ownership is
set to whatever the default uid/gid is for the mount. This makes some
apps work (at least for a little while), but causes other problems. For
instance, someone can create a directory with a restrictive mode but
then can't actually write to it because they don't own it.

This also seems scary to me from a security standpoint. We're basically
allowing someone to create a file with an arbitrary mode that is owned
by a different user. That user is generally root by default.

We have 2 separate but related pieces of info to deal with:

1) uid/gid: for this I'd like to see an idmapping upcall. When that
info isn't available (daemon is down or no mapping exists), then we'd
fall back to using the "default" uid/gid for the mount. This should be
the behavior regardless of whether we have unix extensions enabled or
not. Right now, we trust that when unix extensions are enabled that we 
have unix uid/gid mapped to the same things on all machines. This isn't
necessarily the case.

2) mode: for this we have 3 cases. When cifsacl is specified, we'd
use the mode obtained via them. If not, then when unix extensions are
supported, we should use the mode obtained via them. Otherwise, the mode
should be governed by the file_mode/dir_mode of the share.

At the same time, we should also consider tightening up the default
file_mode/dir_mode. Right now it's 02767 (I think). We should change
that to be 0700, and allow admins to loosen it if they wish.

The current approach of allowing users to have different info in
in-memory inodes vs. what's recorded on the server seems very
problematic to me. This is something that leads to non-deterministic
behavior and that's worse than just breaking some apps outright.

Just my 2 lumps of copper and nickel...
-- 
Jeff Layton <jlayton at redhat.com>