From 8b80fb0fc0aa294203607492d29b314567b1a9c9 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 2 Sep 2011 14:59:31 -0700 Subject: [PATCH 1/5] 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. --- source3/smbd/posix_acls.c | 72 --------------------------------------------- 1 files changed, 0 insertions(+), 72 deletions(-) diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c index 714a4d3..6f51e31 100644 --- a/source3/smbd/posix_acls.c +++ b/source3/smbd/posix_acls.c @@ -1526,50 +1526,6 @@ static void check_owning_objs(canon_ace *ace, DOM_SID *pfile_owner_sid, DOM_SID } /**************************************************************************** - 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 && - !(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 && - !(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 SEC_DESC into two canonical ace lists. ****************************************************************************/ @@ -1820,34 +1776,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 . - */ - - 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. */ -- 1.7.3.1 From 206d1319167d7d64fa4cfc54a63cddaa180f430c Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 2 Sep 2011 15:07:48 -0700 Subject: [PATCH 2/5] 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. --- source3/smbd/posix_acls.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 files changed, 38 insertions(+), 0 deletions(-) diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c index 6f51e31..ec16bf2 100644 --- a/source3/smbd/posix_acls.c +++ b/source3/smbd/posix_acls.c @@ -1748,6 +1748,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 *); /* @@ -1809,6 +1810,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 (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 (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; + } + } } } -- 1.7.3.1 From 03d8acb1496fe8da7effea0207a1f3b782371b1f Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 2 Sep 2011 15:08:42 -0700 Subject: [PATCH 3/5] 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(). --- source3/smbd/posix_acls.c | 13 ++++++------- 1 files changed, 6 insertions(+), 7 deletions(-) diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c index ec16bf2..17860bd 100644 --- a/source3/smbd/posix_acls.c +++ b/source3/smbd/posix_acls.c @@ -1484,6 +1484,7 @@ static bool ensure_canon_entry_valid(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, DOM_SID *pfile_owner_sid, DOM_SID *pfile_grp_sid) @@ -1908,17 +1909,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; -- 1.7.3.1 From 5e57f680f912de774154a1d66394befc95bf5055 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 2 Sep 2011 15:11:46 -0700 Subject: [PATCH 4/5] 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. --- source3/smbd/posix_acls.c | 19 +++++++++++-------- 1 files changed, 11 insertions(+), 8 deletions(-) diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c index 17860bd..4943e5b 100644 --- a/source3/smbd/posix_acls.c +++ b/source3/smbd/posix_acls.c @@ -1397,29 +1397,32 @@ static bool ensure_canon_entry_valid(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(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); -- 1.7.3.1 From 3f87bb7001440bba74f977535d35aaf82da0a151 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 2 Sep 2011 15:12:48 -0700 Subject: [PATCH 5/5] 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. --- source3/smbd/posix_acls.c | 25 +++++++++++++++++++++---- 1 files changed, 21 insertions(+), 4 deletions(-) diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c index 4943e5b..fffb92c 100644 --- a/source3/smbd/posix_acls.c +++ b/source3/smbd/posix_acls.c @@ -1445,12 +1445,29 @@ static bool ensure_canon_entry_valid(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); -- 1.7.3.1