Attachment 12748 Details for Bug 12460 – git-am fix for 4.5.next.

[patch] git-am fix for 4.5.next.

4.5.patch (text/plain), 12.49 KB, created by Jeremy Allison on 2016-12-07 22:12:23 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeremy Allison

Created: 2016-12-07 22:12:23 UTC

Size: 12.49 KB

patch

obsolete

>From 9b60e55018532fcc0871d5c3f78e7376e0a19cd5 Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Mon, 5 Dec 2016 14:13:14 -0800
>Subject: [PATCH 1/4] s3: smbd: rename - missing early error exit if source and
> destination prefixes are different.
>
>Noticed by Michael Zeis <mzeis.quantum@gmail.com>.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12460
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>(cherry picked from commit 2bfad1c9d3237ad8d174b7dc2d1e6e3c53fdb8dc)
>---
> source3/smbd/reply.c | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
>index 0aec433..6ddfa4f 100644
>--- a/source3/smbd/reply.c
>+++ b/source3/smbd/reply.c
>@@ -6812,6 +6812,7 @@ NTSTATUS rename_internals_fsp(connection_struct *conn,
> 
> 	if (rename_path_prefix_equal(fsp->fsp_name, smb_fname_dst)) {
> 		status = NT_STATUS_ACCESS_DENIED;
>+		goto out;
> 	}
> 
> 	lck = get_existing_share_mode_lock(talloc_tos(), fsp->file_id);
>-- 
>2.8.0.rc3.226.g39d4020
>
>
>From 9165b30e522bf06c21ff9d651b10321b5d6f33ee Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Mon, 5 Dec 2016 14:32:03 -0800
>Subject: [PATCH 2/4] s3: smbd: Make check_parent_access() available to rename
> code.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12460
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>(cherry picked from commit beb8a73e95e768565760f79c2a16586bafb4e58c)
>---
> source3/smbd/open.c  | 2 +-
> source3/smbd/proto.h | 3 +++
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
>diff --git a/source3/smbd/open.c b/source3/smbd/open.c
>index 9d10d19..e9bfdec 100644
>--- a/source3/smbd/open.c
>+++ b/source3/smbd/open.c
>@@ -235,7 +235,7 @@ NTSTATUS smbd_check_access_rights(struct connection_struct *conn,
> 	return NT_STATUS_OK;
> }
> 
>-static NTSTATUS check_parent_access(struct connection_struct *conn,
>+NTSTATUS check_parent_access(struct connection_struct *conn,
> 				struct smb_filename *smb_fname,
> 				uint32_t access_mask)
> {
>diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
>index 352d28c..50ede9d 100644
>--- a/source3/smbd/proto.h
>+++ b/source3/smbd/proto.h
>@@ -642,6 +642,9 @@ NTSTATUS smbd_check_access_rights(struct connection_struct *conn,
> 				const struct smb_filename *smb_fname,
> 				bool use_privs,
> 				uint32_t access_mask);
>+NTSTATUS check_parent_access(struct connection_struct *conn,
>+				struct smb_filename *smb_fname,
>+				uint32_t access_mask);
> NTSTATUS fd_open(struct connection_struct *conn, files_struct *fsp,
> 		 int flags, mode_t mode);
> NTSTATUS fd_close(files_struct *fsp);
>-- 
>2.8.0.rc3.226.g39d4020
>
>
>From f37952ce616e4b917e9ec464d8b037fc9c1c35ab Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Mon, 5 Dec 2016 14:32:55 -0800
>Subject: [PATCH 3/4] s3: smbd: Add missing permissions check on destination
> folder.
>
>Based on code from Michael Zeis <mzeis.quantum@gmail.com>.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12460
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>(cherry picked from commit 91b591224ab7f8ea7b4594da9f61efef14353f7f)
>---
> source3/smbd/reply.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
>diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
>index 6ddfa4f..6acbaca 100644
>--- a/source3/smbd/reply.c
>+++ b/source3/smbd/reply.c
>@@ -6615,6 +6615,7 @@ NTSTATUS rename_internals_fsp(connection_struct *conn,
> 	struct smb_filename *smb_fname_dst = NULL;
> 	NTSTATUS status = NT_STATUS_OK;
> 	struct share_mode_lock *lck = NULL;
>+	uint32_t access_mask = SEC_DIR_ADD_FILE;
> 	bool dst_exists, old_is_stream, new_is_stream;
> 
> 	status = check_name(conn, smb_fname_dst_in->base_name);
>@@ -6815,6 +6816,22 @@ NTSTATUS rename_internals_fsp(connection_struct *conn,
> 		goto out;
> 	}
> 
>+	/* Do we have rights to move into the destination ? */
>+	if (S_ISDIR(fsp->fsp_name->st.st_ex_mode)) {
>+		/* We're moving a directory. */
>+		access_mask = SEC_DIR_ADD_SUBDIR;
>+	}
>+	status = check_parent_access(conn,
>+				smb_fname_dst,
>+				access_mask);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		DBG_INFO("check_parent_access on "
>+			"dst %s returned %s\n",
>+			smb_fname_str_dbg(smb_fname_dst),
>+			nt_errstr(status));
>+		goto out;
>+	}
>+
> 	lck = get_existing_share_mode_lock(talloc_tos(), fsp->file_id);
> 
> 	/*
>-- 
>2.8.0.rc3.226.g39d4020
>
>
>From 5e7224de3e7bf678d3b84742ebd98ab1034fe30c Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Mon, 5 Dec 2016 14:34:18 -0800
>Subject: [PATCH 4/4] s3: torture: Regression test case for permissions check
> on rename.
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12460
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>Reviewed-by: Ralph Boehme <slow@samba.org>
>
>Autobuild-User(master): Ralph BÃ¶hme <slow@samba.org>
>Autobuild-Date(master): Wed Dec  7 11:52:03 CET 2016 on sn-devel-144
>
>(cherry picked from commit 52fad16f1c20109f352c25832d841ff778b2518a)
>---
> selftest/skip             |   1 +
> source3/selftest/tests.py |   5 ++
> source3/torture/torture.c | 192 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 198 insertions(+)
>
>diff --git a/selftest/skip b/selftest/skip
>index ba6718a..478e6c9 100644
>--- a/selftest/skip
>+++ b/selftest/skip
>@@ -48,6 +48,7 @@
> ^samba3.smbtorture_s3.plain$ad_dc_ntvfs$.POSIX-SYMLINK-EA # Fails against the s4 ntvfs server
> ^samba3.smbtorture_s3.plain$ad_dc_ntvfs$.POSIX-OFD-LOCK # Fails against the s4 ntvfs server
> ^samba3.smbtorture_s3.plain$ad_dc_ntvfs$.POSIX-STREAM-DELETE # Fails against the s4 ntvfs server
>+^samba3.smbtorture_s3.plain$ad_dc_ntvfs$.RENAME-ACCESS # Fails against the s4 ntvfs server
> ^samba3.smbtorture_s3.plain$ad_dc_ntvfs$.PIDHIGH # Fails against the s4 ntvfs server
> ^samba3.smbtorture_s3.plain$ad_dc_ntvfs$.NTTRANS-FSCTL # Fails against the s4 ntvfs server
> ^samba3.smbtorture_s3.plain$ad_dc_ntvfs$.SMB2-NEGPROT # Fails against the s4 ntvfs server
>diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
>index 7cfebd6..6d06548 100755
>--- a/source3/selftest/tests.py
>+++ b/source3/selftest/tests.py
>@@ -68,6 +68,11 @@ for t in tests:
>         plantestsuite("samba3.smbtorture_s3.crypt_server(nt4_dc).%s" % t, "nt4_dc", [os.path.join(samba3srcdir, "script/tests/test_smbtorture_s3.sh"), t, '//$SERVER_IP/tmpenc', '$USERNAME', '$PASSWORD', smbtorture3, "", "-l $LOCAL_PATH"])
>     plantestsuite("samba3.smbtorture_s3.plain(ad_dc_ntvfs).%s" % t, "ad_dc_ntvfs", [os.path.join(samba3srcdir, "script/tests/test_smbtorture_s3.sh"), t, '//$SERVER_IP/tmp', '$USERNAME', '$PASSWORD', smbtorture3, "", "-l $LOCAL_PATH"])
> 
>+#
>+# RENAME-ACCESS needs to run against a special share - acl_xattr_ign_sysacl_windows
>+#
>+plantestsuite("samba3.smbtorture_s3.plain(nt4_dc).%s" % "RENAME-ACCESS","nt4_dc", [os.path.join(samba3srcdir, "script/tests/test_smbtorture_s3.sh"), "RENAME-ACCESS", '//$SERVER_IP/acl_xattr_ign_sysacl_windows', '$USERNAME', '$PASSWORD', smbtorture3, "", "-l $LOCAL_PATH"])
>+plantestsuite("samba3.smbtorture_s3.crypt_client(nt4_dc).%s" % "RENAME-ACCESS", "nt4_dc", [os.path.join(samba3srcdir, "script/tests/test_smbtorture_s3.sh"), "RENAME-ACCESS", '//$SERVER_IP/acl_xattr_ign_sysacl_windows', '$USERNAME', '$PASSWORD', smbtorture3, "-e", "-l $LOCAL_PATH"])
> # non-crypt only
> 
> tests = ["OPLOCK-CANCEL"]
>diff --git a/source3/torture/torture.c b/source3/torture/torture.c
>index ecacd1a..90d46dd 100644
>--- a/source3/torture/torture.c
>+++ b/source3/torture/torture.c
>@@ -4898,6 +4898,197 @@ static bool run_rename(int dummy)
> 	return correct;
> }
> 
>+/*
>+  Test rename into a directory with an ACL denying it.
>+ */
>+static bool run_rename_access(int dummy)
>+{
>+	static struct cli_state *cli = NULL;
>+	static struct cli_state *posix_cli = NULL;
>+	const char *src = "test.txt";
>+	const char *dname = "dir";
>+	const char *dst = "dir\\test.txt";
>+	const char *dsrc = "test.dir";
>+	const char *ddst = "dir\\test.dir";
>+	uint16_t fnum = (uint16_t)-1;
>+	struct security_descriptor *sd = NULL;
>+	struct security_descriptor *newsd = NULL;
>+	NTSTATUS status;
>+	TALLOC_CTX *frame = NULL;
>+
>+	frame = talloc_stackframe();
>+	printf("starting rename access test\n");
>+
>+	/* Windows connection. */
>+	if (!torture_open_connection(&cli, 0)) {
>+		goto fail;
>+	}
>+
>+	smbXcli_conn_set_sockopt(cli->conn, sockops);
>+
>+	/* Posix connection. */
>+	if (!torture_open_connection(&posix_cli, 0)) {
>+		goto fail;
>+	}
>+
>+	smbXcli_conn_set_sockopt(posix_cli->conn, sockops);
>+
>+	status = torture_setup_unix_extensions(posix_cli);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		goto fail;
>+	}
>+
>+	/* Start with a clean slate. */
>+	cli_unlink(cli, src, FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN);
>+	cli_unlink(cli, dst, FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN);
>+	cli_rmdir(cli, dsrc);
>+	cli_rmdir(cli, ddst);
>+	cli_rmdir(cli, dname);
>+
>+	/*
>+	 * Setup the destination directory with a DENY ACE to
>+	 * prevent new files within it.
>+	 */
>+	status = cli_ntcreate(cli,
>+				dname,
>+				0,
>+				FILE_READ_ATTRIBUTES|READ_CONTROL_ACCESS|
>+					WRITE_DAC_ACCESS|FILE_READ_DATA|
>+					WRITE_OWNER_ACCESS,
>+				FILE_ATTRIBUTE_DIRECTORY,
>+				FILE_SHARE_READ|FILE_SHARE_WRITE,
>+				FILE_CREATE,
>+				FILE_DIRECTORY_FILE,
>+				0,
>+				&fnum,
>+				NULL);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		printf("Create of %s - %s\n", dname, nt_errstr(status));
>+		goto fail;
>+	}
>+
>+	status = cli_query_secdesc(cli,
>+				fnum,
>+				frame,
>+				&sd);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		printf("cli_query_secdesc failed for %s (%s)\n",
>+			dname, nt_errstr(status));
>+		goto fail;
>+	}
>+
>+	newsd = security_descriptor_dacl_create(frame,
>+					0,
>+					NULL,
>+					NULL,
>+					SID_WORLD,
>+					SEC_ACE_TYPE_ACCESS_DENIED,
>+					SEC_DIR_ADD_FILE|SEC_DIR_ADD_SUBDIR,
>+					0,
>+					NULL);
>+	if (newsd == NULL) {
>+		goto fail;
>+	}
>+	sd->dacl = security_acl_concatenate(frame,
>+					newsd->dacl,
>+					sd->dacl);
>+	if (sd->dacl == NULL) {
>+		goto fail;
>+	}
>+	status = cli_set_secdesc(cli, fnum, sd);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		printf("cli_set_secdesc failed for %s (%s)\n",
>+			dname, nt_errstr(status));
>+		goto fail;
>+	}
>+	status = cli_close(cli, fnum);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		printf("close failed for %s (%s)\n",
>+			dname, nt_errstr(status));
>+		goto fail;
>+	}
>+	/* Now go around the back and chmod to 777 via POSIX. */
>+	status = cli_posix_chmod(posix_cli, dname, 0777);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		printf("cli_posix_chmod failed for %s (%s)\n",
>+			dname, nt_errstr(status));
>+		goto fail;
>+	}
>+
>+	/* Check we can't create a file within dname via Windows. */
>+	status = cli_openx(cli, dst, O_RDWR|O_CREAT|O_EXCL, DENY_NONE, &fnum);
>+	if (!NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) {
>+		cli_close(posix_cli, fnum);
>+		printf("Create of %s should be ACCESS denied, was %s\n",
>+			dst, nt_errstr(status));
>+		goto fail;
>+	}
>+
>+	/* Make the sample file/directory. */
>+	status = cli_openx(cli, src, O_RDWR|O_CREAT|O_EXCL, DENY_NONE, &fnum);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		printf("open of %s failed (%s)\n", src, nt_errstr(status));
>+		goto fail;
>+	}
>+	status = cli_close(cli, fnum);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		printf("cli_close failed (%s)\n", nt_errstr(status));
>+		goto fail;
>+	}
>+
>+	status = cli_mkdir(cli, dsrc);
>+	if (!NT_STATUS_IS_OK(status)) {
>+		printf("cli_mkdir of %s failed (%s)\n",
>+			dsrc, nt_errstr(status));
>+		goto fail;
>+	}
>+
>+	/*
>+	 * OK - renames of the new file and directory into the
>+	 * dst directory should fail.
>+	 */
>+
>+	status = cli_rename(cli, src, dst);
>+	if (!NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) {
>+		printf("rename of %s -> %s should be ACCESS denied, was %s\n",
>+			src, dst, nt_errstr(status));
>+		goto fail;
>+	}
>+	status = cli_rename(cli, dsrc, ddst);
>+	if (!NT_STATUS_EQUAL(status, NT_STATUS_ACCESS_DENIED)) {
>+		printf("rename of %s -> %s should be ACCESS denied, was %s\n",
>+			src, dst, nt_errstr(status));
>+		goto fail;
>+	}
>+
>+	TALLOC_FREE(frame);
>+	return true;
>+
>+  fail:
>+
>+	if (posix_cli) {
>+		torture_close_connection(posix_cli);
>+	}
>+
>+	if (cli) {
>+		if (fnum != -1) {
>+			cli_close(cli, fnum);
>+		}
>+		cli_unlink(cli, src,
>+			FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN);
>+		cli_unlink(cli, dst,
>+			FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN);
>+		cli_rmdir(cli, dsrc);
>+		cli_rmdir(cli, ddst);
>+		cli_rmdir(cli, dname);
>+
>+		torture_close_connection(cli);
>+	}
>+
>+	TALLOC_FREE(frame);
>+	return false;
>+}
>+
> static bool run_pipe_number(int dummy)
> {
> 	struct cli_state *cli1;
>@@ -10454,6 +10645,7 @@ static struct {
> #endif
> 	{"XCOPY", run_xcopy, 0},
> 	{"RENAME", run_rename, 0},
>+	{"RENAME-ACCESS", run_rename_access, 0},
> 	{"DELETE", run_deletetest, 0},
> 	{"WILDDELETE", run_wild_deletetest, 0},
> 	{"DELETE-LN", run_deletetest_ln, 0},
>-- 
>2.8.0.rc3.226.g39d4020
>

Flags:

slow: review+

Actions: View

Attachments on bug 12460: 12731 | 12748 | 12749