When creating a new file/directory, we need to obey the create mask/directory mask parameters.

Andrew Bartlett abartlet at samba.org
Tue Oct 2 17:16:42 MDT 2012


On Tue, 2012-10-02 at 22:28 +0200, Jeremy Allison wrote:
> The branch, master has been updated
>        via  c251a6b When creating a new file/directory, we need to obey the create mask/directory mask parameters.
>        via  8f0ecbb Add functions to programatically set the security mask and directory security mask parameters.
>        via  6575d1d When setting a non-default ACL, don't forget to apply masks to SMB_ACL_USER and SMB_ACL_GROUP entries.
>        via  5d5ddbd Only apply masks on non-default ACL entries when setting the ACL.
>        via  82e7132 Use is_default_acl variable in canonicalise_acl().
>        via  efb446a Reformat spacing to be even.
>       from  a168a7c tdb: Fix a typo
> 
> http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
> 
> 
> - Log -----------------------------------------------------------------
> commit c251a6b0442abc13bc8be4ff8de324c1d7706a78
> Author: Jeremy Allison <jra at samba.org>
> Date:   Tue Oct 2 10:25:14 2012 -0700
> 
>     When creating a new file/directory, we need to obey the create mask/directory mask parameters.
>     
>     Currently we call FSET_NT_ACL to inherit any ACLs on create. However
>     FSET_NT_ACL uses the security mask/directory security mask parameters
>     instead of the create mask/directory mask parameters.
>     
>     Swap them temporarily when creating to ensure the correct masks
>     are applied.
>     
>     Autobuild-User(master): Jeremy Allison <jra at samba.org>
>     Autobuild-Date(master): Tue Oct  2 22:27:17 CEST 2012 on sn-devel-104

Jeremy,

Why couldn't we just pass in the information to FSET_NT_ACL so it can
use the correct parameters in the first place? 

I really dislike the idea of changing the smb.conf parameters
mid-execution like this.  It breaks the abstractions horribly, creates
confusion in the code (because the lower layers don't know that they are
not really using this parameter), and it will make it much harder to
reform the smb.conf loading layer.

Can you please rework this to pass in a file/directory flag?

Thanks,

> diff --git a/source3/include/proto.h b/source3/include/proto.h
> index b3fa55a..e42c33d 100644
> --- a/source3/include/proto.h
> +++ b/source3/include/proto.h
> @@ -1188,6 +1188,8 @@ bool lp_getwd_cache(void);
>  int lp_srv_maxprotocol(void);
>  int lp_srv_minprotocol(void);
>  int lp_security(void);
> +int lp_set_security_mask(int snum, int new_val);
> +int lp_set_directory_security_mask(int snum, int new_mask);
>  int lp__server_role(void);
>  int lp__security(void);
>  int lp__domain_master(void);
> diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
> index 61606ce..960a644 100644
> --- a/source3/param/loadparm.c
> +++ b/source3/param/loadparm.c
> @@ -5476,3 +5476,17 @@ int lp_security(void)
>  	return lp_find_security(lp__server_role(),
>  				lp__security());
>  }
> +
> +int lp_set_security_mask(int snum, int new_val)
> +{
> +	int ret = ServicePtrs[snum]->iSecurity_mask;
> +	ServicePtrs[snum]->iSecurity_mask = new_val;
> +	return ret;
> +}
> +
> +int lp_set_directory_security_mask(int snum, int new_val)
> +{
> +	int ret = ServicePtrs[snum]->iDir_Security_mask;
> +	ServicePtrs[snum]->iDir_Security_mask = new_val;
> +	return ret;
> +}
> diff --git a/source3/smbd/open.c b/source3/smbd/open.c
> index d4babd4..bea4d99 100644
> --- a/source3/smbd/open.c
> +++ b/source3/smbd/open.c
> @@ -3436,6 +3436,9 @@ static NTSTATUS inherit_new_acl(files_struct *fsp)
>  	bool inherit_owner = lp_inherit_owner(SNUM(fsp->conn));
>  	bool inheritable_components = false;
>  	size_t size = 0;
> +	int orig_security_mask = 0;
> +	int orig_directory_security_mask = 0;
> +	int snum = SNUM(fsp->conn);
>  
>  	if (!parent_dirname(ctx, fsp->fsp_name->base_name, &parent_name, NULL)) {
>  		return NT_STATUS_NO_MEMORY;
> @@ -3506,6 +3509,14 @@ static NTSTATUS inherit_new_acl(files_struct *fsp)
>  		NDR_PRINT_DEBUG(security_descriptor, psd);
>  	}
>  
> +	/* Temporarily replace the security masks with the create masks,
> +	   as we're actually doing a create here - we only call this
> +	   when we've created a file or directory - but there's no
> +	   way for FSET_NT_ACL to know the difference. */
> +
> +	orig_security_mask = lp_set_security_mask(snum, lp_create_mask(snum));
> +	orig_directory_security_mask = lp_set_directory_security_mask(snum, lp_dir_mask(snum));
> +
>  	if (inherit_owner) {
>  		/* We need to be root to force this. */
>  		become_root();
> @@ -3516,6 +3527,10 @@ static NTSTATUS inherit_new_acl(files_struct *fsp)
>  	if (inherit_owner) {
>  		unbecome_root();
>  	}
> +
> +	(void)lp_set_security_mask(snum, orig_security_mask);
> +	(void)lp_set_directory_security_mask(snum, orig_directory_security_mask);
> +
>  	return status;
>  }



-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org




More information about the samba-technical mailing list