From d6d181e4ec99dbf627991c34f44e3bf60b5e845b Mon Sep 17 00:00:00 2001 From: Michael Adam Date: Thu, 6 Dec 2012 10:38:11 +0100 Subject: [PATCH 1/7] Revert "Another fix needed for bug #9236 - ACL masks incorrectly applied when setting ACLs." This reverts commit ce8beb781f7456e53262bd331ab3fbb8a100356b. The patch will be picked again from master in the proper order to reduce the need for conflict resolution. Signed-off-by: Michael Adam --- source3/smbd/posix_acls.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c index 4e93fef..02a680e 100644 --- a/source3/smbd/posix_acls.c +++ b/source3/smbd/posix_acls.c @@ -1431,11 +1431,7 @@ static bool ensure_canon_entry_valid(connection_struct *conn, for (pace = *pp_ace; pace; pace = pace->next) { if (pace->type == SMB_ACL_USER_OBJ) { - if (setting_acl) { - /* - * Ensure we have default parameters for the - * user (owner) even on default ACLs. - */ + if (setting_acl && !is_default_acl) { apply_default_perms(params, is_directory, pace, S_IRUSR); } pace_user = pace; @@ -1518,11 +1514,9 @@ static bool ensure_canon_entry_valid(connection_struct *conn, pace->perms = pace_other->perms; } - /* - * Ensure we have default parameters for the - * user (owner) even on default ACLs. - */ - apply_default_perms(params, is_directory, pace, S_IRUSR); + if (!is_default_acl) { + apply_default_perms(params, is_directory, pace, S_IRUSR); + } } else { pace->perms = unix_perms_to_acl_perms(pst->st_ex_mode, S_IRUSR, S_IWUSR, S_IXUSR); } -- 1.7.9.5 From b2c5e2c4347c4b887869f60e5cf62640e86fa9f3 Mon Sep 17 00:00:00 2001 From: Michael Adam Date: Thu, 6 Dec 2012 10:38:40 +0100 Subject: [PATCH 2/7] Revert "Fix bug 9376 - ensure_canon_entry_valid generates duplicate SMB_ACL_GROUP, acl_valid fails." This reverts commit e122c7d24b10119c9ea4d65e0099ff1690394457. The patch will be picked again from master in the proper order to reduce the need for conflict resolution. Signed-off-by: Michael Adam --- source3/smbd/posix_acls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c index 02a680e..63ff587 100644 --- a/source3/smbd/posix_acls.c +++ b/source3/smbd/posix_acls.c @@ -1598,7 +1598,7 @@ static bool ensure_canon_entry_valid(connection_struct *conn, /* Already got one. */ got_duplicate_user = true; } else if (pace->type == SMB_ACL_GROUP && - pace->unix_ug.id == pace_group->unix_ug.id) { + pace->unix_ug.id == pace_user->unix_ug.id) { /* Already got one. */ got_duplicate_group = true; } else if ((pace->type == SMB_ACL_GROUP) -- 1.7.9.5 From 1a43bb486a1349a6aada456cf7a80f32f4582072 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 5 Oct 2012 15:48:07 -0700 Subject: [PATCH 3/7] Modify ensure_canon_entry_valid() into ensure_canon_entry_valid_on_set() - makes the logic clearer. (cherry picked from commit 47ebc8fbc93ee1eb9640d9ca30275fcfc3b50026) --- source3/smbd/posix_acls.c | 295 ++++++++++++++++++++++----------------------- 1 file changed, 141 insertions(+), 154 deletions(-) diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c index 63ff587..0b1f459 100644 --- a/source3/smbd/posix_acls.c +++ b/source3/smbd/posix_acls.c @@ -1404,34 +1404,34 @@ static bool ensure_canon_entry_valid_on_get(connection_struct *conn, } /**************************************************************************** - A well formed POSIX file or default ACL has at least 3 entries, a + A well formed POSIX file or default ACL has at least 3 entries, a SMB_ACL_USER_OBJ, SMB_ACL_GROUP_OBJ, SMB_ACL_OTHER_OBJ. In addition, the owner must always have at least read access. - When using this call on get_acl, the pst struct is valid and contains - the mode of the file. When using this call on set_acl, the pst struct has + When using this call on set_acl, the pst struct has been modified to have a mode containing the default for this file or directory type. ****************************************************************************/ -static bool ensure_canon_entry_valid(connection_struct *conn, +static bool ensure_canon_entry_valid_on_set(connection_struct *conn, canon_ace **pp_ace, bool is_default_acl, const struct share_params *params, const bool is_directory, const struct dom_sid *pfile_owner_sid, const struct dom_sid *pfile_grp_sid, - const SMB_STRUCT_STAT *pst, - bool setting_acl) + const SMB_STRUCT_STAT *pst) { canon_ace *pace; canon_ace *pace_user = NULL; canon_ace *pace_group = NULL; canon_ace *pace_other = NULL; + bool got_duplicate_user = false; + bool got_duplicate_group = false; for (pace = *pp_ace; pace; pace = pace->next) { if (pace->type == SMB_ACL_USER_OBJ) { - if (setting_acl && !is_default_acl) { + if (!is_default_acl) { apply_default_perms(params, is_directory, pace, S_IRUSR); } pace_user = pace; @@ -1442,7 +1442,7 @@ static bool ensure_canon_entry_valid(connection_struct *conn, * Ensure create mask/force create mode is respected on set. */ - if (setting_acl && !is_default_acl) { + if (!is_default_acl) { apply_default_perms(params, is_directory, pace, S_IRGRP); } pace_group = pace; @@ -1453,7 +1453,7 @@ static bool ensure_canon_entry_valid(connection_struct *conn, * Ensure create mask/force create mode is respected on set. */ - if (setting_acl && !is_default_acl) { + if (!is_default_acl) { apply_default_perms(params, is_directory, pace, S_IROTH); } pace_other = pace; @@ -1464,16 +1464,18 @@ static bool ensure_canon_entry_valid(connection_struct *conn, * Ensure create mask/force create mode is respected on set. */ - if (setting_acl && !is_default_acl) { + if (!is_default_acl) { apply_default_perms(params, is_directory, pace, S_IRGRP); } } } if (!pace_user) { + canon_ace *pace_iter; + if ((pace = talloc(talloc_tos(), canon_ace)) == NULL) { - DEBUG(0,("ensure_canon_entry_valid: malloc fail.\n")); - return False; + DEBUG(0,("talloc fail.\n")); + return false; } ZERO_STRUCTP(pace); @@ -1487,38 +1489,33 @@ static bool ensure_canon_entry_valid(connection_struct *conn, surprises for the user. */ pace->perms = unix_perms_to_acl_perms(pst->st_ex_mode, S_IRUSR, S_IWUSR, S_IXUSR); - 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 (by uid - or in the case of ID_TYPE_BOTH by SID). - If so, OR in the permissions from that entry. */ + /* See if the owning user is in any of the other groups in + 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; - for (pace_iter = *pp_ace; pace_iter; pace_iter = pace_iter->next) { - if (pace_iter->type == SMB_ACL_USER && - pace_iter->unix_ug.id == pace->unix_ug.id) { + for (pace_iter = *pp_ace; pace_iter; pace_iter = pace_iter->next) { + if (pace_iter->type == SMB_ACL_USER && + pace_iter->unix_ug.id == pace->unix_ug.id) { + pace->perms |= pace_iter->perms; + } else if (pace_iter->type == SMB_ACL_GROUP_OBJ || pace_iter->type == SMB_ACL_GROUP) { + 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; - } else if (pace_iter->type == SMB_ACL_GROUP_OBJ || pace_iter->type == SMB_ACL_GROUP) { - 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; - } } } + } - if (pace->perms == 0) { - /* If we only got an "everyone" perm, just use that. */ - if (pace_other) - pace->perms = pace_other->perms; - } + if (pace->perms == 0) { + /* If we only got an "everyone" perm, just use that. */ + if (pace_other) + pace->perms = pace_other->perms; + } - if (!is_default_acl) { - apply_default_perms(params, is_directory, pace, S_IRUSR); - } - } else { - pace->perms = unix_perms_to_acl_perms(pst->st_ex_mode, S_IRUSR, S_IWUSR, S_IXUSR); + if (!is_default_acl) { + apply_default_perms(params, is_directory, pace, S_IRUSR); } DLIST_ADD(*pp_ace, pace); @@ -1527,8 +1524,8 @@ static bool ensure_canon_entry_valid(connection_struct *conn, if (!pace_group) { if ((pace = talloc(talloc_tos(), canon_ace)) == NULL) { - DEBUG(0,("ensure_canon_entry_valid: malloc fail.\n")); - return False; + DEBUG(0,("talloc fail.\n")); + return false; } ZERO_STRUCTP(pace); @@ -1538,17 +1535,15 @@ static bool ensure_canon_entry_valid(connection_struct *conn, pace->unix_ug.id = pst->st_ex_gid; pace->trustee = *pfile_grp_sid; pace->attr = ALLOW_ACE; - if (setting_acl) { - /* If we only got an "everyone" perm, just use that. */ - if (pace_other) - pace->perms = pace_other->perms; - else - pace->perms = 0; - if (!is_default_acl) { - apply_default_perms(params, is_directory, pace, S_IRGRP); - } + + /* If we only got an "everyone" perm, just use that. */ + if (pace_other) { + pace->perms = pace_other->perms; } else { - pace->perms = unix_perms_to_acl_perms(pst->st_ex_mode, S_IRGRP, S_IWGRP, S_IXGRP); + pace->perms = 0; + } + if (!is_default_acl) { + apply_default_perms(params, is_directory, pace, S_IRGRP); } DLIST_ADD(*pp_ace, pace); @@ -1557,8 +1552,8 @@ static bool ensure_canon_entry_valid(connection_struct *conn, if (!pace_other) { if ((pace = talloc(talloc_tos(), canon_ace)) == NULL) { - DEBUG(0,("ensure_canon_entry_valid: malloc fail.\n")); - return False; + DEBUG(0,("talloc fail.\n")); + return false; } ZERO_STRUCTP(pace); @@ -1568,126 +1563,118 @@ static bool ensure_canon_entry_valid(connection_struct *conn, pace->unix_ug.id = -1; pace->trustee = global_sid_World; pace->attr = ALLOW_ACE; - if (setting_acl) { - pace->perms = 0; - if (!is_default_acl) { - apply_default_perms(params, is_directory, pace, S_IROTH); - } - } else - pace->perms = unix_perms_to_acl_perms(pst->st_ex_mode, S_IROTH, S_IWOTH, S_IXOTH); + pace->perms = 0; + if (!is_default_acl) { + apply_default_perms(params, is_directory, pace, S_IROTH); + } DLIST_ADD(*pp_ace, pace); pace_other = pace; } - if (setting_acl) { - /* Ensure when setting a POSIX ACL, that the uid for a - SMB_ACL_USER_OBJ ACE (the owner ACE entry) has a duplicate - permission entry as an SMB_ACL_USER, and a gid for a - SMB_ACL_GROUP_OBJ ACE (the primary group ACE entry) also has - a duplicate permission entry as an SMB_ACL_GROUP. If not, - then if the ownership or group ownership of this file or - directory gets changed, the user or group can lose their - access. */ - bool got_duplicate_user = false; - bool got_duplicate_group = false; - - for (pace = *pp_ace; pace; pace = pace->next) { - if (pace->type == SMB_ACL_USER && - pace->unix_ug.id == pace_user->unix_ug.id) { - /* Already got one. */ - got_duplicate_user = true; - } else if (pace->type == SMB_ACL_GROUP && - pace->unix_ug.id == pace_user->unix_ug.id) { - /* 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.type = ID_TYPE_GID; - pace->unix_ug.id = pace_group->unix_ug.id; - pace->trustee = pace_group->trustee; - pace->attr = pace_group->attr; - pace->perms = pace_group->perms; - - DLIST_ADD(*pp_ace, pace); + /* Ensure when setting a POSIX ACL, that the uid for a + SMB_ACL_USER_OBJ ACE (the owner ACE entry) has a duplicate + permission entry as an SMB_ACL_USER, and a gid for a + SMB_ACL_GROUP_OBJ ACE (the primary group ACE entry) also has + a duplicate permission entry as an SMB_ACL_GROUP. If not, + then if the ownership or group ownership of this file or + directory gets changed, the user or group can lose their + access. */ - /* We're done here, make sure the - statements below are not executed. */ + for (pace = *pp_ace; pace; pace = pace->next) { + if (pace->type == SMB_ACL_USER && + pace->unix_ug.id == pace_user->unix_ug.id) { + /* Already got one. */ got_duplicate_user = true; + } else if (pace->type == SMB_ACL_GROUP && + pace->unix_ug.id == pace_user->unix_ug.id) { + /* 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 (!got_duplicate_user) { - /* Add a duplicate SMB_ACL_USER entry. */ - if ((pace = talloc(talloc_tos(), canon_ace)) == NULL) { - DEBUG(0,("ensure_canon_entry_valid: talloc fail.\n")); - return false; - } + /* 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. */ - ZERO_STRUCTP(pace); - pace->type = SMB_ACL_USER;; - pace->owner_type = UID_ACE; - pace->unix_ug.type = ID_TYPE_UID; - pace->unix_ug.id = pace_user->unix_ug.id; - pace->trustee = pace_user->trustee; - pace->attr = pace_user->attr; - pace->perms = pace_user->perms; + if ((pace = talloc(talloc_tos(), canon_ace)) == NULL) { + DEBUG(0,("talloc fail.\n")); + return false; + } + + ZERO_STRUCTP(pace); + pace->type = SMB_ACL_GROUP;; + pace->owner_type = GID_ACE; + pace->unix_ug.type = ID_TYPE_GID; + pace->unix_ug.id = pace_group->unix_ug.id; + pace->trustee = pace_group->trustee; + pace->attr = pace_group->attr; + pace->perms = pace_group->perms; - DLIST_ADD(*pp_ace, pace); + DLIST_ADD(*pp_ace, pace); - got_duplicate_user = true; + /* 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) { + DEBUG(0,("talloc fail.\n")); + return false; } - if (!got_duplicate_group) { - /* Add a duplicate SMB_ACL_GROUP entry. */ - 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_USER;; + pace->owner_type = UID_ACE; + pace->unix_ug.type = ID_TYPE_UID; + pace->unix_ug.id = pace_user->unix_ug.id; + pace->trustee = pace_user->trustee; + pace->attr = pace_user->attr; + pace->perms = pace_user->perms; - ZERO_STRUCTP(pace); - pace->type = SMB_ACL_GROUP;; - pace->owner_type = GID_ACE; - pace->unix_ug.type = ID_TYPE_GID; - pace->unix_ug.id = pace_group->unix_ug.id; - pace->trustee = pace_group->trustee; - pace->attr = pace_group->attr; - pace->perms = pace_group->perms; + DLIST_ADD(*pp_ace, pace); - DLIST_ADD(*pp_ace, pace); + got_duplicate_user = true; + } - got_duplicate_group = true; + if (!got_duplicate_group) { + /* Add a duplicate SMB_ACL_GROUP entry. */ + if ((pace = talloc(talloc_tos(), canon_ace)) == NULL) { + DEBUG(0,("talloc fail.\n")); + return false; } + ZERO_STRUCTP(pace); + pace->type = SMB_ACL_GROUP;; + pace->owner_type = GID_ACE; + pace->unix_ug.type = ID_TYPE_GID; + pace->unix_ug.id = pace_group->unix_ug.id; + pace->trustee = pace_group->trustee; + pace->attr = pace_group->attr; + pace->perms = pace_group->perms; + + DLIST_ADD(*pp_ace, pace); + + got_duplicate_group = true; } - return True; + return true; } /**************************************************************************** @@ -2191,7 +2178,7 @@ static bool create_canon_ace_lists(files_struct *fsp, * 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(). + * ensure_canon_entry_valid_on_set(). */ if (file_ace) { check_owning_objs(file_ace, pfile_owner_sid, pfile_grp_sid); @@ -2593,8 +2580,8 @@ static bool unpack_canon_ace(files_struct *fsp, print_canon_ace_list( "file ace - before valid", file_ace); - if (!ensure_canon_entry_valid(fsp->conn, &file_ace, false, fsp->conn->params, - fsp->is_directory, pfile_owner_sid, pfile_grp_sid, pst, True)) { + if (!ensure_canon_entry_valid_on_set(fsp->conn, &file_ace, false, fsp->conn->params, + fsp->is_directory, pfile_owner_sid, pfile_grp_sid, pst)) { free_canon_ace_list(file_ace); free_canon_ace_list(dir_ace); return False; @@ -2602,8 +2589,8 @@ static bool unpack_canon_ace(files_struct *fsp, print_canon_ace_list( "dir ace - before valid", dir_ace); - if (dir_ace && !ensure_canon_entry_valid(fsp->conn, &dir_ace, true, fsp->conn->params, - fsp->is_directory, pfile_owner_sid, pfile_grp_sid, pst, True)) { + if (dir_ace && !ensure_canon_entry_valid_on_set(fsp->conn, &dir_ace, true, fsp->conn->params, + fsp->is_directory, pfile_owner_sid, pfile_grp_sid, pst)) { free_canon_ace_list(file_ace); free_canon_ace_list(dir_ace); return False; -- 1.7.9.5 From c7c7d9732875fc657ee8072b6078a9de0bb330d6 Mon Sep 17 00:00:00 2001 From: Arvid Requate Date: Sat, 10 Nov 2012 10:40:32 +0100 Subject: [PATCH 4/7] s3:smbd: Fix typo in got_duplicate_group check Reviewed by: Jeremy Allison Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Sat Nov 10 20:25:48 CET 2012 on sn-devel-104 (cherry picked from commit c06d602d7f3b8d3da972071a1b5392c6b145133f) --- source3/smbd/posix_acls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c index 0b1f459..fb9cf30 100644 --- a/source3/smbd/posix_acls.c +++ b/source3/smbd/posix_acls.c @@ -1587,7 +1587,7 @@ static bool ensure_canon_entry_valid_on_set(connection_struct *conn, /* Already got one. */ got_duplicate_user = true; } else if (pace->type == SMB_ACL_GROUP && - pace->unix_ug.id == pace_user->unix_ug.id) { + pace->unix_ug.id == pace_group->unix_ug.id) { /* Already got one. */ got_duplicate_group = true; } else if ((pace->type == SMB_ACL_GROUP) -- 1.7.9.5 From b6baefb0f9d03a0af339e18f89a348d8a46b6a12 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 13 Nov 2012 11:22:15 -0800 Subject: [PATCH 5/7] Another fix needed for bug #9236 - ACL masks incorrectly applied when setting ACLs. Not caught by make test as it's an extreme edge case for strange incoming ACLs. I only found this as I'm making raw.acls and smb2.acls pass against 3.6.x and 4.0.0 with acl_xattr mapped onto a POSIX backend. An incoming inheritable ACE entry containing only one permission, WRITE_DATA maps into a POSIX owner perm of "-w-", which violates the principle that the owner of a file/directory can always read. Signed-off-by: Jeremy Allison Reviewed-by: Michael Adam Autobuild-User(master): Michael Adam Autobuild-Date(master): Thu Nov 15 19:52:52 CET 2012 on sn-devel-104 (cherry picked from commit cf1540b73714fac6b25de5942cbd821e5f4f6ffc) --- source3/smbd/posix_acls.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c index fb9cf30..9fbb675 100644 --- a/source3/smbd/posix_acls.c +++ b/source3/smbd/posix_acls.c @@ -1430,10 +1430,11 @@ static bool ensure_canon_entry_valid_on_set(connection_struct *conn, for (pace = *pp_ace; pace; pace = pace->next) { if (pace->type == SMB_ACL_USER_OBJ) { - - if (!is_default_acl) { - apply_default_perms(params, is_directory, pace, S_IRUSR); - } + /* + * Ensure we have default parameters for the + * user (owner) even on default ACLs. + */ + apply_default_perms(params, is_directory, pace, S_IRUSR); pace_user = pace; } else if (pace->type == SMB_ACL_GROUP_OBJ) { @@ -1514,9 +1515,11 @@ static bool ensure_canon_entry_valid_on_set(connection_struct *conn, pace->perms = pace_other->perms; } - if (!is_default_acl) { - apply_default_perms(params, is_directory, pace, S_IRUSR); - } + /* + * Ensure we have default parameters for the + * user (owner) even on default ACLs. + */ + apply_default_perms(params, is_directory, pace, S_IRUSR); DLIST_ADD(*pp_ace, pace); pace_user = pace; -- 1.7.9.5 From 37388139964e2a7a6c7a8e83b457665a925bf41c Mon Sep 17 00:00:00 2001 From: Michael Adam Date: Wed, 5 Dec 2012 15:04:01 +0100 Subject: [PATCH 6/7] s3:smbd: don't apply create/directory mask and modes in apply_default_perms() The mask/mode parameters should only apply to a situation with only pure posix permissions. Once we are dealing with ACLs and inheritance, we need to do it correctly. This fixes bug #9462: Users can not be given write permissions any more by default Signed-off-by: Michael Adam Reviewed by: Jeremy Allison (cherry picked from commit 2013bb9b4dbed747921df2591068e2765428f57d) --- source3/smbd/posix_acls.c | 88 ++++++--------------------------------------- 1 file changed, 11 insertions(+), 77 deletions(-) diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c index 9fbb675..bbc1eed 100644 --- a/source3/smbd/posix_acls.c +++ b/source3/smbd/posix_acls.c @@ -1235,48 +1235,19 @@ NTSTATUS unpack_nt_owners(struct connection_struct *conn, return NT_STATUS_OK; } -/**************************************************************************** - Ensure the enforced permissions for this share apply. -****************************************************************************/ -static void apply_default_perms(const struct share_params *params, - const bool is_directory, canon_ace *pace, - mode_t type) +static void trim_ace_perms(canon_ace *pace) { - mode_t and_bits = (mode_t)0; - mode_t or_bits = (mode_t)0; - - /* Get the initial bits to apply. */ + pace->perms = pace->perms & (S_IXUSR|S_IWUSR|S_IRUSR); +} +static void ensure_minimal_owner_ace_perms(const bool is_directory, + canon_ace *pace) +{ + pace->perms |= S_IRUSR; if (is_directory) { - and_bits = lp_dir_mask(params->service); - or_bits = lp_force_dir_mode(params->service); - } else { - and_bits = lp_create_mask(params->service); - or_bits = lp_force_create_mode(params->service); - } - - /* Now bounce them into the S_USR space. */ - switch(type) { - case S_IRUSR: - /* Ensure owner has read access. */ - pace->perms |= S_IRUSR; - if (is_directory) - pace->perms |= (S_IWUSR|S_IXUSR); - and_bits = unix_perms_to_acl_perms(and_bits, S_IRUSR, S_IWUSR, S_IXUSR); - or_bits = unix_perms_to_acl_perms(or_bits, S_IRUSR, S_IWUSR, S_IXUSR); - break; - case S_IRGRP: - and_bits = unix_perms_to_acl_perms(and_bits, S_IRGRP, S_IWGRP, S_IXGRP); - or_bits = unix_perms_to_acl_perms(or_bits, S_IRGRP, S_IWGRP, S_IXGRP); - break; - case S_IROTH: - and_bits = unix_perms_to_acl_perms(and_bits, S_IROTH, S_IWOTH, S_IXOTH); - or_bits = unix_perms_to_acl_perms(or_bits, S_IROTH, S_IWOTH, S_IXOTH); - break; + pace->perms |= (S_IWUSR|S_IXUSR); } - - pace->perms = ((pace->perms & and_bits)|or_bits); } /**************************************************************************** @@ -1429,45 +1400,14 @@ static bool ensure_canon_entry_valid_on_set(connection_struct *conn, bool got_duplicate_group = false; for (pace = *pp_ace; pace; pace = pace->next) { + trim_ace_perms(pace); if (pace->type == SMB_ACL_USER_OBJ) { - /* - * Ensure we have default parameters for the - * user (owner) even on default ACLs. - */ - apply_default_perms(params, is_directory, pace, S_IRUSR); + ensure_minimal_owner_ace_perms(is_directory, pace); pace_user = pace; - } else if (pace->type == SMB_ACL_GROUP_OBJ) { - - /* - * Ensure create mask/force create mode is respected on set. - */ - - if (!is_default_acl) { - apply_default_perms(params, is_directory, pace, S_IRGRP); - } pace_group = pace; - } else if (pace->type == SMB_ACL_OTHER) { - - /* - * Ensure create mask/force create mode is respected on set. - */ - - if (!is_default_acl) { - apply_default_perms(params, is_directory, pace, S_IROTH); - } pace_other = pace; - - } else if (pace->type == SMB_ACL_USER || pace->type == SMB_ACL_GROUP) { - - /* - * Ensure create mask/force create mode is respected on set. - */ - - if (!is_default_acl) { - apply_default_perms(params, is_directory, pace, S_IRGRP); - } } } @@ -1519,7 +1459,7 @@ static bool ensure_canon_entry_valid_on_set(connection_struct *conn, * Ensure we have default parameters for the * user (owner) even on default ACLs. */ - apply_default_perms(params, is_directory, pace, S_IRUSR); + ensure_minimal_owner_ace_perms(is_directory, pace); DLIST_ADD(*pp_ace, pace); pace_user = pace; @@ -1545,9 +1485,6 @@ static bool ensure_canon_entry_valid_on_set(connection_struct *conn, } else { pace->perms = 0; } - if (!is_default_acl) { - apply_default_perms(params, is_directory, pace, S_IRGRP); - } DLIST_ADD(*pp_ace, pace); pace_group = pace; @@ -1567,9 +1504,6 @@ static bool ensure_canon_entry_valid_on_set(connection_struct *conn, pace->trustee = global_sid_World; pace->attr = ALLOW_ACE; pace->perms = 0; - if (!is_default_acl) { - apply_default_perms(params, is_directory, pace, S_IROTH); - } DLIST_ADD(*pp_ace, pace); pace_other = pace; -- 1.7.9.5 From 2db713e0e6cc5aadc48efd1815e7f26d6ff64ac6 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 4 Dec 2012 15:47:06 -0800 Subject: [PATCH 7/7] Documentation fixes for bug #9462 - Users can not be given write permissions any more by default Ensure we don't apply the masks + force modes on security setting changes, only on create. Signed-off-by: Jeremy Allison Reviewed-by: Michael Adam (cherry picked from commit 1ff1597e1feb45fd54b0d8dc6d8eabc7ace9073a) --- docs-xml/smbdotconf/security/createmask.xml | 5 ----- docs-xml/smbdotconf/security/directorymask.xml | 5 ----- .../smbdotconf/security/directorysecuritymask.xml | 4 +--- docs-xml/smbdotconf/security/forcecreatemode.xml | 6 ------ .../smbdotconf/security/forcedirectorymode.xml | 6 ------ .../security/forcedirectorysecuritymode.xml | 5 +---- docs-xml/smbdotconf/security/forcesecuritymode.xml | 5 +---- docs-xml/smbdotconf/security/securitymask.xml | 4 +--- 8 files changed, 4 insertions(+), 36 deletions(-) diff --git a/docs-xml/smbdotconf/security/createmask.xml b/docs-xml/smbdotconf/security/createmask.xml index 59e208d..5df0718 100644 --- a/docs-xml/smbdotconf/security/createmask.xml +++ b/docs-xml/smbdotconf/security/createmask.xml @@ -26,11 +26,6 @@ This parameter does not affect directory masks. See the parameter for details. - - - New in Samba 4.0.0. This mask is applied whenever permissions are changed on a file. To allow clients full control - over permission changes it should be set to 0777. - force create mode diff --git a/docs-xml/smbdotconf/security/directorymask.xml b/docs-xml/smbdotconf/security/directorymask.xml index 2ebfc16..b17625c 100644 --- a/docs-xml/smbdotconf/security/directorymask.xml +++ b/docs-xml/smbdotconf/security/directorymask.xml @@ -23,11 +23,6 @@ Following this Samba will bit-wise 'OR' the UNIX mode created from this parameter with the value of the parameter. This parameter is set to 000 by default (i.e. no extra mode bits are added). - - - New in Samba 4.0.0. This mask is applied whenever permissions are changed on a directory. To allow clients full control - over permission changes it should be set to 0777. - force directory mode diff --git a/docs-xml/smbdotconf/security/directorysecuritymask.xml b/docs-xml/smbdotconf/security/directorysecuritymask.xml index c5c8c65..ad208f4 100644 --- a/docs-xml/smbdotconf/security/directorysecuritymask.xml +++ b/docs-xml/smbdotconf/security/directorysecuritymask.xml @@ -5,9 +5,7 @@ xmlns:samba="http://www.samba.org/samba/DTD/samba-doc"> - This parameter has been removed for Samba 4.0.0. The parameter - is now used instead to mask - any permission bit changes on directories. + This parameter has been removed for Samba 4.0.0. diff --git a/docs-xml/smbdotconf/security/forcecreatemode.xml b/docs-xml/smbdotconf/security/forcecreatemode.xml index 5a57a29..a3f1c2c 100644 --- a/docs-xml/smbdotconf/security/forcecreatemode.xml +++ b/docs-xml/smbdotconf/security/forcecreatemode.xml @@ -10,12 +10,6 @@ mode after the mask set in the create mask parameter is applied. - - New in Samba 4.0.0. This mode is also 'OR'ed into the mode bits whenever - permissions are changed on a file, not just when the file is created. - This replaces the now removed force security mode. - - The example below would force all newly created files to have read and execute permissions set for 'group' and 'other' as well as the read/write/execute bits set for the 'user'. diff --git a/docs-xml/smbdotconf/security/forcedirectorymode.xml b/docs-xml/smbdotconf/security/forcedirectorymode.xml index e5b37ea..7effc0e 100644 --- a/docs-xml/smbdotconf/security/forcedirectorymode.xml +++ b/docs-xml/smbdotconf/security/forcedirectorymode.xml @@ -12,12 +12,6 @@ mask in the parameter directory mask is applied. - - New in Samba 4.0.0. This mode is also 'OR'ed into the mode bits whenever - permissions are changed on a directory, not just when the file is created. - This replaces the now removed force directory security mode. - - The example below would force all created directories to have read and execute permissions set for 'group' and 'other' as well as the read/write/execute bits set for the 'user'. diff --git a/docs-xml/smbdotconf/security/forcedirectorysecuritymode.xml b/docs-xml/smbdotconf/security/forcedirectorysecuritymode.xml index 3ea3b5c..a45395d 100644 --- a/docs-xml/smbdotconf/security/forcedirectorysecuritymode.xml +++ b/docs-xml/smbdotconf/security/forcedirectorysecuritymode.xml @@ -5,10 +5,7 @@ xmlns:samba="http://www.samba.org/samba/DTD/samba-doc"> - This parameter has been removed for Samba 4.0.0. The parameter - is now used instead to - force any permission changes on directories to include specific UNIX - permission bits. + This parameter has been removed for Samba 4.0.0. diff --git a/docs-xml/smbdotconf/security/forcesecuritymode.xml b/docs-xml/smbdotconf/security/forcesecuritymode.xml index 2568bcc..5a9479e 100644 --- a/docs-xml/smbdotconf/security/forcesecuritymode.xml +++ b/docs-xml/smbdotconf/security/forcesecuritymode.xml @@ -5,10 +5,7 @@ xmlns:samba="http://www.samba.org/samba/DTD/samba-doc"> - This parameter has been removed for Samba 4.0.0. The parameter - is now used instead to - force any permission changes on files to include specific UNIX - permission bits. + This parameter has been removed for Samba 4.0.0. diff --git a/docs-xml/smbdotconf/security/securitymask.xml b/docs-xml/smbdotconf/security/securitymask.xml index cb7fcfa..e535d32 100644 --- a/docs-xml/smbdotconf/security/securitymask.xml +++ b/docs-xml/smbdotconf/security/securitymask.xml @@ -5,9 +5,7 @@ xmlns:samba="http://www.samba.org/samba/DTD/samba-doc"> - This parameter has been removed for Samba 4.0.0. The parameter - is now used instead to mask - any permission bit changes on files. + This parameter has been removed for Samba 4.0.0. -- 1.7.9.5