[SCM] Samba Shared Repository - branch master updated

Jeremy Allison jra at samba.org
Fri Sep 2 16:17:02 MDT 2011


The branch, master has been updated
       via  17f6e02 Part 5 of bugfix for bug #7509 - smb_acl_to_posix: ACL is invalid for set (Invalid argument)
       via  2a1453e Part 4 of bugfix for bug #7509 - smb_acl_to_posix: ACL is invalid for set (Invalid argument)
       via  c528fc5 Part 3 of bugfix for bug #7509 - smb_acl_to_posix: ACL is invalid for set (Invalid argument)
       via  a5038ac Part 2 of bugfix for bug #7509 - smb_acl_to_posix: ACL is invalid for set (Invalid argument)
       via  2b935b4 Part 1 of bugfix for bug #7509 - smb_acl_to_posix: ACL is invalid for set (Invalid argument)
      from  dfbffac s3:registry: fix a debug message typo

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 17f6e0272370f764d4a0053c8e74f20b0444c721
Author: Jeremy Allison <jra at samba.org>
Date:   Fri Sep 2 13:41:24 2011 -0700

    Part 5 of bugfix for bug #7509 - smb_acl_to_posix: ACL is invalid for set (Invalid argument)
    
    Be smarter about setting default permissions when a ACL_GROUP_OBJ isn't given. Use the
    principle of least surprises for the user.
    
    Autobuild-User: Jeremy Allison <jra at samba.org>
    Autobuild-Date: Sat Sep  3 00:16:05 CEST 2011 on sn-devel-104

commit 2a1453e2318af77a79180f3137f8a8d3f1240233
Author: Jeremy Allison <jra at samba.org>
Date:   Fri Sep 2 13:36:10 2011 -0700

    Part 4 of bugfix for bug #7509 - smb_acl_to_posix: ACL is invalid for set (Invalid argument)
    
    Be smarter about setting default permissions when a ACL_USER_OBJ isn't given. Use the
    principle of least surprises for the user.

commit c528fc5cacaae7e0e83041eb98150052b436071e
Author: Jeremy Allison <jra at samba.org>
Date:   Fri Sep 2 12:22:34 2011 -0700

    Part 3 of bugfix for bug #7509 - smb_acl_to_posix: ACL is invalid for set (Invalid argument)
    
    Don't call check_owning_objs() to convert ACL_USER->ACL_USER_OBJ and
    AC_GROUP->ACL_GROUP_OBJ for default (directory) ACLs, we do this separately
    inside ensure_canon_entry_valid().

commit a5038ace24559bb02eec8262d3af5b5e78634d16
Author: Jeremy Allison <jra at samba.org>
Date:   Fri Sep 2 11:58:56 2011 -0700

    Part 2 of bugfix for bug #7509 - smb_acl_to_posix: ACL is invalid for set (Invalid argument)
    
    Only map CREATOR_OWNER/CREATOR_GROUP to ACL_USER_OBJ/ACL_GROUP_OBJ in
    a default(directory) ACL set.

commit 2b935b49f3d975759eb1cbcf2b11bf7c9d982804
Author: Jeremy Allison <jra at samba.org>
Date:   Fri Sep 2 11:21:08 2011 -0700

    Part 1 of bugfix for bug #7509 - smb_acl_to_posix: ACL is invalid for set (Invalid argument)
    
    Remove the code I added for bug "6878 - Cannot change ACL's inherit flag". It is incorrect
    and causes the POSIX ACL ACL_USER_OBJ duplication.

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

Summary of changes:
 source3/smbd/posix_acls.c |  167 ++++++++++++++++++++------------------------
 1 files changed, 76 insertions(+), 91 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c
index da25a52..0d0b5da 100644
--- a/source3/smbd/posix_acls.c
+++ b/source3/smbd/posix_acls.c
@@ -1409,29 +1409,32 @@ static bool ensure_canon_entry_valid(connection_struct *conn, canon_ace **pp_ace
 		pace->unix_ug.uid = pst->st_ex_uid;
 		pace->trustee = *pfile_owner_sid;
 		pace->attr = ALLOW_ACE;
+		/* Start with existing permissions, principle of least
+		   surprises for the user. */
+		pace->perms = pst->st_ex_mode;
 
 		if (setting_acl) {
 			/* See if the owning user is in any of the other groups in
-			   the ACE. If so, OR in the permissions from that group. */
+			   the ACE, or if there's a matching user entry.
+			   If so, OR in the permissions from that entry. */
 
-			bool group_matched = False;
 			canon_ace *pace_iter;
 
 			for (pace_iter = *pp_ace; pace_iter; pace_iter = pace_iter->next) {
-				if (pace_iter->type == SMB_ACL_GROUP_OBJ || pace_iter->type == SMB_ACL_GROUP) {
+				if (pace_iter->type == SMB_ACL_USER &&
+						pace_iter->unix_ug.uid == pace->unix_ug.uid) {
+					pace->perms |= pace_iter->perms;
+				} else if (pace_iter->type == SMB_ACL_GROUP_OBJ || pace_iter->type == SMB_ACL_GROUP) {
 					if (uid_entry_in_group(conn, pace, pace_iter)) {
 						pace->perms |= pace_iter->perms;
-						group_matched = True;
 					}
 				}
 			}
 
-			/* If we only got an "everyone" perm, just use that. */
-			if (!group_matched) {
+			if (pace->perms == 0) {
+				/* If we only got an "everyone" perm, just use that. */
 				if (got_other)
 					pace->perms = pace_other->perms;
-				else
-					pace->perms = 0;
 			}
 
 			apply_default_perms(params, is_directory, pace, S_IRUSR);
@@ -1454,12 +1457,29 @@ static bool ensure_canon_entry_valid(connection_struct *conn, canon_ace **pp_ace
 		pace->unix_ug.uid = pst->st_ex_gid;
 		pace->trustee = *pfile_grp_sid;
 		pace->attr = ALLOW_ACE;
+		/* Start with existing permissions, principle of least
+		   surprises for the user. */
+		pace->perms = pst->st_ex_mode;
+
 		if (setting_acl) {
+			/* See if there's a matching group entry.
+			   If so, OR in the permissions from that entry. */
+
+			canon_ace *pace_iter;
+
+			for (pace_iter = *pp_ace; pace_iter; pace_iter = pace_iter->next) {
+				if (pace_iter->type == SMB_ACL_GROUP &&
+							pace_iter->unix_ug.gid == pace->unix_ug.gid) {
+					pace->perms |= pace_iter->perms;
+					break;
+				}
+			}
+
 			/* If we only got an "everyone" perm, just use that. */
-			if (got_other)
-				pace->perms = pace_other->perms;
-			else
-				pace->perms = 0;
+			if (pace->perms == 0) {
+				if (got_other)
+					pace->perms = pace_other->perms;
+			}
 			apply_default_perms(params, is_directory, pace, S_IRGRP);
 		} else {
 			pace->perms = unix_perms_to_acl_perms(pst->st_ex_mode, S_IRGRP, S_IWGRP, S_IXGRP);
@@ -1496,6 +1516,7 @@ static bool ensure_canon_entry_valid(connection_struct *conn, canon_ace **pp_ace
  Check if a POSIX ACL has the required SMB_ACL_USER_OBJ and SMB_ACL_GROUP_OBJ entries.
  If it does not have them, check if there are any entries where the trustee is the
  file owner or the owning group, and map these to SMB_ACL_USER_OBJ and SMB_ACL_GROUP_OBJ.
+ Note we must not do this to default directory ACLs.
 ****************************************************************************/
 
 static void check_owning_objs(canon_ace *ace, struct dom_sid *pfile_owner_sid, struct dom_sid *pfile_grp_sid)
@@ -1538,50 +1559,6 @@ static void check_owning_objs(canon_ace *ace, struct dom_sid *pfile_owner_sid, s
 }
 
 /****************************************************************************
- If an ACE entry is SMB_ACL_USER_OBJ and not CREATOR_OWNER, map to SMB_ACL_USER.
- If an ACE entry is SMB_ACL_GROUP_OBJ and not CREATOR_GROUP, map to SMB_ACL_GROUP
-****************************************************************************/
-
-static bool dup_owning_ace(canon_ace *dir_ace, canon_ace *ace)
-{
-	/* dir ace must be followings.
-	   SMB_ACL_USER_OBJ : trustee(CREATOR_OWNER) -> Posix ACL d:u::perm
-	   SMB_ACL_USER     : not trustee    -> Posix ACL u:user:perm
-	   SMB_ACL_USER_OBJ : trustee -> convert to SMB_ACL_USER : trustee
-	   Posix ACL u:trustee:perm
-
-	   SMB_ACL_GROUP_OBJ: trustee(CREATOR_GROUP) -> Posix ACL d:g::perm
-	   SMB_ACL_GROUP    : not trustee   -> Posix ACL g:group:perm
-	   SMB_ACL_GROUP_OBJ: trustee -> convert to SMB_ACL_GROUP : trustee
-	   Posix ACL g:trustee:perm
-	*/
-
-	if (ace->type == SMB_ACL_USER_OBJ &&
-			!(dom_sid_equal(&ace->trustee, &global_sid_Creator_Owner))) {
-		canon_ace *dup_ace = dup_canon_ace(ace);
-
-		if (dup_ace == NULL) {
-			return false;
-		}
-		dup_ace->type = SMB_ACL_USER;
-		DLIST_ADD_END(dir_ace, dup_ace, canon_ace *);
-	}
-
-	if (ace->type == SMB_ACL_GROUP_OBJ &&
-			!(dom_sid_equal(&ace->trustee, &global_sid_Creator_Group))) {
-		canon_ace *dup_ace = dup_canon_ace(ace);
-
-		if (dup_ace == NULL) {
-			return false;
-		}
-		dup_ace->type = SMB_ACL_GROUP;
-		DLIST_ADD_END(dir_ace, dup_ace, canon_ace *);
-	}
-
-	return true;
-}
-
-/****************************************************************************
  Unpack a struct security_descriptor into two canonical ace lists.
 ****************************************************************************/
 
@@ -1804,6 +1781,7 @@ static bool create_canon_ace_lists(files_struct *fsp,
 			if ((psa->flags & (SEC_ACE_FLAG_OBJECT_INHERIT|SEC_ACE_FLAG_CONTAINER_INHERIT)) ==
 				(SEC_ACE_FLAG_OBJECT_INHERIT|SEC_ACE_FLAG_CONTAINER_INHERIT)) {
 
+				canon_ace *current_dir_ace = current_ace;
 				DLIST_ADD_END(dir_ace, current_ace, canon_ace *);
 
 				/*
@@ -1832,34 +1810,6 @@ static bool create_canon_ace_lists(files_struct *fsp,
 				}
 
 				/*
-				 * We have a lossy mapping: directory ACE entries
-				 * CREATOR_OWNER ------\
-				 *     (map to)         +---> SMB_ACL_USER_OBJ
-				 * owning sid    ------/
-				 *
-				 * CREATOR_GROUP ------\
-				 *     (map to)         +---> SMB_ACL_GROUP_OBJ
-				 * primary group sid --/
-				 *
-				 * on set. And on read of a directory ACL
-				 *
-				 * SMB_ACL_USER_OBJ ----> CREATOR_OWNER
-				 * SMB_ACL_GROUP_OBJ ---> CREATOR_GROUP.
-				 *
-				 * Deal with this on set by duplicating
-				 * owning sid and primary group sid ACE
-				 * entries into the directory ACL.
-				 * Fix from Tsukasa Hamano <hamano at osstech.co.jp>.
-				 */
-
-				if (!dup_owning_ace(dir_ace, current_ace)) {
-					DEBUG(0,("create_canon_ace_lists: malloc fail !\n"));
-					free_canon_ace_list(file_ace);
-					free_canon_ace_list(dir_ace);
-					return false;
-				}
-
-				/*
 				 * If this is not an inherit only ACE we need to add a duplicate
 				 * to the file acl.
 				 */
@@ -1893,6 +1843,43 @@ static bool create_canon_ace_lists(files_struct *fsp,
 					 */
 					current_ace = NULL;
 				}
+
+				/*
+				 * current_ace is now either owned by file_ace
+				 * or is NULL. We can safely operate on current_dir_ace
+				 * to treat mapping for default acl entries differently
+				 * than access acl entries.
+				 */
+
+				if (current_dir_ace->owner_type == UID_ACE) {
+					/*
+					 * We already decided above this is a uid,
+					 * for default acls ace's only CREATOR_OWNER
+					 * maps to ACL_USER_OBJ. All other uid
+					 * ace's are ACL_USER.
+					 */
+					if (dom_sid_equal(&current_dir_ace->trustee,
+							&global_sid_Creator_Owner)) {
+						current_dir_ace->type = SMB_ACL_USER_OBJ;
+					} else {
+						current_dir_ace->type = SMB_ACL_USER;
+					}
+				}
+
+				if (current_dir_ace->owner_type == GID_ACE) {
+					/*
+					 * We already decided above this is a gid,
+					 * for default acls ace's only CREATOR_GROUP
+					 * maps to ACL_GROUP_OBJ. All other uid
+					 * ace's are ACL_GROUP.
+					 */
+					if (dom_sid_equal(&current_dir_ace->trustee,
+							&global_sid_Creator_Group)) {
+						current_dir_ace->type = SMB_ACL_GROUP_OBJ;
+					} else {
+						current_dir_ace->type = SMB_ACL_GROUP;
+					}
+				}
 			}
 		}
 
@@ -1954,17 +1941,15 @@ static bool create_canon_ace_lists(files_struct *fsp,
 		dir_ace = NULL;
 	} else {
 		/*
-		 * Check if we have SMB_ACL_USER_OBJ and SMB_ACL_GROUP_OBJ entries in each
-		 * ACL. If we don't have them, check if any SMB_ACL_USER/SMB_ACL_GROUP
-		 * entries can be converted to *_OBJ. Usually we will already have these
-		 * entries in the Default ACL, and the Access ACL will not have them.
+		 * Check if we have SMB_ACL_USER_OBJ and SMB_ACL_GROUP_OBJ entries in
+		 * the file ACL. If we don't have them, check if any SMB_ACL_USER/SMB_ACL_GROUP
+		 * entries can be converted to *_OBJ. Don't do this for the default
+		 * ACL, we will create them separately for this if needed inside
+		 * ensure_canon_entry_valid().
 		 */
 		if (file_ace) {
 			check_owning_objs(file_ace, pfile_owner_sid, pfile_grp_sid);
 		}
-		if (dir_ace) {
-			check_owning_objs(dir_ace, pfile_owner_sid, pfile_grp_sid);
-		}
 	}
 
 	*ppfile_ace = file_ace;


-- 
Samba Shared Repository


More information about the samba-cvs mailing list