Re: nfs4_acl restricts copy_up in overlayfs

[Miklos Szeredi <miklos@szeredi.hu>]

On Sat, Jun 2, 2018 at 2:50 AM, bfields@fieldses.org
<bfields@fieldses.org> wrote:
> On Fri, Jun 01, 2018 at 09:14:38PM +0200, Miklos Szeredi wrote:
>> On Fri, Jun 1, 2018 at 7:43 PM, bfields@fieldses.org
>> <bfields@fieldses.org> wrote:
>> > On Fri, Jun 01, 2018 at 07:02:20PM +0200, Miklos Szeredi wrote:
>> >> On Fri, Jun 1, 2018 at 6:08 PM, bfields@fieldses.org
>> >> <bfields@fieldses.org> wrote:
>> >> > On Fri, Jun 01, 2018 at 04:43:51PM +0200, Miklos Szeredi wrote:
>> >> >> 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?
>> >>
>> >> Right.
>> >>
>> >> > 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?
>> >>
>> >> Theoretically, yes.  Not in any sane setup, though.
>> >
>> > If root squashing is enabled and you mount as root, then it will change.
>> >
>> > That's not an unlikely case, it's pretty much the default.
>>
>> Let's see: root squashing will change root creds to nobody's creds, right?
>>
>> That results in a weird situation, where user foo is trying to access
>> a file owned by foo, but which doesn't have read permission for
>> "other" will not be able to perform a read on that file.  In fact, no
>> user will be able to read that file, including root.   Copying up the
>> file will also fail, since that requires read permission.
>>
>> So for that case it's not true that permission will change, since
>> copy-up won't be possible.
>
> Fair enough.  That's still pretty weird behavior.
>
> Arguably if you're going to do something different here then maybe it's
> good for the difference to manifest in a way that's really obvious in
> common cases, so people don't just overlook it.
>
>> That leaves the case where no read or write permissions are involved,
>> which is execute.  I think we should just mask that out from the
>> inode_permission() check.  Makes no sense to ask underlying filesystem
>> about execute permission.
>
> I suppose so.
>
>> With that we'll have consistent permissions before and after copy-up.
>> Right?
>
> Hm.  If inode_permission(realinode, mask) changes across copy-up:
>
>         - if the difference is in execute, you're telling me we'll
>           ignore that.
>         - if it changes from denying to allowing read, you don't care
>           since that will prevent the copy up in the first place.
>         - if it changes from denying to allowing write, that has no
>           effect since you only care about read permission for copyup in
>           the case someone requests write permission.  (Hm, the special
>           file case should be weird.  I guess those just don't get
>           copied up?  Still I think people will be surprised by
>           permission failures here.)

Special file can get copied up on metadata change.  We shoudln't ask
server at all for permissions in this case.  That would be consistent
with the "cp -a" model.

>         - if it changes from allowing read or write to denying--then
>           we're no longer talking about root, assuming the permission
>           check on the copied-up inode wouldn't fail for root.  We don't
>           currently allow non-root to mount NFS, but there are people
>           that would like to see that change.

The "cp -a" model should work even for userns unprivileged mounts.  We
do assume a certain sanity with setups: I'm sure someone could come up
with a setup (e.g. with selinux) where weirdness happens after
copy-up.

Thanks,
Miklos



>
> --b.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html