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 <s...@samba.org>
Date: Tue May 25 19:04:10 2021 +0200
loadparam: add option "acl flag inherited canonicalization"
Signed-off-by: Ralph Boehme <s...@samba.org>
Reviewed-by: Jeremy Allison <j...@samba.org>
Autobuild-User(master): Jeremy Allison <j...@samba.org>
Autobuild-Date(master): Thu May 27 19:51:57 UTC 2021 on sn-devel-184
commit 86e0901392008b5d76e2cb5230609809e752d658
Author: Ralph Boehme <s...@samba.org>
Date: Tue May 25 17:17:17 2021 +0200
smbd: pass fsp to canonicalize_inheritance_bits()
Signed-off-by: Ralph Boehme <s...@samba.org>
Reviewed-by: Jeremy Allison <j...@samba.org>
commit 31ea8ea875f960970661097b25a9d172be1bedb2
Author: Ralph Boehme <s...@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 <s...@samba.org>
Reviewed-by: Jeremy Allison <j...@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
