From 0f5a1f8ea0ea83d25966501e4a7e6ced568d5e27 Mon Sep 17 00:00:00 2001 From: Noel Power Date: Thu, 19 Aug 2021 12:13:27 +0100 Subject: [PATCH 1/4] s4: torture: CHECK ret value and fail if false If we reach 'done' with ret == false without setting the torture result we get unexpected results e.g. Exception: Exception: Unknown error/failure. Missing torture_fail() or torture_assert_*() call? BUG: https://bugzilla.samba.org/show_bug.cgi?id=14760 Signed-off-by: Noel Power Reviewed-by: Jeremy Allison (cherry picked from commit 161cee6f36b1642e2096a64a4eec22a1ebf82aa2) --- source4/torture/smb2/streams.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/source4/torture/smb2/streams.c b/source4/torture/smb2/streams.c index 814ed1666c7..c5ece0d9bcb 100644 --- a/source4/torture/smb2/streams.c +++ b/source4/torture/smb2/streams.c @@ -957,7 +957,7 @@ static bool test_stream_names(struct torture_context *tctx, CHECK_STATUS(status, NT_STATUS_OK); ret &= check_stream_list(tree, tctx, fname, 4, four, io.smb2.out.file.handle); - + CHECK_VALUE(ret, true); smb2_util_close(tree, h1); smb2_util_close(tree, h2); smb2_util_close(tree, h3); @@ -973,6 +973,7 @@ static bool test_stream_names(struct torture_context *tctx, ret &= check_stream_list(tree, tctx, fname, 4, four, io.smb2.out.file.handle); + CHECK_VALUE(ret, true); for (i=0; i < 4; i++) { NTTIME write_time; uint64_t stream_size; @@ -1114,6 +1115,7 @@ static bool test_stream_names(struct torture_context *tctx, ret &= check_stream_list(tree,tctx, fname, 5, five2, io.smb2.out.file.handle); + CHECK_VALUE(ret, true); ZERO_STRUCT(sinfo); sinfo.rename_information.level = RAW_SFILEINFO_RENAME_INFORMATION; sinfo.rename_information.in.file.handle = h1; @@ -1138,6 +1140,7 @@ static bool test_stream_names(struct torture_context *tctx, ret &= check_stream_list(tree,tctx, fname, 5, five2, io.smb2.out.file.handle); + CHECK_VALUE(ret, true); /* TODO: we need to test more rename combinations */ done: @@ -2062,6 +2065,5 @@ struct torture_suite *torture_smb2_streams_init(TALLOC_CTX *ctx) test_basefile_rename_with_open_stream); suite->description = talloc_strdup(suite, "SMB2-STREAM tests"); - return suite; } -- 2.30.2 From fe8d7561a3964d81e88463760bdbfb574361cb63 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 20 Jul 2021 17:50:49 -0700 Subject: [PATCH 2/4] s3: selftest: Add a test for vfs_streams_depot with the target path outside of the share. Mark as knownfail.d/simpleserver_streams BUG: https://bugzilla.samba.org/show_bug.cgi?id=14760 Signed-off-by: Jeremy Allison Reviewed-by: Noel Power (cherry picked from commit 5fdf4219c6db6d81ebe608c4313c9c9aea6dbc7c) --- selftest/knownfail.d/simpleserver_streams | 6 ++++++ selftest/target/Samba3.pm | 10 ++++++++++ source3/selftest/tests.py | 5 +++++ 3 files changed, 21 insertions(+) create mode 100644 selftest/knownfail.d/simpleserver_streams diff --git a/selftest/knownfail.d/simpleserver_streams b/selftest/knownfail.d/simpleserver_streams new file mode 100644 index 00000000000..2b7412999de --- /dev/null +++ b/selftest/knownfail.d/simpleserver_streams @@ -0,0 +1,6 @@ +^samba3.smb2.streams.names\(simpleserver\) +^samba3.smb2.streams.io\(simpleserver\) +^samba3.smb2.streams.names3\(simpleserver\) +^samba3.smb2.streams.delete\(simpleserver\) +^samba3.smb2.streams.zero-byte\(simpleserver\) +^samba3.smb2.streams.sharemodes\(simpleserver\) diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index d0ef659da99..3fe6c194ed8 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -1458,6 +1458,11 @@ sub setup_simpleserver print "PROVISIONING simple server..."; my $prefix_abs = abs_path($path); + mkdir($prefix_abs, 0777); + + my $external_streams_depot="$prefix_abs/external_streams_depot"; + remove_tree($external_streams_depot); + mkdir($external_streams_depot, 0777); my $simpleserver_options = " lanman auth = yes @@ -1531,6 +1536,11 @@ sub setup_simpleserver [hidenewfiles] path = $prefix_abs/share hide new files timeout = 5 + +[external_streams_depot] + path = $prefix_abs/share + read only = no + streams_depot:directory = $external_streams_depot "; my $vars = $self->provision( diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index cf745907219..687a9d9e875 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -978,6 +978,11 @@ plansmbtorture4testsuite( "fileserver:local", '//foo/bar -U%') +plansmbtorture4testsuite( + "smb2.streams", + "simpleserver", + '//$SERVER/external_streams_depot -U$USERNAME%$PASSWORD') + test = 'rpc.lsa.lookupsids' auth_options = ["", "ntlm", "spnego", "spnego,ntlm", "spnego,smb1", "spnego,smb2"] signseal_options = ["", ",connect", ",packet", ",sign", ",seal"] -- 2.30.2 From 0cab17a9bee6834495423b70bae366ca0814f366 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 19 Jul 2021 14:52:32 -0700 Subject: [PATCH 3/4] s3: VFS: vfs_streams_depot: Factor out the code that gets the absolute stream rootdir into a function. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14760 Signed-off-by: Jeremy Allison Reviewed-by: Noel Power (cherry picked from commit 1e3232006d688fa999fb8314ce948ffb45a50e71) --- source3/modules/vfs_streams_depot.c | 50 +++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 14 deletions(-) diff --git a/source3/modules/vfs_streams_depot.c b/source3/modules/vfs_streams_depot.c index d9abf1d71b9..d3b511d3132 100644 --- a/source3/modules/vfs_streams_depot.c +++ b/source3/modules/vfs_streams_depot.c @@ -129,6 +129,39 @@ static bool mark_file_valid(vfs_handle_struct *handle, return true; } +/* + * Return the root of the stream directory. Can be + * external to the share definition but by default + * is "handle->conn->connectpath/.streams". + * + * Note that this is an *absolute* path, starting + * with '/', so the dirfsp being used in the + * calls below isn't looked at. + */ + +static char *stream_rootdir(vfs_handle_struct *handle, + TALLOC_CTX *ctx) +{ + const struct loadparm_substitution *lp_sub = + loadparm_s3_global_substitution(); + char *tmp; + + tmp = talloc_asprintf(ctx, + "%s/.streams", + handle->conn->connectpath); + if (tmp == NULL) { + errno = ENOMEM; + return NULL; + } + + return lp_parm_substituted_string(ctx, + lp_sub, + SNUM(handle->conn), + "streams_depot", + "directory", + tmp); +} + /** * Given an smb_filename, determine the stream directory using the file's * base_name. @@ -137,14 +170,12 @@ static char *stream_dir(vfs_handle_struct *handle, const struct smb_filename *smb_fname, const SMB_STRUCT_STAT *base_sbuf, bool create_it) { - const struct loadparm_substitution *lp_sub = - loadparm_s3_global_substitution(); uint32_t hash; struct smb_filename *smb_fname_hash = NULL; char *result = NULL; SMB_STRUCT_STAT base_sbuf_tmp; + char *tmp = NULL; uint8_t first, second; - char *tmp; char *id_hex; struct file_id id; uint8_t id_buf[16]; @@ -159,17 +190,8 @@ static char *stream_dir(vfs_handle_struct *handle, check_valid = lp_parm_bool(SNUM(handle->conn), "streams_depot", "check_valid", true); - tmp = talloc_asprintf(talloc_tos(), "%s/.streams", - handle->conn->connectpath); - - if (tmp == NULL) { - errno = ENOMEM; - goto fail; - } - - rootdir = lp_parm_substituted_string(talloc_tos(), lp_sub, - SNUM(handle->conn), "streams_depot", "directory", - tmp); + rootdir = stream_rootdir(handle, + talloc_tos()); if (rootdir == NULL) { errno = ENOMEM; goto fail; -- 2.30.2 From 5d53d7becac0a7a0baa8971cc77b2b7ba53a75d3 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 19 Jul 2021 15:10:41 -0700 Subject: [PATCH 4/4] s3: VFS: streams_depot: Allow "streams directory" outside of share path to work again. As we're dealing with absolute paths here, we just need to temporarily replace the connectpath whilst enumerating streams. Remove knownfail file. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14760 Signed-off-by: Jeremy Allison Reviewed-by: Noel Power Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Thu Aug 19 17:04:44 UTC 2021 on sn-devel-184 (cherry picked from commit 649f544ab2cf564cdecf545c549ca9703cb5cda4) --- selftest/knownfail.d/simpleserver_streams | 6 ------ source3/modules/vfs_streams_depot.c | 23 +++++++++++++++++++++++ 2 files changed, 23 insertions(+), 6 deletions(-) delete mode 100644 selftest/knownfail.d/simpleserver_streams diff --git a/selftest/knownfail.d/simpleserver_streams b/selftest/knownfail.d/simpleserver_streams deleted file mode 100644 index 2b7412999de..00000000000 --- a/selftest/knownfail.d/simpleserver_streams +++ /dev/null @@ -1,6 +0,0 @@ -^samba3.smb2.streams.names\(simpleserver\) -^samba3.smb2.streams.io\(simpleserver\) -^samba3.smb2.streams.names3\(simpleserver\) -^samba3.smb2.streams.delete\(simpleserver\) -^samba3.smb2.streams.zero-byte\(simpleserver\) -^samba3.smb2.streams.sharemodes\(simpleserver\) diff --git a/source3/modules/vfs_streams_depot.c b/source3/modules/vfs_streams_depot.c index d3b511d3132..973edeeda24 100644 --- a/source3/modules/vfs_streams_depot.c +++ b/source3/modules/vfs_streams_depot.c @@ -544,6 +544,8 @@ static NTSTATUS walk_streams(vfs_handle_struct *handle, void *private_data) { char *dirname; + char *rootdir = NULL; + char *orig_connectpath = NULL; struct smb_filename *dir_smb_fname = NULL; struct smb_Dir *dir_hnd = NULL; const char *dname = NULL; @@ -576,8 +578,26 @@ static NTSTATUS walk_streams(vfs_handle_struct *handle, return NT_STATUS_NO_MEMORY; } + /* + * For OpenDir to succeed if the stream rootdir is outside + * the share path, we must temporarily swap out the connect + * path for this share. We're dealing with absolute paths + * here so we don't care about chdir calls. + */ + rootdir = stream_rootdir(handle, talloc_tos()); + if (rootdir == NULL) { + TALLOC_FREE(dir_smb_fname); + TALLOC_FREE(dirname); + return NT_STATUS_NO_MEMORY; + } + + orig_connectpath = handle->conn->connectpath; + handle->conn->connectpath = rootdir; + dir_hnd = OpenDir(talloc_tos(), handle->conn, dir_smb_fname, NULL, 0); if (dir_hnd == NULL) { + handle->conn->connectpath = orig_connectpath; + TALLOC_FREE(rootdir); TALLOC_FREE(dir_smb_fname); TALLOC_FREE(dirname); return map_nt_error_from_unix(errno); @@ -600,6 +620,9 @@ static NTSTATUS walk_streams(vfs_handle_struct *handle, TALLOC_FREE(talloced); } + /* Restore the original connectpath. */ + handle->conn->connectpath = orig_connectpath; + TALLOC_FREE(rootdir); TALLOC_FREE(dir_smb_fname); TALLOC_FREE(dir_hnd); -- 2.30.2