On Fri, Jun 01, 2018 at 04:43:51PM +0200, Miklos Szeredi wrote: > On Fri, Jun 1, 2018 at 4:26 PM, bfields@xxxxxxxxxxxx > <bfields@xxxxxxxxxxxx> wrote: > > On Fri, Jun 01, 2018 at 04:00:22PM +0200, Miklos Szeredi wrote: > >> On Fri, Jun 1, 2018 at 3:50 PM, bfields@xxxxxxxxxxxx > >> <bfields@xxxxxxxxxxxx> wrote: > >> > On Fri, Jun 01, 2018 at 03:32:59PM +0200, Miklos Szeredi wrote: > >> >> How do you define "safely"? > >> >> > >> >> Is it safe for root to do > >> >> > >> >> cp -a /nfs/remotedir /tmp/localdir > >> >> > >> >> ? > >> >> > >> >> That's essentially what an overlayfs mount with an NFS layer does with > >> >> respect to access permissions: > >> >> > >> >> - remote files are not modifiable to anyone, unless server allows > >> >> > >> >> - remote files *readable to root* will provide access based on local DAC check. > >> >> > >> >> Does that need to be made clear in the docs? Surely. But it does NOT > >> >> mean it's dangerous or that it's not useful with an arbitrary NFS > >> >> server > >> > > >> > We should definitely have clear documentation, but despite that, in > >> > practice lots of people *will* be surprised when permissions are > >> > enforced differently after copy-up, and those surprises may well have > >> > unpleasant implications. > >> > >> Permissions are enforced exactly the same before and after copy-up. > >> That's one of the good points in doing the permission checks locally. > > > > Whoops, sorry, I missed that. So you always read owners and mode bits > > out of the cached inode and used those to check permissions instead of > > calling access? > > > > That still sounds pretty confusing. E.g. if the server's squashing root > > to a user without permission to read a file, you'll pass local > > permission checks, but the success a given read may actually depend on > > whether the data's already cached? > > You have a point there. I think current code can be inconsistent like > that. But that's only because it doesn't stack file operations. > Stacking f_ops is now queued up for 4.18, which means that *all* calls > into underlying layers should be with the same creds (those of the > mounting task), regardless of the creds of the task performing the > operation. > > So if NFS server is denying read to mounter (because of root squashing > or for other reason), then that file will not be accessible from > overlayfs by anyone and will not be in the cache either. If access to > mounter is allowed, then the access will be based on local DAC. > > Look at ovl_permission(), I think it pretty clearly describes this model. Thanks! Uh, so generic_permission is the thing that just does the usual mode/acl checks on the in-core inode, and inode_permission is the one that also calls into the filesystem? But I'm still a little confused--if I'm reading right, "realinode" is the lower inode before copyup, and the upper inode after, so can't inode_permission(realinode, mask) return different results before and after copyup? --b. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
