From 42f36b5b32252a9447b362260c0da47472a6e5b9 Mon Sep 17 00:00:00 2001 From: Noel Power Date: Fri, 17 Jun 2022 10:15:42 +0100 Subject: [PATCH 1/5] Add new dfs node msdfs-share pointing to new msdfs-share2 Also add another node within msdfs-share2 pointing to normal share This patch is in preperation for creating a test for 'del' & 'deltree' on DFS shares. The extra redirection is necessary to reproduce the bug BUG: https://bugzilla.samba.org/show_bug.cgi?id=15100 Signed-off-by: Noel Power Reviewed-by: Jeremy Allison (cherry picked from commit 39672a9676bff53d3ccc0ad7c1fa65a95cbceaab) --- selftest/target/Samba3.pm | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index df94e358a6e..54d14c813e9 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -2415,6 +2415,9 @@ sub provision($$) my $msdfs_shrdir="$shrdir/msdfsshare"; push(@dirs,$msdfs_shrdir); + my $msdfs_shrdir2="$shrdir/msdfsshare2"; + push(@dirs,$msdfs_shrdir2); + my $msdfs_deeppath="$msdfs_shrdir/deeppath"; push(@dirs,$msdfs_deeppath); @@ -2508,6 +2511,8 @@ sub provision($$) symlink "msdfs:$server_ip\\smbcacls_sharedir_dfs,$server_ipv6\\smbcacls_sharedir_dfs", "$msdfs_shrdir/smbcacls_sharedir_dfs"; + symlink "msdfs:$server_ip\\msdfs-share2,$server_ipv6\\msdfs-share2", "$msdfs_shrdir/dfshop1"; + symlink "msdfs:$server_ip\\tmp,$server_ipv6\\tmp", "$msdfs_shrdir2/dfshop2"; ## ## create bad names in $badnames_shrdir ## @@ -2831,6 +2836,10 @@ sub provision($$) msdfs root = yes msdfs shuffle referrals = yes guest ok = yes +[msdfs-share2] + path = $msdfs_shrdir2 + msdfs root = yes + guest ok = yes [hideunread] copy = tmp hide unreadable = yes -- 2.34.1 From 7cc50e9298701a30b788b7dbcedfba3f5ef526df Mon Sep 17 00:00:00 2001 From: Noel Power Date: Fri, 17 Jun 2022 10:25:49 +0100 Subject: [PATCH 2/5] Add test smbclient 'del' of file (on DFS share) del of a file on a DFS share results in NT_STATUS_OBJECT_PATH_NOT_FOUND Addionally add a knownfail (will be removed in following patch to fix the bug) We also need to add a knownfail (which will not be removed) for the new test which will fail in smb1 envs BUG: https://bugzilla.samba.org/show_bug.cgi?id=15100 Signed-off-by: Noel Power Reviewed-by: Jeremy Allison (back-ported from commit db1b4df0ab3b18821da3c2dbe6d6058f0c3019b8) --- selftest/knownfail.d/smb1-tests | 1 + selftest/knownfail.d/smbclient-smb3 | 1 + source3/script/tests/test_smbclient_s3.sh | 44 +++++++++++++++++++++++ 3 files changed, 46 insertions(+) diff --git a/selftest/knownfail.d/smb1-tests b/selftest/knownfail.d/smb1-tests index 03d299ad7c7..9437da57159 100644 --- a/selftest/knownfail.d/smb1-tests +++ b/selftest/knownfail.d/smb1-tests @@ -10,6 +10,7 @@ ^samba3.blackbox.smbclient_s3.NT1.(plain|sign).member_creds.interactive smbclient -l prompts on stdout\((ad_member|nt4_member)\) ^samba3.blackbox.smbclient_s3.NT1.(plain|sign).member_creds.creating a bad symlink and deleting it\((ad_member|nt4_member)\) ^samba3.blackbox.smbclient_s3.NT1.(plain|sign).member_creds.Accessing an MS-DFS link\((ad_member|nt4_member)\) +^samba3.blackbox.smbclient_s3.NT1.(plain|sign).member_creds.del on MS-DFS share\((ad_member|nt4_member)\) ^samba3.blackbox.smbclient_s3.NT1.(plain|sign).member_creds.Ensure archive bit is set correctly on file/dir rename\((ad_member|nt4_member)\) ^samba3.blackbox.smbclient_s3.NT1.(plain|sign).member_creds.ccache access works for smbclient\((ad_member|nt4_member)\) ^samba3.blackbox.smbclient_s3.NT1.(plain|sign).member_creds.using an authentication file\((ad_member|nt4_member)\) diff --git a/selftest/knownfail.d/smbclient-smb3 b/selftest/knownfail.d/smbclient-smb3 index 119e93e479a..91bed158bfa 100644 --- a/selftest/knownfail.d/smbclient-smb3 +++ b/selftest/knownfail.d/smbclient-smb3 @@ -1,4 +1,5 @@ ^samba3.blackbox.smbclient_s3.SMB3.*.creating.a.bad.symlink.and.deleting.it +^samba3.blackbox.smbclient_s3.SMB3.*.del on MS-DFS share ^samba3.blackbox.acl_xattr.SMB3.nt_affects_posix ^samba3.blackbox.acl_xattr.SMB3.nt_affects_chown ^samba3.blackbox.acl_xattr.SMB3.nt_affects_chgrp diff --git a/source3/script/tests/test_smbclient_s3.sh b/source3/script/tests/test_smbclient_s3.sh index c23f8deecb8..3ca15246291 100755 --- a/source3/script/tests/test_smbclient_s3.sh +++ b/source3/script/tests/test_smbclient_s3.sh @@ -528,6 +528,46 @@ EOF return 0 } +test_msdfs_del() +{ + tmpfile="$PREFIX/smbclient.in.$$" + filename_src="src.$$" + filename_src_path="$PREFIX/$filename_src" + rm -f "$filename_src_path" + touch "$filename_src_path" + + cat > $tmpfile </dev/null 2>&1 + + ret="$?" + if [ "$ret" -eq 0 ] ; then + echo "$out" + echo "del $filename_src NT_STATUS_ error" + return 1 + fi + return 0 +} # Archive bits are correctly set on file/dir creation and rename. test_rename_archive_bit() @@ -2170,6 +2210,10 @@ testit "Hardlink on MS-DFS share" \ test_msdfs_hardlink || \ failed=`expr $failed + 1` +testit "del on MS-DFS share" \ + test_msdfs_del || \ + failed=`expr $failed + 1` + testit "Ensure archive bit is set correctly on file/dir rename" \ test_rename_archive_bit || \ failed=`expr $failed + 1` -- 2.34.1 From 31bb3bf8f24696dfc958481265a35ea9b162153d Mon Sep 17 00:00:00 2001 From: Noel Power Date: Thu, 16 Jun 2022 15:12:05 +0100 Subject: [PATCH 3/5] s3/client: fix dfs delete, resolve dfs path since 4cc4938a2866738aaff4dc91550bb7a5ad05d7fb do_list seems to deal with non dfs root path, hence we need to resolve the path before calling cli_unlink. Also remove the knownfail BUG: https://bugzilla.samba.org/show_bug.cgi?id=15100 Signed-off-by: Noel Power Reviewed-by: Jeremy Allison (cherry picked from commit 7c4cb4982330cd2eda53950e977179920b1e3b04) --- selftest/knownfail.d/smbclient-smb3 | 1 - source3/client/client.c | 13 ++++++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/selftest/knownfail.d/smbclient-smb3 b/selftest/knownfail.d/smbclient-smb3 index 91bed158bfa..119e93e479a 100644 --- a/selftest/knownfail.d/smbclient-smb3 +++ b/selftest/knownfail.d/smbclient-smb3 @@ -1,5 +1,4 @@ ^samba3.blackbox.smbclient_s3.SMB3.*.creating.a.bad.symlink.and.deleting.it -^samba3.blackbox.smbclient_s3.SMB3.*.del on MS-DFS share ^samba3.blackbox.acl_xattr.SMB3.nt_affects_posix ^samba3.blackbox.acl_xattr.SMB3.nt_affects_chown ^samba3.blackbox.acl_xattr.SMB3.nt_affects_chgrp diff --git a/source3/client/client.c b/source3/client/client.c index 8ec4589a1cd..f6bdf29613b 100644 --- a/source3/client/client.c +++ b/source3/client/client.c @@ -2354,6 +2354,9 @@ static NTSTATUS do_del(struct cli_state *cli_state, struct file_info *finfo, { TALLOC_CTX *ctx = talloc_tos(); char *mask = NULL; + struct cli_state *targetcli = NULL; + char *targetname = NULL; + struct cli_credentials *creds = samba_cmdline_get_creds(); NTSTATUS status; mask = talloc_asprintf(ctx, @@ -2370,7 +2373,15 @@ static NTSTATUS do_del(struct cli_state *cli_state, struct file_info *finfo, return NT_STATUS_OK; } - status = cli_unlink(cli_state, mask, FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); + status = cli_resolve_path(ctx, "", + creds, + cli, mask, &targetcli, &targetname); + if (!NT_STATUS_IS_OK(status)) { + goto out; + } + + status = cli_unlink(targetcli, targetname, FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); +out: if (!NT_STATUS_IS_OK(status)) { d_printf("%s deleting remote file %s\n", nt_errstr(status), mask); -- 2.34.1 From 1c6ee853fbe12de375b0b96879465a049e0f1f70 Mon Sep 17 00:00:00 2001 From: Noel Power Date: Fri, 17 Jun 2022 10:58:48 +0100 Subject: [PATCH 4/5] Add test smbclient 'delree' of dir (on DFS share) deltree of a file on a DFS share results in NT_STATUS_OBJECT_PATH_NOT_FOUND Addionally add a knownfail for this (to be removed in subsequent patch to fix bug) We also need to add a knownfail (which will not be removed) for the new test which will fail in smb1 envs BUG: https://bugzilla.samba.org/show_bug.cgi?id=15100 Signed-off-by: Noel Power Reviewed-by: Jeremy Allison (cherry picked from commit 23a5a05db03a8f14ab701005a8bec9a3eeff3d77) --- selftest/knownfail.d/smb1-tests | 1 + selftest/knownfail.d/smbclient-smb3 | 1 + source3/script/tests/test_smbclient_s3.sh | 51 +++++++++++++++++++++++ 3 files changed, 53 insertions(+) diff --git a/selftest/knownfail.d/smb1-tests b/selftest/knownfail.d/smb1-tests index 9437da57159..b5263f28016 100644 --- a/selftest/knownfail.d/smb1-tests +++ b/selftest/knownfail.d/smb1-tests @@ -11,6 +11,7 @@ ^samba3.blackbox.smbclient_s3.NT1.(plain|sign).member_creds.creating a bad symlink and deleting it\((ad_member|nt4_member)\) ^samba3.blackbox.smbclient_s3.NT1.(plain|sign).member_creds.Accessing an MS-DFS link\((ad_member|nt4_member)\) ^samba3.blackbox.smbclient_s3.NT1.(plain|sign).member_creds.del on MS-DFS share\((ad_member|nt4_member)\) +^samba3.blackbox.smbclient_s3.NT1.(plain|sign).member_creds.deltree on MS-DFS share\((ad_member|nt4_member)\) ^samba3.blackbox.smbclient_s3.NT1.(plain|sign).member_creds.Ensure archive bit is set correctly on file/dir rename\((ad_member|nt4_member)\) ^samba3.blackbox.smbclient_s3.NT1.(plain|sign).member_creds.ccache access works for smbclient\((ad_member|nt4_member)\) ^samba3.blackbox.smbclient_s3.NT1.(plain|sign).member_creds.using an authentication file\((ad_member|nt4_member)\) diff --git a/selftest/knownfail.d/smbclient-smb3 b/selftest/knownfail.d/smbclient-smb3 index 119e93e479a..f6ca529c61d 100644 --- a/selftest/knownfail.d/smbclient-smb3 +++ b/selftest/knownfail.d/smbclient-smb3 @@ -1,4 +1,5 @@ ^samba3.blackbox.smbclient_s3.SMB3.*.creating.a.bad.symlink.and.deleting.it +^samba3.blackbox.smbclient_s3.SMB3.*.deltree on MS-DFS share ^samba3.blackbox.acl_xattr.SMB3.nt_affects_posix ^samba3.blackbox.acl_xattr.SMB3.nt_affects_chown ^samba3.blackbox.acl_xattr.SMB3.nt_affects_chgrp diff --git a/source3/script/tests/test_smbclient_s3.sh b/source3/script/tests/test_smbclient_s3.sh index 3ca15246291..0e71f599e34 100755 --- a/source3/script/tests/test_smbclient_s3.sh +++ b/source3/script/tests/test_smbclient_s3.sh @@ -569,6 +569,53 @@ EOF return 0 } +test_msdfs_deltree() +{ + tmpfile="$PREFIX/smbclient.in.$$" + dirname_src="foodir.$$" + filename_src="src.$$" + filename_src_path="$PREFIX/$filename_src" + dirname_src_path="$PREFIX/$dirname" + rm -f "$filename_src_path" + touch "$filename_src_path" + + cat > $tmpfile </dev/null 2>&1 + + ret="$?" + if [ "$ret" -eq 0 ] ; then + echo "$out" + echo "deltree $dirname_src NT_STATUS_ error" + return 1 + fi + return 0 +} + # Archive bits are correctly set on file/dir creation and rename. test_rename_archive_bit() { @@ -2214,6 +2261,10 @@ testit "del on MS-DFS share" \ test_msdfs_del || \ failed=`expr $failed + 1` +testit "deltree on MS-DFS share" \ + test_msdfs_deltree || \ + failed=`expr $failed + 1` + testit "Ensure archive bit is set correctly on file/dir rename" \ test_rename_archive_bit || \ failed=`expr $failed + 1` -- 2.34.1 From 27c8a323a29753e413bfd04337d0446fe47e340d Mon Sep 17 00:00:00 2001 From: Noel Power Date: Thu, 16 Jun 2022 17:17:45 +0100 Subject: [PATCH 5/5] s3/client: fix dfs deltree, resolve dfs path since 4cc4938a2866738aaff4dc91550bb7a5ad05d7fb do_list seems to deal with non dfs root path, hence we need to resolve the path before calling cli_unlink. Also remove the knownfail We additionally have to also remove the fallback to remove 'file3' int the smbcacls_dfs_propagate_inherit.teardown as the deltree that happens in the baseclass now succeeds. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15100 Signed-off-by: Noel Power Reviewed-by: Jeremy Allison Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Fri Jun 17 17:12:07 UTC 2022 on sn-devel-184 (cherry picked from commit 81fdcf95ae92a02f83501753dec0f29ddd555eeb) --- .../smbcacls_dfs_propagate_inherit.py | 8 --- selftest/knownfail.d/smbclient-smb3 | 1 - source3/client/client.c | 50 +++++++++++++++---- 3 files changed, 40 insertions(+), 19 deletions(-) diff --git a/python/samba/tests/blackbox/smbcacls_dfs_propagate_inherit.py b/python/samba/tests/blackbox/smbcacls_dfs_propagate_inherit.py index 36c29c8ccca..42680df0d06 100644 --- a/python/samba/tests/blackbox/smbcacls_dfs_propagate_inherit.py +++ b/python/samba/tests/blackbox/smbcacls_dfs_propagate_inherit.py @@ -85,11 +85,3 @@ class DfsInheritanceSmbCaclsTests(InheritanceSmbCaclsTests): def tearDown(self): super(DfsInheritanceSmbCaclsTests, self).tearDown() - # for dfs tests inevitably we fallback to remove the local files in - # the base class, the base class however doesn't know about the - # target dfs share (or its contents) so we have to assume we need to - # remove the file on the dfs share - smbclient_args = self.build_test_cmd("smbclient", ["//%s/%s" % (self.server, self.dfs_target_share), "-c", "rm file-3"]) - self.check_output(smbclient_args) - - diff --git a/selftest/knownfail.d/smbclient-smb3 b/selftest/knownfail.d/smbclient-smb3 index f6ca529c61d..119e93e479a 100644 --- a/selftest/knownfail.d/smbclient-smb3 +++ b/selftest/knownfail.d/smbclient-smb3 @@ -1,5 +1,4 @@ ^samba3.blackbox.smbclient_s3.SMB3.*.creating.a.bad.symlink.and.deleting.it -^samba3.blackbox.smbclient_s3.SMB3.*.deltree on MS-DFS share ^samba3.blackbox.acl_xattr.SMB3.nt_affects_posix ^samba3.blackbox.acl_xattr.SMB3.nt_affects_chown ^samba3.blackbox.acl_xattr.SMB3.nt_affects_chgrp diff --git a/source3/client/client.c b/source3/client/client.c index f6bdf29613b..69b7c9022c2 100644 --- a/source3/client/client.c +++ b/source3/client/client.c @@ -2439,20 +2439,37 @@ static NTSTATUS delete_remote_files_list(struct cli_state *cli_state, { NTSTATUS status = NT_STATUS_OK; struct file_list *deltree_list_iter = NULL; + char *targetname = NULL; + struct cli_state *targetcli = NULL; + struct cli_credentials *creds = samba_cmdline_get_creds(); + TALLOC_CTX *ctx = talloc_tos(); for (deltree_list_iter = flist; deltree_list_iter != NULL; deltree_list_iter = deltree_list_iter->next) { + status = cli_resolve_path(ctx, + "", + creds, + cli_state, + deltree_list_iter->file_path, + &targetcli, + &targetname); + if (!NT_STATUS_IS_OK(status)) { + d_printf("delete_remote_files %s: %s\n", + deltree_list_iter->file_path, + nt_errstr(status)); + return status; + } if (CLI_DIRSEP_CHAR == '/') { /* POSIX. */ - status = cli_posix_unlink(cli_state, - deltree_list_iter->file_path); + status = cli_posix_unlink(targetcli, + targetname); } else if (deltree_list_iter->isdir) { - status = cli_rmdir(cli_state, - deltree_list_iter->file_path); + status = cli_rmdir(targetcli, + targetname); } else { - status = cli_unlink(cli_state, - deltree_list_iter->file_path, + status = cli_unlink(targetcli, + targetname, FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); } @@ -2561,14 +2578,27 @@ static int cmd_deltree(void) deltree_list_iter = deltree_list_iter->next) { if (deltree_list_iter->isdir == false) { + char *targetname = NULL; + struct cli_state *targetcli = NULL; + struct cli_credentials *creds = samba_cmdline_get_creds(); + status = cli_resolve_path(ctx, + "", + creds, + cli, + deltree_list_iter->file_path, + &targetcli, + &targetname); + if (!NT_STATUS_IS_OK(status)) { + goto err; + } /* Just a regular file. */ if (CLI_DIRSEP_CHAR == '/') { /* POSIX. */ - status = cli_posix_unlink(cli, - deltree_list_iter->file_path); + status = cli_posix_unlink(targetcli, + targetname); } else { - status = cli_unlink(cli, - deltree_list_iter->file_path, + status = cli_unlink(targetcli, + targetname, FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN); } -- 2.34.1