From e1e40835e274485672f99e63552d5631b6d7c4e8 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 8 Sep 2011 12:51:18 -0700 Subject: [PATCH 1/2] First part of fix for bug #8443 - Default user entry is set to minimal permissions on incoming ACL change with no user specified. create_default_mode() is not needed - it's taken care of by code inside ensure_canon_entry_valid(). (cherry picked from commit 793bd527fdd0b188aba8f3b4bffd8fa8f69a9cd1) --- source3/smbd/posix_acls.c | 60 +------------------------------------------- 1 files changed, 2 insertions(+), 58 deletions(-) diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c index d1e5083..37141b5 100644 --- a/source3/smbd/posix_acls.c +++ b/source3/smbd/posix_acls.c @@ -2251,44 +2251,6 @@ static void process_deny_list(connection_struct *conn, canon_ace **pp_ace_list ) } /**************************************************************************** - Create a default mode that will be used if a security descriptor entry has - no user/group/world entries. -****************************************************************************/ - -static mode_t create_default_mode(files_struct *fsp, bool interitable_mode) -{ - int snum = SNUM(fsp->conn); - mode_t and_bits = (mode_t)0; - mode_t or_bits = (mode_t)0; - mode_t mode; - - if (interitable_mode) { - mode = unix_mode(fsp->conn, FILE_ATTRIBUTE_ARCHIVE, - fsp->fsp_name, NULL); - } else { - mode = S_IRUSR; - } - - if (fsp->is_directory) - mode |= (S_IWUSR|S_IXUSR); - - /* - * Now AND with the create mode/directory mode bits then OR with the - * force create mode/force directory mode bits. - */ - - if (fsp->is_directory) { - and_bits = lp_dir_security_mask(snum); - or_bits = lp_force_dir_security_mode(snum); - } else { - and_bits = lp_security_mask(snum); - or_bits = lp_force_security_mode(snum); - } - - return ((mode & and_bits)|or_bits); -} - -/**************************************************************************** Unpack a struct security_descriptor into two canonical ace lists. We don't depend on this succeeding. ****************************************************************************/ @@ -2302,7 +2264,6 @@ static bool unpack_canon_ace(files_struct *fsp, uint32 security_info_sent, const struct security_descriptor *psd) { - SMB_STRUCT_STAT st; canon_ace *file_ace = NULL; canon_ace *dir_ace = NULL; @@ -2366,17 +2327,8 @@ static bool unpack_canon_ace(files_struct *fsp, print_canon_ace_list( "file ace - before valid", file_ace); - st = *pst; - - /* - * A default 3 element mode entry for a file should be r-- --- ---. - * A default 3 element mode entry for a directory should be rwx --- ---. - */ - - st.st_ex_mode = create_default_mode(fsp, False); - if (!ensure_canon_entry_valid(fsp->conn, &file_ace, fsp->conn->params, - fsp->is_directory, pfile_owner_sid, pfile_grp_sid, &st, True)) { + fsp->is_directory, pfile_owner_sid, pfile_grp_sid, pst, True)) { free_canon_ace_list(file_ace); free_canon_ace_list(dir_ace); return False; @@ -2384,16 +2336,8 @@ static bool unpack_canon_ace(files_struct *fsp, print_canon_ace_list( "dir ace - before valid", dir_ace); - /* - * A default inheritable 3 element mode entry for a directory should be the - * mode Samba will use to create a file within. Ensure user rwx bits are set if - * it's a directory. - */ - - st.st_ex_mode = create_default_mode(fsp, True); - if (dir_ace && !ensure_canon_entry_valid(fsp->conn, &dir_ace, fsp->conn->params, - fsp->is_directory, pfile_owner_sid, pfile_grp_sid, &st, True)) { + fsp->is_directory, pfile_owner_sid, pfile_grp_sid, pst, True)) { free_canon_ace_list(file_ace); free_canon_ace_list(dir_ace); return False; -- 1.7.3.1 From cb59c14d417e0e7167edee428efacf953ec13983 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 8 Sep 2011 13:48:27 -0700 Subject: [PATCH 2/2] Second part of fix for bug #8443 - Default user entry is set to minimal permissions on incoming ACL change with no user specified. Be smarter about setting default permissions when a ACL_USER_OBJ isn't given. Use the principle of least surprises for the user. Autobuild-User: Jeremy Allison Autobuild-Date: Fri Sep 9 00:26:08 CEST 2011 on sn-devel-104 (cherry picked from commit e30b8c72def13e2abc14858ea64eb849ea665b80) --- 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 37141b5..278ac0a 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); -- 1.7.3.1