[Samba] Issue with acl_xattr:ignore system acls in 4.5rc2

Ralph Böhme slow at samba.org
Fri Aug 26 16:44:05 UTC 2016


On Fri, Aug 26, 2016 at 09:39:29AM -0700, Jeremy Allison wrote:
> On Fri, Aug 26, 2016 at 06:33:26PM +0200, Ralph Böhme via samba wrote:
> > On Thu, Aug 25, 2016 at 12:14:00PM -0700, Jeremy Allison wrote:
> > > On Wed, Aug 24, 2016 at 04:06:42PM +0200, Ralph Böhme via samba wrote:
> > > > 
> > > > Yeah, as much as I'd like to avoid adding a new option, I guess we
> > > > have to do something about it, my latest take on this is
> > > > 
> > > >        acl_xattr:default acl style = [posix|windows]
> > > > 
> > > >            This parameter determines the type of ACL that is
> > > >            synthesized in case a file or directory lacks an
> > > >            security.NTACL xattr.
> > > > 
> > > >            When set to posix, an ACL will be synthesized based on the
> > > >            POSIX mode permissions for user, group and others, with an
> > > >            additional ACE for NT Authority\SYSTEM will full rights..
> > > > 
> > > >            When set to windows, an ACL is synthesized the same way
> > > >            Windows does it, only inclusing permissions for the owner
> > > >            and NT Authority\SYSTEM
> > > > 
> > > >            The default for this option is posix.
> > > > 
> > > > tldr: this reverts behaviour to what it was before #12028 and make the
> > > > behaviour introduced by #12028 optional.
> > > > 
> > > > Plan? Michael? Uri? Jeremy?
> > > 
> > > I like this. Puts the tweak in the right place IMHO.
> > 
> > ok, final patchset attached. Bonus: it has a test.
> 
> ENOPATCH :-).

*gnah* :)

Cheerio!
-slow
-------------- next part --------------
From dd4665a7930fad64ff649110cb087b69b08464f4 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 24 Aug 2016 10:04:24 +0200
Subject: [PATCH 01/12] Revert "vfs_acl_xattr: objects without NT ACL xattr"

This reverts commit 961c4b591bb102751079d9cc92d7aa1c37f1958c.

Subsequent commits will add the same functionality as an optional
feature.

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_acl_common.c | 43 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 39 insertions(+), 4 deletions(-)

diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index 2fda938e..a287945 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -379,10 +379,12 @@ static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx,
 	gid_to_sid(&group_sid, psbuf->st_ex_gid);
 
 	/*
-	 * We provide 2 ACEs:
-	 * - Owner
-	 * - NT System
-	 */
+	 We provide up to 4 ACEs
+		- Owner
+		- Group
+		- Everyone
+		- NT System
+	*/
 
 	if (mode & S_IRUSR) {
 		if (mode & S_IWUSR) {
@@ -402,6 +404,39 @@ static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx,
 			0);
 	idx++;
 
+	access_mask = 0;
+	if (mode & S_IRGRP) {
+		access_mask |= SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE;
+	}
+	if (mode & S_IWGRP) {
+		/* note that delete is not granted - this matches posix behaviour */
+		access_mask |= SEC_RIGHTS_FILE_WRITE;
+	}
+	if (access_mask) {
+		init_sec_ace(&aces[idx],
+			&group_sid,
+			SEC_ACE_TYPE_ACCESS_ALLOWED,
+			access_mask,
+			0);
+		idx++;
+	}
+
+	access_mask = 0;
+	if (mode & S_IROTH) {
+		access_mask |= SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE;
+	}
+	if (mode & S_IWOTH) {
+		access_mask |= SEC_RIGHTS_FILE_WRITE;
+	}
+	if (access_mask) {
+		init_sec_ace(&aces[idx],
+			&global_sid_World,
+			SEC_ACE_TYPE_ACCESS_ALLOWED,
+			access_mask,
+			0);
+		idx++;
+	}
+
 	init_sec_ace(&aces[idx],
 			&global_sid_System,
 			SEC_ACE_TYPE_ACCESS_ALLOWED,
-- 
2.7.4


From ac9828d7fcc48af51f59f7b271210a6328abed0f Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 23 Aug 2016 13:08:12 +0200
Subject: [PATCH 02/12] vfs_acl_common: rename psd to psd_blob in
 get_nt_acl_internal()

This makes it explicit where the SD is originating from. No change in
behaviour.

This just paves the way for a later change that will simplify the whole
logic and talloc hierarchy, therefor this also strictly renames the
occurences after the out label.

Logically, behind the out label, we're dealing with a variable that
points to what we're going to return, so the name psd_blob is
misleading, but I'm desperately trying to avoid logic changes in this
commit and therefor I'm just strictly renaming.

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_acl_common.c | 58 ++++++++++++++++++++--------------------
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index a287945..1adc875 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -488,7 +488,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 	uint8_t sys_acl_hash[XATTR_SD_HASH_SIZE];
 	uint8_t hash_tmp[XATTR_SD_HASH_SIZE];
 	uint8_t sys_acl_hash_tmp[XATTR_SD_HASH_SIZE];
-	struct security_descriptor *psd = NULL;
+	struct security_descriptor *psd_blob = NULL;
 	struct security_descriptor *pdesc_next = NULL;
 	const struct smb_filename *smb_fname = NULL;
 	bool ignore_file_system_acl = lp_parm_bool(SNUM(handle->conn),
@@ -509,25 +509,25 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(10, ("get_nt_acl_internal: get_acl_blob returned %s\n",
 			nt_errstr(status)));
-		psd = NULL;
+		psd_blob = NULL;
 		goto out;
 	} else {
-		status = parse_acl_blob(&blob, mem_ctx, &psd,
+		status = parse_acl_blob(&blob, mem_ctx, &psd_blob,
 					&hash_type, &xattr_version, &hash[0], &sys_acl_hash[0]);
 		if (!NT_STATUS_IS_OK(status)) {
 			DEBUG(10, ("parse_acl_blob returned %s\n",
 				   nt_errstr(status)));
-			psd = NULL;
+			psd_blob = NULL;
 			goto out;
 		}
 	}
 
-	/* Ensure we don't leak psd if we don't choose it.
+	/* Ensure we don't leak psd_blob if we don't choose it.
 	 *
 	 * We don't allocate it onto frame as it is preferred not to
 	 * steal from a talloc pool.
 	 */
-	talloc_steal(frame, psd);
+	talloc_steal(frame, psd_blob);
 
 	/* determine which type of xattr we got */
 	switch (xattr_version) {
@@ -550,8 +550,8 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 			   "mismatch (%u) for file %s\n",
 			   (unsigned int)hash_type,
 			   smb_fname->base_name));
-		TALLOC_FREE(psd);
-		psd = NULL;
+		TALLOC_FREE(psd_blob);
+		psd_blob = NULL;
 		goto out;
 	}
 
@@ -561,8 +561,8 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 			   "(%u) unexpected for file %s\n",
 			   (unsigned int)hash_type,
 			   smb_fname->base_name));
-		TALLOC_FREE(psd);
-		psd = NULL;
+		TALLOC_FREE(psd_blob);
+		psd_blob = NULL;
 		goto out;
 	}
 
@@ -645,8 +645,8 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 
 		status = hash_sd_sha256(pdesc_next, hash_tmp);
 		if (!NT_STATUS_IS_OK(status)) {
-			TALLOC_FREE(psd);
-			psd = pdesc_next;
+			TALLOC_FREE(psd_blob);
+			psd_blob = pdesc_next;
 			goto out;
 		}
 
@@ -670,12 +670,12 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 			NDR_PRINT_DEBUG(security_descriptor, pdesc_next);
 		}
 
-		TALLOC_FREE(psd);
-		psd = pdesc_next;
+		TALLOC_FREE(psd_blob);
+		psd_blob = pdesc_next;
 	}
   out:
 
-	if (psd == NULL) {
+	if (psd_blob == NULL) {
 		/* Get the full underlying sd, as we failed to get the
 		 * blob for the hash, or the revision/hash type wasn't
 		 * known */
@@ -708,10 +708,10 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 		 * steal from a talloc pool.
 		 */
 		talloc_steal(frame, pdesc_next);
-		psd = pdesc_next;
+		psd_blob = pdesc_next;
 	}
 
-	if (psd != pdesc_next) {
+	if (psd_blob != pdesc_next) {
 		/* We're returning the blob, throw
  		 * away the filesystem SD. */
 		TALLOC_FREE(pdesc_next);
@@ -764,20 +764,20 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 			status = make_default_filesystem_acl(mem_ctx,
 						smb_fname->base_name,
 						psbuf,
-						&psd);
+						&psd_blob);
 			if (!NT_STATUS_IS_OK(status)) {
 				TALLOC_FREE(frame);
 				return status;
 			}
 		} else {
 			if (is_directory &&
-				!sd_has_inheritable_components(psd,
+				!sd_has_inheritable_components(psd_blob,
 							true)) {
 				status = add_directory_inheritable_components(
 							handle,
 							smb_fname->base_name,
 							psbuf,
-							psd);
+							psd_blob);
 				if (!NT_STATUS_IS_OK(status)) {
 					TALLOC_FREE(frame);
 					return status;
@@ -787,23 +787,23 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 			   the ~SEC_DESC_DACL_PROTECTED bit, as ACLs
 			   can't be inherited in this way under POSIX.
 			   Remove it for Windows-style ACLs. */
-			psd->type &= ~SEC_DESC_DACL_PROTECTED;
+			psd_blob->type &= ~SEC_DESC_DACL_PROTECTED;
 		}
 	}
 
 	if (!(security_info & SECINFO_OWNER)) {
-		psd->owner_sid = NULL;
+		psd_blob->owner_sid = NULL;
 	}
 	if (!(security_info & SECINFO_GROUP)) {
-		psd->group_sid = NULL;
+		psd_blob->group_sid = NULL;
 	}
 	if (!(security_info & SECINFO_DACL)) {
-		psd->type &= ~SEC_DESC_DACL_PRESENT;
-		psd->dacl = NULL;
+		psd_blob->type &= ~SEC_DESC_DACL_PRESENT;
+		psd_blob->dacl = NULL;
 	}
 	if (!(security_info & SECINFO_SACL)) {
-		psd->type &= ~SEC_DESC_SACL_PRESENT;
-		psd->sacl = NULL;
+		psd_blob->type &= ~SEC_DESC_SACL_PRESENT;
+		psd_blob->sacl = NULL;
 	}
 
 	TALLOC_FREE(blob.data);
@@ -811,11 +811,11 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 	if (DEBUGLEVEL >= 10) {
 		DEBUG(10,("get_nt_acl_internal: returning acl for %s is:\n",
 			smb_fname->base_name ));
-		NDR_PRINT_DEBUG(security_descriptor, psd);
+		NDR_PRINT_DEBUG(security_descriptor, psd_blob);
 	}
 
 	/* The VFS API is that the ACL is expected to be on mem_ctx */
-	*ppdesc = talloc_move(mem_ctx, &psd);
+	*ppdesc = talloc_move(mem_ctx, &psd_blob);
 
 	TALLOC_FREE(frame);
 	return NT_STATUS_OK;
-- 
2.7.4


From bf5cd7cb4aac0733c8634b62ad29c128a9b3f451 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 23 Aug 2016 13:11:24 +0200
Subject: [PATCH 03/12] vfs_acl_common: rename pdesc_next to psd_fs

In most realistic cases the "next" VFS op will return the permissions
from the filesystem. This rename makes it explicit where the SD is
originating from. No change in behaviour.

This just paves the way for a later change that will simplify the whole
logic and talloc hierarchy.

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_acl_common.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index 1adc875..fb01bf4 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -489,7 +489,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 	uint8_t hash_tmp[XATTR_SD_HASH_SIZE];
 	uint8_t sys_acl_hash_tmp[XATTR_SD_HASH_SIZE];
 	struct security_descriptor *psd_blob = NULL;
-	struct security_descriptor *pdesc_next = NULL;
+	struct security_descriptor *psd_fs = NULL;
 	const struct smb_filename *smb_fname = NULL;
 	bool ignore_file_system_acl = lp_parm_bool(SNUM(handle->conn),
 						ACL_MODULE_NAME,
@@ -618,13 +618,13 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 							  fsp,
 							  HASH_SECURITY_INFO,
 							  mem_ctx,
-							  &pdesc_next);
+							  &psd_fs);
 		} else {
 			status = SMB_VFS_NEXT_GET_NT_ACL(handle,
 							 smb_fname,
 							 HASH_SECURITY_INFO,
 							 mem_ctx,
-							 &pdesc_next);
+							 &psd_fs);
 		}
 
 		if (!NT_STATUS_IS_OK(status)) {
@@ -636,17 +636,17 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 			return status;
 		}
 
-		/* Ensure we don't leak psd_next if we don't choose it.
+		/* Ensure we don't leak psd_fs if we don't choose it.
 		 *
 		 * We don't allocate it onto frame as it is preferred not to
 		 * steal from a talloc pool.
 		 */
-		talloc_steal(frame, pdesc_next);
+		talloc_steal(frame, psd_fs);
 
-		status = hash_sd_sha256(pdesc_next, hash_tmp);
+		status = hash_sd_sha256(psd_fs, hash_tmp);
 		if (!NT_STATUS_IS_OK(status)) {
 			TALLOC_FREE(psd_blob);
-			psd_blob = pdesc_next;
+			psd_blob = psd_fs;
 			goto out;
 		}
 
@@ -667,11 +667,11 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 		if (DEBUGLEVEL >= 10) {
 			DEBUG(10,("get_nt_acl_internal: acl for blob hash for %s is:\n",
 				  smb_fname->base_name ));
-			NDR_PRINT_DEBUG(security_descriptor, pdesc_next);
+			NDR_PRINT_DEBUG(security_descriptor, psd_fs);
 		}
 
 		TALLOC_FREE(psd_blob);
-		psd_blob = pdesc_next;
+		psd_blob = psd_fs;
 	}
   out:
 
@@ -684,13 +684,13 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 							  fsp,
 							  security_info,
 							  mem_ctx,
-							  &pdesc_next);
+							  &psd_fs);
 		} else {
 			status = SMB_VFS_NEXT_GET_NT_ACL(handle,
 							 smb_fname,
 							 security_info,
 							 mem_ctx,
-							 &pdesc_next);
+							 &psd_fs);
 		}
 
 		if (!NT_STATUS_IS_OK(status)) {
@@ -702,19 +702,19 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 			return status;
 		}
 
-		/* Ensure we don't leak psd_next if we don't choose it.
+		/* Ensure we don't leak psd_fs if we don't choose it.
 		 *
 		 * We don't allocate it onto frame as it is preferred not to
 		 * steal from a talloc pool.
 		 */
-		talloc_steal(frame, pdesc_next);
-		psd_blob = pdesc_next;
+		talloc_steal(frame, psd_fs);
+		psd_blob = psd_fs;
 	}
 
-	if (psd_blob != pdesc_next) {
+	if (psd_blob != psd_fs) {
 		/* We're returning the blob, throw
  		 * away the filesystem SD. */
-		TALLOC_FREE(pdesc_next);
+		TALLOC_FREE(psd_fs);
 	} else {
 		SMB_STRUCT_STAT sbuf;
 		SMB_STRUCT_STAT *psbuf = &sbuf;
@@ -760,7 +760,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 		is_directory = S_ISDIR(psbuf->st_ex_mode);
 
 		if (ignore_file_system_acl) {
-			TALLOC_FREE(pdesc_next);
+			TALLOC_FREE(psd_fs);
 			status = make_default_filesystem_acl(mem_ctx,
 						smb_fname->base_name,
 						psbuf,
-- 
2.7.4


From b77ee317be4b59aaffe13aaddd3262ec64ecf914 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 23 Aug 2016 13:14:50 +0200
Subject: [PATCH 04/12] vfs_acl_common: remove redundant NULL assignment

The variables are already set to NULL by TALLOC_FREE.

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_acl_common.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index fb01bf4..0c40f37 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -551,7 +551,6 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 			   (unsigned int)hash_type,
 			   smb_fname->base_name));
 		TALLOC_FREE(psd_blob);
-		psd_blob = NULL;
 		goto out;
 	}
 
@@ -562,7 +561,6 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 			   (unsigned int)hash_type,
 			   smb_fname->base_name));
 		TALLOC_FREE(psd_blob);
-		psd_blob = NULL;
 		goto out;
 	}
 
-- 
2.7.4


From f199343ccafc8ff3c620be730d7a320559733018 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 23 Aug 2016 17:07:20 +0200
Subject: [PATCH 05/12] vfs_acl_common: simplify ACL logic, cleanup and talloc
 hierarchy

No change in behaviour (hopefully! :-). This paves the way for moving
the ACL blob validation to a helper function in the next commit.

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_acl_common.c | 131 ++++++++++++++++++---------------------
 1 file changed, 61 insertions(+), 70 deletions(-)

diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index 0c40f37..66c58e7 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -488,6 +488,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 	uint8_t sys_acl_hash[XATTR_SD_HASH_SIZE];
 	uint8_t hash_tmp[XATTR_SD_HASH_SIZE];
 	uint8_t sys_acl_hash_tmp[XATTR_SD_HASH_SIZE];
+	struct security_descriptor *psd = NULL;
 	struct security_descriptor *psd_blob = NULL;
 	struct security_descriptor *psd_fs = NULL;
 	const struct smb_filename *smb_fname = NULL;
@@ -495,7 +496,9 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 						ACL_MODULE_NAME,
 						"ignore system acls",
 						false);
-	TALLOC_CTX *frame = talloc_stackframe();
+	char *sys_acl_blob_description = NULL;
+	DATA_BLOB sys_acl_blob = { 0 };
+	bool psd_is_from_fs = false;
 
 	if (fsp && smb_fname_in == NULL) {
 		smb_fname = fsp->fsp_name;
@@ -505,11 +508,10 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 
 	DEBUG(10, ("get_nt_acl_internal: name=%s\n", smb_fname->base_name));
 
-	status = get_acl_blob(frame, handle, fsp, smb_fname, &blob);
+	status = get_acl_blob(mem_ctx, handle, fsp, smb_fname, &blob);
 	if (!NT_STATUS_IS_OK(status)) {
 		DEBUG(10, ("get_nt_acl_internal: get_acl_blob returned %s\n",
 			nt_errstr(status)));
-		psd_blob = NULL;
 		goto out;
 	} else {
 		status = parse_acl_blob(&blob, mem_ctx, &psd_blob,
@@ -517,17 +519,12 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 		if (!NT_STATUS_IS_OK(status)) {
 			DEBUG(10, ("parse_acl_blob returned %s\n",
 				   nt_errstr(status)));
-			psd_blob = NULL;
+			TALLOC_FREE(blob.data);
 			goto out;
 		}
 	}
 
-	/* Ensure we don't leak psd_blob if we don't choose it.
-	 *
-	 * We don't allocate it onto frame as it is preferred not to
-	 * steal from a talloc pool.
-	 */
-	talloc_steal(frame, psd_blob);
+	TALLOC_FREE(blob.data);
 
 	/* determine which type of xattr we got */
 	switch (xattr_version) {
@@ -537,10 +534,14 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 		 * require confirmation of the hash.  In particular,
 		 * the NTVFS file server uses version 1, but
 		 * 'samba-tool ntacl' can set these as well */
+		psd = psd_blob;
+		psd_blob = NULL;
 		goto out;
 	case 3:
 	case 4:
 		if (ignore_file_system_acl) {
+			psd = psd_blob;
+			psd_blob = NULL;
 			goto out;
 		}
 
@@ -569,20 +570,18 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 	case 4:
 	{
 		int ret;
-		char *sys_acl_blob_description;
-		DATA_BLOB sys_acl_blob;
 		if (fsp) {
 			/* Get the full underlying sd, then hash. */
 			ret = SMB_VFS_NEXT_SYS_ACL_BLOB_GET_FD(handle,
 							       fsp,
-							       frame,
+							       mem_ctx,
 							       &sys_acl_blob_description,
 							       &sys_acl_blob);
 		} else {
 			/* Get the full underlying sd, then hash. */
 			ret = SMB_VFS_NEXT_SYS_ACL_BLOB_GET_FILE(handle,
 						 smb_fname->base_name,
-						 frame,
+						 mem_ctx,
 						 &sys_acl_blob_description,
 						 &sys_acl_blob);
 		}
@@ -592,16 +591,20 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 		if (ret == 0) {
 			status = hash_blob_sha256(sys_acl_blob, sys_acl_hash_tmp);
 			if (!NT_STATUS_IS_OK(status)) {
-				TALLOC_FREE(frame);
-				return status;
+				goto fail;
 			}
 
+			TALLOC_FREE(sys_acl_blob_description);
+			TALLOC_FREE(sys_acl_blob.data);
+
 			if (memcmp(&sys_acl_hash[0], &sys_acl_hash_tmp[0], 
 				   XATTR_SD_HASH_SIZE) == 0) {
 				/* Hash matches, return blob sd. */
 				DEBUG(10, ("get_nt_acl_internal: blob hash "
 					   "matches for file %s\n",
 					   smb_fname->base_name ));
+				psd = psd_blob;
+				psd_blob = NULL;
 				goto out;
 			}
 		}
@@ -630,21 +633,15 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 				   "returned %s\n",
 				   smb_fname->base_name,
 				   nt_errstr(status)));
-			TALLOC_FREE(frame);
-			return status;
+			goto fail;
 		}
 
-		/* Ensure we don't leak psd_fs if we don't choose it.
-		 *
-		 * We don't allocate it onto frame as it is preferred not to
-		 * steal from a talloc pool.
-		 */
-		talloc_steal(frame, psd_fs);
-
 		status = hash_sd_sha256(psd_fs, hash_tmp);
 		if (!NT_STATUS_IS_OK(status)) {
 			TALLOC_FREE(psd_blob);
-			psd_blob = psd_fs;
+			psd = psd_fs;
+			psd_fs = NULL;
+			psd_is_from_fs = true;
 			goto out;
 		}
 
@@ -653,6 +650,8 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 			DEBUG(10, ("get_nt_acl_internal: blob hash "
 				   "matches for file %s\n",
 				   smb_fname->base_name ));
+			psd = psd_blob;
+			psd_blob = NULL;
 			goto out;
 		}
 
@@ -669,11 +668,13 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 		}
 
 		TALLOC_FREE(psd_blob);
-		psd_blob = psd_fs;
+		psd = psd_fs;
+		psd_fs = NULL;
+		psd_is_from_fs = true;
 	}
-  out:
 
-	if (psd_blob == NULL) {
+out:
+	if (psd == NULL) {
 		/* Get the full underlying sd, as we failed to get the
 		 * blob for the hash, or the revision/hash type wasn't
 		 * known */
@@ -682,13 +683,13 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 							  fsp,
 							  security_info,
 							  mem_ctx,
-							  &psd_fs);
+							  &psd);
 		} else {
 			status = SMB_VFS_NEXT_GET_NT_ACL(handle,
 							 smb_fname,
 							 security_info,
 							 mem_ctx,
-							 &psd_fs);
+							 &psd);
 		}
 
 		if (!NT_STATUS_IS_OK(status)) {
@@ -696,24 +697,13 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 				   "returned %s\n",
 				   smb_fname->base_name,
 				   nt_errstr(status)));
-			TALLOC_FREE(frame);
-			return status;
+			goto fail;
 		}
 
-		/* Ensure we don't leak psd_fs if we don't choose it.
-		 *
-		 * We don't allocate it onto frame as it is preferred not to
-		 * steal from a talloc pool.
-		 */
-		talloc_steal(frame, psd_fs);
-		psd_blob = psd_fs;
+		psd_is_from_fs = true;
 	}
 
-	if (psd_blob != psd_fs) {
-		/* We're returning the blob, throw
- 		 * away the filesystem SD. */
-		TALLOC_FREE(psd_fs);
-	} else {
+	if (psd_is_from_fs) {
 		SMB_STRUCT_STAT sbuf;
 		SMB_STRUCT_STAT *psbuf = &sbuf;
 		bool is_directory = false;
@@ -725,8 +715,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 		if (fsp) {
 			status = vfs_stat_fsp(fsp);
 			if (!NT_STATUS_IS_OK(status)) {
-				TALLOC_FREE(frame);
-				return status;
+				goto fail;
 			}
 			psbuf = &fsp->fsp_name->st;
 		} else {
@@ -751,72 +740,74 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 						smb_fname,
 						&sbuf);
 			if (ret == -1) {
-				TALLOC_FREE(frame);
-				return map_nt_error_from_unix(errno);
+				status = map_nt_error_from_unix(errno);
+				goto fail;
 			}
 		}
 		is_directory = S_ISDIR(psbuf->st_ex_mode);
 
 		if (ignore_file_system_acl) {
-			TALLOC_FREE(psd_fs);
+			TALLOC_FREE(psd);
 			status = make_default_filesystem_acl(mem_ctx,
 						smb_fname->base_name,
 						psbuf,
-						&psd_blob);
+						&psd);
 			if (!NT_STATUS_IS_OK(status)) {
-				TALLOC_FREE(frame);
-				return status;
+				goto fail;
 			}
 		} else {
 			if (is_directory &&
-				!sd_has_inheritable_components(psd_blob,
+				!sd_has_inheritable_components(psd,
 							true)) {
 				status = add_directory_inheritable_components(
 							handle,
 							smb_fname->base_name,
 							psbuf,
-							psd_blob);
+							psd);
 				if (!NT_STATUS_IS_OK(status)) {
-					TALLOC_FREE(frame);
-					return status;
+					goto fail;
 				}
 			}
 			/* The underlying POSIX module always sets
 			   the ~SEC_DESC_DACL_PROTECTED bit, as ACLs
 			   can't be inherited in this way under POSIX.
 			   Remove it for Windows-style ACLs. */
-			psd_blob->type &= ~SEC_DESC_DACL_PROTECTED;
+			psd->type &= ~SEC_DESC_DACL_PROTECTED;
 		}
 	}
 
 	if (!(security_info & SECINFO_OWNER)) {
-		psd_blob->owner_sid = NULL;
+		psd->owner_sid = NULL;
 	}
 	if (!(security_info & SECINFO_GROUP)) {
-		psd_blob->group_sid = NULL;
+		psd->group_sid = NULL;
 	}
 	if (!(security_info & SECINFO_DACL)) {
-		psd_blob->type &= ~SEC_DESC_DACL_PRESENT;
-		psd_blob->dacl = NULL;
+		psd->type &= ~SEC_DESC_DACL_PRESENT;
+		psd->dacl = NULL;
 	}
 	if (!(security_info & SECINFO_SACL)) {
-		psd_blob->type &= ~SEC_DESC_SACL_PRESENT;
-		psd_blob->sacl = NULL;
+		psd->type &= ~SEC_DESC_SACL_PRESENT;
+		psd->sacl = NULL;
 	}
 
-	TALLOC_FREE(blob.data);
-
 	if (DEBUGLEVEL >= 10) {
 		DEBUG(10,("get_nt_acl_internal: returning acl for %s is:\n",
 			smb_fname->base_name ));
-		NDR_PRINT_DEBUG(security_descriptor, psd_blob);
+		NDR_PRINT_DEBUG(security_descriptor, psd);
 	}
 
-	/* The VFS API is that the ACL is expected to be on mem_ctx */
-	*ppdesc = talloc_move(mem_ctx, &psd_blob);
+	*ppdesc = psd;
 
-	TALLOC_FREE(frame);
 	return NT_STATUS_OK;
+
+fail:
+	TALLOC_FREE(psd);
+	TALLOC_FREE(psd_blob);
+	TALLOC_FREE(psd_fs);
+	TALLOC_FREE(sys_acl_blob_description);
+	TALLOC_FREE(sys_acl_blob.data);
+	return status;
 }
 
 /*********************************************************************
-- 
2.7.4


From a063153eeb506a018c84a34801eaddcc416eced9 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 23 Aug 2016 22:32:57 +0200
Subject: [PATCH 06/12] vfs_acl_common: move the ACL blob validation to a
 helper function

No change in behaviour.

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_acl_common.c | 177 +++++++++++++++++++++++++--------------
 1 file changed, 112 insertions(+), 65 deletions(-)

diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index 66c58e7..2cf5012 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -467,20 +467,32 @@ static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx,
 	return NT_STATUS_OK;
 }
 
-/*******************************************************************
- Pull a DATA_BLOB from an xattr given a pathname.
- If the hash doesn't match, or doesn't exist - return the underlying
- filesystem sd.
-*******************************************************************/
-
-static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
-				    files_struct *fsp,
-				    const struct smb_filename *smb_fname_in,
-				    uint32_t security_info,
-				    TALLOC_CTX *mem_ctx,
-				    struct security_descriptor **ppdesc)
+/**
+ * Validate an ACL blob
+ *
+ * This validates an ACL blob against the underlying filesystem ACL. If this
+ * function returns NT_STATUS_OK ppsd can be
+ *
+ * 1. the ACL from the blob (psd_from_fs=false), or
+ * 2. the ACL from the fs (psd_from_fs=true), or
+ * 3. NULL (!)
+ *
+ * If the return value is anything else then NT_STATUS_OK, ppsd is set to NULL
+ * and psd_from_fs set to false.
+ *
+ * Returning the underlying filesystem ACL in case no. 2 is really just an
+ * optimisation, because some validations have to fetch the filesytem ACL as
+ * part of the validation, so we already have it available and callers might
+ * need it as well.
+ **/
+static NTSTATUS validate_nt_acl_blob(TALLOC_CTX *mem_ctx,
+				     vfs_handle_struct *handle,
+				     files_struct *fsp,
+				     const struct smb_filename *smb_fname,
+				     DATA_BLOB *blob,
+				     struct security_descriptor **ppsd,
+				     bool *psd_is_from_fs)
 {
-	DATA_BLOB blob = data_blob_null;
 	NTSTATUS status;
 	uint16_t hash_type = XATTR_SD_HASH_TYPE_NONE;
 	uint16_t xattr_version = 0;
@@ -491,41 +503,28 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 	struct security_descriptor *psd = NULL;
 	struct security_descriptor *psd_blob = NULL;
 	struct security_descriptor *psd_fs = NULL;
-	const struct smb_filename *smb_fname = NULL;
+	char *sys_acl_blob_description = NULL;
+	DATA_BLOB sys_acl_blob = { 0 };
 	bool ignore_file_system_acl = lp_parm_bool(SNUM(handle->conn),
 						ACL_MODULE_NAME,
 						"ignore system acls",
 						false);
-	char *sys_acl_blob_description = NULL;
-	DATA_BLOB sys_acl_blob = { 0 };
-	bool psd_is_from_fs = false;
 
-	if (fsp && smb_fname_in == NULL) {
-		smb_fname = fsp->fsp_name;
-	} else {
-		smb_fname = smb_fname_in;
-	}
+	*ppsd = NULL;
+	*psd_is_from_fs = false;
 
-	DEBUG(10, ("get_nt_acl_internal: name=%s\n", smb_fname->base_name));
-
-	status = get_acl_blob(mem_ctx, handle, fsp, smb_fname, &blob);
+	status = parse_acl_blob(blob,
+				mem_ctx,
+				&psd_blob,
+				&hash_type,
+				&xattr_version,
+				&hash[0],
+				&sys_acl_hash[0]);
 	if (!NT_STATUS_IS_OK(status)) {
-		DEBUG(10, ("get_nt_acl_internal: get_acl_blob returned %s\n",
-			nt_errstr(status)));
-		goto out;
-	} else {
-		status = parse_acl_blob(&blob, mem_ctx, &psd_blob,
-					&hash_type, &xattr_version, &hash[0], &sys_acl_hash[0]);
-		if (!NT_STATUS_IS_OK(status)) {
-			DEBUG(10, ("parse_acl_blob returned %s\n",
-				   nt_errstr(status)));
-			TALLOC_FREE(blob.data);
-			goto out;
-		}
+		DBG_DEBUG("parse_acl_blob returned %s\n", nt_errstr(status));
+		goto fail;
 	}
 
-	TALLOC_FREE(blob.data);
-
 	/* determine which type of xattr we got */
 	switch (xattr_version) {
 	case 1:
@@ -534,15 +533,13 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 		 * require confirmation of the hash.  In particular,
 		 * the NTVFS file server uses version 1, but
 		 * 'samba-tool ntacl' can set these as well */
-		psd = psd_blob;
-		psd_blob = NULL;
-		goto out;
+		*ppsd = psd_blob;
+		return NT_STATUS_OK;
 	case 3:
 	case 4:
 		if (ignore_file_system_acl) {
-			psd = psd_blob;
-			psd_blob = NULL;
-			goto out;
+			*ppsd = psd_blob;
+			return NT_STATUS_OK;
 		}
 
 		break;
@@ -552,7 +549,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 			   (unsigned int)hash_type,
 			   smb_fname->base_name));
 		TALLOC_FREE(psd_blob);
-		goto out;
+		return NT_STATUS_OK;
 	}
 
 	/* determine which type of xattr we got */
@@ -562,7 +559,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 			   (unsigned int)hash_type,
 			   smb_fname->base_name));
 		TALLOC_FREE(psd_blob);
-		goto out;
+		return NT_STATUS_OK;
 	}
 
 	/* determine which type of xattr we got */
@@ -603,9 +600,8 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 				DEBUG(10, ("get_nt_acl_internal: blob hash "
 					   "matches for file %s\n",
 					   smb_fname->base_name ));
-				psd = psd_blob;
-				psd_blob = NULL;
-				goto out;
+				*ppsd = psd_blob;
+				return NT_STATUS_OK;
 			}
 		}
 
@@ -639,10 +635,9 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 		status = hash_sd_sha256(psd_fs, hash_tmp);
 		if (!NT_STATUS_IS_OK(status)) {
 			TALLOC_FREE(psd_blob);
-			psd = psd_fs;
-			psd_fs = NULL;
-			psd_is_from_fs = true;
-			goto out;
+			*ppsd = psd_fs;
+			*psd_is_from_fs = true;
+			return NT_STATUS_OK;
 		}
 
 		if (memcmp(&hash[0], &hash_tmp[0], XATTR_SD_HASH_SIZE) == 0) {
@@ -650,9 +645,8 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 			DEBUG(10, ("get_nt_acl_internal: blob hash "
 				   "matches for file %s\n",
 				   smb_fname->base_name ));
-			psd = psd_blob;
-			psd_blob = NULL;
-			goto out;
+			*ppsd = psd_blob;
+			return NT_STATUS_OK;
 		}
 
 		/* Hash doesn't match, return underlying sd. */
@@ -668,12 +662,69 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 		}
 
 		TALLOC_FREE(psd_blob);
-		psd = psd_fs;
-		psd_fs = NULL;
-		psd_is_from_fs = true;
+		*ppsd = psd_fs;
+		*psd_is_from_fs = true;
+	}
+
+	return NT_STATUS_OK;
+
+fail:
+	TALLOC_FREE(psd);
+	TALLOC_FREE(psd_blob);
+	TALLOC_FREE(psd_fs);
+	TALLOC_FREE(sys_acl_blob_description);
+	TALLOC_FREE(sys_acl_blob.data);
+	return status;
+}
+
+/*******************************************************************
+ Pull a DATA_BLOB from an xattr given a pathname.
+ If the hash doesn't match, or doesn't exist - return the underlying
+ filesystem sd.
+*******************************************************************/
+
+static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
+				    files_struct *fsp,
+				    const struct smb_filename *smb_fname_in,
+				    uint32_t security_info,
+				    TALLOC_CTX *mem_ctx,
+				    struct security_descriptor **ppdesc)
+{
+	DATA_BLOB blob = data_blob_null;
+	NTSTATUS status;
+	struct security_descriptor *psd = NULL;
+	const struct smb_filename *smb_fname = NULL;
+	bool ignore_file_system_acl = lp_parm_bool(SNUM(handle->conn),
+						ACL_MODULE_NAME,
+						"ignore system acls",
+						false);
+	bool psd_is_from_fs = false;
+
+	if (fsp && smb_fname_in == NULL) {
+		smb_fname = fsp->fsp_name;
+	} else {
+		smb_fname = smb_fname_in;
+	}
+
+	DEBUG(10, ("get_nt_acl_internal: name=%s\n", smb_fname->base_name));
+
+	status = get_acl_blob(mem_ctx, handle, fsp, smb_fname, &blob);
+	if (NT_STATUS_IS_OK(status)) {
+		status = validate_nt_acl_blob(mem_ctx,
+					      handle,
+					      fsp,
+					      smb_fname,
+					      &blob,
+					      &psd,
+					      &psd_is_from_fs);
+		TALLOC_FREE(blob.data);
+		if (!NT_STATUS_IS_OK(status)) {
+			DBG_DEBUG("ACL validation for [%s] failed\n",
+				  smb_fname->base_name);
+			goto fail;
+		}
 	}
 
-out:
 	if (psd == NULL) {
 		/* Get the full underlying sd, as we failed to get the
 		 * blob for the hash, or the revision/hash type wasn't
@@ -803,10 +854,6 @@ out:
 
 fail:
 	TALLOC_FREE(psd);
-	TALLOC_FREE(psd_blob);
-	TALLOC_FREE(psd_fs);
-	TALLOC_FREE(sys_acl_blob_description);
-	TALLOC_FREE(sys_acl_blob.data);
 	return status;
 }
 
-- 
2.7.4


From 9a9795133dfc713dd5008eb8705d5b7f72ae1f91 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 24 Aug 2016 10:01:17 +0200
Subject: [PATCH 07/12] vfs_acl_tdb|xattr: use a config handle

Better for performance and a subsequent commit will add one more option
where this will pay off.

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_acl_common.c | 50 ++++++++++++++++++++++++++++++++--------
 source3/modules/vfs_acl_tdb.c    |  7 ++++++
 source3/modules/vfs_acl_xattr.c  |  7 ++++++
 3 files changed, 54 insertions(+), 10 deletions(-)

diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index 2cf5012..fe631e3 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -46,6 +46,34 @@ static NTSTATUS store_acl_blob_fsp(vfs_handle_struct *handle,
 				SECINFO_DACL | \
 				SECINFO_SACL)
 
+struct acl_common_config {
+	bool ignore_system_acls;
+};
+
+static bool init_acl_common_config(vfs_handle_struct *handle)
+{
+	struct acl_common_config *config = NULL;
+
+	config = talloc_zero(handle->conn, struct acl_common_config);
+	if (config == NULL) {
+		DBG_ERR("talloc_zero() failed\n");
+		errno = ENOMEM;
+		return false;
+	}
+
+	config->ignore_system_acls = lp_parm_bool(SNUM(handle->conn),
+						  ACL_MODULE_NAME,
+						  "ignore system acls",
+						  false);
+
+	SMB_VFS_HANDLE_SET_DATA(handle, config, NULL,
+				struct acl_common_config,
+				return false);
+
+	return true;
+}
+
+
 /*******************************************************************
  Hash a security descriptor.
 *******************************************************************/
@@ -505,14 +533,15 @@ static NTSTATUS validate_nt_acl_blob(TALLOC_CTX *mem_ctx,
 	struct security_descriptor *psd_fs = NULL;
 	char *sys_acl_blob_description = NULL;
 	DATA_BLOB sys_acl_blob = { 0 };
-	bool ignore_file_system_acl = lp_parm_bool(SNUM(handle->conn),
-						ACL_MODULE_NAME,
-						"ignore system acls",
-						false);
+	struct acl_common_config *config = NULL;
 
 	*ppsd = NULL;
 	*psd_is_from_fs = false;
 
+	SMB_VFS_HANDLE_GET_DATA(handle, config,
+				struct acl_common_config,
+				return NT_STATUS_UNSUCCESSFUL);
+
 	status = parse_acl_blob(blob,
 				mem_ctx,
 				&psd_blob,
@@ -537,7 +566,7 @@ static NTSTATUS validate_nt_acl_blob(TALLOC_CTX *mem_ctx,
 		return NT_STATUS_OK;
 	case 3:
 	case 4:
-		if (ignore_file_system_acl) {
+		if (config->ignore_system_acls) {
 			*ppsd = psd_blob;
 			return NT_STATUS_OK;
 		}
@@ -694,11 +723,12 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 	NTSTATUS status;
 	struct security_descriptor *psd = NULL;
 	const struct smb_filename *smb_fname = NULL;
-	bool ignore_file_system_acl = lp_parm_bool(SNUM(handle->conn),
-						ACL_MODULE_NAME,
-						"ignore system acls",
-						false);
 	bool psd_is_from_fs = false;
+	struct acl_common_config *config = NULL;
+
+	SMB_VFS_HANDLE_GET_DATA(handle, config,
+				struct acl_common_config,
+				return NT_STATUS_UNSUCCESSFUL);
 
 	if (fsp && smb_fname_in == NULL) {
 		smb_fname = fsp->fsp_name;
@@ -797,7 +827,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 		}
 		is_directory = S_ISDIR(psbuf->st_ex_mode);
 
-		if (ignore_file_system_acl) {
+		if (config->ignore_system_acls) {
 			TALLOC_FREE(psd);
 			status = make_default_filesystem_acl(mem_ctx,
 						smb_fname->base_name,
diff --git a/source3/modules/vfs_acl_tdb.c b/source3/modules/vfs_acl_tdb.c
index e4c8462..0c92b72 100644
--- a/source3/modules/vfs_acl_tdb.c
+++ b/source3/modules/vfs_acl_tdb.c
@@ -308,6 +308,7 @@ static int connect_acl_tdb(struct vfs_handle_struct *handle,
 				const char *user)
 {
 	int ret = SMB_VFS_NEXT_CONNECT(handle, service, user);
+	bool ok;
 
 	if (ret < 0) {
 		return ret;
@@ -318,6 +319,12 @@ static int connect_acl_tdb(struct vfs_handle_struct *handle,
 		return -1;
 	}
 
+	ok = init_acl_common_config(handle);
+	if (!ok) {
+		DBG_ERR("init_acl_common_config failed\n");
+		return -1;
+	}
+
 	/* Ensure we have the parameters correct if we're
 	 * using this module. */
 	DEBUG(2,("connect_acl_tdb: setting 'inherit acls = true' "
diff --git a/source3/modules/vfs_acl_xattr.c b/source3/modules/vfs_acl_xattr.c
index d311c57..307ab6a 100644
--- a/source3/modules/vfs_acl_xattr.c
+++ b/source3/modules/vfs_acl_xattr.c
@@ -180,11 +180,18 @@ static int connect_acl_xattr(struct vfs_handle_struct *handle,
 				const char *user)
 {
 	int ret = SMB_VFS_NEXT_CONNECT(handle, service, user);
+	bool ok;
 
 	if (ret < 0) {
 		return ret;
 	}
 
+	ok = init_acl_common_config(handle);
+	if (!ok) {
+		DBG_ERR("init_acl_common_config failed\n");
+		return -1;
+	}
+
 	/* Ensure we have the parameters correct if we're
 	 * using this module. */
 	DEBUG(2,("connect_acl_xattr: setting 'inherit acls = true' "
-- 
2.7.4


From 8a645460b093d8c61a3507ebb6e3337a3873b10a Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 24 Aug 2016 10:30:15 +0200
Subject: [PATCH 08/12] vfs_acl_common: move stat stuff to a helper function

Will be reused in the next commit when moving the
make_default_filesystem_acl() stuff to a different place.

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_acl_common.c | 79 ++++++++++++++++++++++++----------------
 1 file changed, 48 insertions(+), 31 deletions(-)

diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index fe631e3..d7fe258 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -706,6 +706,48 @@ fail:
 	return status;
 }
 
+static NTSTATUS stat_fsp_or_smb_fname(vfs_handle_struct *handle,
+				      files_struct *fsp,
+				      const struct smb_filename *smb_fname,
+				      SMB_STRUCT_STAT *sbuf,
+				      SMB_STRUCT_STAT **psbuf)
+{
+	NTSTATUS status;
+	int ret;
+
+	if (fsp) {
+		status = vfs_stat_fsp(fsp);
+		if (!NT_STATUS_IS_OK(status)) {
+			return status;
+		}
+		*psbuf = &fsp->fsp_name->st;
+	} else {
+		/*
+		 * https://bugzilla.samba.org/show_bug.cgi?id=11249
+		 *
+		 * We are currently guaranteed that 'name' here is a
+		 * smb_fname->base_name, which *cannot* contain a stream name
+		 * (':'). vfs_stat_smb_fname() splits a name into a base name +
+		 * stream name, which when we get here we know we've already
+		 * done.  So we have to call the stat or lstat VFS calls
+		 * directly here. Else, a base_name that contains a ':' (from a
+		 * demangled name) will get split again.
+		 *
+		 * FIXME.
+		 * This uglyness will go away once smb_fname is fully plumbed
+		 * through the VFS.
+		 */
+		ret = vfs_stat_smb_basename(handle->conn,
+					    smb_fname,
+					    sbuf);
+		if (ret == -1) {
+			return map_nt_error_from_unix(errno);
+		}
+	}
+
+	return NT_STATUS_OK;
+}
+
 /*******************************************************************
  Pull a DATA_BLOB from an xattr given a pathname.
  If the hash doesn't match, or doesn't exist - return the underlying
@@ -793,38 +835,13 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 		 * filesystem. If it's a directory, and has no
 		 * inheritable ACE entries we have to fake them.
 		 */
-		if (fsp) {
-			status = vfs_stat_fsp(fsp);
-			if (!NT_STATUS_IS_OK(status)) {
-				goto fail;
-			}
-			psbuf = &fsp->fsp_name->st;
-		} else {
-			/*
-			 * https://bugzilla.samba.org/show_bug.cgi?id=11249
-			 *
-			 * We are currently guaranteed that 'name' here is
-			 * a smb_fname->base_name, which *cannot* contain
-			 * a stream name (':'). vfs_stat_smb_fname() splits
-			 * a name into a base name + stream name, which
-			 * when we get here we know we've already done.
-			 * So we have to call the stat or lstat VFS
-			 * calls directly here. Else, a base_name that
-			 * contains a ':' (from a demangled name) will
-			 * get split again.
-			 *
-			 * FIXME.
-			 * This uglyness will go away once smb_fname
-			 * is fully plumbed through the VFS.
-			 */
-			int ret = vfs_stat_smb_basename(handle->conn,
-						smb_fname,
-						&sbuf);
-			if (ret == -1) {
-				status = map_nt_error_from_unix(errno);
-				goto fail;
-			}
+
+		status = stat_fsp_or_smb_fname(handle, fsp, smb_fname,
+					       &sbuf, &psbuf);
+		if (!NT_STATUS_IS_OK(status)) {
+			goto fail;
 		}
+
 		is_directory = S_ISDIR(psbuf->st_ex_mode);
 
 		if (config->ignore_system_acls) {
-- 
2.7.4


From 33980bcae5a907f9b8f67c5852a365bd22afc53c Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 24 Aug 2016 10:43:47 +0200
Subject: [PATCH 09/12] vfs_acl_common: check for ignore_system_acls before
 fetching filesystem ACL

If ignore_system_acls is set and we're synthesizing a default ACL, we
were fetching the filesystem ACL just to free it again. This change
avoids this.

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_acl_common.c | 99 ++++++++++++++++++++++------------------
 1 file changed, 55 insertions(+), 44 deletions(-)

diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index d7fe258..9071775 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -801,35 +801,57 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 		/* Get the full underlying sd, as we failed to get the
 		 * blob for the hash, or the revision/hash type wasn't
 		 * known */
-		if (fsp) {
-			status = SMB_VFS_NEXT_FGET_NT_ACL(handle,
-							  fsp,
-							  security_info,
-							  mem_ctx,
-							  &psd);
+
+		if (config->ignore_system_acls) {
+			SMB_STRUCT_STAT sbuf;
+			SMB_STRUCT_STAT *psbuf = &sbuf;
+
+			status = stat_fsp_or_smb_fname(handle, fsp, smb_fname,
+						       &sbuf, &psbuf);
+			if (!NT_STATUS_IS_OK(status)) {
+				goto fail;
+			}
+
+			status = make_default_filesystem_acl(
+				mem_ctx,
+				smb_fname->base_name,
+				psbuf,
+				&psd);
+			if (!NT_STATUS_IS_OK(status)) {
+				goto fail;
+			}
 		} else {
-			status = SMB_VFS_NEXT_GET_NT_ACL(handle,
-							 smb_fname,
-							 security_info,
-							 mem_ctx,
-							 &psd);
-		}
+			if (fsp) {
+				status = SMB_VFS_NEXT_FGET_NT_ACL(handle,
+								  fsp,
+								  security_info,
+								  mem_ctx,
+								  &psd);
+			} else {
+				status = SMB_VFS_NEXT_GET_NT_ACL(handle,
+								 smb_fname,
+								 security_info,
+								 mem_ctx,
+								 &psd);
+			}
 
-		if (!NT_STATUS_IS_OK(status)) {
-			DEBUG(10, ("get_nt_acl_internal: get_next_acl for file %s "
-				   "returned %s\n",
-				   smb_fname->base_name,
-				   nt_errstr(status)));
-			goto fail;
-		}
+			if (!NT_STATUS_IS_OK(status)) {
+				DBG_DEBUG("get_next_acl for file %s "
+					  "returned %s\n",
+					  smb_fname->base_name,
+					  nt_errstr(status));
+				goto fail;
+			}
 
-		psd_is_from_fs = true;
+			psd_is_from_fs = true;
+		}
 	}
 
 	if (psd_is_from_fs) {
 		SMB_STRUCT_STAT sbuf;
 		SMB_STRUCT_STAT *psbuf = &sbuf;
 		bool is_directory = false;
+
 		/*
 		 * We're returning the underlying ACL from the
 		 * filesystem. If it's a directory, and has no
@@ -844,34 +866,23 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 
 		is_directory = S_ISDIR(psbuf->st_ex_mode);
 
-		if (config->ignore_system_acls) {
-			TALLOC_FREE(psd);
-			status = make_default_filesystem_acl(mem_ctx,
-						smb_fname->base_name,
-						psbuf,
-						&psd);
+		if (is_directory && !sd_has_inheritable_components(psd, true)) {
+			status = add_directory_inheritable_components(
+				handle,
+				smb_fname->base_name,
+				psbuf,
+				psd);
 			if (!NT_STATUS_IS_OK(status)) {
 				goto fail;
 			}
-		} else {
-			if (is_directory &&
-				!sd_has_inheritable_components(psd,
-							true)) {
-				status = add_directory_inheritable_components(
-							handle,
-							smb_fname->base_name,
-							psbuf,
-							psd);
-				if (!NT_STATUS_IS_OK(status)) {
-					goto fail;
-				}
-			}
-			/* The underlying POSIX module always sets
-			   the ~SEC_DESC_DACL_PROTECTED bit, as ACLs
-			   can't be inherited in this way under POSIX.
-			   Remove it for Windows-style ACLs. */
-			psd->type &= ~SEC_DESC_DACL_PROTECTED;
 		}
+
+		/*
+		 * The underlying POSIX module always sets the
+		 * ~SEC_DESC_DACL_PROTECTED bit, as ACLs can't be inherited in
+		 * this way under POSIX. Remove it for Windows-style ACLs.
+		 */
+		psd->type &= ~SEC_DESC_DACL_PROTECTED;
 	}
 
 	if (!(security_info & SECINFO_OWNER)) {
-- 
2.7.4


From 1dafd89f8ffa4825d8593fe27069b4d8dcb1ae80 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 24 Aug 2016 20:31:00 +0200
Subject: [PATCH 10/12] vfs_acl_xattr|tdb: add option to control default ACL
 style

Existing behaviour is "posix" style. Next commit will (re)add the
"windows" style. This commit doesn't change behaviour in any way.

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 docs-xml/manpages/vfs_acl_tdb.8.xml   | 25 +++++++++++++++++++
 docs-xml/manpages/vfs_acl_xattr.8.xml | 25 +++++++++++++++++++
 source3/modules/vfs_acl_common.c      | 45 +++++++++++++++++++++++++++++++----
 3 files changed, 91 insertions(+), 4 deletions(-)

diff --git a/docs-xml/manpages/vfs_acl_tdb.8.xml b/docs-xml/manpages/vfs_acl_tdb.8.xml
index 640cec0..607e344 100644
--- a/docs-xml/manpages/vfs_acl_tdb.8.xml
+++ b/docs-xml/manpages/vfs_acl_tdb.8.xml
@@ -63,6 +63,31 @@
 		</para>
 		</listitem>
 		</varlistentry>
+
+		<varlistentry>
+		<term>acl_tdb:default acl style = [posix|windows]</term>
+		<listitem>
+		<para>
+		This parameter determines the type of ACL that is synthesized in
+		case a file or directory lacks an
+		<emphasis>security.NTACL</emphasis> xattr.
+		</para>
+		<para>
+		When set to <emphasis>posix</emphasis>, an ACL will be
+		synthesized based on the POSIX mode permissions for user, group
+		and others, with an additional ACE for <emphasis>NT
+		Authority\SYSTEM</emphasis> will full rights.
+		</para>
+		<para>
+		When set to <emphasis>windows</emphasis>, an ACL is synthesized
+		the same way Windows does it, only including permissions for the
+		owner and <emphasis>NT Authority\SYSTEM</emphasis>.
+		</para>
+		<para>
+		The default for this option is <emphasis>posix</emphasis>.
+		</para>
+		</listitem>
+		</varlistentry>
 	</variablelist>
 
 </refsect1>
diff --git a/docs-xml/manpages/vfs_acl_xattr.8.xml b/docs-xml/manpages/vfs_acl_xattr.8.xml
index 60a1c2d..8da73e0 100644
--- a/docs-xml/manpages/vfs_acl_xattr.8.xml
+++ b/docs-xml/manpages/vfs_acl_xattr.8.xml
@@ -67,6 +67,31 @@
 		</para>
 		</listitem>
 		</varlistentry>
+
+		<varlistentry>
+		<term>acl_xattr:default acl style = [posix|windows]</term>
+		<listitem>
+		<para>
+		This parameter determines the type of ACL that is synthesized in
+		case a file or directory lacks an
+		<emphasis>security.NTACL</emphasis> xattr.
+		</para>
+		<para>
+		When set to <emphasis>posix</emphasis>, an ACL will be
+		synthesized based on the POSIX mode permissions for user, group
+		and others, with an additional ACE for <emphasis>NT
+		Authority\SYSTEM</emphasis> will full rights.
+		</para>
+		<para>
+		When set to <emphasis>windows</emphasis>, an ACL is synthesized
+		the same way Windows does it, only including permissions for the
+		owner and <emphasis>NT Authority\SYSTEM</emphasis>.
+		</para>
+		<para>
+		The default for this option is <emphasis>posix</emphasis>.
+		</para>
+		</listitem>
+		</varlistentry>
 	</variablelist>
 
 </refsect1>
diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index 9071775..919a095 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -46,8 +46,16 @@ static NTSTATUS store_acl_blob_fsp(vfs_handle_struct *handle,
 				SECINFO_DACL | \
 				SECINFO_SACL)
 
+enum default_acl_style {DEFAULT_ACL_POSIX, DEFAULT_ACL_WINDOWS};
+
+static const struct enum_list default_acl_style[] = {
+	{DEFAULT_ACL_POSIX,	"posix"},
+	{DEFAULT_ACL_WINDOWS,	"windows"}
+};
+
 struct acl_common_config {
 	bool ignore_system_acls;
+	enum default_acl_style default_acl_style;
 };
 
 static bool init_acl_common_config(vfs_handle_struct *handle)
@@ -65,6 +73,11 @@ static bool init_acl_common_config(vfs_handle_struct *handle)
 						  ACL_MODULE_NAME,
 						  "ignore system acls",
 						  false);
+	config->default_acl_style = lp_parm_enum(SNUM(handle->conn),
+						 ACL_MODULE_NAME,
+						 "default acl style",
+						 default_acl_style,
+						 DEFAULT_ACL_POSIX);
 
 	SMB_VFS_HANDLE_SET_DATA(handle, config, NULL,
 				struct acl_common_config,
@@ -387,10 +400,10 @@ static NTSTATUS add_directory_inheritable_components(vfs_handle_struct *handle,
 	return NT_STATUS_OK;
 }
 
-static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx,
-					    const char *name,
-					    SMB_STRUCT_STAT *psbuf,
-					    struct security_descriptor **ppdesc)
+static NTSTATUS make_default_acl_posix(TALLOC_CTX *ctx,
+				       const char *name,
+				       SMB_STRUCT_STAT *psbuf,
+				       struct security_descriptor **ppdesc)
 {
 	struct dom_sid owner_sid, group_sid;
 	size_t size = 0;
@@ -495,6 +508,29 @@ static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx,
 	return NT_STATUS_OK;
 }
 
+static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx,
+					    struct acl_common_config *config,
+					    const char *name,
+					    SMB_STRUCT_STAT *psbuf,
+					    struct security_descriptor **ppdesc)
+{
+	NTSTATUS status;
+
+	switch (config->default_acl_style) {
+
+	case DEFAULT_ACL_POSIX:
+		status =  make_default_acl_posix(ctx, name, psbuf, ppdesc);
+		break;
+
+	default:
+		DBG_ERR("unknown acl style %d", config->default_acl_style);
+		status = NT_STATUS_INTERNAL_ERROR;
+		break;
+	}
+
+	return status;
+}
+
 /**
  * Validate an ACL blob
  *
@@ -814,6 +850,7 @@ static NTSTATUS get_nt_acl_internal(vfs_handle_struct *handle,
 
 			status = make_default_filesystem_acl(
 				mem_ctx,
+				config,
 				smb_fname->base_name,
 				psbuf,
 				&psd);
-- 
2.7.4


From 54dbe78c2ec49dff927bd120e25db1f2a9b10e65 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 25 Aug 2016 07:45:34 +0200
Subject: [PATCH 11/12] vfs_acl_common: Windows style default ACL

Reintroduce Windows style default ACL, but this time as an optional
feature, not changing default behaviour.

Original bugreport that got reverted because it changed the default
behaviour: https://bugzilla.samba.org/show_bug.cgi?id=12028

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

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_acl_common.c | 76 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
index 919a095..a3f1e3d 100644
--- a/source3/modules/vfs_acl_common.c
+++ b/source3/modules/vfs_acl_common.c
@@ -508,6 +508,78 @@ static NTSTATUS make_default_acl_posix(TALLOC_CTX *ctx,
 	return NT_STATUS_OK;
 }
 
+static NTSTATUS make_default_acl_windows(TALLOC_CTX *ctx,
+					 const char *name,
+					 SMB_STRUCT_STAT *psbuf,
+					 struct security_descriptor **ppdesc)
+{
+	struct dom_sid owner_sid, group_sid;
+	size_t size = 0;
+	struct security_ace aces[4];
+	uint32_t access_mask = 0;
+	mode_t mode = psbuf->st_ex_mode;
+	struct security_acl *new_dacl = NULL;
+	int idx = 0;
+
+	DBG_DEBUG("file [%s] mode [0%o]\n", name, (int)mode);
+
+	uid_to_sid(&owner_sid, psbuf->st_ex_uid);
+	gid_to_sid(&group_sid, psbuf->st_ex_gid);
+
+	/*
+	 * We provide 2 ACEs:
+	 * - Owner
+	 * - NT System
+	 */
+
+	if (mode & S_IRUSR) {
+		if (mode & S_IWUSR) {
+			access_mask |= SEC_RIGHTS_FILE_ALL;
+		} else {
+			access_mask |= SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE;
+		}
+	}
+	if (mode & S_IWUSR) {
+		access_mask |= SEC_RIGHTS_FILE_WRITE | SEC_STD_DELETE;
+	}
+
+	init_sec_ace(&aces[idx],
+		     &owner_sid,
+		     SEC_ACE_TYPE_ACCESS_ALLOWED,
+		     access_mask,
+		     0);
+	idx++;
+
+	init_sec_ace(&aces[idx],
+		     &global_sid_System,
+		     SEC_ACE_TYPE_ACCESS_ALLOWED,
+		     SEC_RIGHTS_FILE_ALL,
+		     0);
+	idx++;
+
+	new_dacl = make_sec_acl(ctx,
+				NT4_ACL_REVISION,
+				idx,
+				aces);
+
+	if (!new_dacl) {
+		return NT_STATUS_NO_MEMORY;
+	}
+
+	*ppdesc = make_sec_desc(ctx,
+				SECURITY_DESCRIPTOR_REVISION_1,
+				SEC_DESC_SELF_RELATIVE|SEC_DESC_DACL_PRESENT,
+				&owner_sid,
+				&group_sid,
+				NULL,
+				new_dacl,
+				&size);
+	if (!*ppdesc) {
+		return NT_STATUS_NO_MEMORY;
+	}
+	return NT_STATUS_OK;
+}
+
 static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx,
 					    struct acl_common_config *config,
 					    const char *name,
@@ -522,6 +594,10 @@ static NTSTATUS make_default_filesystem_acl(TALLOC_CTX *ctx,
 		status =  make_default_acl_posix(ctx, name, psbuf, ppdesc);
 		break;
 
+	case DEFAULT_ACL_WINDOWS:
+		status =  make_default_acl_windows(ctx, name, psbuf, ppdesc);
+		break;
+
 	default:
 		DBG_ERR("unknown acl style %d", config->default_acl_style);
 		status = NT_STATUS_INTERNAL_ERROR;
-- 
2.7.4


From 521fda43f24d35b8af4ed9fd503b1d42f30e3d03 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 25 Aug 2016 16:30:24 +0200
Subject: [PATCH 12/12] s4/torture: tests for vfs_acl_xattr default ACL styles

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 selftest/target/Samba3.pm       |   8 +
 source3/selftest/tests.py       |   4 +-
 source4/torture/vfs/acl_xattr.c | 314 ++++++++++++++++++++++++++++++++++++++++
 source4/torture/vfs/vfs.c       |   1 +
 source4/torture/wscript_build   |   2 +-
 5 files changed, 327 insertions(+), 2 deletions(-)
 create mode 100644 source4/torture/vfs/acl_xattr.c

diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 8fc3204..f68d7de 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -1792,6 +1792,14 @@ sub provision($$$$$$$$)
 	vfs objects = acl_xattr fake_acls xattr_tdb fake_dfq
 	inherit owner = yes
 	include = $dfqconffile
+[acl_xattr_ign_sysacl_posix]
+	copy = tmp
+	acl_xattr:ignore system acls = yes
+	acl_xattr:default acl style = posix
+[acl_xattr_ign_sysacl_windows]
+	copy = tmp
+	acl_xattr:ignore system acls = yes
+	acl_xattr:default acl style = windows
 	";
 	close(CONF);
 
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 7d0dcc1..e4c31c6 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -322,7 +322,7 @@ nbt = ["nbt.dgram" ]
 
 libsmbclient = ["libsmbclient"]
 
-vfs = ["vfs.fruit"]
+vfs = ["vfs.fruit", "vfs.acl_xattr"]
 
 tests= base + raw + smb2 + rpc + unix + local + rap + nbt + libsmbclient + idmap + vfs
 
@@ -418,6 +418,8 @@ for t in tests:
         plansmbtorture4testsuite(t, "ad_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD --signing=required')
     elif t == "smb2.dosmode":
         plansmbtorture4testsuite(t, "simpleserver", '//$SERVER/dosmode -U$USERNAME%$PASSWORD')
+    elif t == "vfs.acl_xattr":
+        plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
     else:
         plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
         plansmbtorture4testsuite(t, "ad_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD')
diff --git a/source4/torture/vfs/acl_xattr.c b/source4/torture/vfs/acl_xattr.c
new file mode 100644
index 0000000..7fd10d0
--- /dev/null
+++ b/source4/torture/vfs/acl_xattr.c
@@ -0,0 +1,314 @@
+/*
+   Unix SMB/CIFS implementation.
+
+   Copyright (C) Ralph Boehme 2016
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include "includes.h"
+#include "lib/cmdline/popt_common.h"
+#include "libcli/smb2/smb2.h"
+#include "libcli/smb2/smb2_calls.h"
+#include "libcli/smb/smbXcli_base.h"
+#include "torture/torture.h"
+#include "torture/vfs/proto.h"
+#include "libcli/resolve/resolve.h"
+#include "torture/util.h"
+#include "torture/smb2/proto.h"
+#include "libcli/security/security.h"
+#include "librpc/gen_ndr/ndr_security.h"
+#include "lib/param/param.h"
+
+#define BASEDIR "smb2-testsd"
+
+#define CHECK_SECURITY_DESCRIPTOR(_sd1, _sd2) do { \
+	if (!security_descriptor_equal(_sd1, _sd2)) { \
+		torture_warning(tctx, "security descriptors don't match!\n"); \
+		torture_warning(tctx, "got:\n"); \
+		NDR_PRINT_DEBUG(security_descriptor, _sd1); \
+		torture_warning(tctx, "expected:\n"); \
+		NDR_PRINT_DEBUG(security_descriptor, _sd2); \
+		torture_result(tctx, TORTURE_FAIL, \
+			       "%s: security descriptors don't match!\n", \
+			       __location__); \
+		ret = false; \
+	} \
+} while (0)
+
+/**
+ * SMB2 connect with explicit share
+ **/
+static bool torture_smb2_con_share(struct torture_context *tctx,
+                           const char *share,
+                           struct smb2_tree **tree)
+{
+        struct smbcli_options options;
+        NTSTATUS status;
+        const char *host = torture_setting_string(tctx, "host", NULL);
+        struct cli_credentials *credentials = cmdline_credentials;
+
+        lpcfg_smbcli_options(tctx->lp_ctx, &options);
+
+        status = smb2_connect_ext(tctx,
+                                  host,
+                                  lpcfg_smb_ports(tctx->lp_ctx),
+                                  share,
+                                  lpcfg_resolve_context(tctx->lp_ctx),
+                                  credentials,
+                                  0,
+                                  tree,
+                                  tctx->ev,
+                                  &options,
+                                  lpcfg_socket_options(tctx->lp_ctx),
+                                  lpcfg_gensec_settings(tctx, tctx->lp_ctx)
+                                  );
+        if (!NT_STATUS_IS_OK(status)) {
+                printf("Failed to connect to SMB2 share \\\\%s\\%s - %s\n",
+                       host, share, nt_errstr(status));
+                return false;
+        }
+        return true;
+}
+
+static bool test_default_acl_posix(struct torture_context *tctx,
+				   struct smb2_tree *tree_unused)
+{
+	struct smb2_tree *tree = NULL;
+	NTSTATUS status;
+	bool ok;
+	bool ret = true;
+	const char *dname = BASEDIR "\\testdir";
+	const char *fname = BASEDIR "\\testdir\\testfile";
+	struct smb2_handle fhandle, dhandle;
+	union smb_fileinfo q;
+	union smb_setfileinfo set;
+	struct security_descriptor *sd = NULL;
+	struct security_descriptor *exp_sd = NULL;
+	char *owner_sid = NULL;
+	char *group_sid = NULL;
+
+	ok = torture_smb2_con_share(tctx, "acl_xattr_ign_sysacl_posix", &tree);
+	torture_assert_goto(tctx, ok == true, ret, done,
+			    "Unable to connect to 'acl_xattr_ign_sysacl_posix'\n");
+
+	ok = smb2_util_setup_dir(tctx, tree, BASEDIR);
+	torture_assert_goto(tctx, ok == true, ret, done, "Unable to setup testdir\n");
+
+	ZERO_STRUCT(dhandle);
+	status = torture_smb2_testdir(tree, dname, &dhandle);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "torture_smb2_testdir\n");
+
+	torture_comment(tctx, "Get the original sd\n");
+
+	ZERO_STRUCT(q);
+	q.query_secdesc.level = RAW_FILEINFO_SEC_DESC;
+	q.query_secdesc.in.file.handle = dhandle;
+	q.query_secdesc.in.secinfo_flags = SECINFO_DACL | SECINFO_OWNER | SECINFO_GROUP;
+	status = smb2_getinfo_file(tree, tctx, &q);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_getinfo_file\n");
+
+	sd = q.query_secdesc.out.sd;
+	owner_sid = dom_sid_string(tctx, sd->owner_sid);
+	group_sid = dom_sid_string(tctx, sd->group_sid);
+	torture_comment(tctx, "owner [%s] group [%s]\n", owner_sid, group_sid);
+
+	torture_comment(tctx, "Set ACL with no inheritable ACE\n");
+
+	sd = security_descriptor_dacl_create(tctx,
+					     0, NULL, NULL,
+					     owner_sid,
+					     SEC_ACE_TYPE_ACCESS_ALLOWED,
+					     SEC_RIGHTS_DIR_ALL,
+					     0,
+					     NULL);
+
+	ZERO_STRUCT(set);
+	set.set_secdesc.level = RAW_SFILEINFO_SEC_DESC;
+	set.set_secdesc.in.file.handle = dhandle;
+	set.set_secdesc.in.secinfo_flags = SECINFO_DACL;
+	set.set_secdesc.in.sd = sd;
+	status = smb2_setinfo_file(tree, &set);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_setinfo_file\n");
+
+	TALLOC_FREE(sd);
+	smb2_util_close(tree, dhandle);
+
+	torture_comment(tctx, "Create file\n");
+
+	ZERO_STRUCT(fhandle);
+	status = torture_smb2_testfile(tree, fname, &fhandle);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_create_complex_file\n");
+
+	torture_comment(tctx, "Query file SD\n");
+
+	ZERO_STRUCT(q);
+	q.query_secdesc.level = RAW_FILEINFO_SEC_DESC;
+	q.query_secdesc.in.file.handle = fhandle;
+	q.query_secdesc.in.secinfo_flags = SECINFO_DACL | SECINFO_OWNER | SECINFO_GROUP;
+	status = smb2_getinfo_file(tree, tctx, &q);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_getinfo_file\n");
+	sd = q.query_secdesc.out.sd;
+
+	smb2_util_close(tree, fhandle);
+	ZERO_STRUCT(fhandle);
+
+	torture_comment(tctx, "Checking actual file SD against expected SD\n");
+
+	exp_sd = security_descriptor_dacl_create(
+		tctx, 0, owner_sid, group_sid,
+		owner_sid, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_RIGHTS_FILE_ALL, 0,
+		group_sid, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE, 0,
+		SID_WORLD, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_RIGHTS_FILE_READ | SEC_FILE_EXECUTE, 0,
+		SID_NT_SYSTEM, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_RIGHTS_FILE_ALL, 0,
+		NULL);
+
+	CHECK_SECURITY_DESCRIPTOR(sd, exp_sd);
+
+done:
+	if (!smb2_util_handle_empty(fhandle)) {
+		smb2_util_close(tree, fhandle);
+	}
+	if (!smb2_util_handle_empty(dhandle)) {
+		smb2_util_close(tree, dhandle);
+	}
+	if (tree != NULL) {
+		smb2_deltree(tree, BASEDIR);
+		smb2_tdis(tree);
+	}
+
+	return ret;
+}
+
+static bool test_default_acl_win(struct torture_context *tctx,
+				   struct smb2_tree *tree_unused)
+{
+	struct smb2_tree *tree = NULL;
+	NTSTATUS status;
+	bool ok;
+	bool ret = true;
+	const char *dname = BASEDIR "\\testdir";
+	const char *fname = BASEDIR "\\testdir\\testfile";
+	struct smb2_handle fhandle, dhandle;
+	union smb_fileinfo q;
+	union smb_setfileinfo set;
+	struct security_descriptor *sd = NULL;
+	struct security_descriptor *exp_sd = NULL;
+	char *owner_sid = NULL;
+	char *group_sid = NULL;
+
+	ok = torture_smb2_con_share(tctx, "acl_xattr_ign_sysacl_windows", &tree);
+	torture_assert_goto(tctx, ok == true, ret, done,
+			    "Unable to connect to 'acl_xattr_ign_sysacl_windows'\n");
+
+	ok = smb2_util_setup_dir(tctx, tree, BASEDIR);
+	torture_assert_goto(tctx, ok == true, ret, done, "Unable to setup testdir\n");
+
+	ZERO_STRUCT(dhandle);
+	status = torture_smb2_testdir(tree, dname, &dhandle);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "torture_smb2_testdir\n");
+
+	torture_comment(tctx, "Get the original sd\n");
+
+	ZERO_STRUCT(q);
+	q.query_secdesc.level = RAW_FILEINFO_SEC_DESC;
+	q.query_secdesc.in.file.handle = dhandle;
+	q.query_secdesc.in.secinfo_flags = SECINFO_DACL | SECINFO_OWNER | SECINFO_GROUP;
+	status = smb2_getinfo_file(tree, tctx, &q);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_getinfo_file\n");
+
+	sd = q.query_secdesc.out.sd;
+	owner_sid = dom_sid_string(tctx, sd->owner_sid);
+	group_sid = dom_sid_string(tctx, sd->group_sid);
+	torture_comment(tctx, "owner [%s] group [%s]\n", owner_sid, group_sid);
+
+	torture_comment(tctx, "Set ACL with no inheritable ACE\n");
+
+	sd = security_descriptor_dacl_create(tctx,
+					     0, NULL, NULL,
+					     owner_sid,
+					     SEC_ACE_TYPE_ACCESS_ALLOWED,
+					     SEC_RIGHTS_DIR_ALL,
+					     0,
+					     NULL);
+
+	ZERO_STRUCT(set);
+	set.set_secdesc.level = RAW_SFILEINFO_SEC_DESC;
+	set.set_secdesc.in.file.handle = dhandle;
+	set.set_secdesc.in.secinfo_flags = SECINFO_DACL;
+	set.set_secdesc.in.sd = sd;
+	status = smb2_setinfo_file(tree, &set);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_setinfo_file\n");
+
+	TALLOC_FREE(sd);
+	smb2_util_close(tree, dhandle);
+
+	torture_comment(tctx, "Create file\n");
+
+	ZERO_STRUCT(fhandle);
+	status = torture_smb2_testfile(tree, fname, &fhandle);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_create_complex_file\n");
+
+	torture_comment(tctx, "Query file SD\n");
+
+	ZERO_STRUCT(q);
+	q.query_secdesc.level = RAW_FILEINFO_SEC_DESC;
+	q.query_secdesc.in.file.handle = fhandle;
+	q.query_secdesc.in.secinfo_flags = SECINFO_DACL | SECINFO_OWNER | SECINFO_GROUP;
+	status = smb2_getinfo_file(tree, tctx, &q);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_getinfo_file\n");
+	sd = q.query_secdesc.out.sd;
+
+	smb2_util_close(tree, fhandle);
+	ZERO_STRUCT(fhandle);
+
+	torture_comment(tctx, "Checking actual file SD against expected SD\n");
+
+	exp_sd = security_descriptor_dacl_create(
+		tctx, 0, owner_sid, group_sid,
+		owner_sid, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_RIGHTS_FILE_ALL, 0,
+		SID_NT_SYSTEM, SEC_ACE_TYPE_ACCESS_ALLOWED, SEC_RIGHTS_FILE_ALL, 0,
+		NULL);
+
+	CHECK_SECURITY_DESCRIPTOR(sd, exp_sd);
+
+done:
+	if (!smb2_util_handle_empty(fhandle)) {
+		smb2_util_close(tree, fhandle);
+	}
+	if (!smb2_util_handle_empty(dhandle)) {
+		smb2_util_close(tree, dhandle);
+	}
+	if (tree != NULL) {
+		smb2_deltree(tree, BASEDIR);
+		smb2_tdis(tree);
+	}
+
+	return ret;
+}
+
+/*
+   basic testing of vfs_acl_xattr
+*/
+struct torture_suite *torture_acl_xattr(void)
+{
+	struct torture_suite *suite = torture_suite_create(talloc_autofree_context(), "acl_xattr");
+
+	torture_suite_add_1smb2_test(suite, "default-acl-style-posix", test_default_acl_posix);
+	torture_suite_add_1smb2_test(suite, "default-acl-style-windows", test_default_acl_win);
+
+	suite->description = talloc_strdup(suite, "vfs_acl_xattr tests");
+
+	return suite;
+}
diff --git a/source4/torture/vfs/vfs.c b/source4/torture/vfs/vfs.c
index f3ce447..7f805f4 100644
--- a/source4/torture/vfs/vfs.c
+++ b/source4/torture/vfs/vfs.c
@@ -107,6 +107,7 @@ NTSTATUS torture_vfs_init(void)
 	suite->description = talloc_strdup(suite, "VFS modules tests");
 
 	torture_suite_add_suite(suite, torture_vfs_fruit());
+	torture_suite_add_suite(suite, torture_acl_xattr());
 
 	torture_register_suite(suite);
 
diff --git a/source4/torture/wscript_build b/source4/torture/wscript_build
index 382e2c6..ff79c3d 100755
--- a/source4/torture/wscript_build
+++ b/source4/torture/wscript_build
@@ -271,7 +271,7 @@ bld.SAMBA_MODULE('TORTURE_NTP',
 	)
 
 bld.SAMBA_MODULE('TORTURE_VFS',
-	source='vfs/vfs.c vfs/fruit.c',
+	source='vfs/vfs.c vfs/fruit.c vfs/acl_xattr.c',
 	subsystem='smbtorture',
 	deps='LIBCLI_SMB POPT_CREDENTIALS TORTURE_UTIL smbclient-raw TORTURE_RAW',
 	internal_module=True,
-- 
2.7.4



More information about the samba mailing list