[PATCHES][BUG 13673] smbd: Fix DELETE_ON_CLOSE behaviour on files with READ_ONLY attribute
Christof Schmitt
cs at samba.org
Fri Nov 2 20:29:55 UTC 2018
smbd allowed CREATE on a file with the READONLY attribute set and
DELETE_ON_CLOSE requested, even though MS-FSA states that this should be
denied. On the other hand, SETINFO for DELETE_ON_CLOSE is correctly
rejected.
See the attached patches for a fix. This also adds a smbtorture test for
this case and the Samba override 'delete readonly=yes'
gitlab pipeline
https://gitlab.com/samba-team/devel/samba/pipelines/35263504
Christof
-------------- next part --------------
From 26452449e999f3197971f5188660899a352e757d Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Fri, 2 Nov 2018 10:49:53 -0700
Subject: [PATCH 1/4] smbtorture: Add test for DELETE_ON_CLOSE on files with
READ_ONLY attribute
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13673
Signed-off-by: Christof Schmitt <cs at samba.org>
---
selftest/knownfail | 2 +
source4/torture/smb2/delete-on-close.c | 95 ++++++++++++++++++++++++++
2 files changed, 97 insertions(+)
diff --git a/selftest/knownfail b/selftest/knownfail
index 16c2274daec..ed1383cace8 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -356,3 +356,5 @@
^samba.tests.ntlmdisabled.python\(ad_dc_no_ntlm\).ntlmdisabled.NtlmDisabledTests.test_ntlm_connection\(ad_dc_no_ntlm\)
^samba.tests.ntlmdisabled.python\(ktest\).python3.ntlmdisabled.NtlmDisabledTests.test_samr_change_password\(ktest\)
^samba.tests.ntlmdisabled.python\(ad_dc_no_ntlm\).python3.ntlmdisabled.NtlmDisabledTests.test_ntlm_connection\(ad_dc_no_ntlm\)
+^samba3.smb2.delete-on-close-perms.READONLY\(nt4_dc\)
+^samba3.smb2.delete-on-close-perms.READONLY\(ad_dc\)
diff --git a/source4/torture/smb2/delete-on-close.c b/source4/torture/smb2/delete-on-close.c
index 2312df285a3..9651c9dae29 100644
--- a/source4/torture/smb2/delete-on-close.c
+++ b/source4/torture/smb2/delete-on-close.c
@@ -580,6 +580,100 @@ static bool test_doc_find_and_set_doc(struct torture_context *tctx, struct smb2_
return true;
}
+static bool test_doc_read_only(struct torture_context *tctx,
+ struct smb2_tree *tree)
+{
+ struct smb2_handle dir_handle;
+ union smb_setfileinfo sfinfo = { };
+ struct smb2_create create = { };
+ struct smb2_close close = { };
+ NTSTATUS status, expected_status;
+ bool ret = true, delete_readonly;
+
+ /*
+ * Allow testing of the Samba 'delete readonly' option.
+ */
+ delete_readonly = torture_setting_bool(tctx, "delete_readonly", false);
+ expected_status = delete_readonly ?
+ NT_STATUS_OK : NT_STATUS_CANNOT_DELETE;
+
+ smb2_deltree(tree, DNAME);
+
+ status = torture_smb2_testdir(tree, DNAME, &dir_handle);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "CREATE directory failed\n");
+
+ torture_comment(tctx, "Creating file with READ_ONLY attribute.\n");
+
+ create.in.desired_access = SEC_RIGHTS_DIR_ALL;
+ create.in.create_options = NTCREATEX_OPTIONS_NON_DIRECTORY_FILE;
+ create.in.file_attributes = FILE_ATTRIBUTE_READONLY;
+ create.in.share_access = NTCREATEX_SHARE_ACCESS_READ |
+ NTCREATEX_SHARE_ACCESS_WRITE |
+ NTCREATEX_SHARE_ACCESS_DELETE;
+ create.in.create_disposition = NTCREATEX_DISP_CREATE;
+ create.in.fname = FNAME;
+ status = smb2_create(tree, tctx, &create);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "CREATE of READONLY file failed.\n");
+
+ close.in.file.handle = create.out.file.handle;
+ status = smb2_close(tree, &close);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "CLOSE of READONLY file failed.\n");
+
+ torture_comment(tctx, "Testing CREATE with DELETE_ON_CLOSE on "
+ "READ_ONLY attribute file.\n");
+
+ create = (struct smb2_create) { };
+ create.in.desired_access = SEC_RIGHTS_FILE_READ | SEC_STD_DELETE;
+ create.in.create_options = NTCREATEX_OPTIONS_DELETE_ON_CLOSE;
+ create.in.file_attributes = 0;
+ create.in.share_access = NTCREATEX_SHARE_ACCESS_READ |
+ NTCREATEX_SHARE_ACCESS_WRITE |
+ NTCREATEX_SHARE_ACCESS_DELETE;
+ create.in.create_disposition = NTCREATEX_DISP_OPEN;
+ create.in.fname = FNAME;
+ status = smb2_create(tree, tctx, &create);
+ torture_assert_ntstatus_equal_goto(tctx, status,
+ expected_status, ret, done,
+ "CREATE returned unexpected "
+ "status.\n");
+
+ torture_comment(tctx, "Testing setting DELETE_ON_CLOSE disposition on "
+ " file with READONLY attribute.\n");
+
+ create = (struct smb2_create) { };
+ create.in.desired_access = SEC_RIGHTS_FILE_READ | SEC_STD_DELETE;;
+ create.in.create_options = 0;
+ create.in.file_attributes = 0;
+ create.in.share_access = NTCREATEX_SHARE_ACCESS_READ |
+ NTCREATEX_SHARE_ACCESS_WRITE |
+ NTCREATEX_SHARE_ACCESS_DELETE;
+ create.in.create_disposition = NTCREATEX_DISP_OPEN;
+ create.in.fname = FNAME;
+ status = smb2_create(tree, tctx, &create);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "Opening file failed.\n");
+
+ sfinfo.disposition_info.in.delete_on_close = 1;
+ sfinfo.generic.level = RAW_SFILEINFO_DISPOSITION_INFORMATION;
+ sfinfo.generic.in.file.handle = create.out.file.handle;
+
+ status = smb2_setinfo_file(tree, &sfinfo);
+ torture_assert_ntstatus_equal(tctx, status, expected_status,
+ "Set DELETE_ON_CLOSE disposition "
+ "returned un expected status.\n");
+
+ status = smb2_util_close(tree, create.out.file.handle);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "CLOSE failed\n");
+
+done:
+ smb2_deltree(tree, DNAME);
+ return ret;
+}
+
/*
* Extreme testing of Delete On Close and permissions
@@ -595,6 +689,7 @@ struct torture_suite *torture_smb2_doc_init(TALLOC_CTX *ctx)
torture_suite_add_1smb2_test(suite, "CREATE_IF", test_doc_create_if);
torture_suite_add_1smb2_test(suite, "CREATE_IF Existing", test_doc_create_if_exist);
torture_suite_add_1smb2_test(suite, "FIND_and_set_DOC", test_doc_find_and_set_doc);
+ torture_suite_add_1smb2_test(suite, "READONLY", test_doc_read_only);
suite->description = talloc_strdup(suite, "SMB2-Delete-on-Close-Perms tests");
--
2.17.0
From 5ee4b7cc81e7b5dc12d51b401ee9819cab9a3f00 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Fri, 2 Nov 2018 12:08:23 -0700
Subject: [PATCH 2/4] smbd: Fix DELETE_ON_CLOSE behaviour on files with
READ_ONLY attribute
MS-FSA states that a CREATE with FILE_DELETE_ON_CLOSE on an existing
file with READ_ONLY attribute has to return STATUS_CANNOT_DELETE. This
was missing in smbd as the check used the DOS attributes from the CREATE
instead of the DOS attributes on the existing file.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13673
Signed-off-by: Christof Schmitt <cs at samba.org>
---
selftest/knownfail | 2 --
source3/smbd/open.c | 2 +-
2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/selftest/knownfail b/selftest/knownfail
index ed1383cace8..16c2274daec 100644
--- a/selftest/knownfail
+++ b/selftest/knownfail
@@ -356,5 +356,3 @@
^samba.tests.ntlmdisabled.python\(ad_dc_no_ntlm\).ntlmdisabled.NtlmDisabledTests.test_ntlm_connection\(ad_dc_no_ntlm\)
^samba.tests.ntlmdisabled.python\(ktest\).python3.ntlmdisabled.NtlmDisabledTests.test_samr_change_password\(ktest\)
^samba.tests.ntlmdisabled.python\(ad_dc_no_ntlm\).python3.ntlmdisabled.NtlmDisabledTests.test_ntlm_connection\(ad_dc_no_ntlm\)
-^samba3.smb2.delete-on-close-perms.READONLY\(nt4_dc\)
-^samba3.smb2.delete-on-close-perms.READONLY\(ad_dc\)
diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index d6359aac0c6..27222c02807 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -3733,7 +3733,7 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn,
/* Handle strange delete on close create semantics. */
if (create_options & FILE_DELETE_ON_CLOSE) {
- status = can_set_delete_on_close(fsp, new_dos_attributes);
+ status = can_set_delete_on_close(fsp, existing_dos_attributes);
if (!NT_STATUS_IS_OK(status)) {
/* Remember to delete the mode we just added. */
--
2.17.0
From a0bdc438aaf88666d1bd6ea32138a134863db8ad Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Fri, 2 Nov 2018 12:03:51 -0700
Subject: [PATCH 3/4] selftest: Add share to test "delete readonly" option
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13673
Signed-off-by: Christof Schmitt <cs at samba.org>
---
selftest/target/Samba3.pm | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index f7957cf3a7d..c5c62b24bb2 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -2250,6 +2250,10 @@ sub provision($$$$$$$$$)
kernel oplocks = no
posix locking = no
include = $libdir/delay_inject.conf
+
+[delete_readonly]
+ path = $prefix_abs/share
+ delete readonly = yes
";
close(CONF);
--
2.17.0
From 0d23b3986dc503dab075566d80cd62410b58ae1b Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Fri, 2 Nov 2018 12:07:58 -0700
Subject: [PATCH 4/4] selftest: Run smb2.delete-on-close-perms also with
"delete readonly = yes"
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13673
Signed-off-by: Christof Schmitt <cs at samba.org>
---
source3/selftest/tests.py | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 08395c1c096..4253c7f63c0 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -620,6 +620,10 @@ for t in tests:
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.delete-on-close-perms":
+ plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
+ plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/delete_readonly -U$USERNAME%$PASSWORD --option=torture:delete_readonly=true')
+ plansmbtorture4testsuite(t, "ad_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD')
else:
plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD')
plansmbtorture4testsuite(t, "ad_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD')
--
2.17.0
More information about the samba-technical
mailing list