[SCM] Samba Shared Repository - branch master updated

Jeremy Allison jra at samba.org
Tue Mar 30 20:15:02 UTC 2021


The branch, master has been updated
       via  ff48422e639 s3: smbd: Fix SMB_VFS_FGET_NT_ACL/SMB_VFS_FSET_NT_ACL on stream handles.
       via  c7762a2bee2 s3: torture: Add a test for setting and getting ACLs on stream handles (SMB2-STREAM-ACL).
      from  9cff0a0c114 ldb-samba: remove redundant negative check

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


- Log -----------------------------------------------------------------
commit ff48422e63957dd863fda1ede622450312dcb45a
Author: Jeremy Allison <jra at samba.org>
Date:   Thu Mar 25 15:46:45 2021 -0700

    s3: smbd: Fix SMB_VFS_FGET_NT_ACL/SMB_VFS_FSET_NT_ACL on stream handles.
    
    As this is done on existing files, we know that
    fsp->base_fsp != NULL and fsp->base_fsp->fh->fd != -1
    (i.e. it's a pathref fd) for stream handles.
    
    When getting and setting ACLs on stream handles,
    use the fsp->base_fsp instead (as Windows does).
    
    This not only fixes streams_xattr, but will
    allow us to later analyze and remove all
    special casing code for get/set ACLs on streams
    handles.
    
    Remove the knownfail.d/stream-acl file.
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>
    
    Autobuild-User(master): Jeremy Allison <jra at samba.org>
    Autobuild-Date(master): Tue Mar 30 20:14:35 UTC 2021 on sn-devel-184

commit c7762a2bee2573421888b1fab8f3c86ea5348458
Author: Jeremy Allison <jra at samba.org>
Date:   Thu Mar 25 15:43:16 2021 -0700

    s3: torture: Add a test for setting and getting ACLs on stream handles (SMB2-STREAM-ACL).
    
    It shows this isn't done correctly for streams_xattr.
    
    A common config is:
    
    vfs_objects = streams_xattr acl_xattr
    
    to store both streams and Windows ACLs in xattrs.
    
    Unfortunately getting and setting ACLs using handles
    opened on stream files isn't being done correctly
    in Samba.
    
    This test passes against Windows 10.
    
    This adds tests that prove this doesn't work. Next
    patch will add the fix and remove the knownfail.
    
    Signed-off-by: Jeremy Allison <jra at samba.org>
    Reviewed-by: Ralph Boehme <slow at samba.org>

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

Summary of changes:
 source3/selftest/tests.py   |  15 ++++
 source3/smbd/nttrans.c      |  20 ++++-
 source3/torture/proto.h     |   1 +
 source3/torture/test_smb2.c | 207 ++++++++++++++++++++++++++++++++++++++++++++
 source3/torture/torture.c   |   4 +
 5 files changed, 245 insertions(+), 2 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 6efbea8bc4b..450a7159ef9 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -227,6 +227,21 @@ plantestsuite("samba3.smbtorture_s3.smb1.MSDFS-ATTRIBUTE",
                 "-mNT1",
                 "-f msdfs-src1"])
 
+#
+# SMB2-STREAM-ACL needs to run against a special share - vfs_wo_fruit
+#
+plantestsuite("samba3.smbtorture_s3.plain.%s" % "SMB2-STREAM-ACL",
+                "fileserver",
+                [os.path.join(samba3srcdir,
+                              "script/tests/test_smbtorture_s3.sh"),
+                'SMB2-STREAM-ACL',
+                '//$SERVER_IP/vfs_wo_fruit',
+                '$USERNAME',
+                '$PASSWORD',
+                smbtorture3,
+                "",
+                "-l $LOCAL_PATH"])
+
 shares = [
     "vfs_aio_pthread_async_dosmode_default1",
     "vfs_aio_pthread_async_dosmode_default2",
diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c
index 86426c36042..921f3fa692c 100644
--- a/source3/smbd/nttrans.c
+++ b/source3/smbd/nttrans.c
@@ -982,6 +982,7 @@ static void canonicalize_inheritance_bits(struct security_descriptor *psd)
 NTSTATUS set_sd(files_struct *fsp, struct security_descriptor *psd,
 		       uint32_t security_info_sent)
 {
+	files_struct *sd_fsp = fsp;
 	NTSTATUS status;
 
 	if (!CAN_WRITE(fsp->conn)) {
@@ -1058,7 +1059,14 @@ NTSTATUS set_sd(files_struct *fsp, struct security_descriptor *psd,
 		NDR_PRINT_DEBUG(security_descriptor, psd);
 	}
 
-	status = SMB_VFS_FSET_NT_ACL(fsp, security_info_sent, psd);
+	if (fsp->base_fsp != NULL) {
+		/*
+		 * This is a stream handle. Use
+		 * the underlying pathref handle.
+		 */
+		sd_fsp = fsp->base_fsp;
+	}
+	status = SMB_VFS_FSET_NT_ACL(sd_fsp, security_info_sent, psd);
 
 	TALLOC_FREE(psd);
 
@@ -2172,8 +2180,16 @@ NTSTATUS smbd_do_query_security_desc(connection_struct *conn,
 	    ((security_info_wanted & SECINFO_LABEL) == 0) &&
 	    need_to_read_sd)
 	{
+		files_struct *sd_fsp = fsp;
+		if (fsp->base_fsp != NULL) {
+			/*
+			 * This is a stream handle. Use
+			 * the underlying pathref handle.
+			 */
+			sd_fsp = fsp->base_fsp;
+		}
 		status = SMB_VFS_FGET_NT_ACL(
-			fsp, security_info_wanted, frame, &psd);
+			sd_fsp, security_info_wanted, frame, &psd);
 	} else {
 		status = get_null_nt_acl(frame, &psd);
 	}
diff --git a/source3/torture/proto.h b/source3/torture/proto.h
index 521c662e2fb..794a6044427 100644
--- a/source3/torture/proto.h
+++ b/source3/torture/proto.h
@@ -110,6 +110,7 @@ bool run_smb2_dir_fsync(int dummy);
 bool run_smb2_path_slash(int dummy);
 bool run_smb2_sacl(int dummy);
 bool run_smb2_quota1(int dummy);
+bool run_smb2_stream_acl(int dummy);
 bool run_chain3(int dummy);
 bool run_local_conv_auth_info(int dummy);
 bool run_local_sprintf_append(int dummy);
diff --git a/source3/torture/test_smb2.c b/source3/torture/test_smb2.c
index a81e40568e8..feb8b654f3b 100644
--- a/source3/torture/test_smb2.c
+++ b/source3/torture/test_smb2.c
@@ -2937,3 +2937,210 @@ bool run_smb2_quota1(int dummy)
 
 	return true;
 }
+
+bool run_smb2_stream_acl(int dummy)
+{
+	struct cli_state *cli = NULL;
+	NTSTATUS status;
+	uint16_t fnum = (uint16_t)-1;
+	const char *fname = "stream_acl_test_file";
+	const char *sname = "stream_acl_test_file:streamname";
+	struct security_descriptor *sd_dacl = NULL;
+	bool ret = false;
+
+	printf("SMB2 stream acl\n");
+
+	if (!torture_init_connection(&cli)) {
+		return false;
+	}
+
+	status = smbXcli_negprot(cli->conn,
+				cli->timeout,
+				PROTOCOL_SMB2_02,
+				PROTOCOL_SMB3_11);
+	if (!NT_STATUS_IS_OK(status)) {
+		printf("smbXcli_negprot returned %s\n", nt_errstr(status));
+		return false;
+	}
+
+	status = cli_session_setup_creds(cli, torture_creds);
+	if (!NT_STATUS_IS_OK(status)) {
+		printf("cli_session_setup returned %s\n", nt_errstr(status));
+		return false;
+	}
+
+	status = cli_tree_connect(cli, share, "?????", NULL);
+	if (!NT_STATUS_IS_OK(status)) {
+		printf("cli_tree_connect returned %s\n", nt_errstr(status));
+		return false;
+	}
+
+	/* Ensure file doesn't exist. */
+	(void)cli_unlink(cli, fname, 0);
+
+	/* Create the file. */
+	status = cli_ntcreate(cli,
+				fname,
+				0,
+				GENERIC_ALL_ACCESS,
+				FILE_ATTRIBUTE_NORMAL,
+				FILE_SHARE_NONE,
+				FILE_CREATE,
+				0,
+				0,
+				&fnum,
+				NULL);
+
+	if (!NT_STATUS_IS_OK(status)) {
+		printf("Create of %s failed (%s)\n",
+			fname,
+			nt_errstr(status));
+                goto fail;
+	}
+
+	/* Close the handle. */
+	cli_smb2_close_fnum(cli, fnum);
+	fnum = (uint16_t)-1;
+
+	/* Create the stream. */
+	status = cli_ntcreate(cli,
+				sname,
+				0,
+				FILE_READ_DATA|
+					SEC_STD_READ_CONTROL|
+					SEC_STD_WRITE_DAC,
+				FILE_ATTRIBUTE_NORMAL,
+				FILE_SHARE_NONE,
+				FILE_CREATE,
+				0,
+				0,
+				&fnum,
+				NULL);
+
+	if (!NT_STATUS_IS_OK(status)) {
+		printf("Create of %s failed (%s)\n",
+			sname,
+			nt_errstr(status));
+		goto fail;
+	}
+
+	/* Close the handle. */
+	cli_smb2_close_fnum(cli, fnum);
+	fnum = (uint16_t)-1;
+
+	/*
+	 * Open the stream - for Samba this ensures
+	 * we prove we have a pathref fsp.
+	 */
+	status = cli_ntcreate(cli,
+				sname,
+				0,
+				FILE_READ_DATA|
+					SEC_STD_READ_CONTROL|
+					SEC_STD_WRITE_DAC,
+				FILE_ATTRIBUTE_NORMAL,
+				FILE_SHARE_NONE,
+				FILE_OPEN,
+				0,
+				0,
+				&fnum,
+				NULL);
+
+	if (!NT_STATUS_IS_OK(status)) {
+		printf("Open of %s failed (%s)\n",
+			sname,
+			nt_errstr(status));
+                goto fail;
+	}
+
+	/* Read the security descriptor off the stream handle. */
+	status = cli_query_security_descriptor(cli,
+				fnum,
+				SECINFO_DACL,
+				talloc_tos(),
+				&sd_dacl);
+
+	if (!NT_STATUS_IS_OK(status)) {
+		printf("Reading DACL on stream %s got (%s)\n",
+			sname,
+			nt_errstr(status));
+		goto fail;
+	}
+
+	if (sd_dacl == NULL || sd_dacl->dacl == NULL ||
+			sd_dacl->dacl->num_aces < 1) {
+		printf("Invalid DACL returned on stream %s "
+			"(this should not happen)\n",
+			sname);
+		goto fail;
+	}
+
+	/*
+	 * Ensure it allows FILE_READ_DATA in the first ace.
+	 * It always should.
+	 */
+	if ((sd_dacl->dacl->aces[0].access_mask & FILE_READ_DATA) == 0) {
+		printf("DACL->ace[0] returned on stream %s "
+			"doesn't have read access (should not happen)\n",
+			sname);
+		goto fail;
+	}
+
+	/* Remove FILE_READ_DATA from the first ace and set. */
+	sd_dacl->dacl->aces[0].access_mask &= ~FILE_READ_DATA;
+
+	status = cli_set_security_descriptor(cli,
+				fnum,
+				SECINFO_DACL,
+				sd_dacl);
+
+	if (!NT_STATUS_IS_OK(status)) {
+		printf("Setting DACL on stream %s got (%s)\n",
+			sname,
+			nt_errstr(status));
+		goto fail;
+	}
+
+	TALLOC_FREE(sd_dacl);
+
+	/* Read again and check it changed. */
+	status = cli_query_security_descriptor(cli,
+				fnum,
+				SECINFO_DACL,
+				talloc_tos(),
+				&sd_dacl);
+
+	if (!NT_STATUS_IS_OK(status)) {
+		printf("Reading DACL on stream %s got (%s)\n",
+			sname,
+			nt_errstr(status));
+		goto fail;
+	}
+
+	if (sd_dacl == NULL || sd_dacl->dacl == NULL ||
+			sd_dacl->dacl->num_aces < 1) {
+		printf("Invalid DACL (1) returned on stream %s "
+			"(this should not happen)\n",
+			sname);
+		goto fail;
+	}
+
+	/* FILE_READ_DATA should be gone from the first ace. */
+	if ((sd_dacl->dacl->aces[0].access_mask & FILE_READ_DATA) != 0) {
+		printf("DACL on stream %s did not change\n",
+			sname);
+		goto fail;
+	}
+
+	ret = true;
+
+  fail:
+
+	if (fnum != (uint16_t)-1) {
+		cli_smb2_close_fnum(cli, fnum);
+		fnum = (uint16_t)-1;
+	}
+
+	(void)cli_unlink(cli, fname, 0);
+	return ret;
+}
diff --git a/source3/torture/torture.c b/source3/torture/torture.c
index add58414f33..2a78fb92bc4 100644
--- a/source3/torture/torture.c
+++ b/source3/torture/torture.c
@@ -15217,6 +15217,10 @@ static struct {
 		.name  = "SMB2-QUOTA1",
 		.fn    = run_smb2_quota1,
 	},
+	{
+		.name  = "SMB2-STREAM-ACL",
+		.fn    = run_smb2_stream_acl,
+	},
 	{
 		.name  = "CLEANUP1",
 		.fn    = run_cleanup1,


-- 
Samba Shared Repository



More information about the samba-cvs mailing list