[patch] Allow chown of directories from W2k domain clients

[Andrew Bartlett]

On Wed, 2003-02-19 at 19:18, Andrew Furey wrote:
> Hi all,
> 
> I never did get any replies to my original postings to this list (as 
> well as samba@), so I wrote my own patch. I just _know_ that there are 
> going to be several folks point out that this is a hack, and would never 
> survive an audit, etc... but it does the job, which is what our client 
> wanted ;)
> 
> Our existing machine is a domain member server, joined to a W2k-based 
> domain in mixed mode (using winbind+NSS to get user details), which is 
> using ACLs (on ext3) to provide file serving capabilities. The (admin) 
> users are then trying to "take ownership" on files and directories 
> within the shares. This is (I presume) a very specific scenario, or else 
> I presumably would have had more replies before now (changing ownership 
> isn't that uncommon an operation, is it?)

With the implementation of privileges in Samba, the existing
vendor-specific hacks should be replaced with 'proper' support for this
kind of operation.

> Note that this patch isn't perfect; it doesn't check to see if the user 
> actually has write permission on the directory, which is of course a big 
> security hole. Nor does it seem to work from an NT4 client (comes up 
> with "Access Denied"). I didn't get a chance to seriously look at the 
> NT4 problem, but from what I saw of the logs it seems to be using a 
> completely different section of the code.
> 
> Comments (and flames) encouraged...
> 
> Andrew
> 
> -- 
> ANDREW FUREY <andrew at terminus.net.au> - Sysadmin/developer for Terminus.
> Providing online networks of Australian lawyers (http://www.ilaw.com.au)
> and Linux experts (http://www.linuxconsultants.com.au) for instant help!
> Disclaimer: http://www.terminus.net.au/disclaimer.html. GCS L+++ P++ t++
> ----
> 

> diff -u -r samba-2.2.7a-orig/source/smbd/open.c samba-2.2.7a/source/smbd/open.c
> --- samba-2.2.7a-orig/source/smbd/open.c	2002-12-10 22:58:17.000000000 +0800
> +++ samba-2.2.7a/source/smbd/open.c	2003-02-12 08:53:44.000000000 +0800
> @@ -49,6 +49,11 @@
>  		fd = conn->vfs_ops.open(conn,dos_to_unix_static(fname),flags,mode);
>  	}
>  
> +	/* Don't fail automatically if a directory (andrew at terminus.net.au) */
> +	if (strcmp(strerror(errno),"Is a directory") == 0) {
> +	    fd = conn->vfs_ops.opendir(conn,dos_to_unix_static(fname));
> +	}
> +

This needs some serious flamage...  Why can't you just use the errno
directly?

>  	DEBUG(10,("fd_open: name %s, flags = 0%o mode = 0%o, fd = %d. %s\n", fname,
>  		flags, (int)mode, fd, (fd == -1) ? strerror(errno) : "" ));
>  
> diff -u -r samba-2.2.7a-orig/source/smbd/posix_acls.c samba-2.2.7a/source/smbd/posix_acls.c
> --- samba-2.2.7a-orig/source/smbd/posix_acls.c	2002-12-10 22:58:17.000000000 +0800
> +++ samba-2.2.7a/source/smbd/posix_acls.c	2003-02-12 09:00:51.000000000 +0800
> @@ -2173,6 +2173,10 @@
>  	mode_t orig_mode = (mode_t)0;
>  	uid_t orig_uid;
>  	gid_t orig_gid;
> +	int chown_return;
> +	char fullpath[1000];
> +	/* have to specify a size - hopefully 1000 chars for the full file
> +	   name (on the server) should be enough (andrew at terminus.net.au) */

That's what a pstring is for, but we try to use allocated strings now.

>  	DEBUG(10,("set_nt_acl: called for file %s\n", fsp->fsp_name ));
>  
> @@ -2214,7 +2218,8 @@
>  		DEBUG(3,("set_nt_acl: chown %s. uid = %u, gid = %u.\n",
>  				fsp->fsp_name, (unsigned int)user, (unsigned int)grp ));
>  
> -		if(try_chown( fsp->conn, fsp->fsp_name, user, grp) == -1) {
> +		/* also check if it's a directory (andrew at terminus.net.au) */
> +		if((try_chown( fsp->conn, fsp->fsp_name, user, grp) == -1) && (fsp->is_directory == 0)) {
>  			DEBUG(3,("set_nt_acl: chown %s, %u, %u failed. Error = %s.\n",
>  				fsp->fsp_name, (unsigned int)user, (unsigned int)grp, strerror(errno) ));
>  			return False;
> @@ -2254,6 +2259,16 @@
>  									&file_ace_list, &dir_ace_list, security_info_sent, psd);
>  
>  	if ((file_ace_list == NULL) && (dir_ace_list == NULL)) {
> +		/* if we're here we're probably trying to chown a directory
> +		   (fails normally) - andrew at terminus.net.au */
> +		fstrcpy(fullpath, conn->connectpath);
> +		fstrcat(fullpath, "/");
> +		fstrcat(fullpath, fsp->fsp_name);
> +		become_root();

What are you doing to ensure that only your admin users can do this?

> +		chown_return = chown(fullpath, (unsigned int)user, -1);
> +		unbecome_root();
> +		DEBUG(5,("AndrewF: chown of %s returned %u\n",
> +		    fullpath, chown_return));
>  		/* W2K traverse DACL set - ignore. */
>  		return True;
>  	}
-- 
Andrew Bartlett                                 abartlet at pcug.org.au
Manager, Authentication Subsystems, Samba Team  abartlet at samba.org
Student Network Administrator, Hawker College   abartlet at hawkerc.net
http://samba.org     http://build.samba.org     http://hawkerc.net
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : http://lists.samba.org/archive/samba-technical/attachments/20030219/bf592b9b/attachment.bin