Issue with open_file_nt_create() and NFSv4 ACLs (ZFS)

Andrew Walker awalker at ixsystems.com
Thu Jul 9 13:46:07 UTC 2020


In principal, when using NFSv4 ACLs, the ACL on the parent directory will
determine the ACL on newly created files and directories. The problem is as
follows:

1) Admin removes CREATOR-OWNER, CREATOR-GROUP, and WORLD, which maps to
owner@, group@, everyone@ from the ACL, leaving only something like the
following when viewed server-side:

root at fbsd12build:/usr/ports/net/samba # getfacl /zroot/SMB/
# file: /zroot/SMB/
# owner: root
# group: wheel
      user:smbuser:rwxpDdaARWcCos:fd-----:allow
      user:awalker:rwxpDdaARWcCos:fd-----:allow

2) User creates new file, kernel sets correct inherited ACL, and samba then
calls SMB_VFS_CHMOD() on the file in open_file_nt_create() because "inherit
acls" is false and we short-circuit before calling
directory_has_default_acl(). This results in additional entries for owner@,
group@, and everyone@ being added to the new file, and upset sys admins.

I'm somewhat torn between how to handle this. There are very much a finite
number of brands of ACLs. My thought is that perhaps we can expand struct
connection_struct with an "acl_brand" enum (set perhaps in the
fs_capabilities function) so that we can do something like below  to
cleanly separate POSIX1E behavior from NFSv4 behavior (following is
sufficient to avoid the unwanted fchmod()). Then again, we could possibly
move more of this to the VFS.

--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -3638,10 +3638,24 @@ static NTSTATUS
open_file_ntcreate(connection_struct *conn,
        /*
         * Ensure we pay attention to default ACLs on directories if
required.
         */
-
-        if ((flags2 & O_CREAT) && lp_inherit_acls(SNUM(conn)) &&
-           (def_acl = directory_has_default_acl(conn, parent_dir))) {
-               unx_mode = (0777 & lp_create_mask(SNUM(conn)));
+       switch(conn->aclbrand) {
+       case SMB_ACL_BRAND_POSIX:
+               if ((flags2 & O_CREAT) && lp_inherit_acls(SNUM(conn)) &&
+                   (def_acl = directory_has_default_acl(conn,
parent_dir))) {
+                       unx_mode = (0777 & lp_create_mask(SNUM(conn)));
+               }
+               break;
+       default:
+               /*
+                * Pass along to VFS to determine whether the ACL has
+                * any inheriting entries.
+                */
+               if (flags2 & O_CREAT) {
+                       def_acl = directory_has_default_acl(conn,
parent_dir);
+               }
+               break;
        }


More information about the samba-technical mailing list