[SCM] Samba Shared Repository - branch master updated

Jeremy Allison jra at samba.org
Thu May 27 19:52:02 UTC 2021


The branch, master has been updated
       via  9f57a3194a4 loadparam: add option "acl flag inherited canonicalization"
       via  86e09013920 smbd: pass fsp to canonicalize_inheritance_bits()
       via  31ea8ea875f torture/smb2: ACL inheritance flags test with non-canonical behaviour
      from  2f0cfe82907 s3: smbd: Fix uninitialized memory read in process_symlink_open() when used with vfs_shadow_copy2().

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


- Log -----------------------------------------------------------------
commit 9f57a3194a4cf5e0c383a8c6fdcf60c4e922a978
Author: Ralph Boehme <slow at samba.org>
Date:   Tue May 25 19:04:10 2021 +0200

    loadparam: add option "acl flag inherited canonicalization"
    
    Signed-off-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>
    
    Autobuild-User(master): Jeremy Allison <jra at samba.org>
    Autobuild-Date(master): Thu May 27 19:51:57 UTC 2021 on sn-devel-184

commit 86e0901392008b5d76e2cb5230609809e752d658
Author: Ralph Boehme <slow at samba.org>
Date:   Tue May 25 17:17:17 2021 +0200

    smbd: pass fsp to canonicalize_inheritance_bits()
    
    Signed-off-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

commit 31ea8ea875f960970661097b25a9d172be1bedb2
Author: Ralph Boehme <slow at samba.org>
Date:   Wed May 26 12:31:32 2021 +0200

    torture/smb2: ACL inheritance flags test with non-canonical behaviour
    
    Signed-off-by: Ralph Boehme <slow at samba.org>
    Reviewed-by: Jeremy Allison <jra at samba.org>

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

Summary of changes:
 .../security/aclflaginheritedcanonicalization.xml  |  30 ++++
 lib/param/loadparm.c                               |   4 +
 selftest/target/Samba3.pm                          |   4 +
 source3/param/loadparm.c                           |   1 +
 source3/selftest/tests.py                          |   2 +
 source3/smbd/nttrans.c                             |  10 +-
 source4/torture/smb2/acls.c                        | 173 +++++++++++++++++++++
 source4/torture/smb2/smb2.c                        |   1 +
 8 files changed, 223 insertions(+), 2 deletions(-)
 create mode 100644 docs-xml/smbdotconf/security/aclflaginheritedcanonicalization.xml


Changeset truncated at 500 lines:

diff --git a/docs-xml/smbdotconf/security/aclflaginheritedcanonicalization.xml b/docs-xml/smbdotconf/security/aclflaginheritedcanonicalization.xml
new file mode 100644
index 00000000000..676d5b478a3
--- /dev/null
+++ b/docs-xml/smbdotconf/security/aclflaginheritedcanonicalization.xml
@@ -0,0 +1,30 @@
+<samba:parameter name="acl flag inherited canonicalization"
+                 context="S"
+                 type="boolean"
+                 xmlns:samba="http://www.samba.org/samba/DTD/samba-doc">
+<description>
+	<para>This option controls the way Samba handles client requests setting
+	the Security Descriptor of files and directories and the effect the
+	operation has on the Security Descriptor flag "DACL
+	auto-inherited" (DI). Generally, this flag is set on a file (or
+	directory) upon creation if the parent directory has DI set and also has
+	inheritable ACEs.
+	</para>
+
+	<para>On the other hand when a Security Descriptor is explicitly set on
+	a file, the DI flag is cleared, unless the flag "DACL Inheritance
+	Required" (DR) is also set in the new Security Descriptor (fwiw, DR is
+	never stored on disk).</para>
+
+	<para>This is the default behaviour when this option is enabled (the
+	default). When setting this option to <command>no</command>, the
+	resulting value of the DI flag on-disk is directly taken from the DI
+	value of the to-be-set Security Descriptor. This can be used so dump
+	tools like rsync that copy data blobs from xattrs that represent ACLs
+	created by the acl_xattr VFS module will result in copies of the ACL
+	that are identical to the source. Without this option, the copied ACLs
+	would all loose the DI flag if set on the source.</para>
+</description>
+
+<value type="default">yes</value>
+</samba:parameter>
diff --git a/lib/param/loadparm.c b/lib/param/loadparm.c
index b674858e706..54920b85027 100644
--- a/lib/param/loadparm.c
+++ b/lib/param/loadparm.c
@@ -2960,6 +2960,10 @@ struct loadparm_context *loadparm_init(TALLOC_CTX *mem_ctx)
 				  "smbd max xattr size",
 				  "65536");
 
+	lpcfg_do_global_parameter(lp_ctx,
+				  "acl flag inherited canonicalization",
+				  "yes");
+
 	for (i = 0; parm_table[i].label; i++) {
 		if (!(lp_ctx->flags[i] & FLAG_CMDLINE)) {
 			lp_ctx->flags[i] |= FLAG_DEFAULT;
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index a6b3637efbe..84d3fd362ec 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -3067,6 +3067,10 @@ sub provision($$)
 [notify_priv]
 	copy = tmp
 	honor change notify privilege = yes
+
+[acls_non_canonical]
+	copy = tmp
+	acl flag inherited canonicalization = no
 	";
 
 	close(CONF);
diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
index 85e578eda9e..d3b9de4a09a 100644
--- a/source3/param/loadparm.c
+++ b/source3/param/loadparm.c
@@ -240,6 +240,7 @@ static const struct loadparm_service _sDefault =
 	.acl_map_full_control = true,
 	.acl_group_control = false,
 	.acl_allow_execute_always = false,
+	.acl_flag_inherited_canonicalization = true,
 	.aio_read_size = 1,
 	.aio_write_size = 1,
 	.map_readonly = MAP_READONLY_NO,
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index d4f9ea27ba6..4b81947510e 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -891,6 +891,8 @@ for t in tests:
         plansmbtorture4testsuite(t, "ad_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD')
     elif t == "smb2.fileid":
         plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/vfs_fruit_xattr -U$USERNAME%$PASSWORD')
+    elif t == "smb2.acls_non_canonical":
+        plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/acls_non_canonical -U$USERNAME%$PASSWORD')
     elif t == "rpc.wkssvc":
         plansmbtorture4testsuite(t, "ad_member", '//$SERVER/tmp -U$DC_USERNAME%$DC_PASSWORD')
     elif t == "rpc.srvsvc":
diff --git a/source3/smbd/nttrans.c b/source3/smbd/nttrans.c
index ab9b2f06b4f..00f551595d7 100644
--- a/source3/smbd/nttrans.c
+++ b/source3/smbd/nttrans.c
@@ -947,7 +947,8 @@ static void do_nt_transact_create_pipe(connection_struct *conn,
  same.
 *********************************************************************/
 
-static void canonicalize_inheritance_bits(struct security_descriptor *psd)
+static void canonicalize_inheritance_bits(struct files_struct *fsp,
+					  struct security_descriptor *psd)
 {
 	bool set_auto_inherited = false;
 
@@ -964,6 +965,11 @@ static void canonicalize_inheritance_bits(struct security_descriptor *psd)
 	 * for details.
 	 */
 
+	if (!lp_acl_flag_inherited_canonicalization(SNUM(fsp->conn))) {
+		psd->type &= ~SEC_DESC_DACL_AUTO_INHERIT_REQ;
+		return;
+	}
+
 	if ((psd->type & (SEC_DESC_DACL_AUTO_INHERITED|SEC_DESC_DACL_AUTO_INHERIT_REQ))
 			== (SEC_DESC_DACL_AUTO_INHERITED|SEC_DESC_DACL_AUTO_INHERIT_REQ)) {
 		set_auto_inherited = true;
@@ -1052,7 +1058,7 @@ NTSTATUS set_sd(files_struct *fsp, struct security_descriptor *psd,
 		}
 	}
 
-	canonicalize_inheritance_bits(psd);
+	canonicalize_inheritance_bits(fsp, psd);
 
 	if (DEBUGLEVEL >= 10) {
 		DEBUG(10,("set_sd for file %s\n", fsp_str_dbg(fsp)));
diff --git a/source4/torture/smb2/acls.c b/source4/torture/smb2/acls.c
index 4f4538b15e3..c06b1006c2d 100644
--- a/source4/torture/smb2/acls.c
+++ b/source4/torture/smb2/acls.c
@@ -3056,3 +3056,176 @@ struct torture_suite *torture_smb2_acls_init(TALLOC_CTX *ctx)
 
 	return suite;
 }
+
+static bool test_acls_non_canonical_flags(struct torture_context *tctx,
+					  struct smb2_tree *tree)
+{
+	const char *fname = BASEDIR "\\test_acls_non_canonical_flags.txt";
+	struct smb2_create cr;
+	struct smb2_handle testdirh = {{0}};
+	struct smb2_handle handle = {{0}};
+	union smb_fileinfo gi;
+	union smb_setfileinfo si;
+	struct security_descriptor *sd_orig = NULL;
+	struct security_descriptor *sd = NULL;
+	NTSTATUS status;
+	bool ret = true;
+
+	smb2_deltree(tree, BASEDIR);
+
+	status = torture_smb2_testdir(tree, BASEDIR, &testdirh);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+					"torture_smb2_testdir failed\n");
+
+	sd = security_descriptor_dacl_create(tctx,
+					     SEC_DESC_DACL_AUTO_INHERITED
+					     | SEC_DESC_DACL_AUTO_INHERIT_REQ,
+					     NULL,
+					     NULL,
+					     SID_WORLD,
+					     SEC_ACE_TYPE_ACCESS_ALLOWED,
+					     SEC_RIGHTS_DIR_ALL,
+					     SEC_ACE_FLAG_OBJECT_INHERIT
+					     | SEC_ACE_FLAG_CONTAINER_INHERIT,
+					     NULL);
+	torture_assert_not_null_goto(tctx, sd, ret, done,
+					"SD create failed\n");
+
+	si = (union smb_setfileinfo) {
+		.set_secdesc.level = RAW_SFILEINFO_SEC_DESC,
+		.set_secdesc.in.file.handle = testdirh,
+		.set_secdesc.in.secinfo_flags = SECINFO_DACL,
+		.set_secdesc.in.sd = sd,
+	};
+
+	status = smb2_setinfo_file(tree, &si);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+					"smb2_setinfo_file failed\n");
+
+	gi = (union smb_fileinfo) {
+		.query_secdesc.level = RAW_FILEINFO_SEC_DESC,
+		.query_secdesc.in.file.handle = testdirh,
+		.query_secdesc.in.secinfo_flags = SECINFO_DACL,
+	};
+
+	status = smb2_getinfo_file(tree, tctx, &gi);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+				"smb2_getinfo_file failed\n");
+
+	cr = (struct smb2_create) {
+		.in.desired_access = SEC_STD_READ_CONTROL |
+			SEC_STD_WRITE_DAC,
+		.in.file_attributes = FILE_ATTRIBUTE_NORMAL,
+		.in.share_access = NTCREATEX_SHARE_ACCESS_MASK,
+		.in.create_disposition = NTCREATEX_DISP_OPEN_IF,
+		.in.impersonation_level = NTCREATEX_IMPERSONATION_ANONYMOUS,
+		.in.fname = fname,
+	};
+
+	status = smb2_create(tree, tctx, &cr);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+					"smb2_create failed\n");
+	handle = cr.out.file.handle;
+
+	torture_comment(tctx, "get the original sd\n");
+
+	gi = (union smb_fileinfo) {
+		.query_secdesc.level = RAW_FILEINFO_SEC_DESC,
+		.query_secdesc.in.file.handle = handle,
+		.query_secdesc.in.secinfo_flags = SECINFO_DACL,
+	};
+
+	status = smb2_getinfo_file(tree, tctx, &gi);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+				"smb2_getinfo_file failed\n");
+
+	sd_orig = gi.query_secdesc.out.sd;
+
+	torture_assert_goto(tctx, sd_orig->type & SEC_DESC_DACL_AUTO_INHERITED,
+			    ret, done, "Missing SEC_DESC_DACL_AUTO_INHERITED\n");
+
+	/*
+	 * SD with SEC_DESC_DACL_AUTO_INHERITED but without
+	 * SEC_DESC_DACL_AUTO_INHERITED_REQ, so the resulting SD should not have
+	 * SEC_DESC_DACL_AUTO_INHERITED on a Windows box.
+	 *
+	 * But as we're testing against a share with
+	 *
+	 *    "acl flag inherited canonicalization = no"
+	 *
+	 * the resulting SD should have acl flag inherited canonicalization set.
+	 */
+	sd = security_descriptor_dacl_create(tctx,
+					     SEC_DESC_DACL_AUTO_INHERITED,
+					     NULL,
+					     NULL,
+					     SID_WORLD,
+					     SEC_ACE_TYPE_ACCESS_ALLOWED,
+					     SEC_FILE_ALL,
+					     0,
+					     NULL);
+	torture_assert_not_null_goto(tctx, sd, ret, done,
+					"SD create failed\n");
+
+	si = (union smb_setfileinfo) {
+		.set_secdesc.level = RAW_SFILEINFO_SEC_DESC,
+		.set_secdesc.in.file.handle = handle,
+		.set_secdesc.in.secinfo_flags = SECINFO_DACL,
+		.set_secdesc.in.sd = sd,
+	};
+
+	status = smb2_setinfo_file(tree, &si);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+					"smb2_setinfo_file failed\n");
+
+	status = smb2_util_close(tree, handle);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+					"smb2_util_close failed\n");
+	ZERO_STRUCT(handle);
+
+	cr = (struct smb2_create) {
+		.in.desired_access = SEC_FLAG_MAXIMUM_ALLOWED ,
+		.in.file_attributes = FILE_ATTRIBUTE_NORMAL,
+		.in.share_access = NTCREATEX_SHARE_ACCESS_MASK,
+		.in.create_disposition = NTCREATEX_DISP_OPEN,
+		.in.impersonation_level = NTCREATEX_IMPERSONATION_ANONYMOUS,
+		.in.fname = fname,
+	};
+
+	status = smb2_create(tree, tctx, &cr);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+					"smb2_create failed\n");
+	handle = cr.out.file.handle;
+
+	gi = (union smb_fileinfo) {
+		.query_secdesc.level = RAW_FILEINFO_SEC_DESC,
+		.query_secdesc.in.file.handle = handle,
+		.query_secdesc.in.secinfo_flags = SECINFO_DACL,
+	};
+
+	status = smb2_getinfo_file(tree, tctx, &gi);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+				"smb2_getinfo_file failed\n");
+
+	sd_orig = gi.query_secdesc.out.sd;
+	torture_assert_goto(tctx, sd_orig->type & SEC_DESC_DACL_AUTO_INHERITED,
+			    ret, done, "Missing SEC_DESC_DACL_AUTO_INHERITED\n");
+
+done:
+	if (!smb2_util_handle_empty(handle)) {
+		smb2_util_close(tree, testdirh);
+	}
+	if (!smb2_util_handle_empty(handle)) {
+		smb2_util_close(tree, handle);
+	}
+	smb2_deltree(tree, BASEDIR);
+	return ret;
+}
+
+struct torture_suite *torture_smb2_acls_non_canonical_init(TALLOC_CTX *ctx)
+{
+	struct torture_suite *suite = torture_suite_create(ctx, "acls_non_canonical");
+
+	torture_suite_add_1smb2_test(suite, "flags", test_acls_non_canonical_flags);
+	return suite;
+}
diff --git a/source4/torture/smb2/smb2.c b/source4/torture/smb2/smb2.c
index b5bcfe1a7de..f3a5c8ac875 100644
--- a/source4/torture/smb2/smb2.c
+++ b/source4/torture/smb2/smb2.c
@@ -157,6 +157,7 @@ NTSTATUS torture_smb2_init(TALLOC_CTX *ctx)
 	torture_suite_add_suite(suite, torture_smb2_twrp_init(suite));
 	torture_suite_add_suite(suite, torture_smb2_fileid_init(suite));
 	torture_suite_add_suite(suite, torture_smb2_acls_init(suite));
+	torture_suite_add_suite(suite, torture_smb2_acls_non_canonical_init(suite));
 	torture_suite_add_suite(suite, torture_smb2_notify_init(suite));
 	torture_suite_add_suite(suite, torture_smb2_notify_inotify_init(suite));
 	torture_suite_add_suite(suite,


-- 
Samba Shared Repository



More information about the samba-cvs mailing list