Attachment 9397 Details for Bug 10229 – git-am patchset for 4.1.x

[patch] git-am patchset for 4.1.x

bug-10229.patch (text/plain), 10.95 KB, created by Karolin Seeger on 2013-11-07 10:39:52 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Karolin Seeger

Created: 2013-11-07 10:39:52 UTC

Size: 10.95 KB

patch

obsolete

>From b6c8d90ab1686e30f646950636ad64fe9985b662 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Mon, 28 Oct 2013 16:59:20 -0700
>Subject: [PATCH 1/2] 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@samba.org>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: David Disseldorp <ddiss@suse.de>
>(cherry picked from commit 60f922bf1bd8816eacbb32c24793ad1f97a1d9f2)
>---
> source3/smbd/open.c |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 59 insertions(+)
>
>diff --git a/source3/smbd/open.c b/source3/smbd/open.c
>index 10b04e7..cd1bb72 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.
> ****************************************************************************/
> 
>@@ -3764,6 +3804,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. */
>-- 
>1.7.10.4
>
>
>From ebb92082c74b82eccae0f108048edddcf99a5f96 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Tue, 29 Oct 2013 15:57:01 -0700
>Subject: [PATCH 2/2] 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@samba.org>
>Reviewed-by: Stefan Metzmacher <metze@samba.org>
>Reviewed-by: David Disseldorp <ddiss@suse.de>
>
>Autobuild-User(master): Jeremy Allison <jra@samba.org>
>Autobuild-Date(master): Mon Nov  4 23:10:10 CET 2013 on sn-devel-104
>(cherry picked from commit 65882152cc7ccaba0e7903862b99ca93594ed080)
>---
> selftest/knownfail            |    1 +
> source4/torture/raw/streams.c |  181 +++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 182 insertions(+)
>
>diff --git a/selftest/knownfail b/selftest/knownfail
>index 0c501fa..2586947 100644
>--- a/selftest/knownfail
>+++ b/selftest/knownfail
>@@ -144,6 +144,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/source4/torture/raw/streams.c b/source4/torture/raw/streams.c
>index 1611c64..61852f5 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",
>-- 
>1.7.10.4
>

Flags:

metze: review+
ddiss: review+

Actions: View

Attachments on bug 10229: 9351 | 9352 | 9381 | 9382 | 9397