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

Jeremy Allison jra at samba.org
Fri Aug 26 21:46:19 UTC 2016


On Fri, Aug 26, 2016 at 06:44:05PM +0200, Ralph Böhme wrote:
> 
> Cheerio!
> -slow

Still reviewing this - but a few things that will need changing:

When adding the validate_nt_acl_blob() function in
[PATCH 06/12] vfs_acl_common: move the ACL blob validation to a helper function

this makes some of the existing function names in debug statements
incorrect.

Eg.  validate_nt_acl_blob() has debug statements:

688                 DEBUG(10, ("get_nt_acl_internal: ACL blob revision "
 689                            "mismatch (%u) for file %s\n",
 690                            (unsigned int)hash_type,
 691                            smb_fname->base_name));
 692                 TALLOC_FREE(psd_blob);
 693                 return NT_STATUS_OK;
 694         }
 695 
 696         /* determine which type of xattr we got */
 697         if (hash_type != XATTR_SD_HASH_TYPE_SHA256) {
 698                 DEBUG(10, ("get_nt_acl_internal: ACL blob hash type "
 699                            "(%u) unexpected for file %s\n",
 700                            (unsigned int)hash_type,
 701                            smb_fname->base_name));
 702                 TALLOC_FREE(psd_blob);

768                 if (!NT_STATUS_IS_OK(status)) {
 769                         DEBUG(10, ("get_nt_acl_internal: get_next_acl for file %s "
 770                                    "returned %s\n",
 771                                    smb_fname->base_name,
 772                                    nt_errstr(status)));
 773                         goto fail;

784                 if (memcmp(&hash[0], &hash_tmp[0], XATTR_SD_HASH_SIZE) == 0) {
 785                         /* Hash matches, return blob sd. */
 786                         DEBUG(10, ("get_nt_acl_internal: blob hash "
 787                                    "matches for file %s\n",
 788                                    smb_fname->base_name ));
 789                         *ppsd = psd_blob;

794                 DEBUG(10, ("get_nt_acl_internal: blob hash "
 795                            "does not match for file %s - returning "
 796                            "file system SD mapping.\n",
 797                            smb_fname->base_name ));
 798 
 799                 if (DEBUGLEVEL >= 10) {
 800                         DEBUG(10,("get_nt_acl_internal: acl for blob hash for %s is:\n",
 801                                   smb_fname->base_name ));
 802                         NDR_PRINT_DEBUG(security_descriptor, psd_fs);

These are gonna need fixing to remove the function names
from the debugs (they are automatically added now).

Still reviewing...

> 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