From b867343b0c889a1f2eda1858fcb27e828f43ede2 Mon Sep 17 00:00:00 2001 From: Christof Schmitt 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 Reviewed-by: Jeremy Allison (cherry picked from commit dc9bbbe4141d8425e66fe9290ff611845f4bd1ce) --- selftest/knownfail | 2 + source4/torture/smb2/delete-on-close.c | 119 +++++++++++++++++++++++++ 2 files changed, 121 insertions(+) diff --git a/selftest/knownfail b/selftest/knownfail index 84776d4f35d..781c14551fe 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -348,3 +348,5 @@ # Disabling NTLM means you can't use samr to change the password ^samba.tests.ntlmdisabled.python\(ktest\).ntlmdisabled.NtlmDisabledTests.test_samr_change_password\(ktest\) ^samba.tests.ntlmdisabled.python\(ad_dc_no_ntlm\).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..12cdb8540b8 100644 --- a/source4/torture/smb2/delete-on-close.c +++ b/source4/torture/smb2/delete-on-close.c @@ -580,6 +580,124 @@ 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"); + + create = (struct smb2_create) { }; + create.in.desired_access = SEC_RIGHTS_DIR_ALL; + create.in.create_options = NTCREATEX_OPTIONS_NON_DIRECTORY_FILE | + NTCREATEX_OPTIONS_DELETE_ON_CLOSE; + 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_equal_goto(tctx, status, expected_status, ret, + done, "Unexpected status for CREATE " + "of new file.\n"); + + if (delete_readonly) { + 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, "Creating file with READ_ONLY attribute.\n"); + + create = (struct smb2_create) { }; + 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 +713,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 80a482a9890df81edde4c20fed6cb277c352be7f Mon Sep 17 00:00:00 2001 From: Christof Schmitt 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. We need to handle the new file and existing file cases separately. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13673 Signed-off-by: Christof Schmitt Reviewed-by: Jeremy Allison (cherry picked from commit 162a5257c48f20d3752f644e86c9e626b46436c0) --- selftest/knownfail | 2 -- source3/smbd/open.c | 30 ++++++++++++++++++++++-------- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/selftest/knownfail b/selftest/knownfail index 781c14551fe..84776d4f35d 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -348,5 +348,3 @@ # Disabling NTLM means you can't use samr to change the password ^samba.tests.ntlmdisabled.python\(ktest\).ntlmdisabled.NtlmDisabledTests.test_samr_change_password\(ktest\) ^samba.tests.ntlmdisabled.python\(ad_dc_no_ntlm\).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 8a9288dbdb4..97cf458a864 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -3237,6 +3237,18 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, request_time = fsp->open_time; } + if ((create_options & FILE_DELETE_ON_CLOSE) && + (flags2 & O_CREAT) && + !file_existed) { + /* Delete on close semantics for new files. */ + status = can_set_delete_on_close(fsp, + new_dos_attributes); + if (!NT_STATUS_IS_OK(status)) { + fd_close(fsp); + return status; + } + } + /* * Ensure we pay attention to default ACLs on directories if required. */ @@ -3689,15 +3701,17 @@ static NTSTATUS open_file_ntcreate(connection_struct *conn, /* Handle strange delete on close create semantics. */ if (create_options & FILE_DELETE_ON_CLOSE) { + if (!new_file_created) { + status = can_set_delete_on_close(fsp, + existing_dos_attributes); - status = can_set_delete_on_close(fsp, new_dos_attributes); - - if (!NT_STATUS_IS_OK(status)) { - /* Remember to delete the mode we just added. */ - del_share_mode(lck, fsp); - TALLOC_FREE(lck); - fd_close(fsp); - return status; + if (!NT_STATUS_IS_OK(status)) { + /* Remember to delete the mode we just added. */ + del_share_mode(lck, fsp); + TALLOC_FREE(lck); + fd_close(fsp); + return status; + } } /* Note that here we set the *inital* delete on close flag, not the regular one. The magic gets handled in close. */ -- 2.17.0 From 84ff58f642dc40581db6603c6e55d64cb5e87385 Mon Sep 17 00:00:00 2001 From: Christof Schmitt 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 Reviewed-by: Jeremy Allison (cherry picked from commit a8e79decbcfbae1b1a53ec81b942ee06db26bf8f) --- selftest/target/Samba3.pm | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index 968c6aa7ba0..3747a1e087e 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -2231,6 +2231,10 @@ sub provision($$$$$$$$$) vfs objects = delay_inject delay_inject:pread_send = 2000 delay_inject:pwrite_send = 2000 + +[delete_readonly] + path = $prefix_abs/share + delete readonly = yes "; close(CONF); -- 2.17.0 From 2452cc8712b5539a695bcf9e04da2f27bddb7629 Mon Sep 17 00:00:00 2001 From: Christof Schmitt 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 Reviewed-by: Jeremy Allison Autobuild-User(master): Christof Schmitt Autobuild-Date(master): Sat Nov 3 05:55:45 CET 2018 on sn-devel-144 (cherry picked from commit 7dd3585f9c3ae04df45d98bfdc62663c7a69d3e0) --- source3/selftest/tests.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index 0407e368b7c..a6de080b1d3 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -579,6 +579,10 @@ for t in tests: plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/streams_xattr -U$USERNAME%$PASSWORD', 'streams_xattr') elif t == "smb2.aio_delay": plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/aio_delay_inject -U$USERNAME%$PASSWORD') + 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