On Fri, Aug 10 2018, Bruce Fields wrote:
> On Fri, Aug 10, 2018 at 01:00:27PM -0400, Bruce Fields wrote:
>> On Fri, Aug 10, 2018 at 11:29:33AM +1000, NeilBrown wrote:
>> > On Mon, Mar 21 2016, Nelson Elhage wrote:
>> >
>> > > That's correct. The other detail that seems to be important is that
>> > > the user making the call must be different from the user owning the
>> > > file. We've also been using user remapping on the server, so that
>> > > non-xattr calls succeed in that configuration.
>> > >
>> > > The reproducer James added in the bugzilla is:
>> > >
>> > > (on machine with IP address 10.1.1.1)
>> > > sudo mkdir /nfs_test
>> > > sudo useradd -u 10000 test_user
>> > > sudo chown test_user /nfs_test
>> > > echo "/nfs_test 10.1.1.2(rw,all_squash,anonuid=10000)" | sudo tee -a
>> > > /etc/exports
>> > > sudo exportfs -a
>> > >
>> > > (on machine with IP address 10.1.1.2)
>> > > sudo mkdir /nfs_test
>> > > sudo mount -t nfs -o vers=3,noacl 10.1.1.1:/nfs_test /nfs_test
>> > > touch /nfs_test/foo
>> > > install -m 755 /nfs_test/foo /nfs_test/bar
>> >
>> > Did anything ever happen about this?
>> > I have a customer with a similar problem (in 4.4) but I cannot see any
>> > evidence of fixes landing in mainline.
>> >
>> > Problem happens with you have uid mapping on the server
>> > (e.g. anonuid=10000 as above) and a user with a different uid on the
>> > client attempts setacl on a file with that user.
>> > As anon is mapped to the owner of the file, setacl should be allowed.
>> > However set_posix_acl() calls inode_owner_or_capable() which checks if
>> > the client-side uid matches the visible inode->i_uid - they don't.
>> >
>> > Testing i_uid on the client is always incorrect for permission checking
>> > with NFS - the client should always ask the server, either with ACCESS
>> > or, in this case, by simply attempting the operation.
>> >
>> > Any suggestions how best to fix this?
>> > - We could move the responsibility for permission checking into
>> > i_op->set_acl, but that would be a large change and might make it too
>> > easy for other filesystems to get it wrong.
>> > - we could have some sort of flag asking set_posix_acl(), but that's
>> > rather clumsy.... maybe if i_op->set_acl_check_perm use that without
>> > testing ownership first??
>> > - we could copy
>> > posic_acl_xattr_{get,set,list} into nfs together with functions
>> > they call, modify set_posix_acl() to not test ownership,
>> > and provide a local 'struct xattr_handler' structure for NFS.
>> >
>> > I don't really like any of those suggestions. Can someone else do any
>> > better?
>>
>> Do we have important callers of inode_owner_or_capable() in the vfs (as
>> opposed to in individual filesystems), and do any of them pose a similar
>> problem for network filesystems?
>
> do_linkat()->may_linkat() looks kinda suspicious to me. Or what about
> the O_NOATIME check in map_open()? Just engaging in dumb grepping
> here....
>
> --b.
NOATIME, both in open and fcntl, is rejected on NFS. This seems valid
as there is no way in the protocol to ask the server to no update the
atime.
Others I found we just short-cuts to avoid calling i_op->permission() if
the caller was an owner. I don't *think* that would affect NFS much
... though if an owner didn't have write permission, some things might
be incorrectly forbidden. Maybe.
Thanks,
NeilBrown
Attachment:
signature.asc
Description: PGP signature
