Re: [nfsv4] Fwd: New Version Notification for draft-dnoveck-nfsv4-security-04.txt

[David Noveck <[e-mail]>]

On Sun, Jan 9, 2022, 6:15 PM Rick Macklem <[e-mail]> wrote:

> From: David Noveck wrote:
> > Rick Macklem wrote:
> > Here are specific comments related to ACE4_APPEND_DATA:
> >
> > From the draft:
> > The action of modifying a file's data, but only starting at
> >         EOF.  This allows for the specification of append-only files,
> >         by allowing ACE4_APPEND_DATA and denying ACE4_WRITE_DATA to the
> >         same user or group.
> >
> >         [Consensus Needed (Item #8a)]: As there is no way for the
> >         server to decide, in processing an OPEN or ACCESS request,
> >
> > since ACCESS has a separate extend permission bit, it doesn't make sense
> to mentione ACCESS
> > in this paragraph.   Will fix it in -05.
> I think you need to be careful.


I'm trying to be.

Append and Extend are not the same thing.


Perhaps not, but I'm assuming they are closely related.   If they are as
distinct as you suppose, then it is difficult to make sense of NFSv4
handling of these issues.

Append is specifically
> a Write with "offset == size (or file)".
>

I believe that there is text in RFC8881 that says exactly that but I feel
it needs to be corrected.  It doesn't make sense to treat offset == size
and offset == size+1 differently.

I believe Extend refers to any case where file size is increased?
>



> > >         whether subsequent WRITEs will extend the file or not, the
> > >         server will treat masks containing only WRITE_DATA, only
> > >         APPEND_DATA or both, identically.
> > >
> > > I believe it would be preferable to say "...the server will ignore
> > > APPEND_DATA and will only enforce WRITE_DATA".
> >
> > I don't think that will work.   As the spec indicates above, the purpose
> of making
> > these separate mask bits is to allow append-only files.  As a result, if
> the
> > client open such a file (acl allows WRITE_DATA and not APPEND_DATA), he
> has to
> > be able to open it.  Then, the server that supports that (provisions
> need to be
> > made for servers that don't), will check at WRITE time whether the WRITE
> extends the file.
> Yes, agreed, unfortunately. I say "unfortunately" since any old Write that
> happens to have
> an offset == size falls into the "append" category.
>
> > > Why?
> > > Well, my understanding is that APPEND_DATA does not apply to
> > > any old Write that happens to start at EOF, but specifically
> "append-only"
> > >
> > The spec says the contrary.  since there are no NFSv4 append-only
> operations
> > it would not make sense to revise it in line with your understanding.
> >
> > > file operations. (A POSIX write done on a file opened with the O_APPEND
> > > flag would be one example.)
> > Since "append-only" file operations do not exist in NFSv4 (as you noted).
> >
> > true but the server is still able to distinguish writes that extend the
> file from those that don't.
> Be careful with that "extend" word.
>

I think I am.

> > then ignoring APPEND_DATA seems more correct that assuming it to be
> > > equivalent to WRITE_DATA.
> >
> > It doesn't make sense to define this mask bit and then ignore it all the
> time.
> >   When are you supposing it will be looked at?
> By *stupid* Windows only. (Sorry, I couldn't resist;-)
>

It would also be looked at servers supporting Windows clients.

>
> > > For example, assume a file with APPEND_DATA allowed, but WRITE_DATA not
> > > allowed that is of size == 0. If APPEND_DATA is sufficient to allow an
> Open with
> > > OPEN_SHARE_ACCESS_WRITE, then a client could do the following:
> > > - Open the file with OPEN_SHARE_ACCESS_WRITE
> > >  - Write data to the file sequentially (the most common write usage).
> > > --> Not what APPEND_DATA allowed but WRITE_DATA not allowed means,
> > >       from my understanding.
> >
> > The spec  says that this is allowed.
> Yea, I guess you are correct.
>
> > >  *  Servers that do not distinguish between WRITE_DATA and APPEND_DATA
> > >      need to make it clear to clients that support for append-only
> > >      files is not present.  To do that, requests to set NFSv4 ACLs
> > >      where the handling for these two masks are different for any
> > >      specified user or group are to be rejected with
> > >      NFS4ERR_ATTRNOTSUPP.
> > >
> > >First, given that there is no OPEN_SHARE_ACCESS_APPEND, this applies to
> > > all NFSv4 servers.
> >
> > No.   It only applies to servers, and they will be common, that do not
> distinguish WRITE_DATA
> > and APPEND_DATA.   Some if the confusion here resUlts from the fact that,
> > like our old friend owner-override semantics., this is a case in which
> the handling
> > at OPEN time is different from that at IO time.
> >
> > > Second, although I can understand that this would "make it clear to
> clients",
> > > there are issues with doing this for systems like FreeBSD, where NFSv4
> ACLs
> > > are used locally as well as for the NFSv4 server.
> > > For example, on FreeBSD:
> > > - A setfacl(1) can set any combination of APPEND_DATA, WRITE_DATA or
> both
> > >   APPEND_DATA and WRITE_DATA for allow/deny when done locally on UFS
> or ZFS
> > >   file systems.
> > > - A getfacl(1) will show them separately.
> > > If FreeBSD were to enforce the above in the NFSv4 server, the
> getfacl(1) on a FreeBSD
> > > NFSv4 client would show any combination as above, but the setfacl(1)
> would fail
> > > for the case where only one of APPEND_DATA/WRITE_DATA is being set.
> > > Users would find this a weird, irritating inconsistency. (As such, it
> won't be the case
> > > for FreeBSD NFSv4 servers.)
> > >
> > > I'd prefer that setting any combination be allowed, but that
> enforcement of APPEND_DATA
> > > not be done for the NFSv4 server and that APPEND_DATA be handled like
> other non-enforcible
> > > bits, such as SYNCHRONIZE.
> >
> > I see your point but note that there is a significant difference between
> SYNCHRONIZE which
> > is unenforceable since there is no operation to exercise it and
> APPEND_DATA
> > since there is since people extend files all the time.
> There's that "extend" word again. (If Append was defined as allowing files
> to be extended it
> would make a lot more sense, but that does not appear to be the case, from
> the Microsoft
> definition I found and the definition in RFC8881.)
>
> There is an inconsistency in the RFC8881 definition, in that it mentions
> SETATTR of size and
> OPEN as operations affected. (I can see an argument that Setattr of size
> where the new size
> is greater than the old one is modifying the data starting at EOF,


Ok.  That makes sense to me.

but OPEN is completely
> irrelevant, I think?)
>

I don't think so.  Since you have to open a file to do writes whether they
append or not, Open has to be affected.


> >   As such, APPEND_DATA
> > is enforceable although the server might not enforce the distinction
> between
> > APPEND_DATA and WRITE_DATA.
> >
> > If I understand correctly, you are intending to accept an acl that
> APPEND_DATA and not
> > WRITE_DATA  and then not let the user open the file for write.   I don't
> know about your
> > users but I would certainly feel that was a "weird, irritating
> inconsistency".
> Yes, but it is consistent with local behaviour on FreeBSD.
> A file with APPEND_DATA, but not WRITE_DATA cannot be open'd with a POSIX
>   open("foo", O_APPEND | O_WRONLY, 0)
> either locally nor over NFS.
> (O_APPEND is a modifier and cannot be specified by itself.)
>
> You could argue with the FreeBSD ACL guys that this is not correct
> behaviour,
> but it what they chose to do and is what is currently wired into both UFS
> and ZFS.
>
> > It seems to me that ace masks were specified including APPEND_DATA to
> support
> > windows applications with insufficient thought as to how this might  map
> to the
> > world of UNIX.   Still, we are where we are and have to deal with this.
> Definitely agree.
>
> > How would you feel about the following revision of the paragraph for -05?
> >
> >  *  Servers that do not distinguish between WRITE_DATA and APPEND_DATA
> >      may need to make it clear to clients that support for append-only
> >      files is not present.  One way to do that is for requests to set
> NFSv4 ACLs
> >      where the handling for these two masks are different for any
> >      specified user or group to be rejected with     NFS4ERR_ATTRNOTSUPP.
>
> Hmm. I cannot think of any other way that an NFS server can make it clear
> to clients whether or not it supports APPEND_WRITE separately from
> WRITE_DATA
> and, like many statements you've referred to related to ACLs, the above
> seems
> "wishy washy" to me.


That's fair

If you want "wishy washy", just leave it as is, imho.
>

Leaving it as it is not wishy-washy.  It is totally bogus.  It says the
server can choose to support any set of masks it chooses.  I think we need
to cut that down and if being wishy-washy helps, I'm ok with that.

>
> I could add another optional attribute to my draft that reports which
> access mask
> bits in an ACE are enforced vs stored but not enforced.
>

I think that's a good idea.

However, it is not obvious to me how a client (at least the FreeBSD one)
> would
> actually use the attribute?
>

Perhaps not but I feel the information should be available.


> I think that, if the consensus is that "the server must make it clear to
> the client...",
>

We don't have a consensus yet but I think that is a sensible approach.

then you require the NFS4ERR_ATTRNOTSUPP reply when only one of APPEND_DATA,
> WRITE_DATA are specified.
>

Yes.

The FreeBSD server will be non-conformant,


Only if we say "MUST".  Given the history here, I think that "SHOULD" would
be reasonable.  I hope you don't think that's too wishy-washy


but who knows, maybe after it is an RFC,
> the "FreeBSD collective" can be swayed to not allow only one of
> APPEND_DATA/WRITE_DATA
> to be set?
>

"After it's an RFC" sounds nice :-)


> rick
>
>
>
>
>
> rick
>
> ________________________________________
> From: nfsv4 <[e-mail]<mailto:[e-mail]>> on
> behalf of David Noveck <[e-mail]<mailto:[e-mail]>>
> Sent: Friday, December 24, 2021 8:49 AM
> To: NFSv4; nfsv4-chairs; [e-mail]<mailto:[e-mail]>
> Subject: [nfsv4] Fwd: New Version Notification for
> draft-dnoveck-nfsv4-security-04.txt
>
> CAUTION: This email originated from outside of the University of Guelph.
> Do not click links or open attachments unless you recognize the sender and
> know the content is safe. If in doubt, forward suspicious emails to
> [e-mail]<mailto:[e-mail]>
>
>
> I've just posted security-04.   Thanks to Rick Macklem and Chuck Lever who
> made important suggestions that I hope are correctly addressed in this
> version.  An rfcdiff with -03 is not small but it is helpful to see what
> has changed.
>
> As previously discussed, I am proposing that the working group adopt this
> draft as a working group document.   I expect Brian and Zahed to set the
> timeline for that discussion.
>
> Please let me know about your suggestions for -05.
>
> ---------- Forwarded message ---------
> From: <[e-mail]<mailto:[e-mail]><mailto:
> [e-mail]<mailto:[e-mail]>>>
> Date: Fri, Dec 24, 2021 at 8:31 AM
> Subject: New Version Notification for draft-dnoveck-nfsv4-security-04.txt
> To: David Noveck <[e-mail]<mailto:[e-mail]
> ><mailto:[e-mail]<mailto:[e-mail]>>>
>
>
>
> A new version of I-D, draft-dnoveck-nfsv4-security-04.txt
> has been successfully submitted by David Noveck and posted to the
> IETF repository.
>
> Name:           draft-dnoveck-nfsv4-security
> Revision:       04
> Title:          Security for the NFSv4 Protocols
> Document date:  2021-12-24
> Group:          Individual Submission
> Pages:          129
> URL:
> https://www.ietf.org/archive/id/draft-dnoveck-nfsv4-security-04.txt
> Status:
> https://datatracker.ietf.org/doc/draft-dnoveck-nfsv4-security/
> Html:
> https://www.ietf.org/archive/id/draft-dnoveck-nfsv4-security-04.html
> Htmlized:
> https://datatracker.ietf.org/doc/html/draft-dnoveck-nfsv4-security
> Diff:
> https://www.ietf.org/rfcdiff?url2=draft-dnoveck-nfsv4-security-04
>
> Abstract:
>    This document describes the core security features of the NFSv4
>    family of protocols, applying to all minor versions.  The discussion
>    includes the use of security features provided by RPC on a per-
>    connection basis.
>
>    This preliminary version of the document, is intended, in large part,
>    to result in working group discussion regarding existing NFSv4
>    security issues and to provide a framework for addressing these
>    issues and obtaining working group consensus regarding necessary
>    changes.
>
>    When a successor document is eventually published as an RFC, it will
>    supersede the description of security appearing in existing minor
>    version specification documents such as RFC 7530 and RFC 8881.
>
>
>
>
> The IETF Secretariat
>
>
>