The branch, master has been updated via 05e841c Final part of patchset to fix bug #8556 - ACL permissions ignored when SMBsetatr is requested. via 865bc0c Remove the check for FILE_WRITE_ATTRIBUTES from smb_set_file_time(). It is called from places like fileio.c that need to update the write time on a file handle only open for write, without neccessarily having FILE_WRITE_ATTRIBUTES permission. Move all checks to before the smb_set_file_time() callers. via 86c1609 Always set the attribute first, before the time. via edaa747 Move handle-based access check into handle codepath. via c6a62f6 We've already checked fsp must be non-null here. via 93000c9 Remove unneeded access check. This is done inside smb_set_file_time(). via f5cda71 Remove unneeded access check. This is done inside smb_set_file_size(). via c27551b Move handle based access check into handle code path. from dd504b1 HEIMDAL:lib/krb5: add utf8 support to build_logon_name() for the PAC
http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master - Log ----------------------------------------------------------------- commit 05e841c82ce7f0f252b5eb565e457f406b3a1b39 Author: Jeremy Allison <j...@samba.org> Date: Tue Nov 15 17:29:59 2011 -0800 Final part of patchset to fix bug #8556 - ACL permissions ignored when SMBsetatr is requested. This now plumbs access checks through all setattr calls. Autobuild-User: Jeremy Allison <j...@samba.org> Autobuild-Date: Wed Nov 16 04:20:04 CET 2011 on sn-devel-104 commit 865bc0c0ace0a4f8e5eb0277def2315867273071 Author: Jeremy Allison <j...@samba.org> Date: Tue Nov 15 17:41:48 2011 -0800 Remove the check for FILE_WRITE_ATTRIBUTES from smb_set_file_time(). It is called from places like fileio.c that need to update the write time on a file handle only open for write, without neccessarily having FILE_WRITE_ATTRIBUTES permission. Move all checks to before the smb_set_file_time() callers. commit 86c16092194836d8478144b97da9ca08aec7fac6 Author: Jeremy Allison <j...@samba.org> Date: Tue Nov 15 16:49:42 2011 -0800 Always set the attribute first, before the time. commit edaa7479edd9c6472dacb3642fe6d2a6869e4719 Author: Jeremy Allison <j...@samba.org> Date: Tue Nov 15 16:22:09 2011 -0800 Move handle-based access check into handle codepath. commit c6a62f60a23fdadb99da4d4b47694b273342f2a7 Author: Jeremy Allison <j...@samba.org> Date: Tue Nov 15 16:20:44 2011 -0800 We've already checked fsp must be non-null here. commit 93000c98ad42a2e089ba6b371d811263f953c23d Author: Jeremy Allison <j...@samba.org> Date: Tue Nov 15 16:16:54 2011 -0800 Remove unneeded access check. This is done inside smb_set_file_time(). commit f5cda7160ce63abbde6878ca517680f417ffeb17 Author: Jeremy Allison <j...@samba.org> Date: Tue Nov 15 16:14:47 2011 -0800 Remove unneeded access check. This is done inside smb_set_file_size(). commit c27551b1631eff0c91d79fbcadce703eadc3efc1 Author: Jeremy Allison <j...@samba.org> Date: Tue Nov 15 16:14:16 2011 -0800 Move handle based access check into handle code path. ----------------------------------------------------------------------- Summary of changes: source3/smbd/close.c | 6 +-- source3/smbd/proto.h | 4 ++ source3/smbd/reply.c | 26 +++++++++++---- source3/smbd/trans2.c | 86 ++++++++++++++++++++++++++++++------------------ 4 files changed, 79 insertions(+), 43 deletions(-) Changeset truncated at 500 lines: diff --git a/source3/smbd/close.c b/source3/smbd/close.c index b736432..837e360 100644 --- a/source3/smbd/close.c +++ b/source3/smbd/close.c @@ -581,11 +581,9 @@ static NTSTATUS update_write_time_on_close(struct files_struct *fsp) } ft.mtime = fsp->close_write_time; - /* We must use NULL for the fsp handle here, as smb_set_file_time() - checks the fsp access_mask, which may not include FILE_WRITE_ATTRIBUTES. - As this is a close based update, we are not directly changing the + /* As this is a close based update, we are not directly changing the file attributes from a client call, but indirectly from a write. */ - status = smb_set_file_time(fsp->conn, NULL, fsp->fsp_name, &ft, false); + status = smb_set_file_time(fsp->conn, fsp, fsp->fsp_name, &ft, false); if (!NT_STATUS_IS_OK(status)) { DEBUG(10,("update_write_time_on_close: smb_set_file_time " "on file %s returned %s\n", diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index fe90766..151ae78 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -1059,6 +1059,10 @@ int sys_statvfs(const char *path, vfs_statvfs_struct *statbuf); /* The following definitions come from smbd/trans2.c */ +NTSTATUS check_access(connection_struct *conn, + files_struct *fsp, + const struct smb_filename *smb_fname, + uint32_t access_mask); uint64_t smb_roundup(connection_struct *conn, uint64_t val); uint64_t get_FileIndex(connection_struct *conn, const SMB_STRUCT_STAT *psbuf); NTSTATUS get_ea_value(TALLOC_CTX *mem_ctx, connection_struct *conn, diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 0adc4e8..7dd3260 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -1269,19 +1269,19 @@ void reply_setatr(struct smb_request *req) mode = SVAL(req->vwv+0, 0); mtime = srv_make_unix_date3(req->vwv+1); - ft.mtime = convert_time_t_to_timespec(mtime); - status = smb_set_file_time(conn, NULL, smb_fname, &ft, true); - if (!NT_STATUS_IS_OK(status)) { - reply_nterror(req, status); - goto out; - } - if (mode != FILE_ATTRIBUTE_NORMAL) { if (VALID_STAT_OF_DIR(smb_fname->st)) mode |= FILE_ATTRIBUTE_DIRECTORY; else mode &= ~FILE_ATTRIBUTE_DIRECTORY; + status = check_access(conn, NULL, smb_fname, + FILE_WRITE_ATTRIBUTES); + if (!NT_STATUS_IS_OK(status)) { + reply_nterror(req, status); + goto out; + } + if (file_set_dosmode(conn, smb_fname, mode, NULL, false) != 0) { reply_nterror(req, map_nt_error_from_unix(errno)); @@ -1289,6 +1289,13 @@ void reply_setatr(struct smb_request *req) } } + ft.mtime = convert_time_t_to_timespec(mtime); + status = smb_set_file_time(conn, NULL, smb_fname, &ft, true); + if (!NT_STATUS_IS_OK(status)) { + reply_nterror(req, status); + goto out; + } + reply_outbuf(req, 0, 0); DEBUG(3, ("setatr name=%s mode=%d\n", smb_fname_str_dbg(smb_fname), @@ -7952,6 +7959,11 @@ void reply_setattrE(struct smb_request *req) goto out; } + if (!(fsp->access_mask & FILE_WRITE_ATTRIBUTES)) { + reply_nterror(req, NT_STATUS_ACCESS_DENIED); + goto out; + } + status = smb_set_file_time(conn, fsp, fsp->fsp_name, &ft, true); if (!NT_STATUS_IS_OK(status)) { reply_nterror(req, status); diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index f6e62ef..024979c 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -50,6 +50,30 @@ static char *store_file_unix_basic_info2(connection_struct *conn, const SMB_STRUCT_STAT *psbuf); /******************************************************************** + The canonical "check access" based on object handle or path function. +********************************************************************/ + +NTSTATUS check_access(connection_struct *conn, + files_struct *fsp, + const struct smb_filename *smb_fname, + uint32_t access_mask) +{ + if (fsp) { + if (!(fsp->access_mask & access_mask)) { + return NT_STATUS_ACCESS_DENIED; + } + } else { + NTSTATUS status = smbd_check_access_rights(conn, + smb_fname, + access_mask); + if (!NT_STATUS_IS_OK(status)) { + return status; + } + } + return NT_STATUS_OK; +} + +/******************************************************************** Roundup a value to the nearest allocation roundup size boundary. Only do this for Windows clients. ********************************************************************/ @@ -504,14 +528,16 @@ static void canonicalize_ea_name(connection_struct *conn, files_struct *fsp, con NTSTATUS set_ea(connection_struct *conn, files_struct *fsp, const struct smb_filename *smb_fname, struct ea_list *ea_list) { + NTSTATUS status; char *fname = NULL; if (!lp_ea_support(SNUM(conn))) { return NT_STATUS_EAS_NOT_SUPPORTED; } - if (fsp && !(fsp->access_mask & FILE_WRITE_EA)) { - return NT_STATUS_ACCESS_DENIED; + status = check_access(conn, fsp, smb_fname, FILE_WRITE_EA); + if (!NT_STATUS_IS_OK(status)) { + return status; } /* For now setting EAs on streams isn't supported. */ @@ -5440,6 +5466,8 @@ NTSTATUS hardlink_internals(TALLOC_CTX *ctx, /**************************************************************************** Deal with setting the time from any of the setfilepathinfo functions. + NOTE !!!! The check for FILE_WRITE_ATTRIBUTES access must be done *before* + calling this function. ****************************************************************************/ NTSTATUS smb_set_file_time(connection_struct *conn, @@ -5458,10 +5486,6 @@ NTSTATUS smb_set_file_time(connection_struct *conn, return NT_STATUS_OBJECT_NAME_NOT_FOUND; } - if (fsp && !(fsp->access_mask & FILE_WRITE_ATTRIBUTES)) { - return NT_STATUS_ACCESS_DENIED; - } - /* get some defaults (no modifications) if any info is zero or -1. */ if (null_timespec(ft->create_time)) { action &= ~FILE_NOTIFY_CHANGE_CREATION; @@ -5542,6 +5566,8 @@ NTSTATUS smb_set_file_time(connection_struct *conn, /**************************************************************************** Deal with setting the dosmode from any of the setfilepathinfo functions. + NB. The check for FILE_WRITE_ATTRIBUTES access on this path must have been + done before calling this function. ****************************************************************************/ static NTSTATUS smb_set_file_dosmode(connection_struct *conn, @@ -5615,10 +5641,6 @@ static NTSTATUS smb_set_file_size(connection_struct *conn, return NT_STATUS_OBJECT_NAME_NOT_FOUND; } - if (fsp && !(fsp->access_mask & FILE_WRITE_DATA)) { - return NT_STATUS_ACCESS_DENIED; - } - DEBUG(6,("smb_set_file_size: size: %.0f ", (double)size)); if (size == get_file_size_stat(psbuf)) { @@ -5630,6 +5652,10 @@ static NTSTATUS smb_set_file_size(connection_struct *conn, if (fsp && fsp->fh->fd != -1) { /* Handle based call. */ + if (!(fsp->access_mask & FILE_WRITE_DATA)) { + return NT_STATUS_ACCESS_DENIED; + } + if (vfs_set_filelen(fsp, size) == -1) { return map_nt_error_from_unix(errno); } @@ -5726,10 +5752,6 @@ static NTSTATUS smb_info_set_ea(connection_struct *conn, return NT_STATUS_INVALID_PARAMETER; } - if (fsp && !(fsp->access_mask & FILE_WRITE_EA)) { - return NT_STATUS_ACCESS_DENIED; - } - status = set_ea(conn, fsp, smb_fname, ea_list); return status; @@ -5773,10 +5795,6 @@ static NTSTATUS smb_set_file_full_ea_info(connection_struct *conn, return NT_STATUS_INVALID_PARAMETER; } - if (fsp && !(fsp->access_mask & FILE_WRITE_EA)) { - return NT_STATUS_ACCESS_DENIED; - } - status = set_ea(conn, fsp, fsp->fsp_name, ea_list); DEBUG(10, ("smb_set_file_full_ea_info on file %s returned %s\n", @@ -6516,8 +6534,9 @@ static NTSTATUS smb_set_file_basic_info(connection_struct *conn, return NT_STATUS_INVALID_PARAMETER; } - if (fsp && !(fsp->access_mask & FILE_WRITE_ATTRIBUTES)) { - return NT_STATUS_ACCESS_DENIED; + status = check_access(conn, fsp, smb_fname, FILE_WRITE_ATTRIBUTES); + if (!NT_STATUS_IS_OK(status)) { + return status; } /* Set the attributes */ @@ -6556,6 +6575,7 @@ static NTSTATUS smb_set_info_standard(connection_struct *conn, files_struct *fsp, const struct smb_filename *smb_fname) { + NTSTATUS status; struct smb_file_time ft; ZERO_STRUCT(ft); @@ -6564,10 +6584,6 @@ static NTSTATUS smb_set_info_standard(connection_struct *conn, return NT_STATUS_INVALID_PARAMETER; } - if (fsp && !(fsp->access_mask & FILE_WRITE_ATTRIBUTES)) { - return NT_STATUS_ACCESS_DENIED; - } - /* create time */ ft.create_time = convert_time_t_to_timespec(srv_make_unix_date2(pdata)); /* access time */ @@ -6578,6 +6594,11 @@ static NTSTATUS smb_set_info_standard(connection_struct *conn, DEBUG(10,("smb_set_info_standard: file %s\n", smb_fname_str_dbg(smb_fname))); + status = check_access(conn, fsp, smb_fname, FILE_WRITE_ATTRIBUTES); + if (!NT_STATUS_IS_OK(status)) { + return status; + } + return smb_set_file_time(conn, fsp, smb_fname, @@ -6626,16 +6647,16 @@ static NTSTATUS smb_set_file_allocation_info(connection_struct *conn, allocation_size = smb_roundup(conn, allocation_size); } - if (fsp && !(fsp->access_mask & FILE_WRITE_DATA)) { - return NT_STATUS_ACCESS_DENIED; - } - DEBUG(10,("smb_set_file_allocation_info: file %s : setting new " "allocation size to %.0f\n", smb_fname_str_dbg(smb_fname), (double)allocation_size)); if (fsp && fsp->fh->fd != -1) { /* Open file handle. */ + if (!(fsp->access_mask & FILE_WRITE_DATA)) { + return NT_STATUS_ACCESS_DENIED; + } + /* Only change if needed. */ if (allocation_size != get_file_size_stat(&smb_fname->st)) { if (vfs_allocate_file_space(fsp, allocation_size) == -1) { @@ -6727,10 +6748,6 @@ static NTSTATUS smb_set_file_end_of_file_info(connection_struct *conn, "file %s to %.0f\n", smb_fname_str_dbg(smb_fname), (double)size)); - if (fsp && !(fsp->access_mask & FILE_WRITE_DATA)) { - return NT_STATUS_ACCESS_DENIED; - } - return smb_set_file_size(conn, req, fsp, smb_fname, @@ -6952,6 +6969,11 @@ static NTSTATUS smb_set_file_unix_basic(connection_struct *conn, } #endif + status = check_access(conn, fsp, smb_fname, FILE_WRITE_ATTRIBUTES); + if (!NT_STATUS_IS_OK(status)) { + return status; + } + /* * Deal with the UNIX specific mode set. */ -- Samba Shared Repository