Return-Path: Received: from mail-ot0-f172.google.com ([74.125.82.172]:44960 "EHLO mail-ot0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751964AbeFARCV (ORCPT ); Fri, 1 Jun 2018 13:02:21 -0400 Received: by mail-ot0-f172.google.com with SMTP id w13-v6so2648595ote.11 for ; Fri, 01 Jun 2018 10:02:21 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20180601160857.GE10666@fieldses.org> References: <95e00ce46fe1f5fed50fe24947eee0dda51e0140.camel@hammerspace.com> <828f320cde910a45983d91bddb6477d21c5cae33.camel@hammerspace.com> <20180601135033.GA10666@fieldses.org> <20180601142640.GC10666@fieldses.org> <20180601160857.GE10666@fieldses.org> From: Miklos Szeredi Date: Fri, 1 Jun 2018 19:02:20 +0200 Message-ID: Subject: Re: nfs4_acl restricts copy_up in overlayfs To: "bfields@fieldses.org" Cc: Trond Myklebust , "rgoldwyn@suse.de" , "agruenba@redhat.com" , "linux-nfs@vger.kernel.org" , "linux-unionfs@vger.kernel.org" Content-Type: text/plain; charset="UTF-8" Sender: linux-nfs-owner@vger.kernel.org List-ID: On Fri, Jun 1, 2018 at 6:08 PM, bfields@fieldses.org wrote: > On Fri, Jun 01, 2018 at 04:43:51PM +0200, Miklos Szeredi wrote: >> On Fri, Jun 1, 2018 at 4:26 PM, bfields@fieldses.org >> wrote: >> > On Fri, Jun 01, 2018 at 04:00:22PM +0200, Miklos Szeredi wrote: >> >> On Fri, Jun 1, 2018 at 3:50 PM, bfields@fieldses.org >> >> 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? 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. The inode_permission() checks on realinode are for making sure the mounter cannot gain undue privileges (will be especially important with userns mounts). Thanks, Miklos