The branch, v4-15-test has been updated via f2e124e423c s3: smbd: Don't allow setting the delete on close bit on a directory if it contains non-visible files and "delete veto files = no". via bbd8bdc14c8 s3: torture: Add 2 new tests SMB2-DEL-ON-CLOSE-NONWRITE-DELETE-NO, SMB2-DEL-ON-CLOSE-NONWRITE-DELETE-YES. from 1b1a20dd579 WHATSNEW: Mention our matrix room as well
https://git.samba.org/?p=samba.git;a=shortlog;h=v4-15-test - Log ----------------------------------------------------------------- commit f2e124e423c9dbe1f140320b88016be6f6ed84b1 Author: Jeremy Allison <j...@samba.org> Date: Mon Mar 21 13:50:56 2022 -0700 s3: smbd: Don't allow setting the delete on close bit on a directory if it contains non-visible files and "delete veto files = no". Remove knownfail. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15023 Signed-off-by: Jeremy Allison <j...@samba.org> Reviewed-by: Noel Power <npo...@samba.org> Autobuild-User(master): Jeremy Allison <j...@samba.org> Autobuild-Date(master): Tue Mar 22 17:48:25 UTC 2022 on sn-devel-184 (cherry picked from commit 80503b46e7238d0796f5cc9eb6104958c3b3fcc7) Autobuild-User(v4-15-test): Jule Anger <jan...@samba.org> Autobuild-Date(v4-15-test): Thu Mar 24 13:32:14 UTC 2022 on sn-devel-184 commit bbd8bdc14c8e6a76d93b6abde57d55b6a0e0dac9 Author: Jeremy Allison <j...@samba.org> Date: Mon Mar 21 13:45:25 2022 -0700 s3: torture: Add 2 new tests SMB2-DEL-ON-CLOSE-NONWRITE-DELETE-NO, SMB2-DEL-ON-CLOSE-NONWRITE-DELETE-YES. We currently allow setting the delete on close bit for a directory containing only explicitly hidden/vetoed files in the case where "delete veto files = yes" *and* "delete veto files = no". For the "delete veto files = no" case we should be denying setting the delete on close bit when the client tries to set it (that's the only time Windows looks at the bit and returns an error to the user). We already do the in the dangling symlink case, we just missed it in the !is_visible_fsp() case. Mark SMB2-DEL-ON-CLOSE-NONWRITE-DELETE-NO as knownfail for now. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15023 Signed-off-by: Jeremy Allison <j...@samba.org> Reviewed-by: Noel Power <npo...@samba.org> (cherry picked from commit 5fe341d2d67afb7088edcb772b058c747ab341b1) ----------------------------------------------------------------------- Summary of changes: selftest/target/Samba3.pm | 25 +++++ source3/selftest/tests.py | 28 +++++ source3/smbd/dir.c | 16 ++- source3/torture/proto.h | 2 + source3/torture/test_smb2.c | 244 ++++++++++++++++++++++++++++++++++++++++++++ source3/torture/torture.c | 8 ++ 6 files changed, 318 insertions(+), 5 deletions(-) Changeset truncated at 500 lines: diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index c1d0c60d96a..50f9d09a900 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -1691,6 +1691,11 @@ sub setup_fileserver my $virusfilter_sharedir="$share_dir/virusfilter"; push(@dirs,$virusfilter_sharedir); + my $delete_unwrite_sharedir="$share_dir/delete_unwrite"; + push(@dirs,$delete_unwrite_sharedir); + push(@dirs, "$delete_unwrite_sharedir/delete_veto_yes"); + push(@dirs, "$delete_unwrite_sharedir/delete_veto_no"); + my $ip4 = Samba::get_ipv4_addr("FILESERVER"); my $fileserver_options = " kernel change notify = yes @@ -1816,6 +1821,18 @@ sub setup_fileserver path = $veto_sharedir delete veto files = yes +[delete_yes_unwrite] + read only = no + path = $delete_unwrite_sharedir + hide unwriteable files = yes + delete veto files = yes + +[delete_no_unwrite] + read only = no + path = $delete_unwrite_sharedir + hide unwriteable files = yes + delete veto files = no + [virusfilter] path = $virusfilter_sharedir vfs objects = acl_xattr virusfilter @@ -1902,6 +1919,14 @@ sub setup_fileserver ## create_file_chmod("$bad_iconv_sharedir/\xED\x9F\xBF", 0644) or return undef; + ## + ## create unwritable files inside inside the delete unwrite veto share dirs. + ## + unlink("$delete_unwrite_sharedir/delete_veto_yes/file_444"); + create_file_chmod("$delete_unwrite_sharedir/delete_veto_yes/file_444", 0444) or return undef; + unlink("$delete_unwrite_sharedir/delete_veto_no/file_444"); + create_file_chmod("$delete_unwrite_sharedir/delete_veto_no/file_444", 0444) or return undef; + return $vars; } diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index 6b146c76381..81f59071358 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -271,7 +271,35 @@ plantestsuite("samba3.smbtorture_s3.plain.%s" % "SMB2-DEL-ON-CLOSE-NONEMPTY", "", "-l $LOCAL_PATH"]) +# +# SMB2-DEL-ON-CLOSE-NONWRITE-DELETE-YES needs to run against a special fileserver share delete_yes_unwrite +# +plantestsuite("samba3.smbtorture_s3.plain.%s" % "SMB2-DEL-ON-CLOSE-NONWRITE-DELETE-YES", + "fileserver", + [os.path.join(samba3srcdir, + "script/tests/test_smbtorture_s3.sh"), + 'SMB2-DEL-ON-CLOSE-NONWRITE-DELETE-YES', + '//$SERVER_IP/delete_yes_unwrite', + '$USERNAME', + '$PASSWORD', + smbtorture3, + "", + "-l $LOCAL_PATH"]) +# +# SMB2-DEL-ON-CLOSE-NONWRITE-DELETE-NO needs to run against a special fileserver share delete_no_unwrite +# +plantestsuite("samba3.smbtorture_s3.plain.%s" % "SMB2-DEL-ON-CLOSE-NONWRITE-DELETE-NO", + "fileserver", + [os.path.join(samba3srcdir, + "script/tests/test_smbtorture_s3.sh"), + 'SMB2-DEL-ON-CLOSE-NONWRITE-DELETE-NO', + '//$SERVER_IP/delete_no_unwrite', + '$USERNAME', + '$PASSWORD', + smbtorture3, + "", + "-l $LOCAL_PATH"]) shares = [ "vfs_aio_pthread_async_dosmode_default1", diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c index 4d61bb0d56d..77fb3df6a73 100644 --- a/source3/smbd/dir.c +++ b/source3/smbd/dir.c @@ -1997,11 +1997,17 @@ NTSTATUS can_delete_directory_fsp(files_struct *fsp) } if (!is_visible_fsp(direntry_fname->fsp)) { - TALLOC_FREE(talloced); - TALLOC_FREE(fullname); - TALLOC_FREE(smb_dname_full); - TALLOC_FREE(direntry_fname); - continue; + /* + * Hidden file. + * Allow if "delete veto files = yes" + */ + if (lp_delete_veto_files(SNUM(conn))) { + TALLOC_FREE(talloced); + TALLOC_FREE(fullname); + TALLOC_FREE(smb_dname_full); + TALLOC_FREE(direntry_fname); + continue; + } } TALLOC_FREE(talloced); diff --git a/source3/torture/proto.h b/source3/torture/proto.h index d4db60f9dde..551c4ea80ac 100644 --- a/source3/torture/proto.h +++ b/source3/torture/proto.h @@ -122,6 +122,8 @@ bool run_smb2_quota1(int dummy); bool run_smb2_stream_acl(int dummy); bool run_list_dir_async_test(int dummy); bool run_delete_on_close_non_empty(int dummy); +bool run_delete_on_close_nonwrite_delete_yes_test(int dummy); +bool run_delete_on_close_nonwrite_delete_no_test(int dummy); bool run_chain3(int dummy); bool run_local_conv_auth_info(int dummy); bool run_local_sprintf_append(int dummy); diff --git a/source3/torture/test_smb2.c b/source3/torture/test_smb2.c index 0fac5125c08..c3f014100d9 100644 --- a/source3/torture/test_smb2.c +++ b/source3/torture/test_smb2.c @@ -3364,3 +3364,247 @@ bool run_delete_on_close_non_empty(int dummy) (void)cli_rmdir(cli, dname); return ret; } + +static NTSTATUS check_empty_fn(struct file_info *finfo, + const char *mask, + void *private_data) +{ + unsigned int *pcount = (unsigned int *)private_data; + + if (ISDOT(finfo->name) || ISDOTDOT(finfo->name)) { + (*pcount)++; + return NT_STATUS_OK; + } + return NT_STATUS_DIRECTORY_NOT_EMPTY; +} + +/* + * Test setting the delete on close bit on a directory + * containing an unwritable file fails or succeeds + * an a share set with "hide unwritable = yes" + * depending on the setting of "delete veto files". + * BUG: https://bugzilla.samba.org/show_bug.cgi?id=15023 + * + * First version. With "delete veto files = yes" + * setting the delete on close should succeed. + */ + +bool run_delete_on_close_nonwrite_delete_yes_test(int dummy) +{ + struct cli_state *cli = NULL; + NTSTATUS status; + const char *dname = "delete_veto_yes"; + const char *list_dname = "delete_veto_yes\\*"; + uint16_t fnum = (uint16_t)-1; + bool ret = false; + unsigned int list_count = 0; + + printf("SMB2 delete on close nonwrite - delete veto yes\n"); + + if (!torture_init_connection(&cli)) { + return false; + } + + status = smbXcli_negprot(cli->conn, + cli->timeout, + PROTOCOL_SMB2_02, + PROTOCOL_SMB3_11); + if (!NT_STATUS_IS_OK(status)) { + printf("smbXcli_negprot returned %s\n", nt_errstr(status)); + return false; + } + + status = cli_session_setup_creds(cli, torture_creds); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_session_setup returned %s\n", nt_errstr(status)); + return false; + } + + status = cli_tree_connect(cli, share, "?????", NULL); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_tree_connect returned %s\n", nt_errstr(status)); + return false; + } + + /* Ensure target directory is seen as empty. */ + status = cli_list(cli, + list_dname, + FILE_ATTRIBUTE_DIRECTORY | + FILE_ATTRIBUTE_HIDDEN | + FILE_ATTRIBUTE_SYSTEM, + check_empty_fn, + &list_count); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_list of %s returned %s\n", + dname, + nt_errstr(status)); + return false; + } + if (list_count != 2) { + printf("cli_list of %s returned a count of %u\n", + dname, + list_count); + return false; + } + + /* Open target directory. */ + status = cli_ntcreate(cli, + dname, + 0, + DELETE_ACCESS|FILE_READ_DATA, + FILE_ATTRIBUTE_DIRECTORY, + FILE_SHARE_READ| + FILE_SHARE_WRITE| + FILE_SHARE_DELETE, + FILE_OPEN, + FILE_DIRECTORY_FILE, + 0, + &fnum, + NULL); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_ntcreate for directory %s returned %s\n", + dname, + nt_errstr(status)); + goto out; + } + + /* Now set the delete on close bit. */ + status = cli_nt_delete_on_close(cli, fnum, 1); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_cli_nt_delete_on_close set for directory " + "%s returned %s (should have succeeded)\n", + dname, + nt_errstr(status)); + goto out; + } + + ret = true; + + out: + + if (fnum != (uint16_t)-1) { + (void)cli_nt_delete_on_close(cli, fnum, 0); + (void)cli_close(cli, fnum); + } + return ret; +} + +/* + * Test setting the delete on close bit on a directory + * containing an unwritable file fails or succeeds + * an a share set with "hide unwritable = yes" + * depending on the setting of "delete veto files". + * BUG: https://bugzilla.samba.org/show_bug.cgi?id=15023 + * + * Second version. With "delete veto files = no" + * setting the delete on close should fail. + */ + +bool run_delete_on_close_nonwrite_delete_no_test(int dummy) +{ + struct cli_state *cli = NULL; + NTSTATUS status; + const char *dname = "delete_veto_no"; + const char *list_dname = "delete_veto_no\\*"; + uint16_t fnum = (uint16_t)-1; + bool ret = false; + unsigned int list_count = 0; + + printf("SMB2 delete on close nonwrite - delete veto yes\n"); + + if (!torture_init_connection(&cli)) { + return false; + } + + status = smbXcli_negprot(cli->conn, + cli->timeout, + PROTOCOL_SMB2_02, + PROTOCOL_SMB3_11); + if (!NT_STATUS_IS_OK(status)) { + printf("smbXcli_negprot returned %s\n", nt_errstr(status)); + return false; + } + + status = cli_session_setup_creds(cli, torture_creds); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_session_setup returned %s\n", nt_errstr(status)); + return false; + } + + status = cli_tree_connect(cli, share, "?????", NULL); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_tree_connect returned %s\n", nt_errstr(status)); + return false; + } + + /* Ensure target directory is seen as empty. */ + status = cli_list(cli, + list_dname, + FILE_ATTRIBUTE_DIRECTORY | + FILE_ATTRIBUTE_HIDDEN | + FILE_ATTRIBUTE_SYSTEM, + check_empty_fn, + &list_count); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_list of %s returned %s\n", + dname, + nt_errstr(status)); + return false; + } + if (list_count != 2) { + printf("cli_list of %s returned a count of %u\n", + dname, + list_count); + return false; + } + + /* Open target directory. */ + status = cli_ntcreate(cli, + dname, + 0, + DELETE_ACCESS|FILE_READ_DATA, + FILE_ATTRIBUTE_DIRECTORY, + FILE_SHARE_READ| + FILE_SHARE_WRITE| + FILE_SHARE_DELETE, + FILE_OPEN, + FILE_DIRECTORY_FILE, + 0, + &fnum, + NULL); + if (!NT_STATUS_IS_OK(status)) { + printf("cli_ntcreate for directory %s returned %s\n", + dname, + nt_errstr(status)); + goto out; + } + + /* Now set the delete on close bit. */ + status = cli_nt_delete_on_close(cli, fnum, 1); + if (NT_STATUS_IS_OK(status)) { + printf("cli_cli_nt_delete_on_close set for directory " + "%s returned NT_STATUS_OK " + "(should have failed)\n", + dname); + goto out; + } + if (!NT_STATUS_EQUAL(status, NT_STATUS_DIRECTORY_NOT_EMPTY)) { + printf("cli_cli_nt_delete_on_close set for directory " + "%s returned %s " + "(should have returned " + "NT_STATUS_DIRECTORY_NOT_EMPTY)\n", + dname, + nt_errstr(status)); + goto out; + } + + ret = true; + + out: + + if (fnum != (uint16_t)-1) { + (void)cli_nt_delete_on_close(cli, fnum, 0); + (void)cli_close(cli, fnum); + } + return ret; +} diff --git a/source3/torture/torture.c b/source3/torture/torture.c index c5da3836eac..0f80284989c 100644 --- a/source3/torture/torture.c +++ b/source3/torture/torture.c @@ -15300,6 +15300,14 @@ static struct { .name = "SMB2-DEL-ON-CLOSE-NONEMPTY", .fn = run_delete_on_close_non_empty, }, + { + .name = "SMB2-DEL-ON-CLOSE-NONWRITE-DELETE-YES", + .fn = run_delete_on_close_nonwrite_delete_yes_test, + }, + { + .name = "SMB2-DEL-ON-CLOSE-NONWRITE-DELETE-NO", + .fn = run_delete_on_close_nonwrite_delete_no_test, + }, { .name = "CLEANUP1", .fn = run_cleanup1, -- Samba Shared Repository