[SCM] Samba Shared Repository - branch v3-0-test updated -
release-3-0-32-121-g9a95b6c
Jeremy Allison
jra at samba.org
Thu Jan 22 18:59:21 GMT 2009
The branch, v3-0-test has been updated
via 9a95b6cac2dea88cb9e9b428292dfca9d1e3e801 (commit)
from 0098eb45d99373a4d1945e61dda24ea282c377e7 (commit)
http://gitweb.samba.org/?p=samba.git;a=shortlog;h=v3-0-test
- Log -----------------------------------------------------------------
commit 9a95b6cac2dea88cb9e9b428292dfca9d1e3e801
Author: Jeremy Allison <jra at samba.org>
Date: Thu Jan 22 10:58:38 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 | 27 ---------------------------
1 files changed, 0 insertions(+), 27 deletions(-)
Changeset truncated at 500 lines:
diff --git a/source/smbd/posix_acls.c b/source/smbd/posix_acls.c
index 33cba6a..23bf40f 100644
--- a/source/smbd/posix_acls.c
+++ b/source/smbd/posix_acls.c
@@ -3338,7 +3338,6 @@ BOOL set_nt_acl(files_struct *fsp, uint32 security_info_sent, SEC_DESC *psd)
mode_t orig_mode = (mode_t)0;
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 ));
@@ -3377,16 +3376,6 @@ BOOL set_nt_acl(files_struct *fsp, uint32 security_info_sent, SEC_DESC *psd)
*/
if (((user != (uid_t)-1) && (orig_uid != user)) || (( grp != (gid_t)-1) && (orig_gid != grp))) {
- need_chown = True;
- }
-
- /*
- * Chown before setting ACL only if we don't change the user, or
- * if we change to the current user, but not if we want to give away
- * the file.
- */
-
- 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 ));
@@ -3423,9 +3412,6 @@ BOOL 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 it, don't try again */
- need_chown = False;
}
create_file_sids(&sbuf, &file_owner_sid, &file_grp_sid);
@@ -3577,19 +3563,6 @@ BOOL set_nt_acl(files_struct *fsp, uint32 security_info_sent, SEC_DESC *psd)
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) ));
- return False;
- }
- }
-
return True;
}
--
Samba Shared Repository
More information about the samba-cvs
mailing list