[SCM] Samba Shared Repository - branch v3-2-test updated - release-3-2-0pre2-3390-g9c3da89

[Jeremy Allison]

The branch, v3-2-test has been updated
       via  9c3da895e6dd5df2f4e3377e1bf562b376436081 (commit)
      from  0ee05c012e5f58c9132549c59cfd1ed74dd27759 (commit)

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=v3-2-test


- Log -----------------------------------------------------------------
commit 9c3da895e6dd5df2f4e3377e1bf562b376436081
Author: Jeremy Allison <jra at samba.org>
Date:   Thu Jan 22 10:59:47 2009 -0800

    Another attempt to fix bug #4308 - Excel save operation corrupts file ACLs.
    Simo is completely correct. We should be doing the chown *first*, and fail the
    ACL set if this fails. The long standing assumption I made when writing the
    initial POSIX ACL code was that Windows didn't control who could chown a file
    in the same was as POSIX. In POSIX only root can do this whereas I wasn't sure
    who could do this in Windows at the time (I didn't understand the privilege
    model). So the assumption was that setting the ACL was more important (early
    tests showed many failed ACL set's due to inability to chown). But now we have
    privileges in smbd, and we must always fail an ACL set when we can't chown
    first. The key that Simo noticed is that the CREATOR_OWNER bits in the ACL
    incoming are relative to the *new* owner, not the old one. This is why the old
    user owner disappears on ACL set - their access was set via the USER_OBJ in the
    creator POSIX ACL and when the ownership changes they lose their access.
    
    Patch is simple - just ensure we do the chown first before evaluating the
    incoming ACL re-read the owners. We already have code to do this it just wasn't
    rigorously being applied.
    Jeremy.

-----------------------------------------------------------------------

Summary of changes:
 source/smbd/posix_acls.c |   29 ++++-------------------------
 1 files changed, 4 insertions(+), 25 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source/smbd/posix_acls.c b/source/smbd/posix_acls.c
index 09165e7..534c2b9 100644
--- a/source/smbd/posix_acls.c
+++ b/source/smbd/posix_acls.c
@@ -3439,7 +3439,6 @@ NTSTATUS set_nt_acl(files_struct *fsp, uint32 security_info_sent, SEC_DESC *psd)
 	NTSTATUS status;
 	uid_t orig_uid;
 	gid_t orig_gid;
-	bool need_chown = False;
 
 	DEBUG(10,("set_nt_acl: called for file %s\n", fsp->fsp_name ));
 
@@ -3475,14 +3474,12 @@ NTSTATUS set_nt_acl(files_struct *fsp, uint32 security_info_sent, SEC_DESC *psd)
 	}
 
 	/*
-	 * Do we need to chown ?
+	 * Do we need to chown ? If so this must be done first as the incoming
+	 * CREATOR_OWNER acl will be relative to the *new* owner, not the old.
+	 * Noticed by Simo.
 	 */
 
 	if (((user != (uid_t)-1) && (orig_uid != user)) || (( grp != (gid_t)-1) && (orig_gid != grp))) {
-		need_chown = True;
-	}
-
-	if (need_chown && (user == (uid_t)-1 || user == current_user.ut.uid)) {
 
 		DEBUG(3,("set_nt_acl: chown %s. uid = %u, gid = %u.\n",
 				fsp->fsp_name, (unsigned int)user, (unsigned int)grp ));
@@ -3522,9 +3519,6 @@ NTSTATUS set_nt_acl(files_struct *fsp, uint32 security_info_sent, SEC_DESC *psd)
 		orig_mode = sbuf.st_mode;
 		orig_uid = sbuf.st_uid;
 		orig_gid = sbuf.st_gid;
-
-		/* We did chown already, drop the flag */
-		need_chown = False;
 	}
 
 	create_file_sids(&sbuf, &file_owner_sid, &file_grp_sid);
@@ -3673,24 +3667,9 @@ NTSTATUS set_nt_acl(files_struct *fsp, uint32 security_info_sent, SEC_DESC *psd)
 		}
 
 		free_canon_ace_list(file_ace_list);
-		free_canon_ace_list(dir_ace_list); 
+		free_canon_ace_list(dir_ace_list);
 	}
 
-	/* Any chown pending? */
-	if (need_chown) {
-		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) {
-			DEBUG(3,("set_nt_acl: chown %s, %u, %u failed. Error = %s.\n",
-				 fsp->fsp_name, (unsigned int)user, (unsigned int)grp, strerror(errno) ));
-			if (errno == EPERM) {
-				return NT_STATUS_INVALID_OWNER;
-			}
-			return map_nt_error_from_unix(errno);
-		}
-	}
-	
 	return NT_STATUS_OK;
 }
 


-- 
Samba Shared Repository