From fc8a004d45784d8dd686b5ccc33a70625c8ab434 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 baf3d57a31a..6a8c237638b 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -349,3 +349,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.19.1.930.g4563a0d9d0-goog From 97d2f0009d7de7893826461c02ba8367125523f8 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 6a8c237638b..baf3d57a31a 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -349,5 +349,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 db5eb4e459b..3ae8adb5f4f 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -3280,6 +3280,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. */ @@ -3732,15 +3744,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.19.1.930.g4563a0d9d0-goog From ebfb1bfe010b80f8fc0f54535cb3358731e666c0 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 50f579ade3e..438cb3409bb 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -2207,6 +2207,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.19.1.930.g4563a0d9d0-goog From 4b5092180e991de70c1edcfe779b4372b5d10468 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 9b665907395..2763b1bf146 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -585,6 +585,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.19.1.930.g4563a0d9d0-goog