[SCM] Samba Shared Repository - branch master updated

Jeremy Allison jra at samba.org
Mon Nov 4 15:11:02 MST 2013


The branch, master has been updated
       via  6588215 Add regression test for bug #10229 - No access check verification on stream files.
       via  60f922b Fix bug #10229 - No access check verification on stream files.
       via  4061954 torture: Add smb2.rename.rename_dir_bench
      from  03e4037 testparm: Add warning for socket options.

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 65882152cc7ccaba0e7903862b99ca93594ed080
Author: Jeremy Allison <jra at samba.org>
Date:   Tue Oct 29 15:57:01 2013 -0700

    Add regression test for bug #10229 - No access check verification on stream files.
    
    Checks against a file with attribute READONLY, and
    a security descriptor denying WRITE_DATA access.
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: David Disseldorp <ddiss at suse.de>
    
    Autobuild-User(master): Jeremy Allison <jra at samba.org>
    Autobuild-Date(master): Mon Nov  4 23:10:10 CET 2013 on sn-devel-104

commit 60f922bf1bd8816eacbb32c24793ad1f97a1d9f2
Author: Jeremy Allison <jra at samba.org>
Date:   Mon Oct 28 16:59:20 2013 -0700

    Fix bug #10229 - No access check verification on stream files.
    
    https://bugzilla.samba.org/show_bug.cgi?id=10229
    
    We need to check if the requested access mask
    could be used to open the underlying file (if
    it existed), as we're passing in zero for the
    access mask to the base filename.
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Stefan Metzmacher <metze at samba.org>
    Reviewed-by: David Disseldorp <ddiss at suse.de>

commit 4061954990dfa0f898f278a536a9f0995d774a00
Author: Volker Lendecke <vl at samba.org>
Date:   Thu Oct 31 12:37:36 2013 +0100

    torture: Add smb2.rename.rename_dir_bench
    
    This is a little benchmark test excercising parallel directory renames. With
    lots of open files directory renames get pretty slow against some SMB server
    implementations.
    
    Signed-off-by: Volker Lendecke <vl at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

-----------------------------------------------------------------------

Summary of changes:
 selftest/knownfail            |    2 +
 source3/smbd/open.c           |   59 +++++++
 source4/torture/raw/streams.c |  181 ++++++++++++++++++++
 source4/torture/smb2/rename.c |  375 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 617 insertions(+), 0 deletions(-)


Changeset truncated at 500 lines:

diff --git a/selftest/knownfail b/selftest/knownfail
index ab6d45f..201ea6b 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -122,6 +122,7 @@
 ^samba4.smb2.rename.share_delete_no_delete_access\(.*\)$
 ^samba4.smb2.rename.no_share_delete_no_delete_access\(.*\)$
 ^samba4.smb2.rename.msword
+^samba4.smb2.rename.rename_dir_bench\(dc\)
 ^samba4.smb2.oplock.doc
 ^samba4.smb2.compound.related3
 ^samba4.smb2.compound.compound-break
@@ -148,6 +149,7 @@
 ^samba4.raw.streams.*.delete
 ^samba4.raw.streams.*.createdisp
 ^samba4.raw.streams.*.sumtab
+^samba4.raw.streams.*.perms
 ^samba4.raw.acls.INHERITFLAGS
 ^samba4.raw.acls.*.create_dir
 ^samba4.raw.acls.*.create_file
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index 4db673a..105eb09 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -302,6 +302,46 @@ static NTSTATUS check_parent_access(struct connection_struct *conn,
 }
 
 /****************************************************************************
+ Ensure when opening a base file for a stream open that we have permissions
+ to do so given the access mask on the base file.
+****************************************************************************/
+
+static NTSTATUS check_base_file_access(struct connection_struct *conn,
+				struct smb_filename *smb_fname,
+				uint32_t access_mask)
+{
+	NTSTATUS status;
+
+	status = smbd_calculate_access_mask(conn, smb_fname,
+					false,
+					access_mask,
+					&access_mask);
+	if (!NT_STATUS_IS_OK(status)) {
+		DEBUG(10, ("smbd_calculate_access_mask "
+			"on file %s returned %s\n",
+			smb_fname_str_dbg(smb_fname),
+			nt_errstr(status)));
+		return status;
+	}
+
+	if (access_mask & (FILE_WRITE_DATA|FILE_APPEND_DATA)) {
+		uint32_t dosattrs;
+		if (!CAN_WRITE(conn)) {
+			return NT_STATUS_ACCESS_DENIED;
+		}
+		dosattrs = dos_mode(conn, smb_fname);
+		if (IS_DOS_READONLY(dosattrs)) {
+			return NT_STATUS_ACCESS_DENIED;
+		}
+	}
+
+	return smbd_check_access_rights(conn,
+					smb_fname,
+					false,
+					access_mask);
+}
+
+/****************************************************************************
  fd support routines - attempt to do a dos_open.
 ****************************************************************************/
 
@@ -3795,6 +3835,25 @@ static NTSTATUS create_file_unixpath(connection_struct *conn,
 		if (SMB_VFS_STAT(conn, smb_fname_base) == -1) {
 			DEBUG(10, ("Unable to stat stream: %s\n",
 				   smb_fname_str_dbg(smb_fname_base)));
+		} else {
+			/*
+			 * https://bugzilla.samba.org/show_bug.cgi?id=10229
+			 * We need to check if the requested access mask
+			 * could be used to open the underlying file (if
+			 * it existed), as we're passing in zero for the
+			 * access mask to the base filename.
+			 */
+			status = check_base_file_access(conn,
+							smb_fname_base,
+							access_mask);
+
+			if (!NT_STATUS_IS_OK(status)) {
+				DEBUG(10, ("Permission check "
+					"for base %s failed: "
+					"%s\n", smb_fname->base_name,
+					nt_errstr(status)));
+				goto fail;
+			}
 		}
 
 		/* Open the base file. */
diff --git a/source4/torture/raw/streams.c b/source4/torture/raw/streams.c
index cbb7dcf..c1e502f 100644
--- a/source4/torture/raw/streams.c
+++ b/source4/torture/raw/streams.c
@@ -23,6 +23,8 @@
 #include "system/locale.h"
 #include "torture/torture.h"
 #include "libcli/raw/libcliraw.h"
+#include "libcli/security/dom_sid.h"
+#include "libcli/security/security_descriptor.h"
 #include "system/filesys.h"
 #include "libcli/libcli.h"
 #include "torture/util.h"
@@ -1885,6 +1887,184 @@ static bool test_stream_summary_tab(struct torture_context *tctx,
 	return ret;
 }
 
+/* Test how streams interact with base file permissions */
+/* Regression test for bug:
+   https://bugzilla.samba.org/show_bug.cgi?id=10229
+   bug #10229 - No access check verification on stream files.
+*/
+static bool test_stream_permissions(struct torture_context *tctx,
+					   struct smbcli_state *cli)
+{
+	NTSTATUS status;
+	bool ret = true;
+	union smb_open io;
+	const char *fname = BASEDIR "\\stream_permissions.txt";
+	const char *stream = "Stream One:$DATA";
+	const char *fname_stream;
+	union smb_fileinfo finfo;
+	union smb_setfileinfo sfinfo;
+	int fnum = -1;
+	union smb_fileinfo q;
+	union smb_setfileinfo set;
+	struct security_ace ace;
+	struct security_descriptor *sd;
+
+	torture_assert(tctx, torture_setup_dir(cli, BASEDIR),
+		"Failed to setup up test directory: " BASEDIR);
+
+	torture_comment(tctx, "(%s) testing permissions on streams\n", __location__);
+
+	fname_stream = talloc_asprintf(tctx, "%s:%s", fname, stream);
+
+	/* Create a file with a stream with attribute FILE_ATTRIBUTE_ARCHIVE. */
+	ret = create_file_with_stream(tctx, cli, fname_stream);
+	if (!ret) {
+		goto done;
+	}
+
+	ZERO_STRUCT(finfo);
+	finfo.generic.level = RAW_FILEINFO_BASIC_INFO;
+	finfo.generic.in.file.path = fname;
+	status = smb_raw_pathinfo(cli->tree, tctx, &finfo);
+	CHECK_STATUS(status, NT_STATUS_OK);
+
+	torture_assert_int_equal_goto(tctx,
+		finfo.all_info.out.attrib & ~FILE_ATTRIBUTE_NONINDEXED,
+		FILE_ATTRIBUTE_ARCHIVE, ret, done, "attrib incorrect");
+
+	/* Change the attributes on the base file name. */
+	ZERO_STRUCT(sfinfo);
+	sfinfo.generic.level = RAW_SFILEINFO_SETATTR;
+	sfinfo.generic.in.file.path        = fname;
+	sfinfo.setattr.in.attrib           = FILE_ATTRIBUTE_READONLY;
+
+	status = smb_raw_setpathinfo(cli->tree, &sfinfo);
+	CHECK_STATUS(status, NT_STATUS_OK);
+
+	/* Try and open the stream name for WRITE_DATA. Should
+	   fail with ACCESS_DENIED. */
+
+	ZERO_STRUCT(io);
+	io.generic.level = RAW_OPEN_NTCREATEX;
+	io.ntcreatex.in.root_fid.fnum = 0;
+	io.ntcreatex.in.flags = 0;
+	io.ntcreatex.in.access_mask = SEC_FILE_WRITE_DATA;
+	io.ntcreatex.in.create_options = 0;
+	io.ntcreatex.in.file_attr = 0;
+	io.ntcreatex.in.share_access = 0;
+	io.ntcreatex.in.alloc_size = 0;
+	io.ntcreatex.in.open_disposition = NTCREATEX_DISP_OPEN;
+	io.ntcreatex.in.impersonation = NTCREATEX_IMPERSONATION_ANONYMOUS;
+	io.ntcreatex.in.security_flags = 0;
+	io.ntcreatex.in.fname = fname_stream;
+
+	status = smb_raw_open(cli->tree, tctx, &io);
+	CHECK_STATUS(status, NT_STATUS_ACCESS_DENIED);
+
+	/* Change the attributes on the base file back. */
+	ZERO_STRUCT(sfinfo);
+	sfinfo.generic.level = RAW_SFILEINFO_SETATTR;
+	sfinfo.generic.in.file.path        = fname;
+	sfinfo.setattr.in.attrib           = 0;
+
+	status = smb_raw_setpathinfo(cli->tree, &sfinfo);
+	CHECK_STATUS(status, NT_STATUS_OK);
+
+	/* Re-open the file name. */
+
+	ZERO_STRUCT(io);
+	io.generic.level = RAW_OPEN_NTCREATEX;
+	io.ntcreatex.in.root_fid.fnum = 0;
+	io.ntcreatex.in.flags = 0;
+	io.ntcreatex.in.access_mask = (SEC_FILE_READ_DATA|SEC_FILE_WRITE_DATA|
+		SEC_STD_READ_CONTROL|SEC_STD_WRITE_DAC|
+		SEC_FILE_WRITE_ATTRIBUTE);
+	io.ntcreatex.in.create_options = 0;
+	io.ntcreatex.in.file_attr = 0;
+	io.ntcreatex.in.share_access = 0;
+	io.ntcreatex.in.alloc_size = 0;
+	io.ntcreatex.in.open_disposition = NTCREATEX_DISP_OPEN;
+	io.ntcreatex.in.impersonation = NTCREATEX_IMPERSONATION_ANONYMOUS;
+	io.ntcreatex.in.security_flags = 0;
+	io.ntcreatex.in.fname = fname;
+
+	status = smb_raw_open(cli->tree, tctx, &io);
+	CHECK_STATUS(status, NT_STATUS_OK);
+
+	fnum = io.ntcreatex.out.file.fnum;
+
+	/* Get the existing security descriptor. */
+	ZERO_STRUCT(q);
+	q.query_secdesc.level = RAW_FILEINFO_SEC_DESC;
+	q.query_secdesc.in.file.fnum = fnum;
+	q.query_secdesc.in.secinfo_flags =
+		SECINFO_OWNER |
+		SECINFO_GROUP |
+		SECINFO_DACL;
+	status = smb_raw_fileinfo(cli->tree, tctx, &q);
+	CHECK_STATUS(status, NT_STATUS_OK);
+	sd = q.query_secdesc.out.sd;
+
+	/* Now add a DENY WRITE security descriptor for Everyone. */
+	torture_comment(tctx, "add a new ACE to the DACL\n");
+
+	ace.type = SEC_ACE_TYPE_ACCESS_DENIED;
+	ace.flags = 0;
+	ace.access_mask = SEC_FILE_WRITE_DATA;
+	ace.trustee = *dom_sid_parse_talloc(tctx, SID_WORLD);
+
+	status = security_descriptor_dacl_add(sd, &ace);
+	CHECK_STATUS(status, NT_STATUS_OK);
+
+	/* security_descriptor_dacl_add adds to the *end* of
+	   the ace array, we need it at the start. Swap.. */
+	ace = sd->dacl->aces[0];
+	sd->dacl->aces[0] = sd->dacl->aces[sd->dacl->num_aces-1];
+	sd->dacl->aces[sd->dacl->num_aces-1] = ace;
+
+	ZERO_STRUCT(set);
+	set.set_secdesc.level = RAW_SFILEINFO_SEC_DESC;
+	set.set_secdesc.in.file.fnum = fnum;
+	set.set_secdesc.in.secinfo_flags = SECINFO_DACL;
+	set.set_secdesc.in.sd = sd;
+
+	status = smb_raw_setfileinfo(cli->tree, &set);
+	CHECK_STATUS(status, NT_STATUS_OK);
+
+	smbcli_close(cli->tree, fnum);
+	fnum = -1;
+
+	/* Try and open the stream name for WRITE_DATA. Should
+	   fail with ACCESS_DENIED. */
+
+	ZERO_STRUCT(io);
+	io.generic.level = RAW_OPEN_NTCREATEX;
+	io.ntcreatex.in.root_fid.fnum = 0;
+	io.ntcreatex.in.flags = 0;
+	io.ntcreatex.in.access_mask = SEC_FILE_WRITE_DATA;
+	io.ntcreatex.in.create_options = 0;
+	io.ntcreatex.in.file_attr = 0;
+	io.ntcreatex.in.share_access = 0;
+	io.ntcreatex.in.alloc_size = 0;
+	io.ntcreatex.in.open_disposition = NTCREATEX_DISP_OPEN;
+	io.ntcreatex.in.impersonation = NTCREATEX_IMPERSONATION_ANONYMOUS;
+	io.ntcreatex.in.security_flags = 0;
+	io.ntcreatex.in.fname = fname_stream;
+
+	status = smb_raw_open(cli->tree, tctx, &io);
+	CHECK_STATUS(status, NT_STATUS_ACCESS_DENIED);
+
+ done:
+
+	if (fnum != -1) {
+		smbcli_close(cli->tree, fnum);
+	}
+	smbcli_unlink(cli->tree, fname);
+
+	smbcli_deltree(cli->tree, BASEDIR);
+	return ret;
+}
+
 /* 
    basic testing of streams calls
 */
@@ -1905,6 +2085,7 @@ struct torture_suite *torture_raw_streams(TALLOC_CTX *tctx)
 	    test_stream_create_disposition);
 	torture_suite_add_1smb_test(suite, "attr", test_stream_attributes);
 	torture_suite_add_1smb_test(suite, "sumtab", test_stream_summary_tab);
+	torture_suite_add_1smb_test(suite, "perms", test_stream_permissions);
 
 #if 0
 	torture_suite_add_1smb_test(suite, "LARGESTREAMINFO",
diff --git a/source4/torture/smb2/rename.c b/source4/torture/smb2/rename.c
index a452acd..9d0f4e1 100644
--- a/source4/torture/smb2/rename.c
+++ b/source4/torture/smb2/rename.c
@@ -22,6 +22,8 @@
 #include "includes.h"
 #include "libcli/smb2/smb2.h"
 #include "libcli/smb2/smb2_calls.h"
+#include "lib/tevent/tevent.h"
+#include "lib/util/tevent_ntstatus.h"
 
 #include "torture/torture.h"
 #include "torture/smb2/proto.h"
@@ -926,6 +928,375 @@ done:
 	return ret;
 }
 
+struct rename_one_dir_cycle_state {
+	struct tevent_context *ev;
+	struct smb2_tree *tree;
+	struct smb2_handle file;
+	const char *base_name;
+	char *new_name;
+	unsigned *rename_counter;
+
+	unsigned current;
+	unsigned max;
+	union smb_setfileinfo sinfo;
+};
+
+static void rename_one_dir_cycle_done(struct smb2_request *subreq);
+
+static struct tevent_req *rename_one_dir_cycle_send(TALLOC_CTX *mem_ctx,
+						    struct tevent_context *ev,
+						    struct smb2_tree *tree,
+						    struct smb2_handle file,
+						    unsigned max_renames,
+						    const char *base_name,
+						    unsigned *rename_counter)
+{
+	struct tevent_req *req;
+	struct rename_one_dir_cycle_state *state;
+	struct smb2_request *subreq;
+
+	req = tevent_req_create(mem_ctx, &state,
+				struct rename_one_dir_cycle_state);
+	if (req == NULL) {
+		return NULL;
+	}
+	state->ev = ev;
+	state->tree = tree;
+	state->file = file;
+	state->base_name = base_name;
+	state->rename_counter = rename_counter;
+	state->current = 0;
+	state->max = max_renames;
+
+	ZERO_STRUCT(state->sinfo);
+	state->sinfo.rename_information.level =
+		RAW_SFILEINFO_RENAME_INFORMATION;
+	state->sinfo.rename_information.in.file.handle = state->file;
+	state->sinfo.rename_information.in.overwrite = 0;
+	state->sinfo.rename_information.in.root_fid = 0;
+
+	state->new_name = talloc_asprintf(
+		state, "%s-%u", state->base_name, state->current);
+	if (tevent_req_nomem(state->new_name, req)) {
+		return tevent_req_post(req, ev);
+	}
+	state->sinfo.rename_information.in.new_name = state->new_name;
+
+	subreq = smb2_setinfo_file_send(state->tree, &state->sinfo);
+	if (tevent_req_nomem(subreq, req)) {
+		return tevent_req_post(req, ev);
+	}
+	subreq->async.fn = rename_one_dir_cycle_done;
+	subreq->async.private_data = req;
+	return req;
+}
+
+static void rename_one_dir_cycle_done(struct smb2_request *subreq)
+{
+	struct tevent_req *req = talloc_get_type_abort(
+		subreq->async.private_data, struct tevent_req);
+	struct rename_one_dir_cycle_state *state = tevent_req_data(
+		req, struct rename_one_dir_cycle_state);
+	NTSTATUS status;
+
+	status = smb2_setinfo_recv(subreq);
+	if (tevent_req_nterror(req, status)) {
+		return;
+	}
+	TALLOC_FREE(state->new_name);
+
+	*state->rename_counter += 1;
+
+	state->current += 1;
+	if (state->current >= state->max) {
+		tevent_req_done(req);
+		return;
+	}
+
+	ZERO_STRUCT(state->sinfo);
+	state->sinfo.rename_information.level =
+		RAW_SFILEINFO_RENAME_INFORMATION;
+	state->sinfo.rename_information.in.file.handle = state->file;
+	state->sinfo.rename_information.in.overwrite = 0;
+	state->sinfo.rename_information.in.root_fid = 0;
+
+	state->new_name = talloc_asprintf(
+		state, "%s-%u", state->base_name, state->current);
+	if (tevent_req_nomem(state->new_name, req)) {
+		return;
+	}
+	state->sinfo.rename_information.in.new_name = state->new_name;
+
+	subreq = smb2_setinfo_file_send(state->tree, &state->sinfo);
+	if (tevent_req_nomem(subreq, req)) {
+		return;
+	}
+	subreq->async.fn = rename_one_dir_cycle_done;
+	subreq->async.private_data = req;
+}
+
+static NTSTATUS rename_one_dir_cycle_recv(struct tevent_req *req)
+{
+	return tevent_req_simple_recv_ntstatus(req);
+}
+
+struct rename_dir_bench_state {
+	struct tevent_context *ev;
+	struct smb2_tree *tree;
+	const char *base_name;
+	unsigned max_renames;
+	unsigned *rename_counter;
+
+	struct smb2_create io;
+	union smb_setfileinfo sinfo;
+	struct smb2_close cl;
+
+	struct smb2_handle file;
+};
+
+static void rename_dir_bench_opened(struct smb2_request *subreq);
+static void rename_dir_bench_renamed(struct tevent_req *subreq);
+static void rename_dir_bench_set_doc(struct smb2_request *subreq);
+static void rename_dir_bench_closed(struct smb2_request *subreq);
+
+static struct tevent_req *rename_dir_bench_send(TALLOC_CTX *mem_ctx,
+						struct tevent_context *ev,
+						struct smb2_tree *tree,
+						const char *base_name,
+						unsigned max_renames,
+						unsigned *rename_counter)
+{
+	struct tevent_req *req;
+	struct rename_dir_bench_state *state;
+	struct smb2_request *subreq;
+
+	req = tevent_req_create(mem_ctx, &state,
+				struct rename_dir_bench_state);
+	if (req == NULL) {
+		return NULL;
+	}
+	state->ev = ev;
+	state->tree = tree;
+	state->base_name = base_name;
+	state->max_renames = max_renames;
+	state->rename_counter = rename_counter;
+
+	ZERO_STRUCT(state->io);
+	state->io.in.desired_access = SEC_FLAG_MAXIMUM_ALLOWED;
+	state->io.in.share_access =
+		NTCREATEX_SHARE_ACCESS_READ|
+		NTCREATEX_SHARE_ACCESS_WRITE;
+	state->io.in.create_options = NTCREATEX_OPTIONS_DIRECTORY;
+	state->io.in.file_attributes = FILE_ATTRIBUTE_DIRECTORY;
+	state->io.in.create_disposition = NTCREATEX_DISP_OPEN_IF;
+	state->io.in.fname = state->base_name;
+
+	subreq = smb2_create_send(state->tree, &state->io);
+	if (tevent_req_nomem(subreq, req)) {
+		return tevent_req_post(req, ev);
+	}
+	subreq->async.fn = rename_dir_bench_opened;
+	subreq->async.private_data = req;
+	return req;
+}
+
+static void rename_dir_bench_opened(struct smb2_request *subreq)
+{
+	struct tevent_req *req = talloc_get_type_abort(
+		subreq->async.private_data, struct tevent_req);
+	struct rename_dir_bench_state *state = tevent_req_data(
+		req, struct rename_dir_bench_state);
+	struct smb2_create *io;
+	struct tevent_req *subreq2;


-- 
Samba Shared Repository


More information about the samba-cvs mailing list