From 09ff412df885f83ebf05e323f807f48270a87db5 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 --- 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 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..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 db19a5d21dcfc9748cd68aef7a1f429f333ea3e5 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 --- selftest/knownfail | 2 -- source3/smbd/open.c | 30 ++++++++++++++++++++++-------- 2 files changed, 22 insertions(+), 10 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..a323a42609e 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 1a8b729efef7976a4f103b0d0d7f275b9a2f3a87 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 --- selftest/target/Samba3.pm | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index f8fda35366b..91665114661 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -2256,6 +2256,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 70cecb2c62dfec7b4cc088a89b27191ca372b610 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 --- source3/selftest/tests.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index ae569258a78..20b96762e7a 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -622,6 +622,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