Attachment 18436 Details for Bug 15695 – Patch for 4.20 and 4.21 cherry-picked from master

[patch] Patch for 4.20 and 4.21 cherry-picked from master

bug15695-v420,v421.patch (text/plain), 8.24 KB, created by Ralph Böhme on 2024-09-06 08:19:33 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ralph Böhme

Created: 2024-09-06 08:19:33 UTC

Size: 8.24 KB

patch

obsolete

>From 111896618de862cfb19852500100a49c47d9f086 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Sat, 6 Jul 2024 17:10:21 +0200
>Subject: [PATCH 1/2] smbtorture: test creating stream doesn't crash when using
> "inherit permissions = yes"
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15695
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>(cherry picked from commit 09835608307ff2580f1aada84d44feddae17c80f)
>---
> .../samba3.smb2.stream-inherit-perms          |  1 +
> selftest/target/Samba3.pm                     |  5 ++
> source3/selftest/tests.py                     |  2 +
> source4/torture/smb2/smb2.c                   |  2 +
> source4/torture/smb2/streams.c                | 73 +++++++++++++++++++
> 5 files changed, 83 insertions(+)
> create mode 100644 selftest/knownfail.d/samba3.smb2.stream-inherit-perms
>
>diff --git a/selftest/knownfail.d/samba3.smb2.stream-inherit-perms b/selftest/knownfail.d/samba3.smb2.stream-inherit-perms
>new file mode 100644
>index 000000000000..fa311ac924d1
>--- /dev/null
>+++ b/selftest/knownfail.d/samba3.smb2.stream-inherit-perms
>@@ -0,0 +1 @@
>+^samba3.smb2.stream-inherit-perms.stream-inherit-perms$fileserver$
>diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
>index aea64bf5d5df..a7dd1b20e660 100755
>--- a/selftest/target/Samba3.pm
>+++ b/selftest/target/Samba3.pm
>@@ -2126,6 +2126,11 @@ sub setup_fileserver
> 	comment = Home directories
> 	browseable = No
> 	read only = No
>+
>+[inherit_perms]
>+	path = $share_dir
>+	vfs objects = streams_depot
>+	inherit permissions = yes
> ";
> 
> 	if (defined($more_conf)) {
>diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
>index 2de6c8ecd456..88151caea11f 100755
>--- a/source3/selftest/tests.py
>+++ b/source3/selftest/tests.py
>@@ -1355,6 +1355,8 @@ tests = base + raw + smb2 + rpc + unix + local + rap + nbt + idmap + vfs
>         plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
>         plansmbtorture4testsuite(t, "ad_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD')
>         plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/streams_xattr -U$USERNAME%$PASSWORD', 'streams_xattr')
>+    elif t == "smb2.stream-inherit-perms":
>+        plansmbtorture4testsuite(t, "fileserver", '//$SERVER/inherit_perms -U$USERNAME%$PASSWORD')
>     elif t == "smb2.aio_delay":
>         plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/aio_delay_inject -U$USERNAME%$PASSWORD')
>     elif t == "smb2.delete-on-close-perms":
>diff --git a/source4/torture/smb2/smb2.c b/source4/torture/smb2/smb2.c
>index 5b6477e47bc3..28a62f49c47d 100644
>--- a/source4/torture/smb2/smb2.c
>+++ b/source4/torture/smb2/smb2.c
>@@ -178,6 +178,8 @@ NTSTATUS torture_smb2_init(TALLOC_CTX *ctx)
> 	torture_suite_add_suite(suite, torture_smb2_oplocks_init(suite));
> 	torture_suite_add_suite(suite, torture_smb2_kernel_oplocks_init(suite));
> 	torture_suite_add_suite(suite, torture_smb2_streams_init(suite));
>+	torture_suite_add_1smb2_test(suite, "stream-inherit-perms",
>+				     test_stream_inherit_perms);
> 	torture_suite_add_suite(suite, torture_smb2_ioctl_init(suite));
> 	torture_suite_add_simple_test(suite, "set-sparse-ioctl",
> 				      test_ioctl_set_sparse);
>diff --git a/source4/torture/smb2/streams.c b/source4/torture/smb2/streams.c
>index f18048f7762a..abc1fe219608 100644
>--- a/source4/torture/smb2/streams.c
>+++ b/source4/torture/smb2/streams.c
>@@ -30,6 +30,7 @@
> #include "system/filesys.h"
> #include "system/locale.h"
> #include "lib/util/tsort.h"
>+#include "libcli/security/security_descriptor.h"
> 
> #define DNAME "teststreams"
> 
>@@ -2395,6 +2396,78 @@ static bool test_basefile_rename_with_open_stream(struct torture_context *tctx,
> 	return ret;
> }
> 
>+/*
>+ * Simple test creating a stream on a share with "inherit permissions"
>+ * enabled. This tests specifically bug 15695.
>+ */
>+bool test_stream_inherit_perms(struct torture_context *tctx,
>+			       struct smb2_tree *tree)
>+{
>+	NTSTATUS status;
>+	struct smb2_handle h = {};
>+	union smb_fileinfo q = {};
>+	union smb_setfileinfo setinfo = {};
>+	struct security_descriptor *sd = NULL;
>+	struct security_ace ace = {};
>+	const char *fname = DNAME "\\test_stream_inherit_perms:stream";
>+	bool ret = true;
>+
>+	smb2_deltree(tree, DNAME);
>+
>+	status = torture_smb2_testdir(tree, DNAME, &h);
>+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
>+					"torture_smb2_testdir failed\n");
>+
>+	torture_comment(tctx, "getting original sd\n");
>+
>+	q.query_secdesc.level = RAW_FILEINFO_SEC_DESC;
>+	q.query_secdesc.in.file.handle = h;
>+	q.query_secdesc.in.secinfo_flags = SECINFO_DACL | SECINFO_OWNER;
>+
>+	status = smb2_getinfo_file(tree, tctx, &q);
>+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
>+					"smb2_getinfo_file failed\n");
>+
>+	sd = q.query_secdesc.out.sd;
>+
>+	/*
>+	 * Add one explicit non-inheriting ACE which will be stored
>+	 * as a non-inheriting POSIX ACE. These are the ACEs that
>+	 * "inherit permissions" will want to inherit.
>+	 */
>+	ace.type = SEC_ACE_TYPE_ACCESS_ALLOWED;
>+	ace.access_mask = SEC_STD_ALL;
>+	ace.trustee = *(sd->owner_sid);
>+
>+	status = security_descriptor_dacl_add(sd, &ace);
>+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
>+					"security_descriptor_dacl_add failed\n");
>+
>+	setinfo.set_secdesc.level = RAW_SFILEINFO_SEC_DESC;
>+	setinfo.set_secdesc.in.file.handle = h;
>+	setinfo.set_secdesc.in.secinfo_flags = SECINFO_DACL;
>+	setinfo.set_secdesc.in.sd = sd;
>+
>+	status = smb2_setinfo_file(tree, &setinfo);
>+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
>+					"smb2_setinfo_file failed");
>+
>+	smb2_util_close(tree, h);
>+	ZERO_STRUCT(h);
>+
>+	/* This triggers the crash */
>+	status = torture_smb2_testfile(tree, fname, &h);
>+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
>+					"torture_smb2_testfile failed");
>+
>+done:
>+	if (!smb2_util_handle_empty(h)) {
>+		smb2_util_close(tree, h);
>+	}
>+	smb2_deltree(tree, DNAME);
>+	return ret;
>+}
>+
> /*
>    basic testing of streams calls SMB2
> */
>-- 
>2.46.0
>
>
>From 9a37f23e04713ee8e2f64d32b3f33e708759ef86 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Fri, 5 Jul 2024 16:22:18 +0200
>Subject: [PATCH 2/2] smbd: use metadata_fsp(fsp) in copy_access_posix_acl()
> for SMB_VFS_SYS_ACL_SET_FD
>
>When inherting permissions on the created stream, we call into the VFS to fetch
>the streams security descriptor via inherit_access_posix_acl() ->
>copy_access_posix_acl() -> SMB_VFS_SYS_ACL_SET_FD() passing the stream fsp which
>triggers the assert SMB_ASSERT(!fsp_is_alternate_stream(fsp)) in
>vfswrap_sys_acl_set_fd() in vfs_default.
>
>Just passing the base fsp to the VFS fixes this.
>
>vfs_streams_depot which *does use* distinct backend filesystem files for the
>streams, currently does not apply permissions to the stream files at all, so the
>incomplete behaviour of vfs_streams_depot is not affected by this change.
>
>If in the future someone want to fix this defficiency in vfs_streams_depot, the
>module code can use fsp->stream_fsp to base decisions in VFS ops whether the
>module should carry out some action.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15695
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Volker Lendecke <vl@samba.org>
>
>Autobuild-User(master): Volker Lendecke <vl@samba.org>
>Autobuild-Date(master): Mon Sep  2 08:55:28 UTC 2024 on atb-devel-224
>
>(cherry picked from commit ecb8a99a2c7ba36f9adc50ef13cd8465a0c49b19)
>---
> selftest/knownfail.d/samba3.smb2.stream-inherit-perms | 1 -
> source3/smbd/posix_acls.c                             | 4 +++-
> 2 files changed, 3 insertions(+), 2 deletions(-)
> delete mode 100644 selftest/knownfail.d/samba3.smb2.stream-inherit-perms
>
>diff --git a/selftest/knownfail.d/samba3.smb2.stream-inherit-perms b/selftest/knownfail.d/samba3.smb2.stream-inherit-perms
>deleted file mode 100644
>index fa311ac924d1..000000000000
>--- a/selftest/knownfail.d/samba3.smb2.stream-inherit-perms
>+++ /dev/null
>@@ -1 +0,0 @@
>-^samba3.smb2.stream-inherit-perms.stream-inherit-perms$fileserver$
>diff --git a/source3/smbd/posix_acls.c b/source3/smbd/posix_acls.c
>index 0f6a0d52e019..53a9c53f54af 100644
>--- a/source3/smbd/posix_acls.c
>+++ b/source3/smbd/posix_acls.c
>@@ -4000,7 +4000,9 @@ static int copy_access_posix_acl(struct files_struct *from,
> 		goto done;
> 	}
> 
>-	ret = SMB_VFS_SYS_ACL_SET_FD(to, SMB_ACL_TYPE_ACCESS, posix_acl);
>+	ret = SMB_VFS_SYS_ACL_SET_FD(metadata_fsp(to),
>+				     SMB_ACL_TYPE_ACCESS,
>+				     posix_acl);
> 
>  done:
> 
>-- 
>2.46.0
>

Flags:

vl: review+

Actions: View

Attachments on bug 15695: 18436