[PATCHES][BUG 13673] smbd: Fix DELETE_ON_CLOSE behaviour on files with READ_ONLY attribute
Christof Schmitt
cs at samba.org
Sat Nov 3 00:03:45 UTC 2018
On Fri, Nov 02, 2018 at 04:32:04PM -0700, Jeremy Allison wrote:
> On Fri, Nov 02, 2018 at 03:50:27PM -0700, Jeremy Allison via samba-technical wrote:
> > On Fri, Nov 02, 2018 at 03:33:28PM -0700, Christof Schmitt wrote:
> > >
> > > Yes. The attached patches cover both cases now. I also discovered that
> > > when NT_STATUS_CANNOT_DELETE is returne for a new file, the file is
> > > still created. The easiest fix seems to be to unlink again. The question
> > > here is whether there is a good way to move the check for the new file
> > > case before actually creating the file.
> >
> > > /* 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,
> > > + new_dos_attributes);
> > > + if (!NT_STATUS_IS_OK(status)) {
> > > + int ret;
> > >
> > > - status = can_set_delete_on_close(fsp, new_dos_attributes);
> > > + ret = SMB_VFS_UNLINK(conn, fsp->fsp_name);
> > > + if (ret != 0) {
> > > + DBG_INFO("Unlink of new file %s failed "
> > > + "after DELETE_ON_CLOSE check "
> > > + "returned %s: %s\n",
> > > + smb_fname_str_dbg(smb_fname),
> > > + nt_errstr(status),
> > > + strerror(errno));
> > > + }
> > > + }
> > > + } else {
> > > + status = can_set_delete_on_close(fsp,
> > > + existing_dos_attributes);
> > > + }
> > >
> > > if (!NT_STATUS_IS_OK(status)) {
> > > /* Remember to delete the mode we just added. */
> >
> > I have to NAK the above I'm afraid. There's an inherent race
> > condition to do with calling SMB_VFS_UNLINK() even if we just
> > created the file.
>
>
> Here is a version that is rebased on top of the latest master
> I got through autobuild. Take a look at the changes I made
> to the:
>
> smbd: Fix DELETE_ON_CLOSE behaviour on files with READ_ONLY attribute
>
> patch. It handles the delete-on-close of a new file *before*
> we create the new file. Let me know if you're OK with it.
I was also getting there. The two if statements for existing files can be
combined to reduce the indentation. Also, for new files, shouldn't we
set fsp->initial_delete_on_close = True as well? I need that to get the
test to pass. (see attached version).
Christof
>
> Cheers,
>
> Jeremy.
> From 09ff412df885f83ebf05e323f807f48270a87db5 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 | 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 <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.
>
> 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 <cs at samba.org>
> ---
> 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 <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 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 <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 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
>
-------------- next part --------------
From ad3a613691bdb7305275c1aa93ac5e1b3ccd8d3c 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 | 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.17.0
From 151babd8a49bdc559aafad7e7c7def312894510f 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.
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 <cs at samba.org>
---
selftest/knownfail | 2 --
source3/smbd/open.c | 22 +++++++++++++++++++---
2 files changed, 19 insertions(+), 5 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..5c97c8478da 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -3280,6 +3280,22 @@ 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;
+ }
+
+ /* Note that here we set the *inital* delete on close flag,
+ not the regular one. The magic gets handled in close. */
+ fsp->initial_delete_on_close = True;
+ }
+
/*
* Ensure we pay attention to default ACLs on directories if required.
*/
@@ -3731,9 +3747,9 @@ 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);
+ if ((create_options & FILE_DELETE_ON_CLOSE) && !new_file_created) {
+ 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 b87abb0ee4597f3c631109367eee4e711cd96e38 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 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.17.0
From c1f1b2f808d97fa14f9a9cfb4f27fefe91110522 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 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.17.0
More information about the samba-technical
mailing list