[PATCH] Patches required for POSIX ACL support of GPOs
Jeremy Allison
jra at samba.org
Wed May 16 14:40:43 MDT 2012
On Thu, May 10, 2012 at 11:38:48AM +1000, Andrew Bartlett wrote:
> These patches are in my master-devel branch, and are needed for GPO
> support to create the correct POSIX ACL. I would very much appreciate
> review, so we can consider enabling s3fs by default, and making the 4.0
> Beta release.
>
> https://git.samba.org/?p=abartlet/samba.git/.git;a=shortlog;h=refs/heads/master-devel
As I haven't figured out creating remote branches (I'm gonna ask Junio :-)
here is the reviewed patchset for Andrew to look over.
I did make a few changes, mostly to make the patches easier to
digest (for me at least).
I haven't added 2 of the 8 of Andrew's patches, they are the NFSv4
change, and the optimization for tokens:
s3-nfs4acls: Remove lookup_sid and sidmap from NFSv4 ACL mapping and check gid first
s3-smbd: Create a shortcut for building the token of a user by SID for posix_acls
not because I think they're bad or wrong, but they look like
optimizations that aren't needed to make the basic functionality
work. I'm planning to go through them a little later (correctly
this time :-) and push them separately, if that's ok.
Andrew, let me know if this set is ok by you and let's get it in !
Cheers,
Jeremy.
-------------- next part --------------
>From be4d0022ad24d9df79a04b8f9d59d108bb6c682f Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 9 May 2012 12:11:45 +1000
Subject: [PATCH 1/6] 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>
---
source3/smbd/posix_acls.c | 17 ++++++++++++-----
1 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c
index bbf0eae..a313190 100644
--- a/source3/smbd/posix_acls.c
+++ b/source3/smbd/posix_acls.c
@@ -949,15 +949,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 +1011,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");
--
1.7.7.3
>From 3de7799db12cd78709fe47db9387c1c1f2bdf91f Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Mon, 14 May 2012 12:34:39 -0700
Subject: [PATCH 2/6] 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.
---
source3/smbd/posix_acls.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c
index a313190..d58c7c0 100644
--- a/source3/smbd/posix_acls.c
+++ b/source3/smbd/posix_acls.c
@@ -1420,7 +1420,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;
@@ -1430,7 +1431,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;
}
}
--
1.7.7.3
>From e61b9514ba3169d3c3b397f23fb9054343738f17 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Mon, 14 May 2012 17:11:09 -0700
Subject: [PATCH 3/6] 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>
---
source3/smbd/posix_acls.c | 356 +++++++++++++++++++++++----------------------
1 files changed, 184 insertions(+), 172 deletions(-)
diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c
index d58c7c0..a833fbf 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;
@@ -1614,6 +1615,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(¤t_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(¤t_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.
****************************************************************************/
@@ -1807,180 +1983,16 @@ static bool create_canon_ace_lists(files_struct *fsp,
"%s to uid or gid.\n",
sid_string_dbg(¤t_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 (!dup_ace) {
- DEBUG(0,("create_canon_ace_lists: malloc fail !\n"));
- free_canon_ace_list(file_ace);
- free_canon_ace_list(dir_ace);
- 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(¤t_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(¤t_dir_ace->trustee,
- &global_sid_Creator_Group)) {
- current_dir_ace->type = SMB_ACL_GROUP_OBJ;
- } else {
- current_dir_ace->type = SMB_ACL_GROUP;
- }
- }
- }
+ return false;
}
-
- /*
- * 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,("create_canon_ace_lists: malformed "
- "ACL in file 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 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;
+ /* 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;
}
-
- /*
- * Free if ACE was not added.
- */
-
- TALLOC_FREE(current_ace);
}
if (fsp->is_directory && all_aces_are_inherit_only) {
--
1.7.7.3
>From 1af071f5dd86bffcc42f4ca0f0e315eebcf0c796 Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Tue, 15 May 2012 12:33:18 -0700
Subject: [PATCH 4/6] 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>
---
source3/smbd/posix_acls.c | 148 +++++++++++++++++++++++++++++++++------------
1 files changed, 109 insertions(+), 39 deletions(-)
diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c
index a833fbf..6e97dcf 100644
--- a/source3/smbd/posix_acls.c
+++ b/source3/smbd/posix_acls.c
@@ -1938,53 +1938,123 @@ static bool create_canon_ace_lists(files_struct *fsp,
*/
psa->flags |= SEC_ACE_FLAG_INHERIT_ONLY;
- } else if (sid_to_uid( ¤t_ace->trustee, ¤t_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( ¤t_ace->trustee, ¤t_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).
- */
+ struct unixid unixid;
- if (non_mappable_sid(&psa->trustee)) {
- DEBUG(10, ("create_canon_ace_lists: ignoring "
- "non-mappable SID %s\n",
- sid_string_dbg(&psa->trustee)));
+ if (!sids_to_unixids(¤t_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(¤t_ace->trustee)));
+ return false;
}
- 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 (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 ((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;
+ }
+
+ ZERO_STRUCTP(current_ace);
+ }
+
+ sid_copy(¤t_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. */
+ 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 if (unixid.type == ID_TYPE_UID) {
+ current_ace->owner_type = UID_ACE;
+ current_ace->unix_ug.uid = unixid.id;
+ /* 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 (unixid.type == ID_TYPE_GID) {
+ 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. */
+ 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;
+ }
+
+ 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)));
+ TALLOC_FREE(current_ace);
+ continue;
+ }
+
+ 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(¤t_ace->trustee)));
TALLOC_FREE(current_ace);
- continue;
+ 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(¤t_ace->trustee)));
- TALLOC_FREE(current_ace);
- return false;
}
+
/* 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,
--
1.7.7.3
>From 00b765b34be835b4dd695233d66aa5f7d2cf717c Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Thu, 10 May 2012 11:05:41 +1000
Subject: [PATCH 5/6] 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>
---
source3/smbd/posix_acls.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c
index 6e97dcf..99e9156 100644
--- a/source3/smbd/posix_acls.c
+++ b/source3/smbd/posix_acls.c
@@ -1525,6 +1525,13 @@ 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;
}
}
--
1.7.7.3
>From a98b94eeadcd85b00f7a19bd78d1c0a19fb7d62c Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 16 May 2012 13:07:17 -0700
Subject: [PATCH 6/6] 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>
---
source3/smbd/posix_acls.c | 35 +++++++++++++++++++++++++++++++++++
1 files changed, 35 insertions(+), 0 deletions(-)
diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c
index 99e9156..e2571ff 100644
--- a/source3/smbd/posix_acls.c
+++ b/source3/smbd/posix_acls.c
@@ -1535,6 +1535,37 @@ static bool ensure_canon_entry_valid(connection_struct *conn, canon_ace **pp_ace
}
}
+ /* 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) {
@@ -1551,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) {
@@ -1569,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;
}
}
--
1.7.7.3
More information about the samba-technical
mailing list