The branch, v4-16-test has been updated via 188b96164c5 s3/libads: ensure a sockaddr variable is correctly zero initialized via 8cbf38a1b2b s3/libads: simplify storing existing ads->ldap.ss via cdcf23aac2f s3: libsmb: Call cli_dfs_target_check() from cli_smb2_rename_send(). via 35a250f49ee s3: libsmb: Call cli_dfs_target_check() from cli_cifs_rename_send(). via 1304041a4fd s3: libsmb: Call cli_dfs_target_check() from cli_smb1_rename_send(). via 01b06586f19 s3: libsmb: Call cli_dfs_target_check() from cli_ntrename_internal_send(). via 96122869594 s3: libsmb: Call cli_dfs_target_check() from cli_smb2_hardlink_send(). via 62ce0c8f55b s3: libsmb: Add cli_dfs_target_check() function. via 738fbcca544 s3: tests: Add a new test test_msdfs_rename() that does simple renames on MSDFS root shares. via 95aca464c7c s3: tests: Add a new test test_msdfs_hardlink() that does simple hardlinks on MSDFS root shares. from 64aea70f9f8 lib: libsmbclient: Ensure cli_rename() always sets cli->raw_status.
https://git.samba.org/?p=samba.git;a=shortlog;h=v4-16-test - Log ----------------------------------------------------------------- commit 188b96164c5849e28d0349adac6472e2bc4bd000 Author: Ralph Boehme <s...@samba.org> Date: Fri Jan 28 17:51:10 2022 +0100 s3/libads: ensure a sockaddr variable is correctly zero initialized is_zero_addr() doesn't work with addresses that have been zero-initialized. This fixes the logic added in c863cc2ba34025731a18ac735f714b5b888504da. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14674 MR: https://gitlab.com/samba-team/samba/-/merge_requests/2354 Signed-off-by: Ralph Boehme <s...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> Autobuild-User(master): Jeremy Allison <j...@samba.org> Autobuild-Date(master): Tue Feb 8 20:24:12 UTC 2022 on sn-devel-184 (cherry picked from commit 3ee690455eb963dedc7955b79316481387d4ac8c) Autobuild-User(v4-16-test): Jule Anger <jan...@samba.org> Autobuild-Date(v4-16-test): Wed Feb 9 12:03:17 UTC 2022 on sn-devel-184 commit 8cbf38a1b2ba83cde9529d7e9d71bc8fce449293 Author: Ralph Boehme <s...@samba.org> Date: Mon Jan 31 12:54:12 2022 +0100 s3/libads: simplify storing existing ads->ldap.ss We just need temporal storage for ads->ldap.ss, no need to store it as a struct samba_sockaddr. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14674 MR: https://gitlab.com/samba-team/samba/-/merge_requests/2354 Signed-off-by: Ralph Boehme <s...@samba.org> Reviewed-by: Jeremy Allison <j...@samba.org> (cherry picked from commit c266ed40aeb1b1f59a1811cd4511e32e44a4a719) commit cdcf23aac2f979d1ffa33a3a7b3fe98aa47c83d7 Author: Jeremy Allison <j...@samba.org> Date: Thu Feb 3 15:59:51 2022 -0800 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 <j...@samba.org> Reviewed-by: Noel Power <npo...@samba.org> Autobuild-User(master): Noel Power <npo...@samba.org> Autobuild-Date(master): Fri Feb 4 12:02:36 UTC 2022 on sn-devel-184 (cherry picked from commit b9b82f3611c56e837e9189f5275ae9a78e647262) commit 35a250f49ee8ae49fc8a9dfbb704211b189c9435 Author: Jeremy Allison <j...@samba.org> Date: Thu Feb 3 15:56:51 2022 -0800 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 <j...@samba.org> Reviewed-by: Noel Power <npo...@samba.org> (cherry picked from commit 4473aea926fe4ddd23a6e0913009bb1a0a1eaa90) commit 1304041a4fd2fbd5c35b5f5235345b40b5f17bf0 Author: Jeremy Allison <j...@samba.org> Date: Thu Feb 3 15:54:55 2022 -0800 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 <j...@samba.org> Reviewed-by: Noel Power <npo...@samba.org> (cherry picked from commit dd0317f6ecb572a80893405daa83e079dbcdf113) commit 01b06586f195a7eb592f18e5f5657ca9f2fa031d Author: Jeremy Allison <j...@samba.org> Date: Thu Feb 3 14:54:26 2022 -0800 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 <j...@samba.org> Reviewed-by: Noel Power <npo...@samba.org> (cherry picked from commit cf3e5724422d8becd045542be196dfea6ac9ec2b) commit 96122869594760360b2ae26d194c7908bd900bd4 Author: Jeremy Allison <j...@samba.org> Date: Thu Feb 3 14:51:13 2022 -0800 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 <j...@samba.org> Reviewed-by: Noel Power <npo...@samba.org> (cherry picked from commit 4bdbe3c2fc0c35635474ae526896b28f55142aca) commit 62ce0c8f55b52f487b33b49cb4438eff69c138ac Author: Jeremy Allison <j...@samba.org> Date: Thu Feb 3 11:15:30 2022 -0800 s3: libsmb: Add cli_dfs_target_check() function. Strips 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 <j...@samba.org> Reviewed-by: Noel Power <npo...@samba.org> (cherry picked from commit 2abba0ea109d7a3a0b0cb4a7030293f70c2d9d8a) commit 738fbcca5440f3a8ccbeb6e74b2a6560a6b74533 Author: Jeremy Allison <j...@samba.org> Date: Thu Feb 3 14:21:26 2022 -0800 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 <j...@samba.org> Reviewed-by: Noel Power <npo...@samba.org> (cherry picked from commit 44cc9fb0e01b3635804f41e03f9b20afc3bfe36c) commit 95aca464c7c500f798640a7979dade699931cefc Author: Jeremy Allison <j...@samba.org> Date: Thu Feb 3 13:58:28 2022 -0800 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 <j...@samba.org> Reviewed-by: Noel Power <npo...@samba.org> (cherry picked from commit d7deb876053ef45313026b4dea9ee1b376153611) ----------------------------------------------------------------------- Summary of changes: selftest/knownfail.d/smb1-tests | 2 + source3/libads/ldap.c | 14 ++--- source3/libsmb/cli_smb2_fnum.c | 14 +++++ source3/libsmb/clidfs.c | 57 ++++++++++++++++++ source3/libsmb/clifile.c | 52 ++++++++++++++++ source3/libsmb/proto.h | 6 ++ source3/script/tests/test_smbclient_s3.sh | 99 +++++++++++++++++++++++++++++++ 7 files changed, 236 insertions(+), 8 deletions(-) Changeset truncated at 500 lines: diff --git a/selftest/knownfail.d/smb1-tests b/selftest/knownfail.d/smb1-tests index 28a74863c6a..03d299ad7c7 100644 --- a/selftest/knownfail.d/smb1-tests +++ b/selftest/knownfail.d/smb1-tests @@ -29,6 +29,8 @@ ^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.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/libads/ldap.c b/source3/libads/ldap.c index 1bc271785e2..647cdbd0459 100755 --- a/source3/libads/ldap.c +++ b/source3/libads/ldap.c @@ -605,7 +605,9 @@ ADS_STATUS ads_connect(ADS_STRUCT *ads) ADS_STATUS status; NTSTATUS ntstatus; char addr[INET6_ADDRSTRLEN]; - struct samba_sockaddr existing_sa = {0}; + struct sockaddr_storage existing_ss; + + zero_sockaddr(&existing_ss); /* * ads_connect can be passed in a reused ADS_STRUCT @@ -627,11 +629,7 @@ ADS_STATUS ads_connect(ADS_STRUCT *ads) */ if (ads->server.ldap_server == NULL && !is_zero_addr(&ads->ldap.ss)) { /* Save off the address we previously found by ads_find_dc(). */ - bool ok = sockaddr_storage_to_samba_sockaddr(&existing_sa, - &ads->ldap.ss); - if (!ok) { - return ADS_ERROR_NT(NT_STATUS_INVALID_ADDRESS); - } + existing_ss = ads->ldap.ss; } ads_zero_ldap(ads); @@ -679,11 +677,11 @@ ADS_STATUS ads_connect(ADS_STRUCT *ads) } } - if (!is_zero_addr(&existing_sa.u.ss)) { + if (!is_zero_addr(&existing_ss)) { /* We saved off who we should talk to. */ bool ok = ads_try_connect(ads, ads->server.gc, - &existing_sa.u.ss); + &existing_ss); if (ok) { goto got_connection; } 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; 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/clifile.c b/source3/libsmb/clifile.c index 3c3f44923fc..5c570c68eac 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; @@ -1311,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) { @@ -1325,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); @@ -1518,6 +1543,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 +1551,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); @@ -1682,12 +1720,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; 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); diff --git a/source3/script/tests/test_smbclient_s3.sh b/source3/script/tests/test_smbclient_s3.sh index e250d4dd106..3da37e699e6 100755 --- a/source3/script/tests/test_smbclient_s3.sh +++ b/source3/script/tests/test_smbclient_s3.sh @@ -438,6 +438,97 @@ 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 <<EOF +lcd $PREFIX +put $filename_src +ren $filename_src $filename_dst -f +ren $filename_dst $filename_src +del $filename_src +quit +EOF + + cmd='CLI_FORCE_INTERACTIVE=yes $SMBCLIENT "$@" -U$USERNAME%$PASSWORD //$SERVER/msdfs-share -I $SERVER_IP $ADDARGS < $tmpfile 2>&1' + eval echo "$cmd" + out=`eval $cmd` + ret=$? + rm -f "$tmpfile" + rm -f "$filename_src_path" + + if [ $ret != 0 ] ; then + echo "$out" + echo "failed renaming $filename_src $filename_dst with error $ret" + return 1 + fi + + echo "$out" | grep "NT_STATUS" >/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() +{ + 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 <<EOF +lcd $PREFIX +put $filename_src +hardlink $filename_src $filename_dst +del $filename_src +del $filename_dst +quit +EOF + + cmd='CLI_FORCE_INTERACTIVE=yes $SMBCLIENT "$@" -U$USERNAME%$PASSWORD //$SERVER/msdfs-share -I $SERVER_IP $ADDARGS < $tmpfile 2>&1' + eval echo "$cmd" + out=`eval $cmd` + ret=$? + rm -f "$tmpfile" + rm -f "$filename_src_path" + + if [ $ret != 0 ] ; then + echo "$out" + echo "failed hardlink $filename_src $filename_dst with error $ret" + return 1 + fi + + echo "$out" | grep "NT_STATUS" >/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 +2104,14 @@ 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` + testit "Ensure archive bit is set correctly on file/dir rename" \ test_rename_archive_bit || \ failed=`expr $failed + 1` -- Samba Shared Repository