Page MenuHomeFreeBSD

Expose NFSv41 ACL automatic inheritance flags
Needs ReviewPublic

Authored by walker.aj325_gmail.com on Aug 11 2020, 5:35 PM.
Tags
None
Referenced Files
F80144865: D26028.diff
Thu, Mar 28, 1:11 PM
Unknown Object (File)
Sat, Mar 2, 10:34 PM
Unknown Object (File)
Feb 16 2024, 1:34 PM
Unknown Object (File)
Feb 16 2024, 1:34 PM
Unknown Object (File)
Feb 16 2024, 1:34 PM
Unknown Object (File)
Feb 16 2024, 1:21 PM
Unknown Object (File)
Jan 27 2024, 3:09 PM
Unknown Object (File)
Jan 10 2024, 2:03 AM

Details

Reviewers
freqlabs
trasz
delphij
rmacklem
Group Reviewers
manpages
Summary

RFC5661 6.4.3.2 Automatic Inheritance defines an optional mechanism for
propagating changes to inheritable ACEs to an entire directory hierarchy.

These are: ACL4_AUTO_INHERIT, ACL4_PROTECTED, ACL4_DEFAULTED

Support for these flags is a requirement for faithfully reproducing NTFS
ACL inheritance behavior in Samba. This commit provides minimal functionality
to allow correct behavior in Samba and provide basic administrative capabilities
to the end-user. This minimal feature set does the following:

  1. exposes existing ZFS flags
  2. introduces libc ACL API changes to allow getting / setting them through acl_set_aclflag_np(3) and acl_get_aclflag_np(3)
  3. adds support for these flags to getfacl / setfacl

getfacl changes:
Introduce new flag "-f", which adds the acl_flag as a comment to the ACL
output. This is done so as to not impact the ability to pipe output into setfacl.

setfacl changes:
Introduce two new options and one new flag:
-i sets the ACL auto inheritance flag. Possible values are "none", "auto-inherit", "protected", and "defaulted". The only flag that has functional impact is "protected".

-p performs NFSv41 ACL automatic inheritance. This is only permitted on paths that have the "protected" flag set. The automatic inheritance operation recursively traverses the direcory's descendenants and modifies each ACL encounted to remove all ACEs with the INHERITED flag set, and replace them with new inherited ACEs calculated from the parent ACL. ACEs without the INHERITED flag set are left untouched.

New inherited ACEs are appended to the end of the existing ACL to maintain consistency with Windows canonical ordering of ACEs. Due to the nature of this operation, the recursive (-R) flag is _always_ set.

-f forces bypassing of protective measures regarding ACL auto-inheritance.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

walker.aj325_gmail.com edited the summary of this revision. (Show Details)

Added context and basic regression test. Switched to using cwd for parent directory.

Can you run textproc/igor and "mandoc -Tlint" on the man page changes to see if they spit out any warnings?

bin/getfacl/getfacl.1
69

s/infomration/information/

lib/libc/posix1e/acl_get_aclflag_np.3
47

Is there a need to break this line here? I think you can pull up the "is returned" below to this line.

lib/libc/posix1e/acl_set_aclflag_np.3
47

Same here...

sys/sys/acl.h
123

Comment still relevant?

125

May be confused with the acl_flag_t type/meaning

I don't see the Makefile change that hooks up the new man pages to the build.

Also, you might want to take a look at https://github.com/openzfs/zfs/pull/10520 and make sure there's no duplicate work.

bin/getfacl/getfacl.c
254

Should this be upper case?

bin/setfacl/auto_inherit.c
59

I think this should be done the other way round - first call acl_get_file(fl[i], ACL_TYPE_NFS4), and only if it fails, then try to be helpful and call pathconf() to see what's the reason.

112

Likewise.

138

Shouldn't we log a warning in this case?

308

Consistency: we usually don't start warn(3) messages with a capital letter, nor terminate them with a full stop.

362

Consistency: "%s: parent directory does not" etc, perhaps?

bin/setfacl/setfacl.c
548

Why the full stops?

bin/setfacl/setfacl.h
38

This is usually spelled nitems(), see sys/sys/param.h.

lib/libc/posix1e/Symbol.map
82

I _think_ this belongs in FBSD_1.6, not FBSD_1.1. See lib/libc/gen/Symbol.map. Note I might be wrong about this; you might want to ask kib@ to make sure.

lib/libc/posix1e/acl_flag.c
182

Shouldn't we do _acl_brand_may_be(), like eg in acl_set_flagset_np()?

tests/sys/acl/tools-nfs4-auto-inherit.test
1

You might want to update the copyright.