[SCM] Samba Shared Repository - branch master updated

Ralph Böhme slow at samba.org
Wed Dec 7 10:53:03 UTC 2016


The branch, master has been updated
       via  52fad16 s3: torture: Regression test case for permissions check on rename.
       via  91b5912 s3: smbd: Add missing permissions check on destination folder.
       via  beb8a73 s3: smbd: Make check_parent_access() available to rename code.
       via  2bfad1c s3: smbd: rename - missing early error exit if source and destination prefixes are different.
      from  3aecad2 winbind: dom_sid_parse_endp always initializes "endp" when ok

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


- Log -----------------------------------------------------------------
commit 52fad16f1c20109f352c25832d841ff778b2518a
Author: Jeremy Allison <jra at samba.org>
Date:   Mon Dec 5 14:34:18 2016 -0800

    s3: torture: Regression test case for permissions check on rename.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12460
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    
    Autobuild-User(master): Ralph Böhme <slow at samba.org>
    Autobuild-Date(master): Wed Dec  7 11:52:03 CET 2016 on sn-devel-144

commit 91b591224ab7f8ea7b4594da9f61efef14353f7f
Author: Jeremy Allison <jra at samba.org>
Date:   Mon Dec 5 14:32:55 2016 -0800

    s3: smbd: Add missing permissions check on destination folder.
    
    Based on code from Michael Zeis <mzeis.quantum at gmail.com>.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12460
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

commit beb8a73e95e768565760f79c2a16586bafb4e58c
Author: Jeremy Allison <jra at samba.org>
Date:   Mon Dec 5 14:32:03 2016 -0800

    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 at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

commit 2bfad1c9d3237ad8d174b7dc2d1e6e3c53fdb8dc
Author: Jeremy Allison <jra at samba.org>
Date:   Mon Dec 5 14:13:14 2016 -0800

    s3: smbd: rename - missing early error exit if source and destination prefixes are different.
    
    Noticed by Michael Zeis <mzeis.quantum at gmail.com>.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12460
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

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

Summary of changes:
 selftest/skip             |   1 +
 source3/selftest/tests.py |   5 ++
 source3/smbd/open.c       |   2 +-
 source3/smbd/proto.h      |   3 +
 source3/smbd/reply.c      |  18 +++++
 source3/torture/torture.c | 192 ++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 220 insertions(+), 1 deletion(-)


Changeset truncated at 500 lines:

diff --git a/selftest/skip b/selftest/skip
index ebef0e8..0893962 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 7a4e2d7..a678c77 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/smbd/open.c b/source3/smbd/open.c
index c6de2dc..42db659 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);
diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index 0aec433..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);
@@ -6812,6 +6813,23 @@ 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;
+	}
+
+	/* 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);
diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index c7fd5a0..ba50462 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -4895,6 +4895,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;
@@ -10453,6 +10644,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},


-- 
Samba Shared Repository



More information about the samba-cvs mailing list