From 5fe825c4a4d243a55486d9122d66454b0789a45f Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 3 Feb 2022 13:58:28 -0800 Subject: [PATCH 1/8] s3: tests: Add a new test test_msdfs_hardlink() that does simple hardlinks on MSDFS root shares. We pass this already as the cmd_hardlink in smbclient doesn't do the DFS path conversion on the hardlink target. But it's good to have the test. Note we need to add the new test to "selftest/knownfail.d/smb1-tests" as test_smbclient_s3.sh is run against the (ad_member|nt4_member) environments first using NT1 (SMB1) protocol and then using SMB3, but the (ad_member|nt4_member) environments don't support SMB1. Seems a bit strange to me, but all the other SMB1 tests inside test_smbclient_s3.sh have already been added to "selftest/knownfail.d/smb1-tests" so just go with the test environment. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14169 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/smb1-tests | 1 + source3/script/tests/test_smbclient_s3.sh | 48 +++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/selftest/knownfail.d/smb1-tests b/selftest/knownfail.d/smb1-tests index 28a74863c6a..7f304eefc0f 100644 --- a/selftest/knownfail.d/smb1-tests +++ b/selftest/knownfail.d/smb1-tests @@ -29,6 +29,7 @@ ^samba3.blackbox.smbclient_s3.NT1.(plain|sign).member_creds.volume\((ad_member|nt4_member)\) ^samba3.blackbox.smbclient_s3.NT1.(plain|sign).member_creds.delete a non empty directory\((ad_member|nt4_member)\) ^samba3.blackbox.smbclient_s3.NT1.(plain|sign).member_creds.Recursive ls across MS-DFS links\((ad_member|nt4_member)\) +^samba3.blackbox.smbclient_s3.NT1.(plain|sign).member_creds.Hardlink on MS-DFS share\((ad_member|nt4_member)\) ^samba3.blackbox.smbclient_s3.*valid.users.nt4.* ^samba3.blackbox.smbclient_s3.NT1.*valid.users.* ^samba3.unix.whoami machine account.whoami\(ad_member:local\) diff --git a/source3/script/tests/test_smbclient_s3.sh b/source3/script/tests/test_smbclient_s3.sh index e250d4dd106..9866c9dc109 100755 --- a/source3/script/tests/test_smbclient_s3.sh +++ b/source3/script/tests/test_smbclient_s3.sh @@ -438,6 +438,50 @@ EOF return 0 } +# Test doing a normal file hardlink on an msdfs path. +test_msdfs_hardlink() +{ + tmpfile="$PREFIX/smbclient.in.$$" + filename_src="src.$$" + filename_dst="dest.$$" + 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 "hardlink $filename_src $filename_dst got NT_STATUS_ error" + return 1 + fi + return 0 +} + + # Archive bits are correctly set on file/dir creation and rename. test_rename_archive_bit() { @@ -2013,6 +2057,10 @@ testit "Recursive ls across MS-DFS links" \ test_msdfs_recursive_dir || \ failed=`expr $failed + 1` +testit "Hardlink on MS-DFS share" \ + test_msdfs_hardlink || \ + failed=`expr $failed + 1` + testit "Ensure archive bit is set correctly on file/dir rename" \ test_rename_archive_bit || \ failed=`expr $failed + 1` -- 2.32.0 From b987967adcd280658d403b4249c44677a1982560 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 3 Feb 2022 14:21:26 -0800 Subject: [PATCH 2/8] s3: tests: Add a new test test_msdfs_rename() that does simple renames on MSDFS root shares. We fail this on SMB2 for a subtle reason. Our client code called from smbclient only sets the SMB2_HDR_FLAG_DFS flag in the outgoing packet on the SMB2_CREATE call, and SMB2 rename does the following operations: SMB2_CREATE(src_path) // We set SMB2_HDR_FLAG_DFS here for a MSDFS share. SMB2_SETINFO: SMB2_FILE_RENAME_INFO(dst_path). // We don't set SMB2_HDR_FLAG_DFS However, from smbclient, dst_path is a MSDFS path but we don't set the flag, so even though the rename code inside smbd will cope with a MSDFS path (as used in the SMB1 SMBmv call) it fails as the correct flag isn't set. Add knownfail selftest/knownfail.d/msdfs-rename. Note we need to add the new test to "selftest/knownfail.d/smb1-tests" as test_smbclient_s3.sh is run against the (ad_member|nt4_member) environments first using NT1 (SMB1) protocol and then using SMB3, but the (ad_member|nt4_member) environments don't support SMB1. Seems a bit strange to me, but all the other SMB1 tests inside test_smbclient_s3.sh have already been added to "selftest/knownfail.d/smb1-tests" so just go with the test environment. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14169 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/msdfs-rename | 1 + selftest/knownfail.d/smb1-tests | 1 + source3/script/tests/test_smbclient_s3.sh | 51 +++++++++++++++++++++++ 3 files changed, 53 insertions(+) create mode 100644 selftest/knownfail.d/msdfs-rename diff --git a/selftest/knownfail.d/msdfs-rename b/selftest/knownfail.d/msdfs-rename new file mode 100644 index 00000000000..1961e5de1fc --- /dev/null +++ b/selftest/knownfail.d/msdfs-rename @@ -0,0 +1 @@ +^samba3.blackbox.smbclient_s3.SMB3.*.Rename\ on\ MS-DFS\ share\(.*\) diff --git a/selftest/knownfail.d/smb1-tests b/selftest/knownfail.d/smb1-tests index 7f304eefc0f..03d299ad7c7 100644 --- a/selftest/knownfail.d/smb1-tests +++ b/selftest/knownfail.d/smb1-tests @@ -30,6 +30,7 @@ ^samba3.blackbox.smbclient_s3.NT1.(plain|sign).member_creds.delete a non empty directory\((ad_member|nt4_member)\) ^samba3.blackbox.smbclient_s3.NT1.(plain|sign).member_creds.Recursive ls across MS-DFS links\((ad_member|nt4_member)\) ^samba3.blackbox.smbclient_s3.NT1.(plain|sign).member_creds.Hardlink on MS-DFS share\((ad_member|nt4_member)\) +^samba3.blackbox.smbclient_s3.NT1.(plain|sign).member_creds.Rename on MS-DFS share\((ad_member|nt4_member)\) ^samba3.blackbox.smbclient_s3.*valid.users.nt4.* ^samba3.blackbox.smbclient_s3.NT1.*valid.users.* ^samba3.unix.whoami machine account.whoami\(ad_member:local\) diff --git a/source3/script/tests/test_smbclient_s3.sh b/source3/script/tests/test_smbclient_s3.sh index 9866c9dc109..3da37e699e6 100755 --- a/source3/script/tests/test_smbclient_s3.sh +++ b/source3/script/tests/test_smbclient_s3.sh @@ -438,6 +438,53 @@ EOF return 0 } +# Test doing a normal file rename on an msdfs path. +test_msdfs_rename() +{ + tmpfile="$PREFIX/smbclient.in.$$" + filename_src="src.$$" + filename_dst="dest.$$" + filename_src_path="$PREFIX/$filename_src" + rm -f "$filename_src_path" + touch "$filename_src_path" + +# +# Use both non-force and force rename to +# ensure we test both codepaths inside libsmb. +# + cat > $tmpfile </dev/null 2>&1 + + ret="$?" + if [ "$ret" -eq 0 ] ; then + echo "$out" + echo "renaming $filename_src $filename_dst got NT_STATUS_ error" + return 1 + fi + return 0 +} + # Test doing a normal file hardlink on an msdfs path. test_msdfs_hardlink() { @@ -2057,6 +2104,10 @@ testit "Recursive ls across MS-DFS links" \ test_msdfs_recursive_dir || \ failed=`expr $failed + 1` +testit "Rename on MS-DFS share" \ + test_msdfs_rename || \ + failed=`expr $failed + 1` + testit "Hardlink on MS-DFS share" \ test_msdfs_hardlink || \ failed=`expr $failed + 1` -- 2.32.0 From 5d37c4da20ace11069efdc4ea4d800c97756475d Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 3 Feb 2022 11:15:30 -0800 Subject: [PATCH 3/8] s3: libsmb: Add cli_dfs_target_check() function. Stips any DFS prefix from a target name that will be passed to an SMB1/2/3 rename or hardlink call. Returns a pointer into the original target name after the prefix. Not yet used. If the incoming filename is *NOT* a DFS prefix, the original filename is returned unchanged. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14169 Signed-off-by: Jeremy Allison --- source3/libsmb/clidfs.c | 57 +++++++++++++++++++++++++++++++++++++++++ source3/libsmb/proto.h | 6 +++++ 2 files changed, 63 insertions(+) diff --git a/source3/libsmb/clidfs.c b/source3/libsmb/clidfs.c index 5b64858ca33..5288a7efc64 100644 --- a/source3/libsmb/clidfs.c +++ b/source3/libsmb/clidfs.c @@ -1274,3 +1274,60 @@ bool cli_check_msdfs_proxy(TALLOC_CTX *ctx, return true; } + +/******************************************************************** + Windows and NetApp (and arguably the SMB1/2/3 specs) expect a non-DFS + path for the targets of rename and hardlink. If we have been given + a DFS path for these calls, convert it back into a local path by + stripping off the DFS prefix. +********************************************************************/ + +NTSTATUS cli_dfs_target_check(TALLOC_CTX *mem_ctx, + struct cli_state *cli, + const char *fname_src, + const char *fname_dst, + const char **fname_dst_out) +{ + char *dfs_prefix = NULL; + size_t prefix_len = 0; + struct smbXcli_tcon *tcon = NULL; + + if (!smbXcli_conn_dfs_supported(cli->conn)) { + goto copy_fname_out; + } + if (smbXcli_conn_protocol(cli->conn) >= PROTOCOL_SMB2_02) { + tcon = cli->smb2.tcon; + } else { + tcon = cli->smb1.tcon; + } + if (!smbXcli_tcon_is_dfs_share(tcon)) { + goto copy_fname_out; + } + dfs_prefix = cli_dfs_make_full_path(mem_ctx, cli, ""); + if (dfs_prefix == NULL) { + return NT_STATUS_NO_MEMORY; + } + prefix_len = strlen(dfs_prefix); + if (strncmp(fname_dst, dfs_prefix, prefix_len) != 0) { + /* + * Prefix doesn't match. Assume it was + * already stripped or not added in the + * first place. + */ + goto copy_fname_out; + } + /* Return the trailing name after the prefix. */ + *fname_dst_out = &fname_dst[prefix_len]; + TALLOC_FREE(dfs_prefix); + return NT_STATUS_OK; + + copy_fname_out: + + /* + * No change to the destination name. Just + * point it at the incoming destination name. + */ + *fname_dst_out = fname_dst; + TALLOC_FREE(dfs_prefix); + return NT_STATUS_OK; +} diff --git a/source3/libsmb/proto.h b/source3/libsmb/proto.h index 8a32d4b7c2d..bda4058dafe 100644 --- a/source3/libsmb/proto.h +++ b/source3/libsmb/proto.h @@ -160,6 +160,12 @@ bool cli_check_msdfs_proxy(TALLOC_CTX *ctx, char **pp_newshare, struct cli_credentials *creds); +NTSTATUS cli_dfs_target_check(TALLOC_CTX *mem_ctx, + struct cli_state *cli, + const char *fname_src, + const char *fname_dst, + const char **fname_dst_out); + /* The following definitions come from libsmb/clientgen.c */ unsigned int cli_set_timeout(struct cli_state *cli, unsigned int timeout); -- 2.32.0 From fbac001170b2357b135c43e8d1dcfc3520a3a4d5 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 3 Feb 2022 14:51:13 -0800 Subject: [PATCH 4/8] s3: libsmb: Call cli_dfs_target_check() from cli_smb2_hardlink_send(). Currently we don't pass MSDFS names as targets here, but a caller may erroneously do this later, and for non-DFS names this is a no-op. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14169 Signed-off-by: Jeremy Allison --- source3/libsmb/clifile.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/source3/libsmb/clifile.c b/source3/libsmb/clifile.c index 3c3f44923fc..a65b79365ad 100644 --- a/source3/libsmb/clifile.c +++ b/source3/libsmb/clifile.c @@ -1682,12 +1682,26 @@ static struct tevent_req *cli_smb2_hardlink_send( { struct tevent_req *req = NULL, *subreq = NULL; struct cli_smb2_hardlink_state *state = NULL; + NTSTATUS status; req = tevent_req_create( mem_ctx, &state, struct cli_smb2_hardlink_state); if (req == NULL) { return NULL; } + + /* + * Strip a MSDFS path from fname_dst if we were given one. + */ + status = cli_dfs_target_check(state, + cli, + fname_src, + fname_dst, + &fname_dst); + if (tevent_req_nterror(req, status)) { + return tevent_req_post(req, ev); + } + state->ev = ev; state->cli = cli; state->fname_dst = fname_dst; -- 2.32.0 From 62818a9091bfddbee8031e4f19ef97751cf43cce Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 3 Feb 2022 14:54:26 -0800 Subject: [PATCH 5/8] s3: libsmb: Call cli_dfs_target_check() from cli_ntrename_internal_send(). Currently we don't pass MSDFS names as targets here, but a caller may erroneously do this later, and for non-DFS names this is a no-op. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14169 Signed-off-by: Jeremy Allison --- source3/libsmb/clifile.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/source3/libsmb/clifile.c b/source3/libsmb/clifile.c index a65b79365ad..032c11b5e75 100644 --- a/source3/libsmb/clifile.c +++ b/source3/libsmb/clifile.c @@ -1518,6 +1518,7 @@ static struct tevent_req *cli_ntrename_internal_send(TALLOC_CTX *mem_ctx, uint8_t additional_flags = 0; uint16_t additional_flags2 = 0; uint8_t *bytes = NULL; + NTSTATUS status; req = tevent_req_create(mem_ctx, &state, struct cli_ntrename_internal_state); @@ -1525,6 +1526,18 @@ static struct tevent_req *cli_ntrename_internal_send(TALLOC_CTX *mem_ctx, return NULL; } + /* + * Strip a MSDFS path from fname_dst if we were given one. + */ + status = cli_dfs_target_check(state, + cli, + fname_src, + fname_dst, + &fname_dst); + if (tevent_req_nterror(req, status)) { + return tevent_req_post(req, ev); + } + SSVAL(state->vwv+0, 0 ,FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN | FILE_ATTRIBUTE_DIRECTORY); SSVAL(state->vwv+1, 0, rename_flag); -- 2.32.0 From 5df60651f1546c5608926c64ba17014305baa0ee Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 3 Feb 2022 15:54:55 -0800 Subject: [PATCH 6/8] s3: libsmb: Call cli_dfs_target_check() from cli_smb1_rename_send(). Strips off any DFS prefix from the target if passed in. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14169 Signed-off-by: Jeremy Allison --- source3/libsmb/clifile.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/source3/libsmb/clifile.c b/source3/libsmb/clifile.c index 032c11b5e75..9e15843f120 100644 --- a/source3/libsmb/clifile.c +++ b/source3/libsmb/clifile.c @@ -1234,6 +1234,18 @@ static struct tevent_req *cli_smb1_rename_send(TALLOC_CTX *mem_ctx, return NULL; } + /* + * Strip a MSDFS path from fname_dst if we were given one. + */ + status = cli_dfs_target_check(state, + cli, + fname_src, + fname_dst, + &fname_dst); + if (!NT_STATUS_IS_OK(status)) { + goto fail; + } + if (!push_ucs2_talloc(talloc_tos(), &converted_str, fname_dst, &converted_size_bytes)) { status = NT_STATUS_INVALID_PARAMETER; -- 2.32.0 From 0b00076c281fd16f1a4264f32970fe687bcfe0b8 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 3 Feb 2022 15:56:51 -0800 Subject: [PATCH 7/8] s3: libsmb: Call cli_dfs_target_check() from cli_cifs_rename_send(). Strips off any DFS prefix from the target if passed in. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14169 Signed-off-by: Jeremy Allison --- source3/libsmb/clifile.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/source3/libsmb/clifile.c b/source3/libsmb/clifile.c index 9e15843f120..5c570c68eac 100644 --- a/source3/libsmb/clifile.c +++ b/source3/libsmb/clifile.c @@ -1323,6 +1323,7 @@ static struct tevent_req *cli_cifs_rename_send(TALLOC_CTX *mem_ctx, uint8_t additional_flags = 0; uint16_t additional_flags2 = 0; uint8_t *bytes = NULL; + NTSTATUS status; req = tevent_req_create(mem_ctx, &state, struct cli_cifs_rename_state); if (req == NULL) { @@ -1337,6 +1338,18 @@ static struct tevent_req *cli_cifs_rename_send(TALLOC_CTX *mem_ctx, return tevent_req_post(req, ev); } + /* + * Strip a MSDFS path from fname_dst if we were given one. + */ + status = cli_dfs_target_check(state, + cli, + fname_src, + fname_dst, + &fname_dst); + if (tevent_req_nterror(req, status)) { + return tevent_req_post(req, ev); + } + SSVAL(state->vwv+0, 0, FILE_ATTRIBUTE_SYSTEM | FILE_ATTRIBUTE_HIDDEN | FILE_ATTRIBUTE_DIRECTORY); bytes = talloc_array(state, uint8_t, 1); -- 2.32.0 From 8aa37b6cdfc4ae23bad2c5245361562968579ab3 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 3 Feb 2022 15:59:51 -0800 Subject: [PATCH 8/8] s3: libsmb: Call cli_dfs_target_check() from cli_smb2_rename_send(). Strips off any DFS prefix from the target if passed in. Remove knownfail selftest/knownfail.d/msdfs-rename. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14169 Signed-off-by: Jeremy Allison --- selftest/knownfail.d/msdfs-rename | 1 - source3/libsmb/cli_smb2_fnum.c | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) delete mode 100644 selftest/knownfail.d/msdfs-rename diff --git a/selftest/knownfail.d/msdfs-rename b/selftest/knownfail.d/msdfs-rename deleted file mode 100644 index 1961e5de1fc..00000000000 --- a/selftest/knownfail.d/msdfs-rename +++ /dev/null @@ -1 +0,0 @@ -^samba3.blackbox.smbclient_s3.SMB3.*.Rename\ on\ MS-DFS\ share\(.*\) diff --git a/source3/libsmb/cli_smb2_fnum.c b/source3/libsmb/cli_smb2_fnum.c index 87c56b5564d..13d23ed6566 100644 --- a/source3/libsmb/cli_smb2_fnum.c +++ b/source3/libsmb/cli_smb2_fnum.c @@ -3215,12 +3215,26 @@ struct tevent_req *cli_smb2_rename_send( { struct tevent_req *req = NULL, *subreq = NULL; struct cli_smb2_rename_state *state = NULL; + NTSTATUS status; req = tevent_req_create( mem_ctx, &state, struct cli_smb2_rename_state); if (req == NULL) { return NULL; } + + /* + * Strip a MSDFS path from fname_dst if we were given one. + */ + status = cli_dfs_target_check(state, + cli, + fname_src, + fname_dst, + &fname_dst); + if (tevent_req_nterror(req, status)) { + return tevent_req_post(req, ev); + } + state->ev = ev; state->cli = cli; state->fname_dst = fname_dst; -- 2.32.0