[PATCH] Rework patch for bug 7909

[Ralph Böhme]

Hi!

Back in the days a fix for bug 7909 resulted in the common VFS NFS4 ACL
framework to set the SEC_STD_SYNCHRONIZE bit. This was deemed necessary just to
make ZFS happy: the ZFS guys didn't want the SEC_STD_SYNCHRONIZE bit to appear
in the mask in the filesystem and thus are filtering it out of ACEs when setting
ACLs.

Now when fetching ACLs it turned out to be necessary to stick
SEC_STD_SYNCHRONIZE back into the access mask, otherwise rename operation would
fail (client side behaviour).

Other consumers of the framework like vfs_gpfs do not need this behaviour, if
ZFS needs this, it must be put into vfs_zfsacl.

Having divergent behaviour wrt to SEC_STD_SYNCHRONIZE in all modules that use
the NFS4 ACL framework also means that torture tests like "raw.acls.generic"
fail and can't be used to test the modules which severly limits test coverage.

Please review & push if ok. Thanks!

Cheerio!
-slow
-------------- next part --------------
From a2e4e966622db0aa7f8e7be893c8c3d2b2c56741 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 6 Sep 2017 16:28:10 +0200
Subject: [PATCH] vfs/nfs4_acls: move special handling of SMB_ACE4_SYNCHRONIZE
 to vfs_zfsacl

Commit 99a74ff5e6a9f87ad7a650cb44e0f925f834b3a1 added special handling
of SMB_ACE4_SYNCHRONIZE, always setting it in the access_mask when
fabricating an ACL. While at the same time removing it from the
access_mask when setting an ACL, but this is done direclty in
vfs_zfsacl, not it the common code.

Forcing SMB_ACE4_SYNCHRONIZE to be always set is only needed on ZFS, the
other VFS modules using the common NFSv4 infrastructure should not be
made victims of the special ZFS behaviour.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=7909

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/nfs4_acls.c  | 7 -------
 source3/modules/vfs_zfsacl.c | 9 +++++++++
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/source3/modules/nfs4_acls.c b/source3/modules/nfs4_acls.c
index 2007917821b..c715b83390d 100644
--- a/source3/modules/nfs4_acls.c
+++ b/source3/modules/nfs4_acls.c
@@ -385,13 +385,6 @@ static bool smbacl4_nfs42win(TALLOC_CTX *mem_ctx,
 		      ace->aceFlags, win_ace_flags));
 
 		mask = ace->aceMask;
-		/* Windows clients expect SYNC on acls to
-		   correctly allow rename. See bug #7909. */
-		/* But not on DENY ace entries. See
-		   bug #8442. */
-		if(ace->aceType == SMB_ACE4_ACCESS_ALLOWED_ACE_TYPE) {
-			mask = ace->aceMask | SMB_ACE4_SYNCHRONIZE;
-		}
 
 		/* Mapping of owner@ and group@ to creator owner and
 		   creator group. Keep old behavior in mode special. */
diff --git a/source3/modules/vfs_zfsacl.c b/source3/modules/vfs_zfsacl.c
index 76cf5281af7..4cb1b98f01b 100644
--- a/source3/modules/vfs_zfsacl.c
+++ b/source3/modules/vfs_zfsacl.c
@@ -84,6 +84,15 @@ static NTSTATUS zfs_get_nt_acl_common(TALLOC_CTX *mem_ctx,
 		aceprop.aceMask  = (uint32_t) acebuf[i].a_access_mask;
 		aceprop.who.id   = (uint32_t) acebuf[i].a_who;
 
+		/*
+		 * Windows clients expect SYNC on acls to correctly allow
+		 * rename, cf bug #7909. But not on DENY ace entries, cf bug
+		 * #8442.
+		 */
+		if (aceprop.aceType == SMB_ACE4_ACCESS_ALLOWED_ACE_TYPE) {
+			aceprop.aceMask |= SMB_ACE4_SYNCHRONIZE;
+		}
+
 		if(aceprop.aceFlags & ACE_OWNER) {
 			aceprop.flags = SMB_ACE4_ID_SPECIAL;
 			aceprop.who.special_id = SMB_ACE4_WHO_OWNER;
-- 
2.13.5