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

From: David Noveck <[email-masked]>
Date: Sat, 1 Jan 2022 10:23:38 -0500

On Tue, Dec 28, 2021, 11:32 PM Rick Macklem <[email-masked]> wrote:

> David Noveck wrote:
> > On Sun, Dec 26, 2021, 11:45 AM Rick Macklem wrote:
> > > David Noveck wrote:
> > > > Rick Macklem wrote:
> > > > [stuff snipped]
> > > > > Does the Linux client expect "owner-override"?
> > > > >If no one familiar with the Linux client answers this, I'll run a
> test on a Linux
> > > > > mount.
> > > > I ran the following little test program. It ran fine on local file
> systems and on
> > > > an NFSv4 mount for both the FreeBSD and Linux-5.15.1 clients, if
> > > > "owner-override" was applied to Read/Write.
> > >
> > > > Is there documentation for this option?  I have googled for these
> words several
> > > > times and come up empty.  We need a clear definition of these
> semantics to
> > > > resolve this issue in -05 and that documentation might be helpful.
>  If it isn't,
> > > > could you point me to code, preferably in bsd, implementing that
> option.
> > > Well, I'm not sure if/when the term "owner-override" cropped up, but
> here is
> > >a description of it from RFC 1813 pg. 98:
> >
> > Thanks.  This is helpful.
>
> > >   Another problem arises due to the usually stateful open
> > >   operation.  Most operating systems check permission at open
> > >   time, and then check that the file is open on each read and
> > >   write request. With stateless servers, the server cannot
> > >   detect that the file is open and must do permission checking
> > >   on each read and write call. UNIX client semantics of access
> > >   permission checking on open can be provided with the ACCESS
> > >   procedure call in this revision, which allows a client to
> > >   explicitly check access permissions without resorting to
> > >   trying the operation. On a local file system, a user can open
> > >   a file and then change the permissions so that no one is
> > >   allowed to touch it, but will still be able to write to the
> > >   file because it is open. On a remote file system, by
> > >   contrast, the write would fail. To get around this problem,
> > >   the server's permission checking algorithm should allow the
> > >   owner of a file to access it regardless of the permission
> > >   setting. This is needed in a practical NFS version 3 protocol
> > >   server implementation, but it does depart from correct local
> > >   file system semantics. This should not affect the return
> > >   result of access permissions as returned by the ACCESS.
> > >
>

In -05, the Author Aside quotes this in full, but the replacement text
refer to it without quoting it.

> > > See the 3rd sentence from the end.
>

I notice that it use the (lower-case) word "should".   given that RFC1813
is an informational RFC this probably isn't all that relevant.

In the replacement text I make this a MAY, principally because there are
other ways of providing for this

> > >
> > > As far as I know, servers implement the same thing for Read/Write
> > > for NFSv4.
>

This will be a MAY in -05.

> >
> > From your description and the testing you have done, it seems there is a
> > significant difference in that this special handling of the owner
> permissions only
> > applies to an open file in v4.  Is there a v4 server that does this
> v3-like thing even
> > for files that are not open?
> The FreeBSD server does. It does the "owner-override" for Read/Write using
> special stateids (which is what I assume you mean by "not open"?).
>

Yes.   This is not mentioned in -05 so is considered OK.

> It only uses the stateid to check for a conflicting Open_Share_Deny. (For
> special
> stateids, any Open_Share_Deny for the type of I/O would be considered
> conflicting.
>
> > > (FreeBSD does it for Read as well as Write, although the
> > > above only refers to Write.)
> >
> > I don't have a problem with that as long as:
> >  - it is limited to open files
> I can see an argument for not doing "owner-override" for special stateids.

Me too, but I don't present that in -05.

The counter argument would be along the lines of "nothing in the RFCs
> says that it cannot be done for special stateids and defining that in this
> draft changes the protocol.
>

I don't agree with that argument but I expect the working groups does and i
am drafting -05 on that basis.

It looks like we are stuck with this.

>
> To be honest, I am not aware of any extant clients that do Read/Write with
> special stateids at this time.
>

I'm at a loss to understand what they were included in the first place. I
have to check RFC8881 to see if server support is REQUIRED for this.

>
> > - It does not apply to the execute bit
> Not sure why execute would be handled differently that Read?
>

I was just confused about that.   Since this stuff does not apply to
ACCESS, it doesn't matter.

>
> > > For NFSv4 over encryption with client peer authentication, I think you
> > > can argue the following:
> > >
> > Although I don't want to get into arguing in a standards-track document,
> > I think you have the essence of the matter right.
> >
> > Since v4 is  stateful protocol, and now has reasonable security, we can
> reasonably
> > allow the server to rely on permission checking on open, if it chooses.
> Doing this
> > only for the owner would also be covered.
> >
> > > - Since the client can be trusted.
> > > and
> > > - Encryption subverts "man in the middle attacks" and the ability to
> > >   "sniff to acquire valid stateids".
> > > that an appropriate Open/Lock/Delegation stateid is sufficient to
> > > permission check the Read/Write and, as such, checking of mode/ACL
> > > is not required.
> > >
> > > --> Appropriate stateid would refer to a current one issued to that
> client
> > >      where either the Open associated with the Open/Lock stateid
> includes
> > >       the needed OPEN_SHARE_ACCESS or the Delegation is of the correct
> > >     type (READ_DELEGATION or WRITE_DELEGATION).
> >
> > I will do something similar in -05.
>

This will be an alternate way of addressing the issue.

Right now, both ways of doing this use MAY.

A statement you SHOULD do one or the other is meaningless without a
discussions of why you might bypass.   MUST is a possibility but for -05, I
will simply says that clients expect this behavior to be provided in some
way.

> >
> > > The above *might* be preferred to "owner-override" for Read/Write
> > > operations?
>

I try to explain plusses and minuses in -04 with giving a preference.

> > >
> > > rick
> > > ps: I am not sure if any NFSv4 server uses the stateid in Read/Write in
> > >       the way described above.  At this time, FreeBSD only uses these
> > >       stateids to check for conflicting Open/shares (byte range locks
> are
> > >      always advisory).
> This was simply meant to say "I do not know if any extant server does what
> I argued could be reasonable, above.
>
> > >
> > > If that's the case, then, for v4, the it and uw bits would be ignored
> and your test
> > > program would behave the same if you closed it and re-opened it before
> the
> > > write.
> Not sure what you mean? The permission checking would not change for Opens.
> If done locally on the client, due to a delegation being issued, an Access
> operation
> would be used.
>

Right.

>
> > Of that's the case, let me know so that I can address that issue for -05.
> rick
>
>
>
>
> If "owner-override" is turned off on the server, it fails for both the
> FreeBSD
> and Linux-5.15.1 client NFSv4 mounts. (The write(2) fails for FreeBSD and
> the
> close(2) fails for Linux, but that's ok. Unless there is an fsync(2),
> flushing writes
> to the server can be delayed until close(2). Both had empty files after the
> failure.)
>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <fcntl.h>
> #include <errno.h>
> #include <sys/param.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <err.h>
> #include <unistd.h>
>
> char buf[1024 * 1024];
>
> int
> main(int argc, char *argv[])
> {
>         int fd, i;
>
>         fd = open(argv[1], O_CREAT | O_RDWR, 0666);
>         if (fd < 0)
>                 err(1, "cannot open %s", argv[1]);
>         if (fchmod(fd, 0) < 0)
>                 err(1, "cannot chmod %s", argv[1]);
>         for (i = 0; i < 10; i++)
>                 if (write(fd, buf, sizeof(buf)) != sizeof(buf))
>                         err(1, "cannot write");
>         if (lseek(fd, SEEK_SET, 0) < 0)
>                 err(1, "cannot seek");
>         for (i = 0; i < 10; i++)
>                 if (read(fd, buf, sizeof(buf)) != sizeof(buf))
>                         err(1, "cannot read");
>         if (close(fd) < 0)
>                 err(1, "cannot close");
> }
>
> I do not have access to a Solaris or Mac OSX client to test, rick
>
> ________________________________________
> From: nfsv4 <[email-masked]<mailto:[email-masked]><mailto:
> [email-masked]<mailto:[email-masked]>>> on behalf of
> David Noveck <[email-masked]<mailto:[email-masked]><mailto:
> [email-masked]<mailto:[email-masked]>>>
> Sent: Friday, December 24, 2021 8:49 AM
> To: NFSv4; nfsv4-chairs; [email-masked]<mailto:[email-masked]
> ><mailto:[email-masked]<mailto:[email-masked]>>
> 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
> [email-masked]<mailto:[email-masked]><mailto:[email-masked]
> <mailto:[email-masked]>>
>
>
> 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: <[email-masked]<mailto:[email-masked]><mailto:
> [email-masked]<mailto:[email-masked]>><mailto:
> [email-masked]<mailto:[email-masked]><mailto:
> [email-masked]<mailto:[email-masked]>>>>
> Date: Fri, Dec 24, 2021 at 8:31 AM
> Subject: New Version Notification for draft-dnoveck-nfsv4-security-04.txt
> To: David Noveck <[email-masked]<mailto:[email-masked]
> ><mailto:[email-masked]<mailto:[email-masked]>><mailto:
> [email-masked]<mailto:[email-masked]><mailto:
> [email-masked]<mailto:[email-masked]>>>>
>
>
>
> 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
>
>
> _______________________________________________
> nfsv4 mailing list
> [email-masked]<mailto:[email-masked]><mailto:[email-masked]<mailto:
> [email-masked]>>
> https://www.ietf.org/mailman/listinfo/nfsv4
>

---

From: Brian Pawlowski <[email-masked]>
Date: Wed, 5 Jan 2022 23:12:37 -0800

Happy New Year everyone. 

I’d like to determine consensus on adoption of the draft security-04 described below. 

Apologies for delayed ask :-/ Dave posted this on 12/24 and the ask of group was on whether to adopt this draft as a formal working group document and a work item. 

I looked at the detailed comments since Dave’s email, they don’t preclude adoption of the document. We’ll sweep the detailed comments forward as working group business.

Please give me yays (adopt) or nays (with reason and description of what the objection us) please.

— Brian Pawlowski

On Dec 24, 2021, at 05:49, David Noveck <[email-masked]> wrote:

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.

---------- Forwarded message ---------
From: <[email-masked]>
Date: Fri, Dec 24, 2021 at 8:31 AM
Subject: New Version Notification for draft-dnoveck-nfsv4-security-04.txt
To: David Noveck <[email-masked]>

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:

The IETF Secretariat

---

From: Chuck Lever III <[email-masked]>
Date: Thu, 6 Jan 2022 20:30:20 +0000

> On Jan 6, 2022, at 2:12 AM, Brian Pawlowski <[email-masked]> wrote:
> 
> Happy New Year everyone. 
> 

Yay -- no objection to adoption.

--
Chuck Lever

---

From: [email-masked] (J. Bruce Fields)
Date: Fri, 7 Jan 2022 10:48:06 -0500

On Wed, Dec 29, 2021 at 04:32:10AM +0000, Rick Macklem wrote:
> > significant difference in that this special handling of the owner permissions only
> > applies to an open file in v4.  Is there a v4 server that does this v3-like thing even

There's also reboot recovery.  If you open a file, chmod to zero, then
reboot the server, the previous open should still work.

Also, if the client holds a delegation I think the server will allow the
file owner to override permissions for reads or writes or
CLAIM_DELEGATE_CUR opens, because in the face of cached opens the client
doesn't necessarily know if a file open happened before or after a
permission change.  (And a permission change doesn't necessarily revoke
a delegation if it came from the client holding the delegation.)

--b.

---

From: Rick Macklem <[email-masked]>
Date: Fri, 7 Jan 2022 17:11:36 +0000

> a delegation if it came from the client holding the delegation.)
Good point!

Currently, the FreeBSD server does permission checking for all Opens.
but I think you are correct.

For the cases of Claim_Previous, Claim_Delegate_Previous, the state is
being recovered and, as such, file permission checking should not be done.
--> I think these cases should use the same rule as ExchangeID/CreateSession,
      that is "same principal as first ExchangeID or SetClientID".
--> This could also be described as "must be done with machine principal".

Claim_Delegate_Cur is tougher, but I would say at least "owner-override"
should apply. (A client should be doing an Access check when local opens
are done, but if permissions are changed after that and the delegation is
recalled, having the Open/Claim_Delegate_Cur fail would be "unexpected".)

FreeBSD servers do not have delegations enabled by default and *hopefully*
crash infrequently. As such, I have never received a bug report related to a
failed Open/Claim_Previous or Open/Claim_Delegate_Cur, but this
could happen and I think I'll change the code.

This also makes my argument for using an Open stateid as a substitute
for file permission checking on Read/Write weaker. (This is not implemented
in the FreeBSD server and I am not planning on doing so at this time.)

rick

--b.

---

From: Rick Macklem <[email-masked]>
Date: Fri, 7 Jan 2022 17:31:48 +0000

Rick Macklem wrote:
[stuff snipped]
> --> This could also be described as "must be done with machine principal".
I don't think requiring "machine prinicipal" is in any RFC, so I doubt we can
do that now.
I will check the FreeBSD and Linux clients, to see what prinicipal they use
for Open/Claim_Previous in the next few days and report back.
(I have no access to the other clients.)

rick

---

From: David Noveck <[email-masked]>
Date: Fri, 7 Jan 2022 16:41:08 -0500

On Wed, Dec 29, 2021 at 12:42 PM Rick Macklem <[email-masked]> 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.

>          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.

> 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.

> 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?

> 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.

>   *  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.
 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".

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.

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.

> From: nfsv4 <[email-masked]> on behalf of David Noveck <
> [email-masked]>
> To: NFSv4; nfsv4-chairs; [email-masked]
> [email-masked]
> From: <[email-masked]<mailto:[email-masked]>>
> To: David Noveck <[email-masked]<mailto:[email-masked]>>
>
>
>

---

From: Rick Macklem <[email-masked]>
Date: Sat, 8 Jan 2022 16:41:03 +0000

David Noveck wrote:
[> Rick Macklem wrote:
[stuff snipped]
> 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.
Ok, yes. I found a Microsoft document that states APPEND_DATA allows writes
that start at EOF.

But what about:
- Setattr of size, where this size is beyond EOF.
  (Not covered by APPEND_DATA, so presumably allowed/denied via
   WRITE_DATA.)
- A Write with an offset > size (ie past EOF).
  (Also allowed/denied via WRITE_DATA.)

So, does WRITE_DATA being set without APPEND_DATA being set make
sense. Not really, but it is allowed by the RFCs, as far as I can see?
It just makes no sense to allow a Write that starts beyond EOF, but not
one that starts at EOF.

So, I understand your position, but I believe (it is not up to me), that
FreeBSD's NFSv4 server will simply choose to continue to ignore APPEND_DATA.

On the PSARC/2010/029 document:
- The author of the FreeBSD NFSv4 ACL code no longer has a copy, either.
   However, he recalls the main item in it was an alternate algorithm for
   converting mode->ACL. The "canonical six" upset Windows, because it
   mixes allow/deny ACEs and apparently Windows likes all the Allow ACEs
   to preceed all the Deny ACEs.
   - This alternate algorithm creates a set of Allow ACEs followed by a set of
      Deny ACEs that are semnatically equivalent to the "canonical six".

I should have a chance to do some NFSv4 ACL testing against a Linux
server on Sunday and will post after that.

rick

true but the server is still able to distinguish writes that extend the file from those that don't.

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?

allowed that is of size == 0. If APPEND_DATA is sufficient to allow an Open with

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.   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".

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.

      files is not present.  One way to do that is for requests to set NFSv4 ACLs

rick

________________________________________
From: nfsv4 <[email-masked]<mailto:[email-masked]>> on behalf of David Noveck <[email-masked]<mailto:[email-masked]>>
Sent: Friday, December 24, 2021 8:49 AM
To: NFSv4; nfsv4-chairs; [email-masked]<mailto:[email-masked]>
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 [email-masked]<mailto:[email-masked]>

---------- Forwarded message ---------
From: <[email-masked]<mailto:[email-masked]><mailto:[email-masked]<mailto:[email-masked]>>>
Date: Fri, Dec 24, 2021 at 8:31 AM
Subject: New Version Notification for draft-dnoveck-nfsv4-security-04.txt
To: David Noveck <[email-masked]<mailto:[email-masked]><mailto:[email-masked]<mailto:[email-masked]>>>

Abstract:

The IETF Secretariat

---

From: [email-masked] (J. Bruce Fields)
Date: Sat, 8 Jan 2022 17:23:11 -0500

>    to preceed all the Deny ACEs.

It's the other way around.  Googling....:

	https://docs.microsoft.com/en-us/windows/win32/secauthz/order-of-aces-in-a-dacl

>       Deny ACEs that are semnatically equivalent to the "canonical six".

So, we put Deny ACEs before Allow whenever possible.  Or, better yet,
just leave them out.

In theory Windows allows both but I'm told various tools complain if
your ACLs are weird.

--b.

---

From: Rick Macklem <[email-masked]>
Date: Sun, 9 Jan 2022 00:31:09 +0000

[stuff snipped]
> In theory Windows allows both but I'm told various tools complain if
Oops, my mistake.
This algorithm only generates Allow ACEs for simple cases like
# chmod 664
but for
# chmod 646
it generates

owner@:write/append Allow
group@:write/append Deny
owner@:read/write/append + a bunch more Allow
group@read + read acl Allow
everyone@:read/write/append + read acl Allow

So it puts the Deny before the Allow for any given "who".

rick

--b.

---

From: Rick Macklem <[email-masked]>
Date: Sun, 9 Jan 2022 23:15:34 +0000

From: David Noveck wrote:
> 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. Append and Extend are not the same thing. Append is specifically
a Write with "offset == size (or file)".
I believe Extend refers to any case where file size is increased?

> 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.

> 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.
>
Be careful with that "extend" word.
> 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;-)

> The spec  says that this is allowed.
Yea, I guess you are correct.

> 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.
>
> 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, but OPEN is completely
irrelevant, I think?)

>   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.

>      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. If you want "wishy washy", just leave it as is, imho.

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.
However, it is not obvious to me how a client (at least the FreeBSD one) would
actually use the attribute?

I think that, if the consensus is that "the server must make it clear to the client...",
then you require the NFS4ERR_ATTRNOTSUPP reply when only one of APPEND_DATA,
WRITE_DATA are specified.
The FreeBSD server will be non-conformant, 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?

rick

rick

---------- Forwarded message ---------
From: <[email-masked]<mailto:[email-masked]><mailto:[email-masked]<mailto:[email-masked]>>>
Date: Fri, Dec 24, 2021 at 8:31 AM
Subject: New Version Notification for draft-dnoveck-nfsv4-security-04.txt
To: David Noveck <[email-masked]<mailto:[email-masked]><mailto:[email-masked]<mailto:[email-masked]>>>

Abstract:

The IETF Secretariat

---

From: David Noveck <[email-masked]>
Date: Mon, 10 Jan 2022 01:37:28 -0500

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

> 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.

> operations

I think I am.

> time.

It would also be looked at servers supporting Windows clients.

> distinguish WRITE_DATA
> SYNCHRONIZE which
> 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.

> APPEND_DATA and not
> support
> "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.

> 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

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

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

> 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

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

> From: nfsv4 <[email-masked]<mailto:[email-masked]>> on
> behalf of David Noveck <[email-masked]<mailto:[email-masked]>>
> [email-masked]<mailto:[email-masked]>
>
>
> [email-masked]<mailto:[email-masked]>>>
> ><mailto:[email-masked]<mailto:[email-masked]>>>
>
>
>

---

From: Rick Macklem <[email-masked]>
Date: Mon, 10 Jan 2022 16:02:39 +0000

David Noveck wrote:
> 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.
Yes. It does not make sense, but I believe that is just the way it is.

> 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.
Here's a Microsoft doc I found:
https://docs.microsoft.com/en-us/dotnet/api/system.security.accesscontrol.filesystemrights?view=net-6.0
I believe Extend refers to any case where file size is increased?

It states that APPEND_DATA is:
Specifies the right to append data to the end of a file.

When I read this, I think "Write starting at EOF", but maybe you can
include "Write starting beyond EOF".
However, I do not see how you can include a Write where offset < size
and offset +length > size under the above definition?
(But this would "extend" the file.)

It states that WRITE_DATA is:
Specifies the right to open and write to a file or folder. This does not include the right to open and write file system attributes, extended file system attributes, or access and audit rules.

This does not seem to preclude Writes that extend the file.
It is also worth noting that APPEND_WRITE does not say anything
about a "right to open" whereas WRITE_DATA does allow a file
to be opened.

We really need a Windows/CifsSMB wizard to answer what Windows
really expects these to be implemented as.

rick

I think I am.

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.

Definitely agree.

That's fair

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.

Yes.

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

rick

rick

________________________________________
From: nfsv4 <[email-masked]<mailto:[email-masked]><mailto:[email-masked]<mailto:[email-masked]>>> on behalf of David Noveck <[email-masked]<mailto:[email-masked]><mailto:[email-masked]<mailto:[email-masked]>>>
Sent: Friday, December 24, 2021 8:49 AM
To: NFSv4; nfsv4-chairs; [email-masked]<mailto:[email-masked]><mailto:[email-masked]<mailto:[email-masked]>>
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 [email-masked]<mailto:[email-masked]><mailto:[email-masked]<mailto:[email-masked]>>

---------- Forwarded message ---------
From: <[email-masked]<mailto:[email-masked]><mailto:[email-masked]<mailto:[email-masked]>><mailto:[email-masked]<mailto:[email-masked]><mailto:[email-masked]<mailto:[email-masked]>>>>
Date: Fri, Dec 24, 2021 at 8:31 AM
Subject: New Version Notification for draft-dnoveck-nfsv4-security-04.txt
To: David Noveck <[email-masked]<mailto:[email-masked]><mailto:[email-masked]<mailto:[email-masked]>><mailto:[email-masked]<mailto:[email-masked]><mailto:[email-masked]<mailto:[email-masked]>>>>

Abstract:

The IETF Secretariat

---

From: [email-masked] (J. Bruce Fields)
Date: Thu, 13 Jan 2022 17:04:09 -0500

p. 13: As I said before, I'm not OK with this attempt to pin down ACL
semantics and mode/ACL interactions.  See
https://mailarchive.ietf.org/arch/msg/nfsv4/pbGEGwDvMwknwjsBKsWaK2BRXqc/

p. 20:

	"many clients are built on systems whose only ACL-related API is
	based on the UNIX ACL model."

What clients are you thinking of?  The Linux client, for example, does
provide an NFSv4 ACL API, and we have tools built on it that allow users
to get and set ACLs.

	"it is hard to imagine a server incapable of distinguishing a
	write to an offset within existing file and one beyond it."

Most of the filesystems Linux servers export don't support separate
WRITE and APPEND bits.

	"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."

Once upon a time we did have code that rejected any ACL that couldn't be
translated into something storable on our backend filesystems.  It was
painful to use: there's more than one rule governing allowable mask bits
and ALLOW/DENY ordering, and if you mess up, you get one error that
doesn't tell you why.

That's why the Linux server now accepts such ACLs and instead translates
them to something stricter.

You get the guarantee that the ACL won't allow access beyond that you
requested.  And the option of reading back the set ACL gives you at
least some chance of figuring out what happened, where an
NFS4ERR_ATTRNOTSUPP doesn't.

p. 41:

	"The deletion of the file's data is dealt with separately as
	this, like a truncation to length zero, requires
	ACE4_WRITE_DATA."

We can't require write permissions on a file to delete the file.

p. 42:

	"The complexity of the approach without any indication that
	there is a need for such complexity makes me doubtful that
	anything was actually implemented"

I'm pretty sure that section is just a description of what ZFS does, for
what it's worth.

p. 56:

	"Setting only the mode attribute, will result in the access of
	the file being controlled just it would be if the existing acl
	did not exist, with file access decisions as to read made in
	accordance with the mode set."

Neither Linux nor Solaris/ZFS work that way.

p. 61:

	"While the NFSv4.1 provides that many might not need or use, it
	is the one that the working group adopted by the working group,
	and I have to assume that alternatives, such as the withdrawn
	POSIX ACL proposal were considered but not adopted."

Right.  As I understand it, that decision was based in part on the
assumption that it would be possible to map between POSIX and NFSv4
ACLs.  Linux knfsd has been doing that since the beginning.  I don't
agree that this document should now go back and disallow that.

p. 72:

	"The algorithm specified below, now considered the Previous
	Treatment associated with Item #24a, has an important flaw in
	does not deal with the (admittedly uncommon) case in which the
	owner_group has less access than the owner or others have less
	access than the owner-group."

We definitely knew about "reverse-slope" modes and took them into
account.

I'm not entirely sure I understand the algorithm at the end of section
9.3, but I believe it's wrong.  It appears that it would map an ACL
like (ignoring some mask-bit details):

	DENY  OWNER@	 W
	ALLOW EVERYONE@ RW

to the mode r--r--r--.  (Because the OWNER@ ACE applies "in the negative
sense" to the calculation of the group and other bits.)

But the above ACL actually grants write permissions to anyone other than
the file owner, and the correct mode is r--rw-rw-.

(And I also don't like the fact that it ignores named users and groups,
so that, e.g.,

	ALLOW bfields RW

will be mapped to mode 0.)

(I ran out of time here and stopped reading.)

--b.

---

From: [email-masked] (J. Bruce Fields)
Date: Fri, 14 Jan 2022 16:45:09 -0500

> (O_APPEND is a modifier and cannot be specified by itself.)

How would an NFS client write to it, anyway?  Do you just tell people
they need to use O_SYNC for such files?

Otherwise writes are likely to get reordered, and then the writer gets
random EACCES (and random unfillable holes, if you allow writes at
offsets past EOF).  Seems like a bit of a foot-gun.

Oh well, out of scope here, I suppose.

--b.

---

From: Rick Macklem <[email-masked]>
Date: Fri, 14 Jan 2022 23:48:03 +0000

> they need to use O_SYNC for such files?
When a file is POSIX open(...O_APPEND..)'d in FreeBSD on an NFS mount
point, each write is pushed through to the server (with an exclusive lock
held on the vnode) FILESYNC.
The result maintains ordering, but is very slow compared to doing the
same on local file systems. For a test case doing 10,000 writes, it
takes 13sec over NFS vs 0.3sec locally for UFS (even slower for ZFS,
takes 2minutes for ZFS with sync enabled and no ZIL log).

> offsets past EOF).  Seems like a bit of a foot-gun.
Yep, it forces O_SYNC basically.

> Oh well, out of scope here, I suppose.
Not really. If another client writes to the file, it could still get "random"
EACCES failures.
As I mentioned, the FreeBSD server stores APPEND_DATA, but ignores it.
I recall Robert Watson (who designed the FreeBSD NFSv4 ACL stuff)
stating as the reason "because it's such a pita".
For FreeBSD, this is also true of ZFS. (The ACL enforcing part of ZFS
is in the port specific code, so I don't know what Linux/Solaris does
w.r.t. APPEND_DATA for ZFS.)

rick

--b.

---

From: David Noveck <[email-masked]>
Date: Sun, 16 Jan 2022 04:03:12 -0500

I'd like to thank Rick and Bruce for their helpful comments.

It seems to me that, given the absence, of an append-write in the NFSv4
protocol, APPEND_WRITE belongs in the same bucket as SYNCHRONIZE and I am
planning to draft -05 on that basis.

Looking forward to -06, I'd like to get away from the text which makes each
mask bit its own optional feature,. Clearly READ_DATA, WRITE _DATA, and
EXECUTE should be mandatory but what about the others?  Should any of these
be mandatory?  If not, why isn't it implemented? Are there
applications/clients that rely on it?

---

From: "J. Bruce Fields" <[email-masked]>
Date: Mon, 17 Jan 2022 09:51:38 -0500

> applications/clients that rely on it?

Again, I don't agree that it's desirable to specify this level of
detail.

NFS has always been a protocol which allows exporting filesystems with
semantics that vary somewhat.

I'd recommend instead thinking hard about what's essential to do in this
document and looking for ways to reduce its scope and length.

We've got limited resources, and the attempt to produce a really
detailed synthesis of unix and Windows permissions models is an
ambitious project.

--b.

---

From: David Noveck <[email-masked]>
Date: Tue, 18 Jan 2022 14:06:17 -0500

> > planning to draft -05 on that basis.

I take it that you agree with that.

> > EXECUTE should be mandatory

Do you disagree with that.

but what about the others?  Should any of these
> > be mandatory?

It appears that you don't think they should.

  If not, why isn't it implemented? Are there
> > applications/clients that rely on it

> Again, I don't agree that it's desirable to specify this level of

I don't see how you can avoid it. I can see it is undesirable but all those
mask bits are in the spec.  This undesirable level of detail is there and I
can't see specifying it and then essentially saying, obtw, you can pretty
much ignore all of the above and clients just have to deal with it.

> semantics that vary somewhat.

It was able to do that because, for most of its lifetime it was a purely
unix-oriented protocol and did not try to produce an IETF proposed standard
to enable compatible implementations to be produced.  Sigh!

> document and looking for ways to reduce its scope and length.

The scope and length are the result of decisions made in writing RFCs 3530
and 5661.

> detailed synthesis of unix and Windows permissions models is an

It was overambitious and I and others involved failed to point out this
problem before this was published.  I realize this puts the working group
in an unfortunate situation but we are where we are.

I believe the best way forward is to look at the windows support features
as an optional extra and define clearly the Unix oriented subset that
clients can rely on.

I appreciate any help people can provide in drawing that line.

> > > > > > WRITE_DATA  and then not let the user open the file for write.
>  I
> > > don't know about your

---

From: Trond Myklebust <[email-masked]>
Date: Tue, 18 Jan 2022 20:09:56 +0000

> I take it that you agree with that.

I don't. The restriction on extending the file can still be enforced by
the server even without there being an explicit APPEND_WRITE operation.

One use case would be for swap over NFS.

> I don't see how you can avoid it. I can see it is undesirable but all
> those mask bits are in the spec.  This undesirable level of detail is
> there and I can't see specifying it and then essentially saying,
> obtw, you can pretty much ignore all of the above and clients just
> have to deal with it.
> 
> > 
> It was able to do that because, for most of its lifetime it was a
> purely unix-oriented protocol and did not try to produce an IETF
> proposed standard to enable compatible implementations to be
> produced.  Sigh!
> 
> > 
> The scope and length are the result of decisions made in writing RFCs
> 3530 and 5661.
> 
> > 
> It was overambitious and I and others involved failed to point out
> this problem before this was published.  I realize this puts the
> working group in an unfortunate situation but we are where we are.
> 
> I believe the best way forward is to look at the windows support
> features as an optional extra and define clearly the Unix oriented
> subset that clients can rely on.
> 

Do we have any indication that clients have a problem with the current
ACLs? My own testing, with Hammerspace systems that are used for both
SMB and NFSv4 clients has not shown up any protocol level problems yet.

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email-masked]