[SCM] Samba Shared Repository - branch master updated

Jeremy Allison jra at samba.org
Tue Nov 15 20:21:02 MST 2011


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 <jra at 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 <jra at samba.org>
    Autobuild-Date: Wed Nov 16 04:20:04 CET 2011 on sn-devel-104

commit 865bc0c0ace0a4f8e5eb0277def2315867273071
Author: Jeremy Allison <jra at 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 <jra at samba.org>
Date:   Tue Nov 15 16:49:42 2011 -0800

    Always set the attribute first, before the time.

commit edaa7479edd9c6472dacb3642fe6d2a6869e4719
Author: Jeremy Allison <jra at samba.org>
Date:   Tue Nov 15 16:22:09 2011 -0800

    Move handle-based access check into handle codepath.

commit c6a62f60a23fdadb99da4d4b47694b273342f2a7
Author: Jeremy Allison <jra at 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 <jra at 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 <jra at 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 <jra at 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


More information about the samba-cvs mailing list