[nfsv4] Re: Comments for draft-rmacklem-nfsv4-posix-acls-02

[David Noveck <[e-mail]>]

On Wed, Aug 7, 2024, 12:22 PM Pali Rohár <[e-mail]>
wrote:

> Hello Rick, I'm sending comments for the second part of your email.
>
> On Tuesday 06 August 2024 15:20:50 Rick Macklem wrote:
> > On Tue, Aug 6, 2024 at 10:05 AM Pali Rohár <[e-mail]
> >
> > wrote:
> > > > A server that is configured for a acl_trueform_scope other that
> > > > ACL_SCOPE_FILE_OBJECT MUST not support the posix_default_acl and
> > > > posix_access_acl unless the true form for the file system is
> > > > ACL_MODEL_POSIX_DRAFT.
> > >
> > > This is a restriction which basically means that no existing NFS4
> > > server with native NFS4 ACL support can support "posix_access_acl".
> > > In multiprotocol environment where is NFSv4.0 and NFSv4.1, the NFS4
> > > server would have to provide server scope native NFS4 model (to support
> > > NFSv4.1 as as your document is only for NFSv4.2) which implies that
> > > server must not support POSIX ACL. And same applies in the
> multiprotocol
> > > environment with NFS4 and SMB.
> > >
> > Not exactly. posix_access_acl refers to the proposed optional
> > attribute for NFSv4.2 (same goes for acl_trueform_scope).
> > What the server does w.r.t. "true form" is up to the server.
> > Same goes for the scope of it.
> > (For example, Linux server always use POSIX draft ACLs.)
> >
> > The difference is that NFSv4.0 and 4.1 clients cannot know
> > what the "true form" is and can only see NFSv4 ACLs (possibly
> > mapped to/from POSIX draft ACLs).
> >
> > What the above restriction is meant to do is allow a NFSv4.2
> > client to know that the "true form" is ACL_MODEL_POSIX_DRAFT
> > when the scope is ACL_SCOPE_FILE_SYSTEM or ACL_SCOPE_SERVER
> > by seeing that posix_access_acl and posix_default_acl are
> > supported_attrs for the file system.
> > --> Note that acl_trueform_scope is "per server", so a client
> >     only needs to GETATTR this attribute once for the server.
> >     Then, if that is ACL_SCOPE_FILE_SYSTEM or ACL_SCOPE_SERVER,
> >     it can tell that the "true form" is ACL_MODEL_POSIX_DRAFT
> >     as soon as it sees posix_access_acl and posix_default_acl
> >     in supported_attrs.
> >     --> It does not have to bother to check acl_trueform,
> >         which would often result in an extra RPC.
>
> Ok, I will try to describe this issue in different way on example.
>
> NFS4 client can decide that will not implement support for ACL and DACL
> attributes because, NFS4 client is POSIX only and mapping from NFS4 to
> POSIX cause issues for that client.
>
> NFS4 client will implement this extension, so it would understand only
> POSIX_ACCESS_ACL and POSIX_DEFAULT_ACL attributes.
>
> There are existing NFS4 server which implements multiprotocol
> environment. Such server would always use NFS4 true form ACLs and
> nothing different


That is not always true .  It can uses either form on a per-file basis.

>for example because POSIX ACL cannot store SMB ACLs).

> So such server would return for acl_trueform_scope ACL_SCOPE_SERVER (or
> ACL_SCOPE_FILE_SYSTEM in case server supports more filesystems and want
> such feature).
>
> Now according to your description, such server "MUST not support the
> posix_default_acl and posix_access_acl" (because its true form is NFS4).
>
> So it means for NFS4 clients which would implement only POSIX_ACCESS_ACL
> and POSIX_DEFAULT_ACL attributes, those NFS4 servers are not able to
> serve any ACL.
>
> And if customers of such NFS4 server start asking for "ACL compatibility
> with that NFS4 client" then such NFS4 server would ignore the part "MUST
> not support the posix_default_acl and posix_access_acl" and will start
> providing these attributes via some translation or mapping (it would be
> even worse mess).


Or they could switch to a per-file scope.  I think should (note lower case).


> This is why I think it is too restrictive. It is disallowing NFS4
> servers to provide compatibility for POSIX NFS4 clients.
>

It would be too restrictive, if it were the case.


> >
> > > I think that this is too strict and what would happen is either that
> > > this extension would not be implemented OR section option (which I
> think
> > > everybody would do) is ignoring this restriction.
> > >
> > As above, this does not define what the server does for NFSv4.0
> > and NFSv4.1, it just simplifies what a NFSv4.2 client needs to
> > check, if it knows about and the server supports the extension
> > proposed by this draft.
> >
> > >
> > > I guess that for compatibility with FreeBSD and Linux, servers would
> > > implement it even on NFS4 server scope model.
> > >
> > Not sure what you mean by this.
>
> I explained above the whole example. I did not mentioned in it explicit
> client and servers as it nos tight to FreeBSD or Linux. Hopefully it
> would be clear now what I mean.
>

 Not to me.


> > Linux always uses a "true form"
> > of POSIX ACL and FreeBSD supports one of POSIX ACL or NFSv4 ACL
> > on a per-file system basic (at least for now).
> > - That is not changed by this extension. All this extension does
> >   is allow NFSv4.2 clients to use the server more effectively
> >   by choosing to get/set the "true form" and avoiding mapping,
> >   assuming the client and server both implement the extension.
> > Other file systems (IBM's GPFS seems to be the extant example
> > I am aware of) also supports a "true form" on a per-file basis.
> > (Again this is a server file system configuration choice and
> > is not affected by whether or not this extension is supported
> > by a NFSv4.2 server.)
> >
> >
> > >
> > > And I have there two additional points without quoting your document.
> > >
> > > 1)
> > >
> > > Your document does not describe what existing RFC 8881 acl and dacl
> > > attributes should return when user sets new "posix_default_acl" or
> > > "posix_access_acl" attribute. Also what should happen when another
> > > client tries to change acl or dacl attribute when POSIX ACL is already
> > > active.
> > >
> > The intent (if that is not clear, the words need to be improved)
> > is that the server file system shall have one "true form" for each
> > file object. If the attribute(s) for the other "true form" is used
> > on the object (which assumes the server has chosen to put attributes for
> > both in supported_attrs for the file syetem, the GETATTR/READIR/SETATTR
> > will use mapping to translate it to/from the "true form".
>
> Ok, I think this is important to know and was not clear for me.
> So it means that if server's true form is POSIX and NFS4 client call
> GETATTR(DACL) command then server is required to map its POSIX ACL into
> NFS4 ACL. I think that this should be explicitly mentioned to prevent
> any confusion.
>

As I understand things it needs to report that there is no NFSv4 DACL
(length zero or whatever else we decide).


> > The intent is to discourage use of mapping. As such, the client using
> > these extensions can avoid that by determining what the "true form" is
> > for the object and using the appropriate attributes.
>
> Now I understand. So NFS4 client without this extension would use just
> ACL and DACL attributes. And NFS4 client with this extension can use
> POSIX_ACCESS_ACL if wants and can benefit from it.
>
> > My thinking is that servers like FreeBSD (where the "true form" is
> per-file
> > system and mapping is not supported) would:
> > (A) - Support posix_access_acl and posix_default_acl, but not acl, for
> file
> >   systems that use POSIX ACLs as their "true form".
>
> Why not ACL/DACL attribute at least via mapping? This will degrade NFS4
> clients which use NFS4 true form ACL or similar (e.g. Windows or SMB).
>
> RFC 8881 suggest implementations to implement ACL and DACL attributes
> for access control. So servers which will not implement ACL and DACL
> attribute would for access control would not be compatible with RFC 8881
> clients which expects access control.
>

  Not true.  The ACL and DACL attributes are OPTIONAL.  If they are not
supported, access control is provided POSIX-style using the attributes
mode, owner, owner_group which are now REQUIRED.  If this is not clear in
RFC8881, I  hope it is in draft-dnoveck-nfsv4-security.


> > (B) - Support acl, but not posix_access_acl and posix_default_acl, for
> file
> >   systems that ise NFSv4 ACLs as their "true form".
>
> That makes sense. Export POSIX_ACCESS_ACL/POSIX_DEFAULT_ACL only in case
> POSIX is server's true form.
>
> > For Linux servers, they might choose (A) above, or they might choose
> > to support all of acl, posix_access_acl and posix_default_acl, where acl
> > is supported via the mapping algorithms they already use.
> >
> > It is the case of a per-file scope where things get difficult:
> > The server will probably want to support all of acl, posix_access_acl
> > and posix_default_acl, but since the "true form" is defined a one
> > model for each file, then how should the attribute(s) for the other
> > model be handled?
> > - I would like to avoid mapping, so I think being able to return
> >   "no ACL" would be preferred.
>
> I think that this is against RFC 8881 where it is expected that server
> construct ACL attribute which will describe the current access
> permission.
>

I don't think so.


> Returning "no ACL" for NFS4 clients which do not understand this
> extension would be very confusing. User will ask for ACL, user will see
> that it is empty and would think that nobody has access to that file.
>

According to RFC8881, user are not supposed to interpret Acls but use
ACCESS/OPEN for this purpose.

And then would be surprised that somebody can access that file just
> because some optional extension attribute returns some more information
> about access, which the client does not implement. This can lead also to
> security or audit issues.
>
> > - I had thought a zero length ACL would work for this, but that no
> >   longer appears to be the case.
>
> Yes, it would not work.
>

I don't agree.


> > Maybe, defining an ACL with a single ACE that says "not valid" would
> > work?
>
> I think not. ACL and DACL attribute should always say current access
> permissions for the object. And if server is using some other (e.g.
> POSIX or any other strange scheme) then server should provide the best
> effort translation for such clients.
>
> > This could be the reply for GETATTR/READDIR and could be used to
> > "erase" the ACL for SETATTR.
> > --> I think this is what the next draft will propose.
> >
> >
> > > If interop between mode, dacl and posix_default_acl, posix_access_acl
> > > attributes is not explicitly defined in specification then every server
> > > implementation will invent its own rules and it would just lead to the
> > > mess in multiprotocol environment where clients would not be sure what
> > > can happen. Like it is already with "mode" attribute in NFS v4.1 (see
> 2).
> > >
> > For posix_access_acl and posix_default_acl the intent is that the
> > POSIX draft specification defines the interaction between mode and
> > the POSIX ACL.
>
> Ok. But the problem is that POSIX mode != NFS4 mode. Below in 2)
> I described one difference. And because POSIX ACL does not define
> interaction between NFS4 mode and POSIX ACL then I think it is required
> to define it in extension specification to make it clear.
>
> > For NFSv4 ACLs, you are correct. It is a mess.
> > And I wish David N. the best of luck w.r.t. cleaning it up.
>
> Interesting, is David working on cleaning it up? If you are interesting
> I can provide feedback what needs to be fixed (something I have already
> written in my first email to the list).
>

Yes, in draft-dnoveck-nfsv4-security and draft-dnoveck-nfsv4-acls.   Note
that the current draft of the latter, -04 needs to be updates to be
compatible with Rick's work.  Looking for getting -05 out in next few weeks.


> > (OpenZFS already has a knob that specifies 4 different ways
> > of doing this. Another knob setting may be needed.)
> >
> > Thanks for the comments, rick
> >
> > >
> > > 2)
> > >
> > > Then there is a problem with "group" bits of "mode" attribute.
> > > In RFC 8881 section 6.2.4 is written:
> > >
> > > "Bits MODE4_RGRP, MODE4_WGRP, and MODE4_XGRP apply to principals
> > > identified in the owner_group attribute but who are not identified in
> > > the owner attribute."
> > >
> > > And in section 6.3.2.1 is written:
> > >
> > > "Some server implementations also add bits permitted to named users and
> > > groups to the group bits (MODE4_RGRP, MODE4_WGRP, and MODE4_XGRP).
> > > Implementations are discouraged from doing this."
> > > "The same user confusion seen when fetching the mode also results if
> > > setting the mode does not effectively control permissions for the
> owner,
> > > group, and other users; this motivates some of the requirements that
> > > follow."
> > >
> > > POSIX ACL with at least one named user or named group requires that
> > > mode group bits refers to POSIX ACL mask, and not to the POSIX ACL
> > > group_obj (which represents owner_group).
> > >
> > > Those RFC 8881 parts of NFS mode attribute are basically not compatible
> > > with POSIX ACL. Your extension does not mention this problem and does
> > > not handle it.
> > >
> > > Existing NFS4 servers which follows the RFC 8881 meaning of mode group
> > > bits, would not be able to implement this your extension for POSIX ACL,
> > > because existing servers cannot change behavior of mode attribute due
> > > backward compatibility. I think that this is a big problem.
> > > Servers in multiprotocol environment mostly follows the RFC 8881 advice
> > > to never return something like "POSIX mask" in group bits.
> > >
> > > How to solve this problem. One option could be including all mode bits
> > > into your new "posix_access_acl" attribute and let NFS4 clients to use
> > > only "posix_access_acl" on chown operation, and never use existing
> > > "mode" attribute at all (if the extension is supported by server).
> > >
> > > Another option could be to introduce a new "posix_mode" attribute which
> > > would be strictly POSIX and old "mode" attribute can stay as before for
> > > backward compatibility. Again clients would not use "mode" attribute
> > > anymore.
> > >
> > >
> > > Regards,
> > > Pali
> > >
>
> _______________________________________________
> nfsv4 mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]
>