[SCM] Samba Shared Repository - branch master updated

Andrew Bartlett abartlet at samba.org
Wed May 16 21:09:02 MDT 2012


The branch, master has been updated
       via  6bafb4a s3-smbd: Avoid creating a UID ACL entry for SIDs that are mapped as ID_TYPE_BOTH The GID ACL entry is what will be mapped in most cases, and so is sufficient.
       via  f38638d s3-smbd: Consider a group with the same SID as sufficient duplication
       via  5b1c422 s3-smbd: Handle ID_TYPE_BOTH by mapping to both a group ACL entry and file ownership This will allow groups, such as domain administrators, to own files while correctly handling the rest of the ACL permissions.
       via  367a644 We need to split things up into a new helper function add_current_ace_to_acl() in order for there to be more posix ACL elements than NT ACL elements (so a group SID can own a file, but also get the group permissions that will be honoured)
       via  173f818 This covers a case where an ID_TYPE_BOTH mapping creates group permissions, but must own the file. Based on an original patch by Andrew Bartlett.
       via  5910647 s3-smbd: Do not merge UID ACE values with GID ACE values for posix ACL
      from  70be41c s3:onefs: remove all onefs related code as it not maintained anymore

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


- Log -----------------------------------------------------------------
commit 6bafb4ac25989fd5d637db0da4afab5ae36bad1c
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed May 16 13:07:17 2012 -0700

    s3-smbd: Avoid creating a UID ACL entry for SIDs that are mapped as ID_TYPE_BOTH The GID ACL entry is what will be mapped in most cases, and so is sufficient.
    
    Andrew Bartlett
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    
    Autobuild-User: Andrew Bartlett <abartlet at samba.org>
    Autobuild-Date: Thu May 17 05:08:44 CEST 2012 on sn-devel-104

commit f38638d4511814e2b541665df2f56c7ce357682f
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Thu May 10 11:05:41 2012 +1000

    s3-smbd: Consider a group with the same SID as sufficient duplication
    
    This code is to ensure that the user does not loose rights when their file
    ownership is taken away.  If the owner (an IDMAP_BOTH SID) appears as a group
    then a duplicate user is not required.
    
    Signed-off-by: Jeremy Allison <jra at samba.org>

commit 5b1c42228b8badbc7e7a4446c33f590bd1257f1f
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Tue May 15 12:33:18 2012 -0700

    s3-smbd: Handle ID_TYPE_BOTH by mapping to both a group ACL entry and file ownership This will allow groups, such as domain administrators, to own files while correctly handling the rest of the ACL permissions.
    
    Andrew Bartlett
    
    Signed-off-by: Jeremy Allison <jra at samba.org>

commit 367a644c4d91531faf8b2ce9a167fc196da12422
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Mon May 14 17:11:09 2012 -0700

    We need to split things up into a new helper function add_current_ace_to_acl() in order for there to be more posix ACL elements than NT ACL elements (so a group SID can own a file, but also get the group permissions that will be honoured)
    
    Andrew Bartlett
    
    Slightly modified by Jeremy to reduce diff size.
    
    Signed-off-by: Jeremy Allison <jra at samba.org>

commit 173f818a294d89cc97ba22856c334c451772fbe5
Author: Jeremy Allison <jra at samba.org>
Date:   Mon May 14 12:34:39 2012 -0700

    This covers a case where an ID_TYPE_BOTH mapping creates group permissions, but must own the file. Based on an original patch by Andrew Bartlett.

commit 59106473d37044adf5f1edde24221e1f70f15972
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed May 9 12:11:45 2012 +1000

    s3-smbd: Do not merge UID ACE values with GID ACE values for posix ACL
    
    This might happen when we get a SID mapped to IDMAP_BOTH.
    
    Andrew Bartlett
    
    Signed-off-by: Jeremy Allison <jra at samba.org>

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

Summary of changes:
 source3/smbd/posix_acls.c |  534 ++++++++++++++++++++++++++++-----------------
 1 files changed, 334 insertions(+), 200 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c
index bbf0eae..e2571ff 100644
--- a/source3/smbd/posix_acls.c
+++ b/source3/smbd/posix_acls.c
@@ -26,6 +26,7 @@
 #include "trans2.h"
 #include "passdb/lookup_sid.h"
 #include "auth.h"
+#include "../librpc/gen_ndr/idmap.h"
 
 extern const struct generic_mapping file_generic_mapping;
 
@@ -949,15 +950,21 @@ static void merge_aces( canon_ace **pp_list_head, bool dir_acl)
 
 			/* For file ACLs we can merge if the SIDs and ALLOW/DENY
 			 * types are the same. For directory acls we must also
-			 * ensure the POSIX ACL types are the same. */
+			 * ensure the POSIX ACL types are the same.
+			 *
+			 * For the IDMAP_BOTH case, we must not merge
+			 * the UID and GID ACE values for same SID
+			 */
 
 			if (!dir_acl) {
 				can_merge = (dom_sid_equal(&curr_ace->trustee, &curr_ace_outer->trustee) &&
-						(curr_ace->attr == curr_ace_outer->attr));
+					     curr_ace->owner_type == curr_ace_outer->owner_type &&
+					     (curr_ace->attr == curr_ace_outer->attr));
 			} else {
 				can_merge = (dom_sid_equal(&curr_ace->trustee, &curr_ace_outer->trustee) &&
-						(curr_ace->type == curr_ace_outer->type) &&
-						(curr_ace->attr == curr_ace_outer->attr));
+					     curr_ace->owner_type == curr_ace_outer->owner_type &&
+					     (curr_ace->type == curr_ace_outer->type) &&
+					     (curr_ace->attr == curr_ace_outer->attr));
 			}
 
 			if (can_merge) {
@@ -1005,7 +1012,8 @@ static void merge_aces( canon_ace **pp_list_head, bool dir_acl)
 			 */
 
 			if (dom_sid_equal(&curr_ace->trustee, &curr_ace_outer->trustee) &&
-				(curr_ace_outer->attr == DENY_ACE) && (curr_ace->attr == ALLOW_ACE)) {
+			    (curr_ace->owner_type == curr_ace_outer->owner_type) &&
+			    (curr_ace_outer->attr == DENY_ACE) && (curr_ace->attr == ALLOW_ACE)) {
 
 				if( DEBUGLVL( 10 )) {
 					dbgtext("merge_aces: Masking ACE's\n");
@@ -1413,7 +1421,8 @@ static bool ensure_canon_entry_valid(connection_struct *conn, canon_ace **pp_ace
 
 		if (setting_acl) {
 			/* See if the owning user is in any of the other groups in
-			   the ACE, or if there's a matching user entry.
+			   the ACE, or if there's a matching user entry (by uid
+			   or in the case of ID_TYPE_BOTH by SID).
 			   If so, OR in the permissions from that entry. */
 
 			canon_ace *pace_iter;
@@ -1423,7 +1432,9 @@ static bool ensure_canon_entry_valid(connection_struct *conn, canon_ace **pp_ace
 						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)) {
+					if (dom_sid_equal(&pace->trustee, &pace_iter->trustee)) {
+						pace->perms |= pace_iter->perms;
+					} else if (uid_entry_in_group(conn, pace, pace_iter)) {
 						pace->perms |= pace_iter->perms;
 					}
 				}
@@ -1514,9 +1525,47 @@ static bool ensure_canon_entry_valid(connection_struct *conn, canon_ace **pp_ace
 					pace->unix_ug.gid == pace_user->unix_ug.gid) {
 				/* Already got one. */
 				got_duplicate_group = true;
+			} else if ((pace->type == SMB_ACL_GROUP)
+				   && (dom_sid_equal(&pace->trustee, &pace_user->trustee))) {
+				/* If the SID owning the file appears
+				 * in a group entry, then we have
+				 * enough duplication, they will still
+				 * have access */
+				got_duplicate_user = true;
 			}
 		}
 
+		/* If the SID is equal for the user and group that we need
+		   to add the duplicate for, add only the group */
+		if (!got_duplicate_user && !got_duplicate_group
+				&& dom_sid_equal(&pace_group->trustee,
+						&pace_user->trustee)) {
+			/* Add a duplicate SMB_ACL_GROUP entry, this
+			 * will cover the owning SID as well, as it
+			 * will always be mapped to both a uid and
+			 * gid. */
+
+			if ((pace = talloc(talloc_tos(), canon_ace)) == NULL) {
+				DEBUG(0,("ensure_canon_entry_valid: talloc fail.\n"));
+				return false;
+			}
+
+			ZERO_STRUCTP(pace);
+			pace->type = SMB_ACL_GROUP;;
+			pace->owner_type = GID_ACE;
+			pace->unix_ug.gid = pace_group->unix_ug.gid;
+			pace->trustee = pace_group->trustee;
+			pace->attr = pace_group->attr;
+			pace->perms = pace_group->perms;
+
+			DLIST_ADD(*pp_ace, pace);
+
+			/* We're done here, make sure the
+			   statements below are not executed. */
+			got_duplicate_user = true;
+			got_duplicate_group = true;
+		}
+
 		if (!got_duplicate_user) {
 			/* Add a duplicate SMB_ACL_USER entry. */
 			if ((pace = talloc(talloc_tos(), canon_ace)) == NULL) {
@@ -1533,6 +1582,8 @@ static bool ensure_canon_entry_valid(connection_struct *conn, canon_ace **pp_ace
 			pace->perms = pace_user->perms;
 
 			DLIST_ADD(*pp_ace, pace);
+
+			got_duplicate_user = true;
 		}
 
 		if (!got_duplicate_group) {
@@ -1551,6 +1602,8 @@ static bool ensure_canon_entry_valid(connection_struct *conn, canon_ace **pp_ace
 			pace->perms = pace_group->perms;
 
 			DLIST_ADD(*pp_ace, pace);
+
+			got_duplicate_group = true;
 		}
 
 	}
@@ -1604,6 +1657,181 @@ static void check_owning_objs(canon_ace *ace, struct dom_sid *pfile_owner_sid, s
 		DEBUG(10,("check_owning_objs: ACL is missing an owning group entry.\n"));
 }
 
+static bool add_current_ace_to_acl(files_struct *fsp, struct security_ace *psa,
+				   canon_ace **file_ace, canon_ace **dir_ace,
+				   bool *got_file_allow, bool *got_dir_allow,
+				   bool *all_aces_are_inherit_only,
+				   canon_ace *current_ace)
+{
+
+	/*
+	 * Map the given NT permissions into a UNIX mode_t containing only
+	 * S_I(R|W|X)USR bits.
+	 */
+
+	current_ace->perms |= map_nt_perms( &psa->access_mask, S_IRUSR);
+	current_ace->attr = (psa->type == SEC_ACE_TYPE_ACCESS_ALLOWED) ? ALLOW_ACE : DENY_ACE;
+
+	/* Store the ace_flag. */
+	current_ace->ace_flags = psa->flags;
+
+	/*
+	 * Now add the created ace to either the file list, the directory
+	 * list, or both. We *MUST* preserve the order here (hence we use
+	 * DLIST_ADD_END) as NT ACLs are order dependent.
+	 */
+
+	if (fsp->is_directory) {
+
+		/*
+		 * We can only add to the default POSIX ACE list if the ACE is
+		 * designed to be inherited by both files and directories.
+		 */
+
+		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 *);
+
+			/*
+			 * Note if this was an allow ace. We can't process
+			 * any further deny ace's after this.
+			 */
+
+			if (current_ace->attr == ALLOW_ACE)
+				*got_dir_allow = True;
+
+			if ((current_ace->attr == DENY_ACE) && *got_dir_allow) {
+				DEBUG(0,("add_current_ace_to_acl: "
+					 "malformed ACL in "
+					 "inheritable ACL! Deny entry "
+					 "after Allow entry. Failing "
+					 "to set on file %s.\n",
+					 fsp_str_dbg(fsp)));
+				return False;
+			}
+
+			if( DEBUGLVL( 10 )) {
+				dbgtext("add_current_ace_to_acl: adding dir ACL:\n");
+				print_canon_ace( current_ace, 0);
+			}
+
+			/*
+			 * If this is not an inherit only ACE we need to add a duplicate
+			 * to the file acl.
+			 */
+
+			if (!(psa->flags & SEC_ACE_FLAG_INHERIT_ONLY)) {
+				canon_ace *dup_ace = dup_canon_ace(current_ace);
+
+				if (!dup_ace) {
+					DEBUG(0,("add_current_ace_to_acl: malloc fail !\n"));
+					return False;
+				}
+
+				/*
+				 * We must not free current_ace here as its
+				 * pointer is now owned by the dir_ace list.
+				 */
+				current_ace = dup_ace;
+				/* We've essentially split this ace into two,
+				 * and added the ace with inheritance request
+				 * bits to the directory ACL. Drop those bits for
+				 * the ACE we're adding to the file list. */
+				current_ace->ace_flags &= ~(SEC_ACE_FLAG_OBJECT_INHERIT|
+							    SEC_ACE_FLAG_CONTAINER_INHERIT|
+							    SEC_ACE_FLAG_INHERIT_ONLY);
+			} else {
+				/*
+				 * We must not free current_ace here as its
+				 * pointer is now owned by the dir_ace list.
+				 */
+				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;
+				}
+			}
+		}
+	}
+
+	/*
+	 * Only add to the file ACL if not inherit only.
+	 */
+
+	if (current_ace && !(psa->flags & SEC_ACE_FLAG_INHERIT_ONLY)) {
+		DLIST_ADD_END(*file_ace, current_ace, canon_ace *);
+
+		/*
+		 * Note if this was an allow ace. We can't process
+		 * any further deny ace's after this.
+		 */
+
+		if (current_ace->attr == ALLOW_ACE)
+			*got_file_allow = True;
+
+		if ((current_ace->attr == DENY_ACE) && got_file_allow) {
+			DEBUG(0,("add_current_ace_to_acl: malformed "
+				 "ACL in file ACL ! Deny entry after "
+				 "Allow entry. Failing to set on file "
+				 "%s.\n", fsp_str_dbg(fsp)));
+			return False;
+		}
+
+		if( DEBUGLVL( 10 )) {
+			dbgtext("add_current_ace_to_acl: adding file ACL:\n");
+			print_canon_ace( current_ace, 0);
+		}
+		*all_aces_are_inherit_only = False;
+		/*
+		 * We must not free current_ace here as its
+		 * pointer is now owned by the file_ace list.
+		 */
+		current_ace = NULL;
+	}
+
+	/*
+	 * Free if ACE was not added.
+	 */
+
+	TALLOC_FREE(current_ace);
+	return true;
+}
+
 /****************************************************************************
  Unpack a struct security_descriptor into two canonical ace lists.
 ****************************************************************************/
@@ -1752,225 +1980,131 @@ static bool create_canon_ace_lists(files_struct *fsp,
 			 */
 			psa->flags |= SEC_ACE_FLAG_INHERIT_ONLY;
 
-		} else if (sid_to_uid( &current_ace->trustee, &current_ace->unix_ug.uid)) {
-			current_ace->owner_type = UID_ACE;
-			/* If it's the owning user, this is a user_obj, not
-			 * a user. */
-			if (current_ace->unix_ug.uid == pst->st_ex_uid) {
-				current_ace->type = SMB_ACL_USER_OBJ;
-			} else {
-				current_ace->type = SMB_ACL_USER;
-			}
-		} else if (sid_to_gid( &current_ace->trustee, &current_ace->unix_ug.gid)) {
-			current_ace->owner_type = GID_ACE;
-			/* If it's the primary group, this is a group_obj, not
-			 * a group. */
-			if (current_ace->unix_ug.gid == pst->st_ex_gid) {
-				current_ace->type = SMB_ACL_GROUP_OBJ;
-			} else {
-				current_ace->type = SMB_ACL_GROUP;
-			}
 		} else {
-			/*
-			 * Silently ignore map failures in non-mappable SIDs (NT Authority, BUILTIN etc).
-			 */
-
-			if (non_mappable_sid(&psa->trustee)) {
-				DEBUG(10, ("create_canon_ace_lists: ignoring "
-					   "non-mappable SID %s\n",
-					   sid_string_dbg(&psa->trustee)));
-				TALLOC_FREE(current_ace);
-				continue;
-			}
+			struct unixid unixid;
 
-			if (lp_force_unknown_acl_user(SNUM(fsp->conn))) {
-				DEBUG(10, ("create_canon_ace_lists: ignoring "
-					"unknown or foreign SID %s\n",
-					sid_string_dbg(&psa->trustee)));
+			if (!sids_to_unixids(&current_ace->trustee, 1, &unixid)) {
+				free_canon_ace_list(file_ace);
+				free_canon_ace_list(dir_ace);
 				TALLOC_FREE(current_ace);
-				continue;
+				DEBUG(0, ("create_canon_ace_lists: sids_to_unixids "
+					"failed for %s (allocation failure)\n",
+					sid_string_dbg(&current_ace->trustee)));
+				return false;
 			}
 
-			free_canon_ace_list(file_ace);
-			free_canon_ace_list(dir_ace);
-			DEBUG(0, ("create_canon_ace_lists: unable to map SID "
-				  "%s to uid or gid.\n",
-				  sid_string_dbg(&current_ace->trustee)));
-			TALLOC_FREE(current_ace);
-			return False;
-		}
-
-		/*
-		 * Map the given NT permissions into a UNIX mode_t containing only
-		 * S_I(R|W|X)USR bits.
-		 */
-
-		current_ace->perms |= map_nt_perms( &psa->access_mask, S_IRUSR);
-		current_ace->attr = (psa->type == SEC_ACE_TYPE_ACCESS_ALLOWED) ? ALLOW_ACE : DENY_ACE;
-
-		/* Store the ace_flag. */
-		current_ace->ace_flags = psa->flags;
-
-		/*
-		 * Now add the created ace to either the file list, the directory
-		 * list, or both. We *MUST* preserve the order here (hence we use
-		 * DLIST_ADD_END) as NT ACLs are order dependent.
-		 */
-
-		if (fsp->is_directory) {
-
-			/*
-			 * We can only add to the default POSIX ACE list if the ACE is
-			 * designed to be inherited by both files and directories.
-			 */
-
-			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 *);
-
-				/*
-				 * Note if this was an allow ace. We can't process
-				 * any further deny ace's after this.
-				 */
-
-				if (current_ace->attr == ALLOW_ACE)
-					got_dir_allow = True;
-
-				if ((current_ace->attr == DENY_ACE) && got_dir_allow) {
-					DEBUG(0,("create_canon_ace_lists: "
-						 "malformed ACL in "
-						 "inheritable ACL! Deny entry "
-						 "after Allow entry. Failing "
-						 "to set on file %s.\n",
-						 fsp_str_dbg(fsp)));
-					free_canon_ace_list(file_ace);
-					free_canon_ace_list(dir_ace);
-					return False;
-				}	
-
-				if( DEBUGLVL( 10 )) {
-					dbgtext("create_canon_ace_lists: adding dir ACL:\n");
-					print_canon_ace( current_ace, 0);
-				}
-
-				/*
-				 * If this is not an inherit only ACE we need to add a duplicate
-				 * to the file acl.
-				 */
-
-				if (!(psa->flags & SEC_ACE_FLAG_INHERIT_ONLY)) {
-					canon_ace *dup_ace = dup_canon_ace(current_ace);
+			if (unixid.type == ID_TYPE_BOTH) {
+				/* If it's the owning user, this is a
+				 * user_obj, not a user.  This way, we
+				 * get a valid ACL for groups that own
+				 * files, without putting user ACL
+				 * entries in for groups otherwise */
+				if (unixid.id == pst->st_ex_uid) {
+					current_ace->owner_type = UID_ACE;
+					current_ace->unix_ug.uid = unixid.id;
+					current_ace->type = SMB_ACL_USER_OBJ;
+
+					/* Add the user object to the posix ACL,
+					   and proceed to the group mapping
+					   below. This handles the talloc_free
+					   of current_ace if not added for some
+					   reason */
+					if (!add_current_ace_to_acl(fsp,
+							psa,
+							&file_ace,
+							&dir_ace,
+							&got_file_allow,
+							&got_dir_allow,
+							&all_aces_are_inherit_only,
+							current_ace)) {
+						free_canon_ace_list(file_ace);
+						free_canon_ace_list(dir_ace);
+						return false;
+					}
 
-					if (!dup_ace) {
-						DEBUG(0,("create_canon_ace_lists: malloc fail !\n"));
+					if ((current_ace = talloc(talloc_tos(),
+							canon_ace)) == NULL) {
 						free_canon_ace_list(file_ace);
 						free_canon_ace_list(dir_ace);
+						DEBUG(0,("create_canon_ace_lists: "
+							"malloc fail.\n"));
 						return False;
 					}
 
-					/*
-					 * We must not free current_ace here as its
-					 * pointer is now owned by the dir_ace list.
-					 */
-					current_ace = dup_ace;
-					/* We've essentially split this ace into two,
-					 * and added the ace with inheritance request
-					 * bits to the directory ACL. Drop those bits for
-					 * the ACE we're adding to the file list. */
-					current_ace->ace_flags &= ~(SEC_ACE_FLAG_OBJECT_INHERIT|
-								SEC_ACE_FLAG_CONTAINER_INHERIT|
-								SEC_ACE_FLAG_INHERIT_ONLY);
+					ZERO_STRUCTP(current_ace);
+				}
+
+				sid_copy(&current_ace->trustee, &psa->trustee);
+
+				current_ace->unix_ug.gid = unixid.id;
+				current_ace->owner_type = GID_ACE;
+				/* If it's the primary group, this is a
+				   group_obj, not a group. */


-- 
Samba Shared Repository


More information about the samba-cvs mailing list