RE: Better interop for NFS/SMB file share mode/reservation

[Pavel Shilovskiy <pshilov@microsoft.com>]

пн, 29 апр. 2019 г. в 13:29, Jeff Layton <jlayton@kernel.org>:
>
> On Mon, 2019-04-29 at 00:57 +0000, Trond Myklebust wrote:
> > On Sun, 2019-04-28 at 18:33 -0400, Amir Goldstein wrote:
> > > On Sun, Apr 28, 2019 at 6:08 PM Trond Myklebust <
> > > trondmy@hammerspace.com> wrote:
> > > > On Sun, 2019-04-28 at 18:00 -0400, Amir Goldstein wrote:
> > > > > On Sun, Apr 28, 2019 at 11:06 AM Trond Myklebust
> > > > > <trondmy@hammerspace.com> wrote:
> > > > > > On Sun, 2019-04-28 at 09:45 -0400, Amir Goldstein wrote:
> > > > > > > On Sun, Apr 28, 2019 at 8:09 AM Jeff Layton <
> > > > > > > jlayton@kernel.org>
> > > > > > > wrote:
> > > > > > > > On Sat, 2019-04-27 at 16:16 -0400, Amir Goldstein wrote:
> > > > > > > > > [adding back samba/nfs and fsdevel]
> > > > > > > > >
> > > > > > > >
> > > > > > > > cc'ing Pavel too -- he did a bunch of work in this area a
> > > > > > > > few
> > > > > > > > years
> > > > > > > > ago.
> > > > > > > >
> > > > > > > > > On Fri, Apr 26, 2019 at 6:22 PM Jeff Layton <
> > > > > > > > > jlayton@kernel.org>
> > > > > > > > > wrote:
> > > > > > > > > > On Fri, 2019-04-26 at 10:50 -0400, J. Bruce Fields
> > > > > > > > > > wrote:
> > > > > > > > > > > On Fri, Apr 26, 2019 at 04:11:00PM +0200, Amir
> > > > > > > > > > > Goldstein
> > > > > > > > > > > wrote:
> > > > > > > > > > > > On Fri, Apr 26, 2019, 4:00 PM J. Bruce Fields <
> > > > > > > > > > > > bfields@fieldses.org> wrote:
> > > > > > > > > > > >
> > > > > > > > > > That said, we could also look at a vfs-level mount
> > > > > > > > > > option
> > > > > > > > > > that
> > > > > > > > > > would
> > > > > > > > > > make the kernel enforce these for any opener. That
> > > > > > > > > > could
> > > > > > > > > > also
> > > > > > > > > > be useful,
> > > > > > > > > > and shouldn't be too hard to implement. Maybe even make
> > > > > > > > > > it
> > > > > > > > > > a
> > > > > > > > > > vfsmount-
> > > > > > > > > > level option (like -o ro is).
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > Yeh, I am humbly going to leave this struggle to someone
> > > > > > > > > else.
> > > > > > > > > Not important enough IMO and completely independent
> > > > > > > > > effort to
> > > > > > > > > the
> > > > > > > > > advisory atomic open&lock API.
> > > > > > > >
> > > > > > > > Having the kernel allow setting deny modes on any open call
> > > > > > > > is
> > > > > > > > a
> > > > > > > > non-
> > > > > > > > starter, for the reasons Bruce outlined earlier. This
> > > > > > > > _must_ be
> > > > > > > > restricted in some fashion or we'll be opening up a
> > > > > > > > ginormous
> > > > > > > > DoS
> > > > > > > > mechanism.
> > > > > > > >
> > > > > > > > My proposal was to make this only be enforced by
> > > > > > > > applications
> > > > > > > > that
> > > > > > > > explicitly opt-in by setting O_SH*/O_EX* flags. It wouldn't
> > > > > > > > be
> > > > > > > > too
> > > > > > > > difficult to also allow them to be enforced on a per-fs
> > > > > > > > basis
> > > > > > > > via
> > > > > > > > mount
> > > > > > > > option or something. Maybe we could expand the meaning of
> > > > > > > > '-o
> > > > > > > > mand'
> > > > > > > > ?
> > > > > > > >
> > > > > > > > How would you propose that we restrict this?
> > > > > > > >
> > > > > > >
> > > > > > > Our communication channel is broken.
> > > > > > > I did not intend to propose any implicit locking.
> > > > > > > If samba and nfsd can opt-in with O_SHARE flags, I do not
> > > > > > > understand why a mount option is helpful for the cause of
> > > > > > > samba/nfsd interop.
> > > > > > >
> > > > > > > If someone else is interested in samba/local interop than
> > > > > > > yes, a mount option like suggested by Pavel could be a good
> > > > > > > option,
> > > > > > > but it is an orthogonal effort IMO.
> > > > > >
> > > > > > If an NFS client 'opts in' to set share deny, then that still
> > > > > > makes
> > > > > > it
> > > > > > a non-optional lock for the other NFS clients, because all
> > > > > > ordinary
> > > > > > open() calls will be gated by the server whether or not their
> > > > > > application specifies the O_SHARE flag. There is no flag in the
> > > > > > NFS
> > > > > > protocol that could tell the server to ignore deny modes.
> > > > > >
> > > > > > IOW: it would suffice for 1 client to use O_SHARE|O_DENY* to
> > > > > > opt
> > > > > > all
> > > > > > the other clients in.
> > > > > >
> > > > >
> > > > > Sorry for being thick, I don't understand if we are in agreement
> > > > > or
> > > > > not.
> > > > >
> > > > > My understanding is that the network file server implementations
> > > > > (i.e. samba, knfds, Ganesha) will always use share/deny modes.
> > > > > So for example nfs v3 opens will always use O_DENY_NONE
> > > > > in order to have correct interop with samba and nfs v4.
> > > > >
> > > > > If I am misunderstanding something, please enlighten me.
> > > > > If there is a reason why mount option is needed for the sole
> > > > > purpose
> > > > > of interop between network filesystem servers, please enlighten
> > > > > me.
> > > > >
> > > > >
> > > >
> > > > Same difference. As long as nfsd and/or Ganesha are translating
> > > > OPEN4_SHARE_ACCESS_READ and OPEN4_SHARE_ACCESS_WRITE into share
> > > > access
> > > > locks, then those will conflict with any deny locks set by whatever
> > > > application that uses them.
> > > >
> > > > IOW: any open(O_RDONLY) and open(O_RDWR) will conflict with an
> > > > O_DENY_READ that is set on the server, and any open(O_WRONLY) and
> > > > open(O_RDWR) will conflict with an O_DENY_WRITE that is set on the
> > > > server. There is no opt-out for NFS clients on this issue, because
> > > > stateful NFSv4 opens MUST set one or more of
> > > > OPEN4_SHARE_ACCESS_READ
> > > > and OPEN4_SHARE_ACCESS_WRITE.
> > > >
> > >
> > > Urgh! I *think* I understand the confusion.
> > >
> > > I believe Jeff was talking about implementing a mount option
> > > similar to -o mand for local fs on the server.
> > > With that mount option, *any* open() by any app of file from
> > > that mount will use O_DENY_NONE to interop correctly with
> > > network servers that explicitly opt-in for interop on share modes.
> > > I agree its a nice feature that is easy to implement - not important
> > > for first version IMO.
> > >
> > > I *think* you are talking on nfs client mount option for
> > > opt-in/out of share modes? there was no such intention.
> > >
> >
> > No. I'm saying that whether you intended to or not, you _are_
> > implementing a mandatory lock over NFS. No talk about O_SHARE flags and
> > it being an opt-in process for local applications changes the fact that
> > non-local applications (i.e. the ones that count ) are being subjected
> > to a mandatory lock with all the potential for denial of service that
> > implies.
> > So we need a mechanism beyond O_SHARE in order to ensure this system
> > cannot be used on sensitive files that need to be accessible to all. It
> > could be an export option, or a mount option, or it could be a more
> > specific mechanism (e.g. the setgid with no execute mode bit as using
> > in POSIX mandatory locks).
> >
>
> That's a great point.
>
> I was focused on the local fs piece in order to support NFS/SMB serving,
> but we also have to consider that people using nfs or cifs filesystems
> would want to use this interface to have their clients set deny bits as
> well.
>
> So, I think you're right that we can't really do this without involving
> non-cooperating processes in some way.

It's been 5+ years since I touched that code but I still like the idea of having a separate mount option for mountpoints used by Samba and NFS servers and clients to avoid security attacks on the sensitive files. For some sensitive files on such mountpoints a more selective mechanism may be used to prevent deny flags to be set (like mentioned above). Or we may think about adding another flag e.g. O_DENYFORCE available to root only that tells the kernel to not take into account deny flags already set on a file - might be useful for recovery tools.

About O_DENYDELETE: I don't understand how we may reach a good interop story without a proper implementation of this flag. Windows apps may set it and Samba needs to respect it. If an NFS client removes such an opened file, what will Samba tell the Windows client?

>
> A mount option sounds like the simplest way to do this. We have
> SB_MANDLOCK now, so we'd just need a SB_DENYLOCK or something that would
> enable the use of O_DENY_READ/WRITE on a file. Maybe '-o denymode' or
> something.

I remember it was 'sharelock' in my patchset but naming here is a least important I guess.

>
> You might still get back EBUSY on a nfs or cifs filesystem even without
> that option, but there's not much we can do about that.

I ended up with a new ESHAREDENIED error code which I found better for detectability - might be useful to know an exact reason of the open call being failed. Let's say a DB instance wants to be sure that a partition file is already being served by another instance before giving up on trying to open it.

--
Best regards,
Pavel Shilovsky