From f1d1d30c3b2880255d79bc7f5e99d0435b326f4f Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 13 Mar 2012 13:44:37 -0700 Subject: [PATCH] Fix bug #7933 - samba fails to honor SEC_STD_WRITE_OWNER bit with the acl_xattr module. Contains both parts of the fix from master (45a3b14beab20445ecffea5f66d278d1bd102c74 and 30edc41e3f87a17c4ff5436b5e9a9aac5714d7d6). --- source3/modules/vfs_acl_common.c | 37 ++++++++++++++++- source3/smbd/posix_acls.c | 87 ++++++++++++++++++++++++++++++++----- 2 files changed, 111 insertions(+), 13 deletions(-) diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c index a537011..891be86 100644 --- a/source3/modules/vfs_acl_common.c +++ b/source3/modules/vfs_acl_common.c @@ -757,6 +757,7 @@ static NTSTATUS fset_nt_acl_common(vfs_handle_struct *handle, files_struct *fsp, struct security_descriptor *pdesc_next = NULL; struct security_descriptor *psd = NULL; uint8_t hash[XATTR_SD_HASH_SIZE]; + bool chown_needed = false; if (DEBUGLEVEL >= 10) { DEBUG(10,("fset_nt_acl_xattr: incoming sd for file %s\n", @@ -779,9 +780,17 @@ static NTSTATUS fset_nt_acl_common(vfs_handle_struct *handle, files_struct *fsp, psd->type = orig_psd->type | SEC_DESC_SELF_RELATIVE; if ((security_info_sent & SECINFO_OWNER) && (orig_psd->owner_sid != NULL)) { + if (!dom_sid_equal(orig_psd->owner_sid, psd->owner_sid)) { + /* We're changing the owner. */ + chown_needed = true; + } psd->owner_sid = orig_psd->owner_sid; } if ((security_info_sent & SECINFO_GROUP) && (orig_psd->group_sid != NULL)) { + if (!dom_sid_equal(orig_psd->group_sid, psd->group_sid)) { + /* We're changing the group. */ + chown_needed = true; + } psd->group_sid = orig_psd->group_sid; } if (security_info_sent & SECINFO_DACL) { @@ -795,7 +804,33 @@ static NTSTATUS fset_nt_acl_common(vfs_handle_struct *handle, files_struct *fsp, status = SMB_VFS_NEXT_FSET_NT_ACL(handle, fsp, security_info_sent, psd); if (!NT_STATUS_IS_OK(status)) { - return status; + if (!NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) { + return status; + } + /* We got access denied here. If we're already root, + or we didn't need to do a chown, or the fsp isn't + open with WRITE_OWNER access, just return. */ + if (get_current_uid(handle->conn) == 0 || + chown_needed == false || + !(fsp->access_mask & SEC_STD_WRITE_OWNER)) { + return NT_STATUS_ACCESS_DENIED; + } + + DEBUG(10,("fset_nt_acl_common: overriding chown on file %s " + "for sid %s\n", + fsp_str_dbg(fsp), + sid_string_tos(psd->owner_sid) + )); + + /* Ok, we failed to chown and we have + SEC_STD_WRITE_OWNER access - override. */ + become_root(); + status = SMB_VFS_NEXT_FSET_NT_ACL(handle, fsp, + security_info_sent, psd); + unbecome_root(); + if (!NT_STATUS_IS_OK(status)) { + return status; + } } /* Get the full underlying sd, then hash. */ diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c index 34747d3..91cb209 100644 --- a/source3/smbd/posix_acls.c +++ b/source3/smbd/posix_acls.c @@ -1362,9 +1362,8 @@ static bool ensure_canon_entry_valid(connection_struct *conn, canon_ace **pp_ace bool setting_acl) { canon_ace *pace; - bool got_user = False; - bool got_grp = False; - bool got_other = False; + canon_ace *pace_user = NULL; + canon_ace *pace_group = NULL; canon_ace *pace_other = NULL; for (pace = *pp_ace; pace; pace = pace->next) { @@ -1372,7 +1371,7 @@ static bool ensure_canon_entry_valid(connection_struct *conn, canon_ace **pp_ace if (setting_acl) apply_default_perms(params, is_directory, pace, S_IRUSR); - got_user = True; + pace_user = pace; } else if (pace->type == SMB_ACL_GROUP_OBJ) { @@ -1382,7 +1381,7 @@ static bool ensure_canon_entry_valid(connection_struct *conn, canon_ace **pp_ace if (setting_acl) apply_default_perms(params, is_directory, pace, S_IRGRP); - got_grp = True; + pace_group = pace; } else if (pace->type == SMB_ACL_OTHER) { @@ -1392,12 +1391,11 @@ static bool ensure_canon_entry_valid(connection_struct *conn, canon_ace **pp_ace if (setting_acl) apply_default_perms(params, is_directory, pace, S_IROTH); - got_other = True; pace_other = pace; } } - if (!got_user) { + if (!pace_user) { if ((pace = SMB_MALLOC_P(canon_ace)) == NULL) { DEBUG(0,("ensure_canon_entry_valid: malloc fail.\n")); return False; @@ -1433,7 +1431,7 @@ static bool ensure_canon_entry_valid(connection_struct *conn, canon_ace **pp_ace if (pace->perms == 0) { /* If we only got an "everyone" perm, just use that. */ - if (got_other) + if (pace_other) pace->perms = pace_other->perms; } @@ -1443,9 +1441,10 @@ static bool ensure_canon_entry_valid(connection_struct *conn, canon_ace **pp_ace } DLIST_ADD(*pp_ace, pace); + pace_user = pace; } - if (!got_grp) { + if (!pace_group) { if ((pace = SMB_MALLOC_P(canon_ace)) == NULL) { DEBUG(0,("ensure_canon_entry_valid: malloc fail.\n")); return False; @@ -1454,12 +1453,12 @@ static bool ensure_canon_entry_valid(connection_struct *conn, canon_ace **pp_ace ZERO_STRUCTP(pace); pace->type = SMB_ACL_GROUP_OBJ; pace->owner_type = GID_ACE; - pace->unix_ug.uid = pst->st_ex_gid; + pace->unix_ug.gid = 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 (got_other) + if (pace_other) pace->perms = pace_other->perms; else pace->perms = 0; @@ -1469,9 +1468,10 @@ static bool ensure_canon_entry_valid(connection_struct *conn, canon_ace **pp_ace } DLIST_ADD(*pp_ace, pace); + pace_group = pace; } - if (!got_other) { + if (!pace_other) { if ((pace = SMB_MALLOC_P(canon_ace)) == NULL) { DEBUG(0,("ensure_canon_entry_valid: malloc fail.\n")); return False; @@ -1490,6 +1490,69 @@ static bool ensure_canon_entry_valid(connection_struct *conn, canon_ace **pp_ace pace->perms = unix_perms_to_acl_perms(pst->st_ex_mode, S_IROTH, S_IWOTH, S_IXOTH); 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.uid == pace_user->unix_ug.uid) { + /* Already got one. */ + got_duplicate_user = true; + } else if (pace->type == SMB_ACL_GROUP && + pace->unix_ug.uid == pace_user->unix_ug.uid) { + /* Already got one. */ + got_duplicate_group = true; + } + } + + if (!got_duplicate_user) { + /* Add a duplicate SMB_ACL_USER entry. */ + if ((pace = SMB_MALLOC_P(canon_ace)) == NULL) { + DEBUG(0,("ensure_canon_entry_valid: malloc fail.\n")); + return false; + } + + ZERO_STRUCTP(pace); + pace->type = SMB_ACL_USER;; + pace->owner_type = UID_ACE; + pace->unix_ug.uid = pace_user->unix_ug.uid; + pace->trustee = pace_user->trustee; + pace->attr = pace_user->attr; + pace->perms = pace_user->perms; + + DLIST_ADD(*pp_ace, pace); + } + + if (!got_duplicate_group) { + /* Add a duplicate SMB_ACL_GROUP entry. */ + if ((pace = SMB_MALLOC_P(canon_ace)) == NULL) { + DEBUG(0,("ensure_canon_entry_valid: malloc 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); + } + } return True; -- 1.7.7.3