[SCM] Samba Shared Repository - branch master updated

[email protected] (Ralph B??hme)
Newsgroups gmane.network.samba.cvs
Message-ID <[email protected]>
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 <[email protected]>
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 <[email protected]>
    Reviewed-by: Ralph Boehme <[email protected]>
    
    Autobuild-User(master): Ralph Böhme <[email protected]>
    Autobuild-Date(master): Wed Dec  7 11:52:03 CET 2016 on sn-devel-144

commit 91b591224ab7f8ea7b4594da9f61efef14353f7f
Author: Jeremy Allison <[email protected]>
Date:   Mon Dec 5 14:32:55 2016 -0800

    s3: smbd: Add missing permissions check on destination folder.
    
    Based on code from Michael Zeis <[email protected]>.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12460
    
    Signed-off-by: Jeremy Allison <[email protected]>
    Reviewed-by: Ralph Boehme <[email protected]>

commit beb8a73e95e768565760f79c2a16586bafb4e58c
Author: Jeremy Allison <[email protected]>
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 <[email protected]>
    Reviewed-by: Ralph Boehme <[email protected]>

commit 2bfad1c9d3237ad8d174b7dc2d1e6e3c53fdb8dc
Author: Jeremy Allison <[email protected]>
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 <[email protected]>.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12460
    
    Signed-off-by: Jeremy Allison <[email protected]>
    Reviewed-by: Ralph Boehme <[email protected]>

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

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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.