Attachment 14197 Details for Bug 13432 – git-am fix for 4.8.next, 4.7.next

[patch] git-am fix for 4.8.next, 4.7.next

bug-13432-4.x (text/plain), 12.82 KB, created by Jeremy Allison on 2018-05-11 22:45:10 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeremy Allison

Created: 2018-05-11 22:45:10 UTC

Size: 12.82 KB

patch

obsolete

>From 62feea3df2242833133c4705ce1997811ddd4abf Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Thu, 10 May 2018 12:28:43 +0200
>Subject: [PATCH 1/2] s4:torture/smb2: new test for interaction between chown
> and SD flags
>
>This passes against Windows, but fails against Samba.
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=13432
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>(cherry picked from commit 12f6d56c4814fca64e0e3c636018e70d71ad0be5)
>---
> selftest/knownfail.d/samba3.smb2.acls |   1 +
> source4/torture/smb2/acls.c           | 278 ++++++++++++++++++++++++++
> 2 files changed, 279 insertions(+)
> create mode 100644 selftest/knownfail.d/samba3.smb2.acls
>
>diff --git a/selftest/knownfail.d/samba3.smb2.acls b/selftest/knownfail.d/samba3.smb2.acls
>new file mode 100644
>index 00000000000..68966c951a9
>--- /dev/null
>+++ b/selftest/knownfail.d/samba3.smb2.acls
>@@ -0,0 +1 @@
>+^samba3.smb2.acls.SDFLAGSVSCHOWN.*
>diff --git a/source4/torture/smb2/acls.c b/source4/torture/smb2/acls.c
>index 7365554470c..6178e211034 100644
>--- a/source4/torture/smb2/acls.c
>+++ b/source4/torture/smb2/acls.c
>@@ -1500,6 +1500,283 @@ done:
> 	return ret;
> }
> 
>+/*
>+ * This is basically a copy of test_inheritance_flags() with an additional twist
>+ * to change the owner of the testfile, verifying that the security descriptor
>+ * flags are not altered.
>+ */
>+static bool test_sd_flags_vs_chown(struct torture_context *tctx,
>+				   struct smb2_tree *tree)
>+{
>+	NTSTATUS status;
>+	struct smb2_create io;
>+	const char *dname = BASEDIR "\\inheritance";
>+	const char *fname1 = BASEDIR "\\inheritance\\testfile";
>+	bool ret = true;
>+	struct smb2_handle handle = {{0}};
>+	struct smb2_handle handle2 = {{0}};
>+	int i, j;
>+	union smb_fileinfo q;
>+	union smb_setfileinfo set;
>+	struct security_descriptor *sd, *sd2, *sd_orig=NULL;
>+	struct security_descriptor *owner_sd = NULL;
>+	const char *owner_sid_string = NULL;
>+	struct dom_sid *owner_sid = NULL;
>+	struct dom_sid world_sid = global_sid_World;
>+	struct {
>+		uint32_t parent_set_sd_type; /* 3 options */
>+		uint32_t parent_set_ace_inherit; /* 1 option */
>+		uint32_t parent_get_sd_type;
>+		uint32_t parent_get_ace_inherit;
>+		uint32_t child_get_sd_type;
>+		uint32_t child_get_ace_inherit;
>+	} tflags[16] = {{0}}; /* 2^4 */
>+
>+	owner_sd = security_descriptor_dacl_create(tctx,
>+						   0,
>+						   SID_WORLD,
>+						   NULL,
>+						   NULL);
>+	torture_assert_not_null_goto(tctx, owner_sd, ret, done,
>+				     "security_descriptor_dacl_create failed\n");
>+
>+	for (i = 0; i < 15; i++) {
>+		torture_comment(tctx, "i=%d:", i);
>+
>+		if (i & 1) {
>+			tflags[i].parent_set_sd_type |=
>+			    SEC_DESC_DACL_AUTO_INHERITED;
>+			torture_comment(tctx, "AUTO_INHERITED, ");
>+		}
>+		if (i & 2) {
>+			tflags[i].parent_set_sd_type |=
>+			    SEC_DESC_DACL_AUTO_INHERIT_REQ;
>+			torture_comment(tctx, "AUTO_INHERIT_REQ, ");
>+		}
>+		if (i & 4) {
>+			tflags[i].parent_set_sd_type |=
>+			    SEC_DESC_DACL_PROTECTED;
>+			torture_comment(tctx, "PROTECTED, ");
>+			tflags[i].parent_get_sd_type |=
>+			    SEC_DESC_DACL_PROTECTED;
>+		}
>+		if (i & 8) {
>+			tflags[i].parent_set_ace_inherit |=
>+			    SEC_ACE_FLAG_INHERITED_ACE;
>+			torture_comment(tctx, "INHERITED, ");
>+			tflags[i].parent_get_ace_inherit |=
>+			    SEC_ACE_FLAG_INHERITED_ACE;
>+		}
>+
>+		if ((tflags[i].parent_set_sd_type &
>+		    (SEC_DESC_DACL_AUTO_INHERITED | SEC_DESC_DACL_AUTO_INHERIT_REQ)) ==
>+		    (SEC_DESC_DACL_AUTO_INHERITED | SEC_DESC_DACL_AUTO_INHERIT_REQ)) {
>+			tflags[i].parent_get_sd_type |=
>+			    SEC_DESC_DACL_AUTO_INHERITED;
>+			tflags[i].child_get_sd_type |=
>+			    SEC_DESC_DACL_AUTO_INHERITED;
>+			tflags[i].child_get_ace_inherit |=
>+			    SEC_ACE_FLAG_INHERITED_ACE;
>+			torture_comment(tctx, "  ... parent is AUTO INHERITED");
>+		}
>+
>+		if (tflags[i].parent_set_ace_inherit &
>+		    SEC_ACE_FLAG_INHERITED_ACE) {
>+			tflags[i].parent_get_ace_inherit =
>+			    SEC_ACE_FLAG_INHERITED_ACE;
>+			torture_comment(tctx, "  ... parent ACE is INHERITED");
>+		}
>+
>+		torture_comment(tctx, "\n");
>+	}
>+
>+	if (!smb2_util_setup_dir(tctx, tree, BASEDIR))
>+		return false;
>+
>+	torture_comment(tctx, "TESTING ACL INHERITANCE FLAGS\n");
>+
>+	ZERO_STRUCT(io);
>+	io.level = RAW_OPEN_SMB2;
>+	io.in.create_flags = 0;
>+	io.in.desired_access = SEC_RIGHTS_FILE_ALL;
>+	io.in.create_options = NTCREATEX_OPTIONS_DIRECTORY;
>+	io.in.file_attributes = FILE_ATTRIBUTE_DIRECTORY;
>+	io.in.share_access = NTCREATEX_SHARE_ACCESS_MASK;
>+	io.in.alloc_size = 0;
>+	io.in.create_disposition = NTCREATEX_DISP_CREATE;
>+	io.in.impersonation_level = NTCREATEX_IMPERSONATION_ANONYMOUS;
>+	io.in.security_flags = 0;
>+	io.in.fname = dname;
>+
>+	torture_comment(tctx, "creating initial directory %s\n", dname);
>+	status = smb2_create(tree, tctx, &io);
>+	CHECK_STATUS(status, NT_STATUS_OK);
>+	handle = io.out.file.handle;
>+
>+	torture_comment(tctx, "getting original sd\n");
>+	q.query_secdesc.level = RAW_FILEINFO_SEC_DESC;
>+	q.query_secdesc.in.file.handle = handle;
>+	q.query_secdesc.in.secinfo_flags = SECINFO_DACL | SECINFO_OWNER;
>+	status = smb2_getinfo_file(tree, tctx, &q);
>+	CHECK_STATUS(status, NT_STATUS_OK);
>+	sd_orig = q.query_secdesc.out.sd;
>+
>+	owner_sid = sd_orig->owner_sid;
>+	owner_sid_string = dom_sid_string(tctx, sd_orig->owner_sid);
>+	torture_comment(tctx, "owner_sid is %s\n", owner_sid_string);
>+
>+	for (i=0; i < ARRAY_SIZE(tflags); i++) {
>+		torture_comment(tctx, "setting a new sd on directory, pass #%d\n", i);
>+
>+		sd = security_descriptor_dacl_create(tctx,
>+						tflags[i].parent_set_sd_type,
>+						NULL, NULL,
>+						owner_sid_string,
>+						SEC_ACE_TYPE_ACCESS_ALLOWED,
>+						SEC_FILE_WRITE_DATA | SEC_STD_WRITE_DAC,
>+						SEC_ACE_FLAG_OBJECT_INHERIT |
>+						SEC_ACE_FLAG_CONTAINER_INHERIT |
>+						tflags[i].parent_set_ace_inherit,
>+						SID_WORLD,
>+						SEC_ACE_TYPE_ACCESS_ALLOWED,
>+						SEC_FILE_ALL | SEC_STD_ALL,
>+						0,
>+						NULL);
>+		set.set_secdesc.level = RAW_SFILEINFO_SEC_DESC;
>+		set.set_secdesc.in.file.handle = handle;
>+		set.set_secdesc.in.secinfo_flags = SECINFO_DACL;
>+		set.set_secdesc.in.sd = sd;
>+		status = smb2_setinfo_file(tree, &set);
>+		CHECK_STATUS(status, NT_STATUS_OK);
>+
>+		/*
>+		 * Check DACL we just set, except change the bits to what they
>+		 * should be.
>+		 */
>+		torture_comment(tctx, "  checking new sd\n");
>+
>+		/* REQ bit should always be false. */
>+		sd->type &= ~SEC_DESC_DACL_AUTO_INHERIT_REQ;
>+
>+		if ((tflags[i].parent_get_sd_type & SEC_DESC_DACL_AUTO_INHERITED) == 0)
>+			sd->type &= ~SEC_DESC_DACL_AUTO_INHERITED;
>+
>+		q.query_secdesc.in.file.handle = handle;
>+		q.query_secdesc.in.secinfo_flags = SECINFO_DACL;
>+		status = smb2_getinfo_file(tree, tctx, &q);
>+		CHECK_STATUS(status, NT_STATUS_OK);
>+		CHECK_SECURITY_DESCRIPTOR(q.query_secdesc.out.sd, sd);
>+
>+		/* Create file. */
>+		torture_comment(tctx, "  creating file %s\n", fname1);
>+		io.in.fname = fname1;
>+		io.in.create_options = 0;
>+		io.in.file_attributes = FILE_ATTRIBUTE_NORMAL;
>+		io.in.desired_access = SEC_RIGHTS_FILE_ALL;
>+		io.in.create_disposition = NTCREATEX_DISP_CREATE;
>+		status = smb2_create(tree, tctx, &io);
>+		CHECK_STATUS(status, NT_STATUS_OK);
>+		handle2 = io.out.file.handle;
>+		CHECK_ACCESS_FLAGS(handle2, SEC_RIGHTS_FILE_ALL);
>+
>+		q.query_secdesc.in.file.handle = handle2;
>+		q.query_secdesc.in.secinfo_flags = SECINFO_DACL | SECINFO_OWNER;
>+		status = smb2_getinfo_file(tree, tctx, &q);
>+		CHECK_STATUS(status, NT_STATUS_OK);
>+
>+		torture_comment(tctx, "  checking sd on file %s\n", fname1);
>+		sd2 = security_descriptor_dacl_create(tctx,
>+						 tflags[i].child_get_sd_type,
>+						 owner_sid_string, NULL,
>+						 owner_sid_string,
>+						 SEC_ACE_TYPE_ACCESS_ALLOWED,
>+						 SEC_FILE_WRITE_DATA | SEC_STD_WRITE_DAC,
>+						 tflags[i].child_get_ace_inherit,
>+						 NULL);
>+		CHECK_SECURITY_DESCRIPTOR(q.query_secdesc.out.sd, sd2);
>+
>+		/*
>+		 * Set new sd on file ... prove that the bits have nothing to
>+		 * do with the parents bits when manually setting an ACL. The
>+		 * _AUTO_INHERITED bit comes directly from the ACL set.
>+		 */
>+		for (j = 0; j < ARRAY_SIZE(tflags); j++) {
>+			torture_comment(tctx, "  setting new file sd, pass #%d\n", j);
>+
>+			/* Change sd type. */
>+			sd2->type &= ~(SEC_DESC_DACL_AUTO_INHERITED |
>+			    SEC_DESC_DACL_AUTO_INHERIT_REQ |
>+			    SEC_DESC_DACL_PROTECTED);
>+			sd2->type |= tflags[j].parent_set_sd_type;
>+
>+			sd2->dacl->aces[0].flags &=
>+			    ~SEC_ACE_FLAG_INHERITED_ACE;
>+			sd2->dacl->aces[0].flags |=
>+			    tflags[j].parent_set_ace_inherit;
>+
>+			set.set_secdesc.level = RAW_SFILEINFO_SEC_DESC;
>+			set.set_secdesc.in.file.handle = handle2;
>+			set.set_secdesc.in.secinfo_flags = SECINFO_DACL;
>+			set.set_secdesc.in.sd = sd2;
>+			status = smb2_setinfo_file(tree, &set);
>+			CHECK_STATUS(status, NT_STATUS_OK);
>+
>+			/* Check DACL we just set. */
>+			sd2->type &= ~SEC_DESC_DACL_AUTO_INHERIT_REQ;
>+			if ((tflags[j].parent_get_sd_type & SEC_DESC_DACL_AUTO_INHERITED) == 0)
>+				sd2->type &= ~SEC_DESC_DACL_AUTO_INHERITED;
>+
>+			q.query_secdesc.in.file.handle = handle2;
>+			q.query_secdesc.in.secinfo_flags = SECINFO_DACL | SECINFO_OWNER;
>+			status = smb2_getinfo_file(tree, tctx, &q);
>+			CHECK_STATUS(status, NT_STATUS_OK);
>+
>+			CHECK_SECURITY_DESCRIPTOR(q.query_secdesc.out.sd, sd2);
>+
>+			/*
>+			 * Check that changing ownder doesn't affect SD flags.
>+			 *
>+			 * Do this by first changing ownder to world and then
>+			 * back to the original ownder. Afterwards compare SD,
>+			 * should be the same.
>+			 */
>+			owner_sd->owner_sid = &world_sid;
>+			set.set_secdesc.level = RAW_SFILEINFO_SEC_DESC;
>+			set.set_secdesc.in.file.handle = handle2;
>+			set.set_secdesc.in.secinfo_flags = SECINFO_OWNER;
>+			set.set_secdesc.in.sd = owner_sd;
>+			status = smb2_setinfo_file(tree, &set);
>+			CHECK_STATUS(status, NT_STATUS_OK);
>+
>+			owner_sd->owner_sid = owner_sid;
>+			set.set_secdesc.level = RAW_SFILEINFO_SEC_DESC;
>+			set.set_secdesc.in.file.handle = handle2;
>+			set.set_secdesc.in.secinfo_flags = SECINFO_OWNER;
>+			set.set_secdesc.in.sd = owner_sd;
>+			status = smb2_setinfo_file(tree, &set);
>+			CHECK_STATUS(status, NT_STATUS_OK);
>+
>+			q.query_secdesc.in.file.handle = handle2;
>+			q.query_secdesc.in.secinfo_flags = SECINFO_DACL | SECINFO_OWNER;
>+			status = smb2_getinfo_file(tree, tctx, &q);
>+			CHECK_STATUS(status, NT_STATUS_OK);
>+
>+			CHECK_SECURITY_DESCRIPTOR(q.query_secdesc.out.sd, sd2);
>+			torture_assert_goto(tctx, ret, ret, done, "CHECK_SECURITY_DESCRIPTOR failed\n");
>+		}
>+
>+		smb2_util_close(tree, handle2);
>+		smb2_util_unlink(tree, fname1);
>+	}
>+
>+done:
>+	smb2_util_close(tree, handle);
>+	smb2_deltree(tree, BASEDIR);
>+	smb2_tdis(tree);
>+	smb2_logoff(tree->session);
>+	return ret;
>+}
>+
> /*
>   test dynamic acl inheritance
>   Note: This test was copied from raw/acls.c.
>@@ -2098,6 +2375,7 @@ struct torture_suite *torture_smb2_acls_init(TALLOC_CTX *ctx)
> 	torture_suite_add_1smb2_test(suite, "OWNER", test_owner_bits);
> 	torture_suite_add_1smb2_test(suite, "INHERITANCE", test_inheritance);
> 	torture_suite_add_1smb2_test(suite, "INHERITFLAGS", test_inheritance_flags);
>+	torture_suite_add_1smb2_test(suite, "SDFLAGSVSCHOWN", test_sd_flags_vs_chown);
> 	torture_suite_add_1smb2_test(suite, "DYNAMIC", test_inheritance_dynamic);
> #if 0
> 	/* XXX This test does not work against XP or Vista. */
>-- 
>2.17.0.441.gb46fe60e1d-goog
>
>
>From 8cca6dbd105ce3545201059ea5a9236c4607bcc6 Mon Sep 17 00:00:00 2001
>From: Ralph Boehme <slow@samba.org>
>Date: Thu, 10 May 2018 12:29:35 +0200
>Subject: [PATCH 2/2] s3:smbd: fix interaction between chown and SD flags
>
>A change ownership operation that doesn't set the NT ACLs must not touch
>the SD flags (type).
>
>Bug: https://bugzilla.samba.org/show_bug.cgi?id=13432
>
>Signed-off-by: Ralph Boehme <slow@samba.org>
>Reviewed-by: Jeremy Allison <jra@samba.org>
>
>Autobuild-User(master): Jeremy Allison <jra@samba.org>
>Autobuild-Date(master): Fri May 11 23:30:32 CEST 2018 on sn-devel-144
>
>(cherry picked from commit ced55850034a3653525823bf9623912a4fcf18a0)
>---
> selftest/knownfail.d/samba3.smb2.acls | 1 -
> source3/modules/vfs_acl_common.c      | 7 +++++--
> 2 files changed, 5 insertions(+), 3 deletions(-)
> delete mode 100644 selftest/knownfail.d/samba3.smb2.acls
>
>diff --git a/selftest/knownfail.d/samba3.smb2.acls b/selftest/knownfail.d/samba3.smb2.acls
>deleted file mode 100644
>index 68966c951a9..00000000000
>--- a/selftest/knownfail.d/samba3.smb2.acls
>+++ /dev/null
>@@ -1 +0,0 @@
>-^samba3.smb2.acls.SDFLAGSVSCHOWN.*
>diff --git a/source3/modules/vfs_acl_common.c b/source3/modules/vfs_acl_common.c
>index 546e97b9b5d..f2b2df19400 100644
>--- a/source3/modules/vfs_acl_common.c
>+++ b/source3/modules/vfs_acl_common.c
>@@ -941,8 +941,11 @@ NTSTATUS fset_nt_acl_common(
> 	}
> 
> 	psd->revision = orig_psd->revision;
>-	/* All our SD's are self relative. */
>-	psd->type = orig_psd->type | SEC_DESC_SELF_RELATIVE;
>+	if (security_info_sent & SECINFO_DACL) {
>+		psd->type = orig_psd->type;
>+		/* All our SD's are self relative. */
>+		psd->type |= SEC_DESC_SELF_RELATIVE;
>+	}
> 
> 	if ((security_info_sent & SECINFO_OWNER) && (orig_psd->owner_sid != NULL)) {
> 		if (!dom_sid_equal(orig_psd->owner_sid, psd->owner_sid)) {
>-- 
>2.17.0.441.gb46fe60e1d-goog
>

Flags:

slow: review+

Actions: View

Attachments on bug 13432: 14197