2005-11-28 16:51:51

by Steve French

[permalink] [raw]
Subject: inode_change_ok

Why are there no calls to inode_change_ok in nfs (on the client), but
there are in most other filesytsems? Seems like there are some cases
in nfs in which a local permission check is done via a call to
nfs_permission which calls generic_permission ... if that is the case
why not do a call to inode_change_ok in similar cases?

For the case of cifs vfs, which is also missing this call - I was
thinking of adding something like:
if (!cifs_sb->mnt_cifs_flags & CIFS_MOUNT_NO_PERM)
inode_change_ok(direntry->d_inode, attrs);
to fs/cifs/inode.c near the beginning of cifs_setattr. Although the
permissions (actually ACLs) are checked on the server during setattr
from cifs to Samba or cifs servers such as Windows, it is common for
convenience for users to mount with one id, rather than authenticating
each user (so that there are multiple smb uids) in which case the
permission check on setattr on the client can be important since
apparently the ".permission" entry point does not seem to get invoked in
the chown/chmod path.


2005-11-28 17:06:45

by Trond Myklebust

[permalink] [raw]
Subject: Re: inode_change_ok

On Mon, 2005-11-28 at 11:48 -0600, Steve French wrote:
> Why are there no calls to inode_change_ok in nfs (on the client), but
> there are in most other filesytsems? Seems like there are some cases
> in nfs in which a local permission check is done via a call to
> nfs_permission which calls generic_permission ... if that is the case
> why not do a call to inode_change_ok in similar cases?

Under the NFS model, the server manages the permissions, not the client.

The purpose of inode_change_ok() is to perform a load of local checks
which are simply alien to that model:

a) your capabilities don't mean anything to the server. Its decision to
grant the ability to change owner of a file is based on your
credentials, not your capabilities.

b) Even the uid/gid checks don't take into account the fact that the
server may be mapping you into different users/groups (c.f. root
squashing etc.).

All, in all, a call to inode_change_ok() would at best be redundant, and
at worst, be plain incorrect.

Cheers,
Trond

2005-11-28 17:15:31

by Trond Myklebust

[permalink] [raw]
Subject: Re: inode_change_ok

On Mon, 2005-11-28 at 12:06 -0500, Trond Myklebust wrote:
> On Mon, 2005-11-28 at 11:48 -0600, Steve French wrote:
> > Why are there no calls to inode_change_ok in nfs (on the client), but
> > there are in most other filesytsems? Seems like there are some cases
> > in nfs in which a local permission check is done via a call to
> > nfs_permission which calls generic_permission ... if that is the case
> > why not do a call to inode_change_ok in similar cases?
>
> Under the NFS model, the server manages the permissions, not the client.
>
> The purpose of inode_change_ok() is to perform a load of local checks
> which are simply alien to that model:
>
> a) your capabilities don't mean anything to the server. Its decision to
> grant the ability to change owner of a file is based on your
> credentials, not your capabilities.
>
> b) Even the uid/gid checks don't take into account the fact that the
> server may be mapping you into different users/groups (c.f. root
> squashing etc.).
>
> All, in all, a call to inode_change_ok() would at best be redundant, and
> at worst, be plain incorrect.

BTW: The call to permission() is thoroughly redundant too, and really
wants to be optimised away. Permissions checking is done by the server
on the actual SETATTR RPC call...

Cheers,
Trond

2005-11-28 20:08:35

by Steve French

[permalink] [raw]
Subject: Re: inode_change_ok

Trond Myklebust wrote:

>CAP_CHOWN would therefore only be useful in the corner case where the
>server is allowing full root privileges to a client, and where the
>client is using capabilities to limit the root account's (or a setuid
>processes') ability to exercise those full privileges.
>
>Cheers,
> Trond
>
>
>
>
It sounds like I do need to make the change to cifs to add the optional
(based on the mount parm the admin can choose which model) call to
inode_change_ok to make the permissions check consistent when "noperm"
is not enabled on that mount , but I was trying to figure out how
important that change to the cifs code was and whether it was a priority
to get in. I realize that this corner case is far less common in the
wild for nfs. The corner case you describe above did seem to come up
today for cifs though (where the admin had in effect, trusted the
client, and allowed the client to mount as root with full root priv on
the server - I can imagine this being valid in their particular case,
with a trusted client os, over a trusted lan, but obviously would not be
the most common scenario). Obviously they did not turn on
multiusermount in /proc/fs/cifs (ie the ability to send different uids
on the wire depending on the uid of the client calling process) so the
only perm check that would be useful that was occuring at the client the
way they had chosen to configure/mount was happening in
generic_permission in the client. In their particular configuration
open and other paths in the vfs that call permission behaved fine and
the permission checks worked as expected - but chown always worked
(which is of course wrong) since the vfs does not call permission but
expects the vfs (if at all) to call inode_change_ok - so open failing,
but chown working was confusing to them.

Of course it does not make it much easier trying to compare with other
filesystems - ncpfs does seem to call both (generic_permission
inode_change_ok) and but the afs model (openafs) is far more complicated
and somewhat confusing (and interestingly afs does not call
generic_permission but does call inode_change_ok) so it is not really
possible to draw too many conclusions for how afs approached the same
two usage models (apparently afs also had a third usage model based on
machine address)