On Wed, 2023-07-19 at 19:02 +0000, Chuck Lever III wrote: > > > On Jul 19, 2023, at 1:49 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote: > > > > A well-formed NFSv4 ACL will always contain OWNER@/GROUP@/EVERYONE@ > > ACEs, but there is no requirement for inheritable entries for those > > entities. POSIX ACLs must always have owner/group/other entries, even for a > > default ACL. > > > > nfsd builds the default ACL from inheritable ACEs, but the current code > > just leaves any unspecified ACEs zeroed out. The result is that adding a > > default user or group ACE to an inode can leave it with unwanted deny > > entries. > > > > For instance, a newly created directory with no acl will look something > > like this: > > > > # NFSv4 translation by server > > A::OWNER@:rwaDxtTcCy > > A::GROUP@:rxtcy > > A::EVERYONE@:rxtcy > > > > # POSIX ACL of underlying file > > user::rwx > > group::r-x > > other::r-x > > > > ...if I then add new v4 ACE: > > > > nfs4_setfacl -a A:fd:1000:rwx /mnt/local/test > > > > ...I end up with a result like this today: > > > > user::rwx > > user:1000:rwx > > group::r-x > > mask::rwx > > other::r-x > > default:user::--- > > default:user:1000:rwx > > default:group::--- > > default:mask::rwx > > default:other::--- > > > > A::OWNER@:rwaDxtTcCy > > A::1000:rwaDxtcy > > A::GROUP@:rxtcy > > A::EVERYONE@:rxtcy > > D:fdi:OWNER@:rwaDx > > A:fdi:OWNER@:tTcCy > > A:fdi:1000:rwaDxtcy > > A:fdi:GROUP@:tcy > > A:fdi:EVERYONE@:tcy > > > > ...which is not at all expected. Adding a single inheritable allow ACE > > should not result in everyone else losing access. > > > > The setfacl command solves a silimar issue by copying owner/group/other > > entries from the effective ACL when none of them are set: > > > > "If a Default ACL entry is created, and the Default ACL contains no > > owner, owning group, or others entry, a copy of the ACL owner, > > owning group, or others entry is added to the Default ACL. > > > > Having nfsd do the same provides a more sane result (with no deny ACEs > > in the resulting set): > > > > user::rwx > > user:1000:rwx > > group::r-x > > mask::rwx > > other::r-x > > default:user::rwx > > default:user:1000:rwx > > default:group::r-x > > default:mask::rwx > > default:other::r-x > > > > A::OWNER@:rwaDxtTcCy > > A::1000:rwaDxtcy > > A::GROUP@:rxtcy > > A::EVERYONE@:rxtcy > > A:fdi:OWNER@:rwaDxtTcCy > > A:fdi:1000:rwaDxtcy > > A:fdi:GROUP@:rxtcy > > A:fdi:EVERYONE@:rxtcy > > > > Link: https://bugzilla.redhat.com/show_bug.cgi?id=2136452 > > Reported-by: Ondrej Valousek <ondrej.valousek@xxxxxxxxxxx> > > Suggested-by: Andreas Gruenbacher <agruen@xxxxxxxxxx> > > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> > > As you pointed out in the bug report, there is not much testing > infrastructure for NFSv4 ACLs. It will be hard to tell in > advance if this change results in a behavior regression. > > On the other hand, I'm not sure we have a large cohort of > NFSv4 ACL users on Linux. > > I can certainly apply this to nfsd-next at least for a few > weeks to see if anyone yelps. > Thanks, that's probably the best we can do, given the state of v4 ACL test coverage. -- Jeff Layton <jlayton@xxxxxxxxxx>
