From 7bf82ece36c384784b1ba672667c5461fd0d7c29 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sat, 26 May 2018 16:30:47 +0200 Subject: [PATCH 1/6] selftest: run smb2.streams tests against a share with vfs_streams_xattr The tests are currently only run against streams_depot, where stream IO is handle based, compared to streams_xattr which is path based. vfs_streams_xattr is also used much more in real world setups, so we should run our tests against it. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13451 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (backported from commit aa096ab70a466388a9947f73a525b2dcbb9821e5) --- selftest/knownfail | 3 +++ source3/selftest/tests.py | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/selftest/knownfail b/selftest/knownfail index eaddaece25c..ba16fd72290 100644 --- a/selftest/knownfail +++ b/selftest/knownfail @@ -177,6 +177,9 @@ ^samba3.smb2.streams.rename ^samba3.smb2.streams.rename2 ^samba3.smb2.streams.attributes +^samba3.smb2.streams streams_xattr.rename\(nt4_dc\) +^samba3.smb2.streams streams_xattr.rename2\(nt4_dc\) +^samba3.smb2.streams streams_xattr.attributes\(nt4_dc\) ^samba3.smb2.getinfo.complex ^samba3.smb2.getinfo.fsinfo # quotas don't work yet ^samba3.smb2.setinfo.setinfo diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index f43d2b14d3a..9092c1776c8 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -559,6 +559,10 @@ tests= base + raw + smb2 + rpc + unix + local + rap + nbt + libsmbclient + idmap elif t == "rpc.samba3.netlogon" or t == "rpc.samba3.sessionkey": plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD --option=torture:wksname=samba3rpctest') plansmbtorture4testsuite(t, "ad_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD --option=torture:wksname=samba3rpctest') + elif t == "smb2.streams": + plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD') + plansmbtorture4testsuite(t, "ad_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD') + plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/streams_xattr -U$USERNAME%$PASSWORD', 'streams_xattr') else: plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD') plansmbtorture4testsuite(t, "ad_dc", '//$SERVER/tmp -U$USERNAME%$PASSWORD') -- 2.13.6 From 495303b80c7cc87a5b2c6a8b6c6d545db7b48d8b Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sat, 26 May 2018 16:07:14 +0200 Subject: [PATCH 2/6] s4:torture/smb2/streams: try to rename basefile while is has open streams This tests the following: - create a file with a stream - open the the stream and keep it open - on a second connection, try to rename the basefile, this should fail with NT_STATUS_ACCESS_DENIED Bug: https://bugzilla.samba.org/show_bug.cgi?id=13451 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (cherry picked from commit 00d19bdab15102083b8ba395ede161824c898be1) --- selftest/knownfail.d/samba3.smb2.streams | 2 + source4/torture/smb2/streams.c | 82 ++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+) create mode 100644 selftest/knownfail.d/samba3.smb2.streams diff --git a/selftest/knownfail.d/samba3.smb2.streams b/selftest/knownfail.d/samba3.smb2.streams new file mode 100644 index 00000000000..26d40a67bda --- /dev/null +++ b/selftest/knownfail.d/samba3.smb2.streams @@ -0,0 +1,2 @@ +samba3.smb2.streams.basefile-rename-with-open-stream\(.*\) +samba3.smb2.streams streams_xattr.basefile-rename-with-open-stream\(nt4_dc\) diff --git a/source4/torture/smb2/streams.c b/source4/torture/smb2/streams.c index d302bf923c9..b39d96d4924 100644 --- a/source4/torture/smb2/streams.c +++ b/source4/torture/smb2/streams.c @@ -1830,6 +1830,86 @@ static bool test_stream_attributes(struct torture_context *tctx, return ret; } +static bool test_basefile_rename_with_open_stream(struct torture_context *tctx, + struct smb2_tree *tree) +{ + bool ret = true; + NTSTATUS status; + struct smb2_tree *tree2 = NULL; + struct smb2_create create, create2; + struct smb2_handle h1 = {{0}}, h2 = {{0}}; + const char *fname = "test_rename_openfile"; + const char *sname = "test_rename_openfile:foo"; + const char *fname_renamed = "test_rename_openfile_renamed"; + union smb_setfileinfo sinfo; + const char *data = "test data"; + + ret = torture_smb2_connection(tctx, &tree2); + torture_assert_goto(tctx, ret == true, ret, done, + "torture_smb2_connection failed\n"); + + torture_comment(tctx, "Creating file with stream\n"); + + ZERO_STRUCT(create); + create.in.desired_access = SEC_FILE_ALL; + create.in.share_access = NTCREATEX_SHARE_ACCESS_MASK; + create.in.file_attributes = FILE_ATTRIBUTE_NORMAL; + create.in.create_disposition = NTCREATEX_DISP_OPEN_IF; + create.in.impersonation_level = SMB2_IMPERSONATION_IMPERSONATION; + create.in.fname = sname; + + status = smb2_create(tree, tctx, &create); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_create failed\n"); + + h1 = create.out.file.handle; + + torture_comment(tctx, "Writing to stream\n"); + + status = smb2_util_write(tree, h1, data, 0, strlen(data)); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_util_write failed\n"); + + torture_comment(tctx, "Renaming base file\n"); + + ZERO_STRUCT(create2); + create2.in.desired_access = SEC_FILE_ALL; + create2.in.file_attributes = FILE_ATTRIBUTE_NORMAL; + create2.in.share_access = NTCREATEX_SHARE_ACCESS_MASK; + create2.in.create_disposition = NTCREATEX_DISP_OPEN; + create2.in.impersonation_level = SMB2_IMPERSONATION_IMPERSONATION; + create2.in.fname = fname; + + status = smb2_create(tree2, tctx, &create2); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_create failed\n"); + + h2 = create2.out.file.handle; + + ZERO_STRUCT(sinfo); + sinfo.rename_information.level = RAW_SFILEINFO_RENAME_INFORMATION; + sinfo.rename_information.in.file.handle = h2; + sinfo.rename_information.in.new_name = fname_renamed; + + status = smb2_setinfo_file(tree2, &sinfo); + torture_assert_ntstatus_equal_goto( + tctx, status, NT_STATUS_ACCESS_DENIED, ret, done, + "smb2_setinfo_file didn't return NT_STATUS_ACCESS_DENIED\n"); + + smb2_util_close(tree2, h2); + +done: + if (!smb2_util_handle_empty(h1)) { + smb2_util_close(tree, h1); + } + if (!smb2_util_handle_empty(h2)) { + smb2_util_close(tree2, h2); + } + smb2_util_unlink(tree, fname); + smb2_util_unlink(tree, fname_renamed); + + return ret; +} /* basic testing of streams calls SMB2 @@ -1850,6 +1930,8 @@ struct torture_suite *torture_smb2_streams_init(TALLOC_CTX *ctx) torture_suite_add_1smb2_test(suite, "attributes", test_stream_attributes); torture_suite_add_1smb2_test(suite, "delete", test_stream_delete); torture_suite_add_1smb2_test(suite, "zero-byte", test_zero_byte_stream); + torture_suite_add_1smb2_test(suite, "basefile-rename-with-open-stream", + test_basefile_rename_with_open_stream); suite->description = talloc_strdup(suite, "SMB2-STREAM tests"); -- 2.13.6 From fbdb42c19526ff2ddeab378f384526156da161b0 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sat, 26 May 2018 18:33:00 +0200 Subject: [PATCH 3/6] s4:torture/vfs/fruit: adjust test testing basefile rename to expect failure Renaming a basefile that has open streams must fail with NT_STATUS_ACCESS_DENIED. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13451 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (cherry picked from commit f166207fc0344b51879d863857055ab7ff36a09b) --- selftest/knownfail.d/samba3.vfs.fruit | 3 +++ source4/torture/vfs/fruit.c | 25 ++++--------------------- 2 files changed, 7 insertions(+), 21 deletions(-) diff --git a/selftest/knownfail.d/samba3.vfs.fruit b/selftest/knownfail.d/samba3.vfs.fruit index 8df25bccb79..bf97dbc5822 100644 --- a/selftest/knownfail.d/samba3.vfs.fruit +++ b/selftest/knownfail.d/samba3.vfs.fruit @@ -1 +1,4 @@ ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion\(nt4_dc\) +^samba3.vfs.fruit metadata_netatalk.read open rsrc after rename\(nt4_dc\) +^samba3.vfs.fruit metadata_stream.read open rsrc after rename\(nt4_dc\) +^samba3.vfs.fruit streams_depot.read open rsrc after rename\(nt4_dc\) diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c index 65109cc1934..4564047e0fb 100644 --- a/source4/torture/vfs/fruit.c +++ b/source4/torture/vfs/fruit.c @@ -3897,7 +3897,6 @@ static bool test_rename_and_read_rsrc(struct torture_context *tctx, const char *fname_renamed = "test_rename_openfile_renamed"; const char *data = "1234567890"; union smb_setfileinfo sinfo; - struct smb2_read r; ret = enable_aapl(tctx, tree); torture_assert_goto(tctx, ret == true, ret, done, "enable_aapl failed"); @@ -3949,28 +3948,12 @@ static bool test_rename_and_read_rsrc(struct torture_context *tctx, sinfo.rename_information.in.new_name = fname_renamed; status = smb2_setinfo_file(tree, &sinfo); - torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_setinfo_file failed"); - - smb2_util_close(tree, h2); - - ZERO_STRUCT(r); - r.in.file.handle = h1; - r.in.length = 10; - r.in.offset = 0; - - torture_comment(tctx, "Read resource fork of renamed file\n"); - - status = smb2_read(tree, tree, &r); - torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_read failed"); + torture_assert_ntstatus_equal_goto( + tctx, status, NT_STATUS_ACCESS_DENIED, ret, done, + "smb2_setinfo_file failed"); smb2_util_close(tree, h1); - - torture_assert_goto(tctx, r.out.data.length == 10, ret, done, - talloc_asprintf(tctx, "smb2_read returned %jd bytes, expected 10\n", - (intmax_t)r.out.data.length)); - - torture_assert_goto(tctx, memcmp(r.out.data.data, data, 10) == 0, ret, done, - talloc_asprintf(tctx, "Bad data in stream\n")); + smb2_util_close(tree, h2); done: smb2_util_unlink(tree, fname); -- 2.13.6 From 33e52b7e4e7d54b3488a54e2620f5e07b3042b9c Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sun, 27 May 2018 13:01:50 +0200 Subject: [PATCH 4/6] s3:smbd: add private option NTCREATEX_OPTIONS_PRIVATE_STREAM_BASEOPEN This will be used to mark basefile opens of streams opens. This is needed to later implement a function that can determine if a file has stream opens. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13451 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (cherry picked from commit 37e7ff05ab9443c0330e68f5c701ffecedf2d738) --- source3/include/smb.h | 3 +++ source3/smbd/open.c | 7 ++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/source3/include/smb.h b/source3/include/smb.h index 3316f09d94f..5e83ee90afe 100644 --- a/source3/include/smb.h +++ b/source3/include/smb.h @@ -419,6 +419,9 @@ Offset Data length. /* Private options for printer support */ #define NTCREATEX_OPTIONS_PRIVATE_DELETE_ON_CLOSE 0x0008 +/* Private option for streams support */ +#define NTCREATEX_OPTIONS_PRIVATE_STREAM_BASEOPEN 0x0010 + /* Flag for NT transact rename call. */ #define RENAME_REPLACE_IF_EXISTS 1 diff --git a/source3/smbd/open.c b/source3/smbd/open.c index 3708bdd10fa..8a9288dbdb4 100644 --- a/source3/smbd/open.c +++ b/source3/smbd/open.c @@ -5091,6 +5091,7 @@ static NTSTATUS create_file_unixpath(connection_struct *conn, && (!(private_flags & NTCREATEX_OPTIONS_PRIVATE_STREAM_DELETE))) { uint32_t base_create_disposition; struct smb_filename *smb_fname_base = NULL; + uint32_t base_privflags; if (create_options & FILE_DIRECTORY_FILE) { status = NT_STATUS_NOT_A_DIRECTORY; @@ -5141,13 +5142,17 @@ static NTSTATUS create_file_unixpath(connection_struct *conn, } } + base_privflags = NTCREATEX_OPTIONS_PRIVATE_STREAM_BASEOPEN; + /* Open the base file. */ status = create_file_unixpath(conn, NULL, smb_fname_base, 0, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, base_create_disposition, - 0, 0, 0, NULL, 0, 0, NULL, NULL, + 0, 0, 0, NULL, 0, + base_privflags, + NULL, NULL, &base_fsp, NULL); TALLOC_FREE(smb_fname_base); -- 2.13.6 From f2f02d9b9f2d6c38b2813757ee942b8910985839 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sun, 27 May 2018 13:03:25 +0200 Subject: [PATCH 5/6] s3:locking: add file_has_open_streams() This can be used to check if a file opened by fsp also has stream opens. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13451 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (cherry picked from commit dd8cf54c79fe8536e34cde15801d60931cd47b8b) --- source3/locking/locking.c | 31 +++++++++++++++++++++++++++++++ source3/locking/proto.h | 1 + 2 files changed, 32 insertions(+) diff --git a/source3/locking/locking.c b/source3/locking/locking.c index 4e9f1bbc681..f71cd176029 100644 --- a/source3/locking/locking.c +++ b/source3/locking/locking.c @@ -1318,3 +1318,34 @@ struct timespec get_share_mode_write_time(struct share_mode_lock *lck) } return d->old_write_time; } + +bool file_has_open_streams(files_struct *fsp) +{ + struct share_mode_lock *lock = NULL; + struct share_mode_data *d = NULL; + uint32_t i; + + lock = get_existing_share_mode_lock(talloc_tos(), fsp->file_id); + if (lock == NULL) { + return false; + } + d = lock->data; + + for (i = 0; i < d->num_share_modes; i++) { + struct share_mode_entry *e = &d->share_modes[i]; + + if (share_mode_stale_pid(d, i)) { + continue; + } + + if (e->private_options & + NTCREATEX_OPTIONS_PRIVATE_STREAM_BASEOPEN) + { + TALLOC_FREE(lock); + return true; + } + } + + TALLOC_FREE(lock); + return false; +} diff --git a/source3/locking/proto.h b/source3/locking/proto.h index 33184e0fa0a..4cd38091f3c 100644 --- a/source3/locking/proto.h +++ b/source3/locking/proto.h @@ -205,6 +205,7 @@ bool is_delete_on_close_set(struct share_mode_lock *lck, uint32_t name_hash); bool set_sticky_write_time(struct file_id fileid, struct timespec write_time); bool set_write_time(struct file_id fileid, struct timespec write_time); struct timespec get_share_mode_write_time(struct share_mode_lock *lck); +bool file_has_open_streams(files_struct *fsp); int share_mode_forall(int (*fn)(struct file_id fid, const struct share_mode_data *data, void *private_data), -- 2.13.6 From 809c3b9f13d5d22847a94fcfdec27b022fdb099d Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sat, 26 May 2018 18:32:21 +0200 Subject: [PATCH 6/6] s3:smbd: don't allow renaming basefile if streams are open Bug: https://bugzilla.samba.org/show_bug.cgi?id=13451 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (cherry picked from commit 465b7d07e5db787c3d6330371e5e42ecbb1b57b9) --- selftest/knownfail.d/samba3.smb2.streams | 2 -- selftest/knownfail.d/samba3.vfs.fruit | 3 --- source3/smbd/reply.c | 4 ++++ 3 files changed, 4 insertions(+), 5 deletions(-) delete mode 100644 selftest/knownfail.d/samba3.smb2.streams diff --git a/selftest/knownfail.d/samba3.smb2.streams b/selftest/knownfail.d/samba3.smb2.streams deleted file mode 100644 index 26d40a67bda..00000000000 --- a/selftest/knownfail.d/samba3.smb2.streams +++ /dev/null @@ -1,2 +0,0 @@ -samba3.smb2.streams.basefile-rename-with-open-stream\(.*\) -samba3.smb2.streams streams_xattr.basefile-rename-with-open-stream\(nt4_dc\) diff --git a/selftest/knownfail.d/samba3.vfs.fruit b/selftest/knownfail.d/samba3.vfs.fruit index bf97dbc5822..8df25bccb79 100644 --- a/selftest/knownfail.d/samba3.vfs.fruit +++ b/selftest/knownfail.d/samba3.vfs.fruit @@ -1,4 +1 @@ ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion\(nt4_dc\) -^samba3.vfs.fruit metadata_netatalk.read open rsrc after rename\(nt4_dc\) -^samba3.vfs.fruit metadata_stream.read open rsrc after rename\(nt4_dc\) -^samba3.vfs.fruit streams_depot.read open rsrc after rename\(nt4_dc\) diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c index 623f83b1250..2b5bb8f1ed6 100644 --- a/source3/smbd/reply.c +++ b/source3/smbd/reply.c @@ -6642,6 +6642,10 @@ NTSTATUS rename_internals_fsp(connection_struct *conn, return status; } + if (file_has_open_streams(fsp)) { + return NT_STATUS_ACCESS_DENIED; + } + /* Make a copy of the dst smb_fname structs */ smb_fname_dst = cp_smb_filename(ctx, smb_fname_dst_in); -- 2.13.6