From 9ca83fc902e9f36cf19083a673f3d6e77df8bb54 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 19 Oct 2018 12:15:42 +0200 Subject: [PATCH 01/43] vfs_fruit: remove check for number of xattrs from ad_convert_xattr Turns out that there exist AppleDouble files with an extended FinderInfo entry that includes the xattr marshall buffer, but the count of xattrs in the buffer is just zero. We do want to discard this extended FinderInfo entry and convert it to a simple fixed size FinderInfo entry, so remove the check. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649 Signed-off-by: Ralph Boehme --- source3/modules/vfs_fruit.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c index 58ef3cc68d3..4b0e6e3d21f 100644 --- a/source3/modules/vfs_fruit.c +++ b/source3/modules/vfs_fruit.c @@ -1014,10 +1014,6 @@ static bool ad_convert_xattr(struct adouble *ad, return true; } - if (ad->adx_header.adx_num_attrs == 0) { - return true; - } - if (string_replace_cmaps == NULL) { const char **mappings = NULL; -- 2.17.2 From ad2cb8561c387f84ce3b4e41c9950e6d68b92b3e Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Tue, 2 Oct 2018 16:31:15 +0200 Subject: [PATCH 02/43] docs:vfs_fruit: add "wipe_intentionally_left_blank_rfork" option Bug: https://bugzilla.samba.org/show_bug.cgi?id=13642 Signed-off-by: Ralph Boehme --- docs-xml/manpages/vfs_fruit.8.xml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/docs-xml/manpages/vfs_fruit.8.xml b/docs-xml/manpages/vfs_fruit.8.xml index 25833038d96..0df5a7e5a79 100644 --- a/docs-xml/manpages/vfs_fruit.8.xml +++ b/docs-xml/manpages/vfs_fruit.8.xml @@ -378,6 +378,20 @@ + + fruit:wipe_intentionally_left_blank_rfork = yes | no + + Whether to wipe Resource Fork data that matches the special + 286 bytes sized placeholder blob that macOS client create on + occasion. The blob contains a string This resource fork + intentionally left blank, the remaining bytes being mostly + zero. There being no one use of this data, it is probably safe to + discard it. When this option is enabled, this module truncates the + Resource Fork stream to 0 bytes. + The default is no. + + + -- 2.17.2 From 27122bdf3410ebcfdb51497909db4f728ba268ec Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 4 Oct 2018 18:22:31 +0200 Subject: [PATCH 03/43] docs:vfs_fruit: add "delete_empty_adfiles" option Bug: https://bugzilla.samba.org/show_bug.cgi?id=13642 Signed-off-by: Ralph Boehme --- docs-xml/manpages/vfs_fruit.8.xml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs-xml/manpages/vfs_fruit.8.xml b/docs-xml/manpages/vfs_fruit.8.xml index 0df5a7e5a79..e4ca7bd1828 100644 --- a/docs-xml/manpages/vfs_fruit.8.xml +++ b/docs-xml/manpages/vfs_fruit.8.xml @@ -392,6 +392,17 @@ + + fruit:delete_empty_adfiles = yes | no + + Whether to delete empty AppleDouble files. Empty means that + the resource fork entry in the AppleDouble files is of size 0, or + the size is exactly 286 bytes and the content matches a special + boilerplate resource fork created my macOS. + The default is no. + + + -- 2.17.2 From 5d0f3fda1ef64025b28e15e49eb021c3dcdc6a98 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 4 Oct 2018 13:47:20 +0200 Subject: [PATCH 04/43] s3:selftest: list vfs testssuites one per line Bug: https://bugzilla.samba.org/show_bug.cgi?id=13642 Signed-off-by: Ralph Boehme --- source3/selftest/tests.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index 069213e53c7..29b324e9b68 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -416,7 +416,13 @@ nbt = ["nbt.dgram" ] libsmbclient = ["libsmbclient"] -vfs = ["vfs.fruit", "vfs.acl_xattr", "vfs.fruit_netatalk", "vfs.fruit_file_id", "vfs.fruit_timemachine"] +vfs = [ + "vfs.fruit", + "vfs.acl_xattr", + "vfs.fruit_netatalk", + "vfs.fruit_file_id", + "vfs.fruit_timemachine", +] tests= base + raw + smb2 + rpc + unix + local + rap + nbt + libsmbclient + idmap + vfs -- 2.17.2 From 30496945f50e19672c75760446ad450ec4935b3b Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 4 Oct 2018 14:28:15 +0200 Subject: [PATCH 05/43] s4:torture: add test for AppleDouble ResourceFork conversion Bug: https://bugzilla.samba.org/show_bug.cgi?id=13642 Signed-off-by: Ralph Boehme --- selftest/knownfail.d/samba3.vfs.fruit | 2 + selftest/target/Samba3.pm | 18 +++ source3/selftest/tests.py | 4 + source4/torture/vfs/fruit.c | 191 ++++++++++++++++++++++++++ source4/torture/vfs/vfs.c | 1 + 5 files changed, 216 insertions(+) diff --git a/selftest/knownfail.d/samba3.vfs.fruit b/selftest/knownfail.d/samba3.vfs.fruit index 6307e2b3404..f2a27040a8e 100644 --- a/selftest/knownfail.d/samba3.vfs.fruit +++ b/selftest/knownfail.d/samba3.vfs.fruit @@ -1,2 +1,4 @@ ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion$nt4_dc$ ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion without embedded xattr$nt4_dc$ +^samba3.vfs.fruit_conversion wipe_intentionally_left_blank_rfork.convert_xattr_and_empty_rfork_then_delete$nt4_dc$ +^samba3.vfs.fruit_conversion delete_empty_adfiles.convert_xattr_and_empty_rfork_then_delete$nt4_dc$ diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm index fdbf1fa9a9a..a5835d2f309 100755 --- a/selftest/target/Samba3.pm +++ b/selftest/target/Samba3.pm @@ -1957,6 +1957,24 @@ sub provision($$$$$$$$$) fruit:time machine = yes fruit:time machine max size = 32K +[vfs_fruit_wipe_intentionally_left_blank_rfork] + path = $shrdir + vfs objects = fruit streams_xattr acl_xattr xattr_tdb + fruit:resource = file + fruit:metadata = stream + fruit:wipe_intentionally_left_blank_rfork = true + fruit:delete_empty_adfiles = false + fruit:veto_appledouble = no + +[vfs_fruit_delete_empty_adfiles] + path = $shrdir + vfs objects = fruit streams_xattr acl_xattr xattr_tdb + fruit:resource = file + fruit:metadata = stream + fruit:wipe_intentionally_left_blank_rfork = true + fruit:delete_empty_adfiles = true + fruit:veto_appledouble = no + [badname-tmp] path = $badnames_shrdir guest ok = yes diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index 29b324e9b68..ced19268611 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -422,6 +422,7 @@ vfs = [ "vfs.fruit_netatalk", "vfs.fruit_file_id", "vfs.fruit_timemachine", + "vfs.fruit_conversion", ] tests= base + raw + smb2 + rpc + unix + local + rap + nbt + libsmbclient + idmap + vfs @@ -532,6 +533,9 @@ tests= base + raw + smb2 + rpc + unix + local + rap + nbt + libsmbclient + idmap plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/vfs_fruit_timemachine -U$USERNAME%$PASSWORD --option=torture:localdir=$SELFTEST_PREFIX/nt4_dc/share') elif t == "vfs.fruit_file_id": plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/vfs_fruit -U$USERNAME%$PASSWORD') + elif t == "vfs.fruit_conversion": + plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD --option=torture:share2=vfs_fruit_wipe_intentionally_left_blank_rfork --option=torture:delete_empty_adfiles=false', 'wipe_intentionally_left_blank_rfork') + plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$USERNAME%$PASSWORD --option=torture:share2=vfs_fruit_delete_empty_adfiles --option=torture:delete_empty_adfiles=true', 'delete_empty_adfiles') elif t == "rpc.schannel_anon_setpw": plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmp -U$%', description="anonymous password set") plansmbtorture4testsuite(t, "nt4_dc_schannel", '//$SERVER_IP/tmp -U$%', description="anonymous password set (schannel enforced server-side)") diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c index 58a94dd143c..2eaf698c4e0 100644 --- a/source4/torture/vfs/fruit.c +++ b/source4/torture/vfs/fruit.c @@ -5412,3 +5412,194 @@ struct torture_suite *torture_vfs_fruit_timemachine(TALLOC_CTX *ctx) return suite; } + +static bool test_convert_xattr_and_empty_rfork_then_delete( + struct torture_context *tctx, + struct smb2_tree *tree1, + struct smb2_tree *tree2) +{ + TALLOC_CTX *mem_ctx = talloc_new(tctx); + const char *fname = BASEDIR "\\test_adouble_conversion"; + const char *adname = BASEDIR "/._test_adouble_conversion"; + const char *rfork = BASEDIR "\\test_adouble_conversion" AFPRESOURCE_STREAM_NAME; + NTSTATUS status; + struct smb2_handle testdirh; + bool ret = true; + const char *streams[] = { + "::$DATA", + AFPINFO_STREAM, + ":com.apple.metadata" "\xef\x80\xa2" "_kMDItemUserTags:$DATA", + ":foo" "\xef\x80\xa2" "bar:$DATA", /* "foo:bar:$DATA" */ + }; + struct smb2_create create; + struct smb2_find find; + unsigned int count; + union smb_search_data *d; + bool delete_empty_adfiles; + int expected_num_files; + + delete_empty_adfiles = torture_setting_bool(tctx, + "delete_empty_adfiles", + false); + + smb2_deltree(tree1, BASEDIR); + + status = torture_smb2_testdir(tree1, BASEDIR, &testdirh); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "torture_smb2_testdir failed\n"); + smb2_util_close(tree1, testdirh); + + ret = torture_setup_file(tctx, tree1, fname, false); + torture_assert_goto(tctx, ret == true, ret, done, + "torture_setup_file failed\n"); + + ret = torture_setup_file(tctx, tree1, adname, false); + torture_assert_goto(tctx, ret == true, ret, done, + "torture_setup_file failed\n"); + + ret = write_stream(tree1, __location__, tctx, mem_ctx, + adname, NULL, + 0, sizeof(osx_adouble_w_xattr), osx_adouble_w_xattr); + torture_assert_goto(tctx, ret == true, ret, done, + "write_stream failed\n"); + + ret = enable_aapl(tctx, tree2); + torture_assert_goto(tctx, ret == true, ret, done, "enable_aapl failed"); + + /* + * Issue a smb2_find(), this triggers the server-side conversion + */ + + create = (struct smb2_create) { + .in.desired_access = SEC_RIGHTS_DIR_READ, + .in.create_options = NTCREATEX_OPTIONS_DIRECTORY, + .in.file_attributes = FILE_ATTRIBUTE_DIRECTORY, + .in.share_access = NTCREATEX_SHARE_ACCESS_READ, + .in.create_disposition = NTCREATEX_DISP_OPEN, + .in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS, + .in.fname = BASEDIR, + }; + + status = smb2_create(tree2, tctx, &create); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_create failed\n"); + + find = (struct smb2_find) { + .in.file.handle = create.out.file.handle, + .in.pattern = "*", + .in.max_response_size = 0x1000, + .in.level = SMB2_FIND_ID_BOTH_DIRECTORY_INFO, + }; + + status = smb2_find_level(tree2, tree2, &find, &count, &d); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_find_level failed\n"); + + status = smb2_util_close(tree2, create.out.file.handle); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_util_close failed"); + + /* + * Check number of streams + */ + + ret = check_stream_list(tree2, tctx, fname, 4, streams, false); + torture_assert_goto(tctx, ret == true, ret, done, "check_stream_list"); + + /* + * Check Resource Fork is gone + */ + + create = (struct smb2_create) { + .in.desired_access = SEC_RIGHTS_FILE_READ|SEC_RIGHTS_FILE_WRITE, + .in.file_attributes = FILE_ATTRIBUTE_NORMAL, + .in.share_access = NTCREATEX_SHARE_ACCESS_READ, + .in.create_disposition = NTCREATEX_DISP_OPEN, + .in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS, + .in.fname = rfork, + }; + + status = smb2_create(tree2, mem_ctx, &create); + torture_assert_ntstatus_equal_goto( + tctx, status, NT_STATUS_OBJECT_NAME_NOT_FOUND, + ret, done, "Bad smb2_create return\n"); + + /* + * Check xattr data has been migrated from the AppleDouble file to + * streams. + */ + + ret = check_stream(tree2, __location__, tctx, mem_ctx, + fname, AFPINFO_STREAM, + 0, 60, 16, 8, "TESTSLOW"); + torture_assert_goto(tctx, ret == true, ret, done, + "check AFPINFO_STREAM failed\n"); + + ret = check_stream(tree2, __location__, tctx, mem_ctx, + fname, ":foo" "\xef\x80\xa2" "bar", /* foo:bar */ + 0, 3, 0, 3, "baz"); + torture_assert_goto(tctx, ret == true, ret, done, + "check foo stream failed\n"); + + /* + * Now check number of files. If delete_empty_adfiles is set, the + * AppleDouble files should have been deleted. + */ + + create = (struct smb2_create) { + .in.desired_access = SEC_RIGHTS_DIR_READ, + .in.create_options = NTCREATEX_OPTIONS_DIRECTORY, + .in.file_attributes = FILE_ATTRIBUTE_DIRECTORY, + .in.share_access = NTCREATEX_SHARE_ACCESS_READ, + .in.create_disposition = NTCREATEX_DISP_OPEN, + .in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS, + .in.fname = BASEDIR, + }; + + status = smb2_create(tree2, tctx, &create); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_create failed\n"); + + find = (struct smb2_find) { + .in.file.handle = create.out.file.handle, + .in.pattern = "*", + .in.max_response_size = 0x1000, + .in.level = SMB2_FIND_ID_BOTH_DIRECTORY_INFO, + }; + + status = smb2_find_level(tree2, tree2, &find, &count, &d); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_find_level failed\n"); + + status = smb2_util_close(tree2, create.out.file.handle); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_util_close failed"); + + if (delete_empty_adfiles) { + expected_num_files = 3; + } else { + expected_num_files = 4; + } + torture_assert_int_equal_goto(tctx, count, expected_num_files, ret, done, + "Wrong number of files\n"); + +done: + smb2_deltree(tree1, BASEDIR); + talloc_free(mem_ctx); + return ret; +} + +struct torture_suite *torture_vfs_fruit_conversion(TALLOC_CTX *ctx) +{ + struct torture_suite *suite = torture_suite_create( + ctx, "fruit_conversion"); + + suite->description = talloc_strdup( + suite, "vfs_fruit conversion tests"); + + torture_suite_add_2ns_smb2_test( + suite, "convert_xattr_and_empty_rfork_then_delete", + test_convert_xattr_and_empty_rfork_then_delete); + + return suite; +} diff --git a/source4/torture/vfs/vfs.c b/source4/torture/vfs/vfs.c index 64cb0e3d6c4..39cd573c9d6 100644 --- a/source4/torture/vfs/vfs.c +++ b/source4/torture/vfs/vfs.c @@ -113,6 +113,7 @@ NTSTATUS torture_vfs_init(TALLOC_CTX *ctx) torture_suite_add_suite(suite, torture_acl_xattr(suite)); torture_suite_add_suite(suite, torture_vfs_fruit_file_id(suite)); torture_suite_add_suite(suite, torture_vfs_fruit_timemachine(suite)); + torture_suite_add_suite(suite, torture_vfs_fruit_conversion(suite)); torture_register_suite(ctx, suite); -- 2.17.2 From c9584419a75f271933fe7d0c0a9d45d79bf1028c Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 3 Oct 2018 12:01:00 +0200 Subject: [PATCH 06/43] vfs_fruit: add option "wipe_intentionally_left_blank_rfork" Bug: https://bugzilla.samba.org/show_bug.cgi?id=13642 Signed-off-by: Ralph Boehme --- source3/modules/vfs_fruit.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c index 4b0e6e3d21f..89307a06f26 100644 --- a/source3/modules/vfs_fruit.c +++ b/source3/modules/vfs_fruit.c @@ -142,6 +142,7 @@ struct fruit_config_data { const char *model; bool time_machine; off_t time_machine_max_size; + bool wipe_intentionally_left_blank_rfork; /* * Additional options, all enabled by default, @@ -2094,6 +2095,10 @@ static int init_fruit_config(vfs_handle_struct *handle) config->time_machine_max_size = conv_str_size(tm_size_str); } + config->wipe_intentionally_left_blank_rfork = lp_parm_bool( + SNUM(handle->conn), FRUIT_PARAM_TYPE_NAME, + "wipe_intentionally_left_blank_rfork", false); + SMB_VFS_HANDLE_SET_DATA(handle, config, NULL, struct fruit_config_data, return -1); -- 2.17.2 From 50cc7c0e0959135a783f98486f8da1d7889f2004 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Tue, 2 Oct 2018 16:05:28 +0200 Subject: [PATCH 07/43] vfs_fruit: detect empty resource forks in ad_convert() For some reason the macOS client often writes AppleDouble files with a non-zero sized resource fork, but the resource fork data is just boilerplate data with the following string close to the start This resource fork intentionally left blank A dump with apple_dump looks like this: Entry ID : 00000002 : Resource Fork Offset : 00000052 : 82 Length : 0000011E : 286 -RAW DUMP--: 0 1 2 3 4 5 6 7 8 9 A B C D E F : (ASCII) 00000000 : 00 00 01 00 00 00 01 00 00 00 00 00 00 00 00 1E : ................ 00000010 : 54 68 69 73 20 72 65 73 6F 75 72 63 65 20 66 6F : This resource fo 00000020 : 72 6B 20 69 6E 74 65 6E 74 69 6F 6E 61 6C 6C 79 : rk intentionally 00000030 : 20 6C 65 66 74 20 62 6C 61 6E 6B 20 20 20 00 00 : left blank .. 00000040 : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................ 00000050 : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................ 00000060 : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................ 00000070 : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................ 00000080 : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................ 00000090 : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................ 000000A0 : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................ 000000B0 : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................ 000000C0 : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................ 000000D0 : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................ 000000E0 : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................ 000000F0 : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................ 00000100 : 00 00 01 00 00 00 01 00 00 00 00 00 00 00 00 1E : ................ 00000110 : 00 00 00 00 00 00 00 00 00 1C 00 1E FF FF : .............. We can safely discard this Resource Fork data. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13642 Signed-off-by: Ralph Boehme --- selftest/knownfail.d/samba3.vfs.fruit | 1 - source3/modules/vfs_fruit.c | 111 +++++++++++++++++++++++++- 2 files changed, 110 insertions(+), 2 deletions(-) diff --git a/selftest/knownfail.d/samba3.vfs.fruit b/selftest/knownfail.d/samba3.vfs.fruit index f2a27040a8e..8b9aed48b63 100644 --- a/selftest/knownfail.d/samba3.vfs.fruit +++ b/selftest/knownfail.d/samba3.vfs.fruit @@ -1,4 +1,3 @@ ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion$nt4_dc$ ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion without embedded xattr$nt4_dc$ -^samba3.vfs.fruit_conversion wipe_intentionally_left_blank_rfork.convert_xattr_and_empty_rfork_then_delete$nt4_dc$ ^samba3.vfs.fruit_conversion delete_empty_adfiles.convert_xattr_and_empty_rfork_then_delete$nt4_dc$ diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c index 89307a06f26..a4795700299 100644 --- a/source3/modules/vfs_fruit.c +++ b/source3/modules/vfs_fruit.c @@ -468,6 +468,45 @@ static const uint32_t set_eid[] = { AD_DEV, AD_INO, AD_SYN, AD_ID }; +static char empty_resourcefork[] = { + 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x1E, + 0x54, 0x68, 0x69, 0x73, 0x20, 0x72, 0x65, 0x73, + 0x6F, 0x75, 0x72, 0x63, 0x65, 0x20, 0x66, 0x6F, + 0x72, 0x6B, 0x20, 0x69, 0x6E, 0x74, 0x65, 0x6E, + 0x74, 0x69, 0x6F, 0x6E, 0x61, 0x6C, 0x6C, 0x79, + 0x20, 0x6C, 0x65, 0x66, 0x74, 0x20, 0x62, 0x6C, + 0x61, 0x6E, 0x6B, 0x20, 0x20, 0x20, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x1E, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x1C, 0x00, 0x1E, 0xFF, 0xFF +}; + struct fio { /* tcon config handle */ struct fruit_config_data *config; @@ -1282,6 +1321,70 @@ static bool ad_convert_truncate(struct adouble *ad, return true; } +static bool ad_convert_blank_rfork(struct adouble *ad, + bool *blank) +{ + struct fruit_config_data *config = NULL; + uint8_t *map = MAP_FAILED; + size_t maplen; + int cmp; + ssize_t len; + int rc; + bool ok; + + *blank = false; + + SMB_VFS_HANDLE_GET_DATA(ad->ad_handle, config, + struct fruit_config_data, return false); + + if (!config->wipe_intentionally_left_blank_rfork) { + return true; + } + + if (ad_getentrylen(ad, ADEID_RFORK) != sizeof(empty_resourcefork)) { + return true; + } + + maplen = ad_getentryoff(ad, ADEID_RFORK) + + ad_getentrylen(ad, ADEID_RFORK); + + /* FIXME: direct use of mmap(), vfs_aio_fork does it too */ + map = mmap(NULL, maplen, PROT_READ|PROT_WRITE, MAP_SHARED, + ad->ad_fd, 0); + if (map == MAP_FAILED) { + DBG_ERR("mmap AppleDouble: %s\n", strerror(errno)); + return false; + } + + cmp = memcmp(map + ADEDOFF_RFORK_DOT_UND, + empty_resourcefork, + sizeof(empty_resourcefork)); + rc = munmap(map, maplen); + if (rc != 0) { + DBG_ERR("munmap failed: %s\n", strerror(errno)); + return false; + } + + if (cmp != 0) { + return true; + } + + ad_setentrylen(ad, ADEID_RFORK, 0); + + ok = ad_pack(ad); + if (!ok) { + return false; + } + + len = sys_pwrite(ad->ad_fd, ad->ad_data, AD_DATASZ_DOT_UND, 0); + if (len != AD_DATASZ_DOT_UND) { + return false; + } + + *blank = true; + return true; +} + /** * Convert from Apple's ._ file to Netatalk * @@ -1296,13 +1399,19 @@ static int ad_convert(struct adouble *ad, { bool ok; bool converted_xattr = false; + bool blank; ok = ad_convert_xattr(ad, smb_fname, &converted_xattr); if (!ok) { return -1; } - if (converted_xattr) { + ok = ad_convert_blank_rfork(ad, &blank); + if (!ok) { + return -1; + } + + if (converted_xattr || blank) { ok = ad_convert_truncate(ad, smb_fname); if (!ok) { return -1; -- 2.17.2 From f4ccb47907c1f549a7d44007a5c8eaf77ebfa0cb Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 3 Oct 2018 12:01:00 +0200 Subject: [PATCH 08/43] vfs_fruit: add option "delete_empty_adfiles" Bug: https://bugzilla.samba.org/show_bug.cgi?id=13642 Signed-off-by: Ralph Boehme --- source3/modules/vfs_fruit.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c index a4795700299..6c00ce8048a 100644 --- a/source3/modules/vfs_fruit.c +++ b/source3/modules/vfs_fruit.c @@ -143,6 +143,7 @@ struct fruit_config_data { bool time_machine; off_t time_machine_max_size; bool wipe_intentionally_left_blank_rfork; + bool delete_empty_adfiles; /* * Additional options, all enabled by default, @@ -2208,6 +2209,10 @@ static int init_fruit_config(vfs_handle_struct *handle) SNUM(handle->conn), FRUIT_PARAM_TYPE_NAME, "wipe_intentionally_left_blank_rfork", false); + config->delete_empty_adfiles = lp_parm_bool( + SNUM(handle->conn), FRUIT_PARAM_TYPE_NAME, + "delete_empty_adfiles", false); + SMB_VFS_HANDLE_SET_DATA(handle, config, NULL, struct fruit_config_data, return -1); -- 2.17.2 From b953a3b7239b61747851a9e0c0a49b2c3d0429e9 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Tue, 9 Oct 2018 14:54:31 +0200 Subject: [PATCH 09/43] vfs_fruit: optionally delete AppleDouble files without Resourcefork data Bug: https://bugzilla.samba.org/show_bug.cgi?id=13642 Signed-off-by: Ralph Boehme --- selftest/knownfail.d/samba3.vfs.fruit | 2 +- source3/modules/vfs_fruit.c | 42 +++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/selftest/knownfail.d/samba3.vfs.fruit b/selftest/knownfail.d/samba3.vfs.fruit index 8b9aed48b63..2b49e827d7e 100644 --- a/selftest/knownfail.d/samba3.vfs.fruit +++ b/selftest/knownfail.d/samba3.vfs.fruit @@ -1,3 +1,3 @@ ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion$nt4_dc$ ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion without embedded xattr$nt4_dc$ -^samba3.vfs.fruit_conversion delete_empty_adfiles.convert_xattr_and_empty_rfork_then_delete$nt4_dc$ + diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c index 6c00ce8048a..5c986a7b3ea 100644 --- a/source3/modules/vfs_fruit.c +++ b/source3/modules/vfs_fruit.c @@ -1386,6 +1386,43 @@ static bool ad_convert_blank_rfork(struct adouble *ad, return true; } +static bool ad_convert_delete_adfile(struct adouble *ad, + const struct smb_filename *smb_fname) +{ + struct fruit_config_data *config = NULL; + struct smb_filename *ad_name = NULL; + int rc; + + if (ad_getentrylen(ad, ADEID_RFORK) > 0) { + return true; + } + + SMB_VFS_HANDLE_GET_DATA(ad->ad_handle, config, + struct fruit_config_data, return false); + + if (!config->delete_empty_adfiles) { + return true; + } + + rc = adouble_path(talloc_tos(), smb_fname, &ad_name); + if (rc != 0) { + return false; + } + + rc = SMB_VFS_NEXT_UNLINK(ad->ad_handle, ad_name); + if (rc != 0) { + DBG_ERR("Unlinking [%s] failed: %s\n", + smb_fname_str_dbg(ad_name), strerror(errno)); + TALLOC_FREE(ad_name); + return false; + } + + DBG_WARNING("Unlinked [%s] after conversion\n", smb_fname_str_dbg(ad_name)); + TALLOC_FREE(ad_name); + + return true; +} + /** * Convert from Apple's ._ file to Netatalk * @@ -1426,6 +1463,11 @@ static int ad_convert(struct adouble *ad, return -1; } + ok = ad_convert_delete_adfile(ad, smb_fname); + if (!ok) { + return -1; + } + return 0; } -- 2.17.2 From d23ded0fa5053aa539c60112d5c8ec45d4941cd4 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 23 Aug 2018 12:07:20 +0200 Subject: [PATCH 10/43] vfs_streams_xattr: fix open implementation Since a long time the modules's open function happily returned success when opening a non existent stream without O_CREAT. This change fixes it to return -1 and errno=ENOATTR if o get_ea_value() returns NT_STATUS_NOT_FOUND (eg mapped from getxattr() = -1, errno=ENOATTR) and o flags doesn't contain O_CREAT Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- source3/modules/vfs_streams_xattr.c | 64 +++++++++++++++++------------ 1 file changed, 38 insertions(+), 26 deletions(-) diff --git a/source3/modules/vfs_streams_xattr.c b/source3/modules/vfs_streams_xattr.c index 8714007cb8d..87c5589395a 100644 --- a/source3/modules/vfs_streams_xattr.c +++ b/source3/modules/vfs_streams_xattr.c @@ -407,6 +407,7 @@ static int streams_xattr_open(vfs_handle_struct *handle, char *xattr_name = NULL; int pipe_fds[2]; int fakefd = -1; + bool set_empty_xattr = false; int ret; SMB_VFS_HANDLE_GET_DATA(handle, config, struct streams_xattr_config, @@ -440,39 +441,37 @@ static int streams_xattr_open(vfs_handle_struct *handle, goto fail; } - /* - * Return a valid fd, but ensure any attempt to use it returns an error - * (EPIPE). - */ - ret = pipe(pipe_fds); - if (ret != 0) { - goto fail; - } - - close(pipe_fds[1]); - pipe_fds[1] = -1; - fakefd = pipe_fds[0]; - status = get_ea_value(talloc_tos(), handle->conn, NULL, smb_fname, xattr_name, &ea); DEBUG(10, ("get_ea_value returned %s\n", nt_errstr(status))); - if (!NT_STATUS_IS_OK(status) - && !NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) { - /* - * The base file is not there. This is an error even if we got - * O_CREAT, the higher levels should have created the base - * file for us. - */ - DEBUG(10, ("streams_xattr_open: base file %s not around, " - "returning ENOENT\n", smb_fname->base_name)); - errno = ENOENT; - goto fail; + if (!NT_STATUS_IS_OK(status)) { + if (!NT_STATUS_EQUAL(status, NT_STATUS_NOT_FOUND)) { + /* + * The base file is not there. This is an error even if + * we got O_CREAT, the higher levels should have created + * the base file for us. + */ + DBG_DEBUG("streams_xattr_open: base file %s not around, " + "returning ENOENT\n", smb_fname->base_name); + errno = ENOENT; + goto fail; + } + + if (!(flags & O_CREAT)) { + errno = ENOATTR; + goto fail; + } + + set_empty_xattr = true; } - if ((!NT_STATUS_IS_OK(status) && (flags & O_CREAT)) || - (flags & O_TRUNC)) { + if (flags & O_TRUNC) { + set_empty_xattr = true; + } + + if (set_empty_xattr) { /* * The attribute does not exist or needs to be truncated */ @@ -495,6 +494,19 @@ static int streams_xattr_open(vfs_handle_struct *handle, } } + /* + * Return a valid fd, but ensure any attempt to use it returns an error + * (EPIPE). + */ + ret = pipe(pipe_fds); + if (ret != 0) { + goto fail; + } + + close(pipe_fds[1]); + pipe_fds[1] = -1; + fakefd = pipe_fds[0]; + sio = VFS_ADD_FSP_EXTENSION(handle, fsp, struct stream_io, NULL); if (sio == NULL) { errno = ENOMEM; -- 2.17.2 From fc0235f50fd26b8704d41aaa5665dd588123c4e9 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 17 Oct 2018 10:54:10 +0200 Subject: [PATCH 11/43] vfs_streams_xattr: consistently log dev/ino as hex open_file_ntcreate() logs dev/ino in hex format if it hits a "dev/ino mismatch". Logging it consistently as hex helps debugging such mismatch issues. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- source3/modules/vfs_streams_xattr.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/source3/modules/vfs_streams_xattr.c b/source3/modules/vfs_streams_xattr.c index 87c5589395a..6fbf568ba18 100644 --- a/source3/modules/vfs_streams_xattr.c +++ b/source3/modules/vfs_streams_xattr.c @@ -52,9 +52,9 @@ static SMB_INO_T stream_inode(const SMB_STRUCT_STAT *sbuf, const char *sname) SMB_INO_T result; char *upper_sname; - DEBUG(10, ("stream_inode called for %lu/%lu [%s]\n", - (unsigned long)sbuf->st_ex_dev, - (unsigned long)sbuf->st_ex_ino, sname)); + DBG_DEBUG("stream_inode called for 0x%jx/0x%jx [%s]\n", + (uintmax_t)sbuf->st_ex_dev, + (uintmax_t)sbuf->st_ex_ino, sname); upper_sname = talloc_strdup_upper(talloc_tos(), sname); SMB_ASSERT(upper_sname != NULL); @@ -73,7 +73,7 @@ static SMB_INO_T stream_inode(const SMB_STRUCT_STAT *sbuf, const char *sname) /* Hopefully all the variation is in the lower 4 (or 8) bytes! */ memcpy(&result, hash, sizeof(result)); - DEBUG(10, ("stream_inode returns %lu\n", (unsigned long)result)); + DBG_DEBUG("stream_inode returns 0x%jx\n", (uintmax_t)result); return result; } -- 2.17.2 From a5a16d8d2596d881eb9e8ab615bb7ae954e509ef Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 19 Oct 2018 22:21:10 +0200 Subject: [PATCH 12/43] s4:torture/vfs/fruit: skip a few tests when running against a macOS SMB server These tests are designed to test specific vfs_fruit functionality. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- source4/torture/vfs/fruit.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c index 2eaf698c4e0..b15958afe07 100644 --- a/source4/torture/vfs/fruit.c +++ b/source4/torture/vfs/fruit.c @@ -2111,6 +2111,11 @@ static bool test_adouble_conversion(struct torture_context *tctx, ":com.apple.metadata" "\xef\x80\xa2" "_kMDItemUserTags:$DATA", ":foo" "\xef\x80\xa2" "bar:$DATA", /* "foo:bar:$DATA" */ }; + bool is_osx = torture_setting_bool(tctx, "osx", false); + + if (is_osx) { + torture_skip(tctx, "Test only works with Samba\n"); + } smb2_deltree(tree, BASEDIR); @@ -2185,6 +2190,11 @@ static bool test_adouble_conversion_wo_xattr(struct torture_context *tctx, union smb_search_data *d; const char *data = "This resource fork intentionally left blank"; size_t datalen = strlen(data); + bool is_osx = torture_setting_bool(tctx, "osx", false); + + if (is_osx) { + torture_skip(tctx, "Test only works with Samba\n"); + } smb2_deltree(tree, BASEDIR); @@ -4742,6 +4752,11 @@ static bool test_nfs_aces(struct torture_context *tctx, struct security_descriptor *psd = NULL; NTSTATUS status; bool ret = true; + bool is_osx = torture_setting_bool(tctx, "osx", false); + + if (is_osx) { + torture_skip(tctx, "Test only works with Samba\n"); + } ret = enable_aapl(tctx, tree); torture_assert(tctx, ret == true, "enable_aapl failed"); -- 2.17.2 From 4f1ab9e6deff67fe7afcd5d3fa1b65c20af5caf8 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 17 Oct 2018 10:51:45 +0200 Subject: [PATCH 13/43] s4:torture/vfs/fruit: fix a few error checks in "delete AFP_AfpInfo by writing all 0" Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- source4/torture/vfs/fruit.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c index b15958afe07..3039c4ecc5b 100644 --- a/source4/torture/vfs/fruit.c +++ b/source4/torture/vfs/fruit.c @@ -3675,8 +3675,8 @@ static bool test_afpinfo_all0(struct torture_context *tctx, create.in.fname = fname; status = smb2_create(tree, mem_ctx, &create); - torture_assert_goto(tctx, ret == true, ret, done, - "smb2_create failed\n"); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_create failed\n"); baseh = create.out.file.handle; ZERO_STRUCT(create); @@ -3686,8 +3686,8 @@ static bool test_afpinfo_all0(struct torture_context *tctx, create.in.fname = sname; status = smb2_create(tree, mem_ctx, &create); - torture_assert_goto(tctx, ret == true, ret, done, - "smb2_create failed\n"); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_create failed\n"); h1 = create.out.file.handle; status = smb2_util_write(tree, h1, infobuf, 0, AFP_INFO_SIZE); -- 2.17.2 From 0206cb029e393de74adb4d3db143583017402cd3 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 11 Oct 2018 17:13:52 +0200 Subject: [PATCH 14/43] s4:torture/vfs/fruit: set share_access to NTCREATEX_SHARE_ACCESS_MASK in check_stream_list Avoid sharing conflicts with other opens on the basefile. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- source4/torture/vfs/fruit.c | 1 + 1 file changed, 1 insertion(+) diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c index 3039c4ecc5b..d067a9c97e2 100644 --- a/source4/torture/vfs/fruit.c +++ b/source4/torture/vfs/fruit.c @@ -3053,6 +3053,7 @@ static bool check_stream_list(struct smb2_tree *tree, create.in.desired_access = SEC_FILE_ALL; create.in.create_options = is_dir ? NTCREATEX_OPTIONS_DIRECTORY : 0; create.in.file_attributes = is_dir ? FILE_ATTRIBUTE_DIRECTORY : FILE_ATTRIBUTE_NORMAL; + create.in.share_access = NTCREATEX_SHARE_ACCESS_MASK; status = smb2_create(tree, tmp_ctx, &create); torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_create"); h = create.out.file.handle; -- 2.17.2 From ff782eafc6df73259bc47716853e18af67c5f361 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Mon, 15 Oct 2018 15:31:21 +0200 Subject: [PATCH 15/43] s4:torture/vfs/fruit: update test "SMB2/CREATE context AAPL" to work against macOS Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- source4/torture/vfs/fruit.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c index d067a9c97e2..0e463a7e451 100644 --- a/source4/torture/vfs/fruit.c +++ b/source4/torture/vfs/fruit.c @@ -2306,6 +2306,7 @@ static bool test_aapl(struct torture_context *tctx, uint32_t aapl_reply_bitmap; uint32_t aapl_server_caps; uint32_t aapl_vol_caps; + uint32_t expected_vol_caps = 0; char *model; struct smb2_find f; unsigned int count; @@ -2424,8 +2425,11 @@ static bool test_aapl(struct torture_context *tctx, goto done; } + if (is_osx_server) { + expected_vol_caps = 5; + } aapl_vol_caps = BVAL(aapl->data.data, 24); - if (aapl_vol_caps != 0) { + if (aapl_vol_caps != expected_vol_caps) { /* this will fail on a case insensitive fs ... */ torture_result(tctx, TORTURE_FAIL, "(%s) unexpected vol_caps: %d", -- 2.17.2 From b79798c9bad486baca70d074c71f8492897ab674 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Mon, 15 Oct 2018 15:39:12 +0200 Subject: [PATCH 16/43] s4:torture/vfs/fruit: update test "stream names" to work with macOS o create the basefile before trying to create a stream on it, otherwise this fails on macOS o write something to the stream, otherwise the stream is not listed as macOS hides 0-byte sized streams Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- source4/torture/vfs/fruit.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c index 0e463a7e451..681dde53d06 100644 --- a/source4/torture/vfs/fruit.c +++ b/source4/torture/vfs/fruit.c @@ -3132,6 +3132,9 @@ static bool test_stream_names(struct torture_context *tctx, CHECK_STATUS(status, NT_STATUS_OK); smb2_util_close(tree, h); + ret = torture_setup_file(mem_ctx, tree, fname, false); + torture_assert_goto(tctx, ret, ret, done, "torture_setup_file"); + torture_comment(tctx, "(%s) testing stream names\n", __location__); ZERO_STRUCT(create); create.in.desired_access = SEC_FILE_WRITE_DATA; @@ -3146,6 +3149,11 @@ static bool test_stream_names(struct torture_context *tctx, status = smb2_create(tree, mem_ctx, &create); CHECK_STATUS(status, NT_STATUS_OK); + + status = smb2_util_write(tree, create.out.file.handle, "foo", 0, 3); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_util_write failed\n"); + smb2_util_close(tree, create.out.file.handle); ret = check_stream_list(tree, tctx, fname, 2, streams, false); -- 2.17.2 From 27575e607e1b07b90f2062db99d0b3327c4d23b9 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Mon, 15 Oct 2018 16:03:03 +0200 Subject: [PATCH 17/43] s4:torture/vfs/fruit: update test "rename_dir_openfile" to work against macOS The test opens a directory twice where the first open didn't allow sharing. No idea why this works against Samba, against macOS the second open got a sharing violation. Obvious fix: add proper sharing. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- source4/torture/vfs/fruit.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c index 681dde53d06..0b5f7726c9d 100644 --- a/source4/torture/vfs/fruit.c +++ b/source4/torture/vfs/fruit.c @@ -3190,7 +3190,7 @@ static bool test_rename_dir_openfile(struct torture_context *torture, io.smb2.in.desired_access = 0x0017019f; io.smb2.in.create_options = NTCREATEX_OPTIONS_DIRECTORY; io.smb2.in.file_attributes = FILE_ATTRIBUTE_DIRECTORY; - io.smb2.in.share_access = 0; + io.smb2.in.share_access = NTCREATEX_SHARE_ACCESS_MASK; io.smb2.in.alloc_size = 0; io.smb2.in.create_disposition = NTCREATEX_DISP_CREATE; io.smb2.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS; @@ -3252,7 +3252,7 @@ static bool test_rename_dir_openfile(struct torture_context *torture, io.generic.level = RAW_OPEN_SMB2; io.smb2.in.desired_access = 0x0017019f; io.smb2.in.file_attributes = FILE_ATTRIBUTE_DIRECTORY; - io.smb2.in.share_access = 0; + io.smb2.in.share_access = NTCREATEX_SHARE_ACCESS_MASK; io.smb2.in.alloc_size = 0; io.smb2.in.create_disposition = NTCREATEX_DISP_OPEN; io.smb2.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS; -- 2.17.2 From d63926e14e998b6b90480d9045dc4b3a6359598a Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Mon, 15 Oct 2018 16:24:19 +0200 Subject: [PATCH 18/43] s4:torture/vfs/fruit: update test "read open rsrc after rename" to work with macOS macOS SMB server seems to return NT_STATUS_SHARING_VIOLATION in this case while Windows 2016 returns NT_STATUS_ACCESS_DENIED. Lets stick with the Windows error code for now in the Samba fileserver, but let the test pass against macOS. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- source4/torture/vfs/fruit.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c index 0b5f7726c9d..13c8cc9288c 100644 --- a/source4/torture/vfs/fruit.c +++ b/source4/torture/vfs/fruit.c @@ -4182,6 +4182,8 @@ 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; + bool server_is_macos = torture_setting_bool(tctx, "osx", false); + NTSTATUS expected_status; ret = enable_aapl(tctx, tree); torture_assert_goto(tctx, ret == true, ret, done, "enable_aapl failed"); @@ -4232,14 +4234,25 @@ static bool test_rename_and_read_rsrc(struct torture_context *tctx, sinfo.rename_information.in.root_fid = 0; sinfo.rename_information.in.new_name = fname_renamed; + if (server_is_macos) { + expected_status = NT_STATUS_SHARING_VIOLATION; + } else { + expected_status = NT_STATUS_ACCESS_DENIED; + } + status = smb2_setinfo_file(tree, &sinfo); torture_assert_ntstatus_equal_goto( - tctx, status, NT_STATUS_ACCESS_DENIED, ret, done, + tctx, status, expected_status, ret, done, "smb2_setinfo_file failed"); - smb2_util_close(tree, h1); smb2_util_close(tree, h2); + status = smb2_util_write(tree, h1, "foo", 0, 3); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "write failed\n"); + + smb2_util_close(tree, h1); + done: smb2_util_unlink(tree, fname); smb2_util_unlink(tree, fname_renamed); -- 2.17.2 From cac085de923b04f3fe64e215e143b4b160f99536 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Tue, 9 Oct 2018 18:48:08 +0200 Subject: [PATCH 19/43] s4:torture/vfs/fruit: expand existing test "setinfo delete-on-close AFP_AfpInfo" a little bit Add a check that verifies a create on a stream gets NT_STATUS_DELETE_PENDING after delete-on-close has been set. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- source4/torture/vfs/fruit.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c index 13c8cc9288c..85643038764 100644 --- a/source4/torture/vfs/fruit.c +++ b/source4/torture/vfs/fruit.c @@ -3453,6 +3453,10 @@ static bool test_setinfo_delete_on_close(struct torture_context *tctx, const char *sname = BASEDIR "\\file" AFPINFO_STREAM_NAME; const char *type_creator = "SMB,OLE!"; AfpInfo *info = NULL; + const char *streams[] = { + AFPINFO_STREAM, + "::$DATA" + }; const char *streams_basic[] = { "::$DATA" }; @@ -3494,6 +3498,19 @@ static bool test_setinfo_delete_on_close(struct torture_context *tctx, status = smb2_setinfo_file(tree, &sfinfo); torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "set delete-on-close failed"); + ret = check_stream_list(tree, tctx, fname, 2, streams, false); + torture_assert_goto(tctx, ret == true, ret, done, "Bad streams"); + + ZERO_STRUCT(create); + create.in.create_disposition = NTCREATEX_DISP_OPEN; + create.in.desired_access = SEC_FILE_ALL; + create.in.fname = sname; + create.in.file_attributes = FILE_ATTRIBUTE_NORMAL; + create.in.impersonation_level = NTCREATEX_IMPERSONATION_IMPERSONATION; + status = smb2_create(tree, mem_ctx, &create); + torture_assert_ntstatus_equal_goto(tctx, status, NT_STATUS_DELETE_PENDING, + ret, done, "Got unexpected AFP_AfpInfo stream"); + smb2_util_close(tree, h1); ret = check_stream_list(tree, tctx, fname, 1, streams_basic, false); -- 2.17.2 From 624bfed60b29112d3b8c5d798e96b4b7a8900fdf Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 10 Oct 2018 12:47:07 +0200 Subject: [PATCH 20/43] s4:torture/vfs/fruit: expand existing vfs_test "null afpinfo" This adds a check that a read on a seperate handle also sees the previously created AFP_AfpInfo stream. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- source4/torture/vfs/fruit.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c index 85643038764..3b0c3cdae85 100644 --- a/source4/torture/vfs/fruit.c +++ b/source4/torture/vfs/fruit.c @@ -4094,6 +4094,8 @@ static bool test_null_afpinfo(struct torture_context *tctx, AfpInfo *afpinfo = NULL; char *afpinfo_buf = NULL; const char *type_creator = "SMB,OLE!"; + struct smb2_handle handle2; + struct smb2_read r; torture_comment(tctx, "Checking create of AfpInfo stream\n"); @@ -4132,6 +4134,20 @@ static bool test_null_afpinfo(struct torture_context *tctx, status = smb2_read_recv(req[1], tree, &read); torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "smb2_read_recv failed"); + status = torture_smb2_testfile_access(tree, sname, &handle2, + SEC_FILE_READ_DATA); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "torture_smb2_testfile failed\n"); + r = (struct smb2_read) { + .in.file.handle = handle2, + .in.length = AFP_INFO_SIZE, + }; + + status = smb2_read(tree, tree, &r); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "torture_smb2_testfile failed\n"); + smb2_util_close(tree, handle2); + afpinfo = torture_afpinfo_new(mem_ctx); torture_assert_goto(tctx, afpinfo != NULL, ret, done, "torture_afpinfo_new failed"); -- 2.17.2 From 676842e2d68243ecc88935db610247e71a1cfc4e Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Mon, 15 Oct 2018 15:17:08 +0200 Subject: [PATCH 21/43] s4:torture/vfs/fruit: update test "creating rsrc with read-only access" for newer macOS versions While this operation failed against older macOS versions, it passes against versions 10.12 and newer. Update the test accordingly, a subsequent commit will then update our implementation. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- selftest/knownfail.d/samba3.vfs.fruit | 4 +++- source4/torture/vfs/fruit.c | 28 +-------------------------- 2 files changed, 4 insertions(+), 28 deletions(-) diff --git a/selftest/knownfail.d/samba3.vfs.fruit b/selftest/knownfail.d/samba3.vfs.fruit index 2b49e827d7e..a2758ffeded 100644 --- a/selftest/knownfail.d/samba3.vfs.fruit +++ b/selftest/knownfail.d/samba3.vfs.fruit @@ -1,3 +1,5 @@ ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion$nt4_dc$ ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion without embedded xattr$nt4_dc$ - +^samba3.vfs.fruit metadata_netatalk.creating rsrc with read-only access$nt4_dc$ +^samba3.vfs.fruit metadata_stream.creating rsrc with read-only access$nt4_dc$ +^samba3.vfs.fruit streams_depot.creating rsrc with read-only access$nt4_dc$ diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c index 3b0c3cdae85..f4de09576a7 100644 --- a/source4/torture/vfs/fruit.c +++ b/source4/torture/vfs/fruit.c @@ -2045,35 +2045,9 @@ static bool test_rfork_create_ro(struct torture_context *tctx, } torture_comment(tctx, "(%s) Try opening read-only with " - "open_if create disposition, should return ENOENT\n", + "open_if create disposition, should work\n", __location__); - ZERO_STRUCT(create); - create.in.fname = rfork; - create.in.create_disposition = NTCREATEX_DISP_OPEN_IF; - create.in.desired_access = SEC_FILE_READ_DATA | SEC_STD_READ_CONTROL; - create.in.file_attributes = FILE_ATTRIBUTE_NORMAL; - create.in.share_access = FILE_SHARE_READ | FILE_SHARE_DELETE; - status = smb2_create(tree, mem_ctx, &(create)); - torture_assert_ntstatus_equal_goto(tctx, status, - NT_STATUS_OBJECT_NAME_NOT_FOUND, - ret, done, "smb2_create failed\n"); - - torture_comment(tctx, "(%s) Now write something to the " - "rsrc stream, then the same open should succeed\n", - __location__); - - ret = write_stream(tree, __location__, tctx, mem_ctx, - fname, AFPRESOURCE_STREAM_NAME, - 0, 3, "foo"); - torture_assert_goto(tctx, ret == true, ret, done, - "write_stream failed\n"); - - ret = check_stream(tree, __location__, tctx, mem_ctx, - fname, AFPRESOURCE_STREAM, - 0, 3, 0, 3, "foo"); - torture_assert_goto(tctx, ret == true, ret, done, "check_stream"); - ZERO_STRUCT(create); create.in.fname = rfork; create.in.create_disposition = NTCREATEX_DISP_OPEN_IF; -- 2.17.2 From 02b0916e0cc296fc1fcad1a7eb95365bd7cdfd83 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Mon, 22 Oct 2018 12:32:09 +0200 Subject: [PATCH 22/43] vfs_fruit: update handling of read-only creation of resource fork macOS SMB server versions supports this since 10.12, so we adapt our behaviour. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- selftest/knownfail.d/samba3.vfs.fruit | 3 --- source3/modules/vfs_fruit.c | 23 +++-------------------- 2 files changed, 3 insertions(+), 23 deletions(-) diff --git a/selftest/knownfail.d/samba3.vfs.fruit b/selftest/knownfail.d/samba3.vfs.fruit index a2758ffeded..6307e2b3404 100644 --- a/selftest/knownfail.d/samba3.vfs.fruit +++ b/selftest/knownfail.d/samba3.vfs.fruit @@ -1,5 +1,2 @@ ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion$nt4_dc$ ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion without embedded xattr$nt4_dc$ -^samba3.vfs.fruit metadata_netatalk.creating rsrc with read-only access$nt4_dc$ -^samba3.vfs.fruit metadata_stream.creating rsrc with read-only access$nt4_dc$ -^samba3.vfs.fruit streams_depot.creating rsrc with read-only access$nt4_dc$ diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c index 5c986a7b3ea..b51eda1146b 100644 --- a/source3/modules/vfs_fruit.c +++ b/source3/modules/vfs_fruit.c @@ -3575,12 +3575,9 @@ static int fruit_open_rsrc_adouble(vfs_handle_struct *handle, goto exit; } - /* Sanitize flags */ - if (flags & O_WRONLY) { - /* We always need read access for the metadata header too */ - flags &= ~O_WRONLY; - flags |= O_RDWR; - } + /* We always need read/write access for the metadata header too */ + flags &= ~(O_RDONLY | O_WRONLY); + flags |= O_RDWR; hostfd = SMB_VFS_NEXT_OPEN(handle, smb_fname_base, fsp, flags, mode); @@ -3667,20 +3664,6 @@ static int fruit_open_rsrc(vfs_handle_struct *handle, SMB_VFS_HANDLE_GET_DATA(handle, config, struct fruit_config_data, return -1); - if (((flags & O_ACCMODE) == O_RDONLY) - && (flags & O_CREAT) - && !VALID_STAT(fsp->fsp_name->st)) - { - /* - * This means the stream doesn't exist. macOS SMB server fails - * this with NT_STATUS_OBJECT_NAME_NOT_FOUND, so must we. Cf bug - * 12565 and the test for this combination in - * test_rfork_create(). - */ - errno = ENOENT; - return -1; - } - switch (config->rsrc) { case FRUIT_RSRC_STREAM: fd = SMB_VFS_NEXT_OPEN(handle, smb_fname, fsp, flags, mode); -- 2.17.2 From 1e97f46a8cdbdeb51e66383791b97292cf07df09 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Mon, 22 Oct 2018 12:43:16 +0200 Subject: [PATCH 23/43] s4:torture/vfs/fruit: expand test "setinfo eof stream" o Adds checks verifying that after setting eof to 0 on a stream, a subsequent open gets ENOENT, before and after closing the handle that had been used to set eof to 0. o Verify that a write to a handle succeeds after that handle has been used to set eof to 0 on a stream. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- selftest/knownfail.d/samba3.vfs.fruit | 3 ++ source4/torture/vfs/fruit.c | 61 +++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/selftest/knownfail.d/samba3.vfs.fruit b/selftest/knownfail.d/samba3.vfs.fruit index 6307e2b3404..5ecba522d4a 100644 --- a/selftest/knownfail.d/samba3.vfs.fruit +++ b/selftest/knownfail.d/samba3.vfs.fruit @@ -1,2 +1,5 @@ ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion$nt4_dc$ ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion without embedded xattr$nt4_dc$ +^samba3.vfs.fruit metadata_netatalk.setinfo eof stream$nt4_dc$ +^samba3.vfs.fruit metadata_stream.setinfo eof stream$nt4_dc$ +^samba3.vfs.fruit streams_depot.setinfo eof stream$nt4_dc$ diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c index f4de09576a7..ee2c5e3fb75 100644 --- a/source4/torture/vfs/fruit.c +++ b/source4/torture/vfs/fruit.c @@ -4984,8 +4984,32 @@ static bool test_setinfo_stream_eof(struct torture_context *tctx, torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "set eof 0 failed\n"); + ZERO_STRUCT(create); + create.in.desired_access = SEC_FILE_READ_ATTRIBUTE; + create.in.share_access = NTCREATEX_SHARE_ACCESS_MASK; + create.in.file_attributes = FILE_ATTRIBUTE_NORMAL; + create.in.create_disposition = NTCREATEX_DISP_OPEN; + create.in.fname = sname; + + status = smb2_create(tree, tctx, &create); + torture_assert_ntstatus_equal_goto( + tctx, status, NT_STATUS_OBJECT_NAME_NOT_FOUND, ret, done, + "Unexpected status\n"); + smb2_util_close(tree, h1); + ZERO_STRUCT(create); + create.in.desired_access = SEC_FILE_READ_ATTRIBUTE; + create.in.share_access = NTCREATEX_SHARE_ACCESS_MASK; + create.in.file_attributes = FILE_ATTRIBUTE_NORMAL; + create.in.create_disposition = NTCREATEX_DISP_OPEN; + create.in.fname = sname; + + status = smb2_create(tree, tctx, &create); + torture_assert_ntstatus_equal_goto( + tctx, status, NT_STATUS_OBJECT_NAME_NOT_FOUND, ret, done, + "Unexpected status\n"); + status = torture_smb2_testfile_access(tree, sname, &h1, SEC_FILE_WRITE_DATA); torture_assert_ntstatus_ok_goto(tctx, status, ret, done, @@ -5064,6 +5088,18 @@ static bool test_setinfo_stream_eof(struct torture_context *tctx, torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "set eof 0 failed\n"); + ZERO_STRUCT(create); + create.in.desired_access = SEC_FILE_READ_ATTRIBUTE; + create.in.share_access = NTCREATEX_SHARE_ACCESS_MASK; + create.in.file_attributes = FILE_ATTRIBUTE_NORMAL; + create.in.create_disposition = NTCREATEX_DISP_OPEN; + create.in.fname = sname; + + status = smb2_create(tree, tctx, &create); + torture_assert_ntstatus_equal_goto( + tctx, status, NT_STATUS_OBJECT_NAME_NOT_FOUND, ret, done, + "Unexpected status\n"); + smb2_util_close(tree, h1); ZERO_STRUCT(create); @@ -5123,6 +5159,31 @@ static bool test_setinfo_stream_eof(struct torture_context *tctx, torture_assert_ntstatus_ok_goto(tctx, status, ret, done, "torture_smb2_testfile failed\n"); smb2_util_close(tree, h1); + + torture_comment(tctx, "Writing to stream after setting EOF to 0\n"); + status = torture_smb2_testfile_access(tree, sname, &h1, + SEC_FILE_WRITE_DATA); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "torture_smb2_testfile failed\n"); + + status = smb2_util_write(tree, h1, "1234567890", 0, 10); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_util_write failed\n"); + + ZERO_STRUCT(sfinfo); + sfinfo.generic.in.file.handle = h1; + sfinfo.generic.level = RAW_SFILEINFO_END_OF_FILE_INFORMATION; + sfinfo.position_information.in.position = 0; + status = smb2_setinfo_file(tree, &sfinfo); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "set eof 0 failed\n"); + + status = smb2_util_write(tree, h1, "1234567890", 0, 10); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_util_write failed\n"); + + smb2_util_close(tree, h1); + done: smb2_util_unlink(tree, fname); smb2_util_rmdir(tree, BASEDIR); -- 2.17.2 From f812760e576399ca598d58a44c124c3ba5bf08e5 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sat, 20 Oct 2018 14:52:23 +0200 Subject: [PATCH 24/43] s4:torture/vfs/fruit: write some data to a just created teststream Doesn't currently make a difference, but this prepares for a later change in vfs_fruit that will filter out empty streams (which is the macOS behaviour). Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- source4/torture/vfs/fruit.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c index ee2c5e3fb75..c77cc981b84 100644 --- a/source4/torture/vfs/fruit.c +++ b/source4/torture/vfs/fruit.c @@ -5285,6 +5285,11 @@ static bool test_stream_names_local(struct torture_context *tctx, status = smb2_create(tree, mem_ctx, &create); CHECK_STATUS(status, NT_STATUS_OK); + + status = smb2_util_write(tree, create.out.file.handle, "foo", 0, 3); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_util_write failed\n"); + smb2_util_close(tree, create.out.file.handle); ret = torture_setup_local_xattr(tctx, "localdir", BASEDIR "/stream_names.txt", -- 2.17.2 From 79599c70cf4643ddb51c5b9179c68148b3e978a6 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sat, 20 Oct 2018 14:54:48 +0200 Subject: [PATCH 25/43] vfs_fruit: don't unlink 0-byte size truncated streams This caused all sort of havoc with subsequent SMB request that acted on the handle of the then deleted backend storage (file or blob, depending on the used streams module). Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- source3/modules/vfs_fruit.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c index b51eda1146b..1446461a8a3 100644 --- a/source3/modules/vfs_fruit.c +++ b/source3/modules/vfs_fruit.c @@ -5816,13 +5816,6 @@ static int fruit_ftruncate(struct vfs_handle_struct *handle, (intmax_t)offset); if (fio == NULL) { - if (offset == 0 && - global_fruit_config.nego_aapl && - is_ntfs_stream_smb_fname(fsp->fsp_name) && - !is_ntfs_default_stream_smb_fname(fsp->fsp_name)) - { - return SMB_VFS_NEXT_UNLINK(handle, fsp->fsp_name); - } return SMB_VFS_NEXT_FTRUNCATE(handle, fsp, offset); } -- 2.17.2 From f2fe4968808de7d631a605fdeeb7037d11e050c9 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Mon, 22 Oct 2018 14:01:34 +0200 Subject: [PATCH 26/43] s4:torture/vfs/fruit: enable AAPL extensions in a bunch of tests These tests check for macOS SMB server specific behaviour. They work currently against Samba without enabling AAPL because in vfs_fruit we're currently don't check whether AAPL has been negotiated in one place. A subsequent commit will change that and this commit prepares for that change. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- source4/torture/vfs/fruit.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c index c77cc981b84..589c25cfda2 100644 --- a/source4/torture/vfs/fruit.c +++ b/source4/torture/vfs/fruit.c @@ -1810,6 +1810,9 @@ static bool test_rfork_truncate(struct torture_context *tctx, struct smb2_handle fh1, fh2, fh3; union smb_setfileinfo sinfo; + ret = enable_aapl(tctx, tree); + torture_assert_goto(tctx, ret == true, ret, done, "enable_aapl failed"); + smb2_util_unlink(tree, fname); status = torture_smb2_testdir(tree, BASEDIR, &testdirh); @@ -1928,6 +1931,9 @@ static bool test_rfork_create(struct torture_context *tctx, }; union smb_fileinfo finfo; + ret = enable_aapl(tctx, tree); + torture_assert_goto(tctx, ret == true, ret, done, "enable_aapl failed"); + smb2_util_unlink(tree, fname); status = torture_smb2_testdir(tree, BASEDIR, &testdirh); @@ -3938,6 +3944,9 @@ static bool test_setinfo_eof_resource(struct torture_context *tctx, torture_assert_goto(tctx, mem_ctx != NULL, ret, done, "talloc_new"); + ret = enable_aapl(tctx, tree); + torture_assert_goto(tctx, ret == true, ret, done, "enable_aapl failed"); + torture_comment(tctx, "Set AFP_AfpResource EOF to 1 and 0\n"); smb2_deltree(tree, BASEDIR); @@ -4904,6 +4913,9 @@ static bool test_setinfo_stream_eof(struct torture_context *tctx, torture_assert_goto(tctx, mem_ctx != NULL, ret, done, "talloc_new failed\n"); + ret = enable_aapl(tctx, tree); + torture_assert(tctx, ret == true, "enable_aapl failed"); + torture_comment(tctx, "Test setting EOF on a stream\n"); smb2_deltree(tree, BASEDIR); -- 2.17.2 From 3c76b503a2b03b5408f71987a56ecd82f05b3960 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sat, 20 Oct 2018 15:28:06 +0200 Subject: [PATCH 27/43] vfs_fruit: use check on global_fruit_config.nego_aapl for macOS specific behaviour Ensure any non MS compliant protocol behaviour targetted at supporting macOS clients are only effective if the client negotiated AAPL. Currently this only guards the resource fork which only macOS client are going to use, but subsequent commits add more this at this place. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- source3/modules/vfs_fruit.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c index 1446461a8a3..62e741c4ae2 100644 --- a/source3/modules/vfs_fruit.c +++ b/source3/modules/vfs_fruit.c @@ -5892,7 +5892,8 @@ static NTSTATUS fruit_create_file(vfs_handle_struct *handle, * * Cf the vfs_fruit torture tests in test_rfork_create(). */ - if (is_afpresource_stream(fsp->fsp_name) && + if (global_fruit_config.nego_aapl && + is_afpresource_stream(fsp->fsp_name) && create_disposition == FILE_OPEN) { if (fsp->fsp_name->st.st_ex_size == 0) { -- 2.17.2 From dfaf4a6c2ca45c91b8311274ccf03afdb7b26d6c Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sat, 20 Oct 2018 14:53:50 +0200 Subject: [PATCH 28/43] vfs_fruit: filter empty streams First step in achieving macOS compliant behaviour wrt to empty streams: - hide empty streams in streaminfo - prevent opens of empty streams This means that we may carry 0-byte sized streams in our streams backend, but this shouldn't really hurt. The previous attempt of deleting the streams when an SMB setinfo eof to 0 request came in, turned out be a road into desaster. We could set delete-on-close on the stream, but that means we'd have to check for it for every write on a stream and checking the delete-on-close bits requires fetching the locking.tdb record, so this is expensive and I'd like to avoid that overhead. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- selftest/knownfail.d/samba3.vfs.fruit | 3 -- source3/modules/vfs_fruit.c | 44 +++++++++++++++++++++++---- 2 files changed, 38 insertions(+), 9 deletions(-) diff --git a/selftest/knownfail.d/samba3.vfs.fruit b/selftest/knownfail.d/samba3.vfs.fruit index 5ecba522d4a..6307e2b3404 100644 --- a/selftest/knownfail.d/samba3.vfs.fruit +++ b/selftest/knownfail.d/samba3.vfs.fruit @@ -1,5 +1,2 @@ ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion$nt4_dc$ ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion without embedded xattr$nt4_dc$ -^samba3.vfs.fruit metadata_netatalk.setinfo eof stream$nt4_dc$ -^samba3.vfs.fruit metadata_stream.setinfo eof stream$nt4_dc$ -^samba3.vfs.fruit streams_depot.setinfo eof stream$nt4_dc$ diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c index 62e741c4ae2..181038c9817 100644 --- a/source3/modules/vfs_fruit.c +++ b/source3/modules/vfs_fruit.c @@ -5598,6 +5598,36 @@ static NTSTATUS fruit_streaminfo_rsrc(vfs_handle_struct *handle, return status; } +static void fruit_filter_empty_streams(unsigned int *pnum_streams, + struct stream_struct **pstreams) +{ + unsigned num_streams = *pnum_streams; + struct stream_struct *streams = *pstreams; + unsigned i = 0; + + if (!global_fruit_config.nego_aapl) { + return; + } + + while (i < num_streams) { + struct smb_filename smb_fname = (struct smb_filename) { + .stream_name = streams[i].name, + }; + + if (is_ntfs_default_stream_smb_fname(&smb_fname) + || streams[i].size > 0) + { + i++; + continue; + } + + streams[i] = streams[num_streams - 1]; + num_streams--; + } + + *pnum_streams = num_streams; +} + static NTSTATUS fruit_streaminfo(vfs_handle_struct *handle, struct files_struct *fsp, const struct smb_filename *smb_fname, @@ -5619,6 +5649,8 @@ static NTSTATUS fruit_streaminfo(vfs_handle_struct *handle, return status; } + fruit_filter_empty_streams(pnum_streams, pstreams); + status = fruit_streaminfo_meta(handle, fsp, smb_fname, mem_ctx, pnum_streams, pstreams); if (!NT_STATUS_IS_OK(status)) { @@ -5893,13 +5925,13 @@ static NTSTATUS fruit_create_file(vfs_handle_struct *handle, * Cf the vfs_fruit torture tests in test_rfork_create(). */ if (global_fruit_config.nego_aapl && - is_afpresource_stream(fsp->fsp_name) && - create_disposition == FILE_OPEN) + create_disposition == FILE_OPEN && + smb_fname->st.st_ex_size == 0 && + is_ntfs_stream_smb_fname(smb_fname) && + !(is_ntfs_default_stream_smb_fname(smb_fname))) { - if (fsp->fsp_name->st.st_ex_size == 0) { - status = NT_STATUS_OBJECT_NAME_NOT_FOUND; - goto fail; - } + status = NT_STATUS_OBJECT_NAME_NOT_FOUND; + goto fail; } if (is_ntfs_stream_smb_fname(smb_fname) -- 2.17.2 From 8a12a00823d672eb77c42b76c226d9da0b01f2c6 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 10 Oct 2018 18:45:56 +0200 Subject: [PATCH 29/43] s4:torture/util: add torture_smb2_open() This seems to be missing: a simple wrapper to just open a file without fancy options. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- source4/torture/smb2/util.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/source4/torture/smb2/util.c b/source4/torture/smb2/util.c index 65090b0e8b7..ac8a0d5df77 100644 --- a/source4/torture/smb2/util.c +++ b/source4/torture/smb2/util.c @@ -527,6 +527,36 @@ NTSTATUS torture_smb2_testfile(struct smb2_tree *tree, const char *fname, SEC_RIGHTS_FILE_ALL); } +/* + create and return a handle to a test file + with a specific access mask +*/ +NTSTATUS torture_smb2_open(struct smb2_tree *tree, + const char *fname, + uint32_t desired_access, + struct smb2_handle *handle) +{ + struct smb2_create io; + NTSTATUS status; + + io = (struct smb2_create) { + .in.fname = fname, + .in.desired_access = desired_access, + .in.file_attributes = FILE_ATTRIBUTE_NORMAL, + .in.create_disposition = NTCREATEX_DISP_OPEN, + .in.share_access = NTCREATEX_SHARE_ACCESS_MASK, + }; + + status = smb2_create(tree, tree, &io); + if (!NT_STATUS_IS_OK(status)) { + return status; + } + + *handle = io.out.file.handle; + + return NT_STATUS_OK; +} + /* create and return a handle to a test directory with specific desired access -- 2.17.2 From 8948096d058d7c4bc019a9b0faed44f6f1651837 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Thu, 11 Oct 2018 17:14:50 +0200 Subject: [PATCH 30/43] s4:torture/vfs/fruit: add check_stream_list_handle() Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- source4/torture/vfs/fruit.c | 62 +++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c index 589c25cfda2..20e5e3f3495 100644 --- a/source4/torture/vfs/fruit.c +++ b/source4/torture/vfs/fruit.c @@ -3082,6 +3082,68 @@ static bool check_stream_list(struct smb2_tree *tree, return ret; } +#if 0 +static bool check_stream_list_handle(struct smb2_tree *tree, + struct torture_context *tctx, + struct smb2_handle h, + int num_exp, + const char **exp, + bool is_dir) +{ + bool ret = true; + union smb_fileinfo finfo; + NTSTATUS status; + int i; + TALLOC_CTX *tmp_ctx = talloc_new(tctx); + char **exp_sort; + struct stream_struct *stream_sort; + + torture_assert_goto(tctx, tmp_ctx != NULL, ret, done, + "talloc_new failed\n"); + + finfo = (union smb_fileinfo) { + .stream_info.level = RAW_FILEINFO_STREAM_INFORMATION, + .stream_info.in.file.handle = h, + }; + + status = smb2_getinfo_file(tree, tctx, &finfo); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "get stream info\n"); + + torture_assert_int_equal_goto(tctx, finfo.stream_info.out.num_streams, + num_exp, ret, done, "stream count\n"); + + if (num_exp == 0) { + TALLOC_FREE(tmp_ctx); + goto done; + } + + exp_sort = talloc_memdup(tmp_ctx, exp, num_exp * sizeof(*exp)); + torture_assert_goto(tctx, exp_sort != NULL, ret, done, __location__); + + TYPESAFE_QSORT(exp_sort, num_exp, qsort_string); + + stream_sort = talloc_memdup(tmp_ctx, finfo.stream_info.out.streams, + finfo.stream_info.out.num_streams * + sizeof(*stream_sort)); + torture_assert_goto(tctx, stream_sort != NULL, ret, done, __location__); + + TYPESAFE_QSORT(stream_sort, finfo.stream_info.out.num_streams, qsort_stream); + + for (i=0; i Date: Mon, 22 Oct 2018 16:21:21 +0200 Subject: [PATCH 31/43] s4:torture/vfs/fruit: add test "empty_stream" One to rule them all: consistently test critical operations on all streams relevant to macOS clients: the FinderInfo stream, the Resource Fork stream and an arbitrary stream that macOS maps to xattrs when written to on a macOS SMB server. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- selftest/knownfail.d/samba3.vfs.fruit | 3 + source4/torture/vfs/fruit.c | 613 +++++++++++++++++++++++++- 2 files changed, 614 insertions(+), 2 deletions(-) diff --git a/selftest/knownfail.d/samba3.vfs.fruit b/selftest/knownfail.d/samba3.vfs.fruit index 6307e2b3404..2d3c21caf91 100644 --- a/selftest/knownfail.d/samba3.vfs.fruit +++ b/selftest/knownfail.d/samba3.vfs.fruit @@ -1,2 +1,5 @@ ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion$nt4_dc$ ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion without embedded xattr$nt4_dc$ +^samba3.vfs.fruit metadata_netatalk.empty_stream$nt4_dc$ +^samba3.vfs.fruit metadata_stream.empty_stream$nt4_dc$ +^samba3.vfs.fruit streams_depot.empty_stream$nt4_dc$ diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c index 20e5e3f3495..a88576b7e25 100644 --- a/source4/torture/vfs/fruit.c +++ b/source4/torture/vfs/fruit.c @@ -3082,7 +3082,6 @@ static bool check_stream_list(struct smb2_tree *tree, return ret; } -#if 0 static bool check_stream_list_handle(struct smb2_tree *tree, struct torture_context *tctx, struct smb2_handle h, @@ -3142,7 +3141,6 @@ static bool check_stream_list_handle(struct smb2_tree *tree, TALLOC_FREE(tmp_ctx); return ret; } -#endif /* test stream names @@ -5264,6 +5262,616 @@ static bool test_setinfo_stream_eof(struct torture_context *tctx, return ret; } +#define MAX_STREAMS 16 + +struct tcase { + const char *name; + uint32_t access; + const char *write_data; + size_t write_size; + struct tcase_results { + size_t size; + NTSTATUS initial_status; + NTSTATUS final_status; + int num_streams_open_handle; + const char *streams_open_handle[MAX_STREAMS]; + int num_streams_closed_handle; + const char *streams_closed_handle[MAX_STREAMS]; + } create, write, overwrite, eof, doc; +}; + +typedef enum {T_CREATE, T_WRITE, T_OVERWRITE, T_EOF, T_DOC} subtcase_t; + +static bool test_empty_stream_do_checks( + struct torture_context *tctx, + struct smb2_tree *tree, + struct smb2_tree *tree2, + struct tcase *tcase, + TALLOC_CTX *mem_ctx, + struct smb2_handle baseh, + struct smb2_handle streamh, + subtcase_t subcase) +{ + bool ret = false; + NTSTATUS status; + struct smb2_handle h1; + union smb_fileinfo finfo; + struct tcase_results *tcase_results = NULL; + + switch (subcase) { + case T_CREATE: + tcase_results = &tcase->create; + break; + case T_OVERWRITE: + tcase_results = &tcase->overwrite; + break; + case T_WRITE: + tcase_results = &tcase->write; + break; + case T_EOF: + tcase_results = &tcase->eof; + break; + case T_DOC: + tcase_results = &tcase->doc; + break; + } + + finfo = (union smb_fileinfo) { + .generic.level = RAW_FILEINFO_STANDARD_INFORMATION, + .generic.in.file.handle = streamh, + }; + + /* + * Test: check size, same client + */ + + status = smb2_getinfo_file(tree, mem_ctx, &finfo); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "torture_smb2_testfile failed\n"); + + torture_assert_int_equal_goto(tctx, finfo.standard_info.out.size, + tcase_results->size, + ret, done, "Wrong size\n"); + + /* + * Test: open, same client + */ + + status = torture_smb2_open(tree, tcase->name, + SEC_FILE_READ_ATTRIBUTE, &h1); + torture_assert_ntstatus_equal_goto(tctx, status, + tcase_results->initial_status, + ret, done, + "smb2_create failed\n"); + if (NT_STATUS_IS_OK(status)) { + status = smb2_util_close(tree, h1); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_util_close failed\n"); + } + + /* + * Test: check streams, same client + */ + + ret = check_stream_list_handle(tree, tctx, baseh, + tcase_results->num_streams_open_handle, + tcase_results->streams_open_handle, + false); + torture_assert_goto(tctx, ret == true, ret, done, "Bad streams"); + + /* + * Test: open, different client + */ + + status = torture_smb2_open(tree2, tcase->name, + SEC_FILE_READ_ATTRIBUTE, &h1); + torture_assert_ntstatus_equal_goto(tctx, status, + tcase_results->initial_status, + ret, done, + "smb2_create failed\n"); + if (NT_STATUS_IS_OK(status)) { + finfo = (union smb_fileinfo) { + .generic.level = RAW_FILEINFO_STANDARD_INFORMATION, + .generic.in.file.handle = h1, + }; + + /* + * Test: check size, different client + */ + + status = smb2_getinfo_file(tree2, mem_ctx, &finfo); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_getinfo_file failed\n"); + + torture_assert_int_equal_goto(tctx, finfo.standard_info.out.size, + tcase_results->size, + ret, done, "Wrong size\n"); + + /* + * Test: check streams, different client + */ + + ret = check_stream_list(tree2, tctx, BASEDIR "\\file", + tcase_results->num_streams_open_handle, + tcase_results->streams_open_handle, + false); + torture_assert_goto(tctx, ret == true, ret, done, "Bad streams"); + + status = smb2_util_close(tree2, h1); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_util_close failed\n"); + } + + status = smb2_util_close(tree, streamh); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_util_close failed\n"); + + /* + * Test: open after close, same client + */ + + status = torture_smb2_open(tree, tcase->name, + SEC_FILE_READ_DATA, &h1); + torture_assert_ntstatus_equal_goto(tctx, status, + tcase_results->final_status, + ret, done, + "smb2_create failed\n"); + if (NT_STATUS_IS_OK(status)) { + status = smb2_util_close(tree, h1); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_util_close failed\n"); + } + + /* + * Test: open after close, different client + */ + + status = torture_smb2_open(tree2, tcase->name, + SEC_FILE_READ_DATA, &h1); + torture_assert_ntstatus_equal_goto(tctx, status, + tcase_results->final_status, + ret, done, + "smb2_create failed\n"); + if (NT_STATUS_IS_OK(status)) { + status = smb2_util_close(tree2, h1); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_util_close failed\n"); + } + + /* + * Test: check streams after close, same client + */ + + ret = check_stream_list_handle(tree, tctx, baseh, + tcase_results->num_streams_closed_handle, + tcase_results->streams_closed_handle, + false); + torture_assert_goto(tctx, ret == true, ret, done, "Bad streams"); + + ret = true; + +done: + smb2_util_close(tree, streamh); + smb2_util_close(tree, baseh); + return ret; +} + +static bool test_empty_stream_do_one( + struct torture_context *tctx, + struct smb2_tree *tree, + struct smb2_tree *tree2, + struct tcase *tcase) +{ + bool ret = false; + NTSTATUS status; + struct smb2_handle baseh; + struct smb2_handle streamh; + struct smb2_create create; + union smb_setfileinfo sfinfo; + TALLOC_CTX *mem_ctx = talloc_new(tctx); + + torture_comment(tctx, "Testing stream [%s]\n", tcase->name); + + torture_assert_goto(tctx, mem_ctx != NULL, ret, done, "talloc_new\n"); + + /* + * Subtest: create + */ + torture_comment(tctx, "Subtest: T_CREATE\n"); + + status = smb2_util_unlink(tree, BASEDIR "\\file"); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_util_unlink failed\n"); + + status = torture_smb2_testfile_access(tree, BASEDIR "\\file", + &baseh, SEC_FILE_ALL); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "torture_smb2_testfile_access failed\n"); + + status = torture_smb2_testfile_access(tree, tcase->name, &streamh, + tcase->access); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "torture_smb2_testfile_access failed\n"); + + ret = test_empty_stream_do_checks(tctx, tree, tree2, tcase, + mem_ctx, baseh, streamh, T_CREATE); + torture_assert_goto(tctx, ret, ret, done, "test failed\n"); + + if (!(tcase->access & SEC_FILE_WRITE_DATA)) { + /* + * All subsequent tests require write access + */ + ret = true; + goto done; + } + + /* + * Subtest: create and write + */ + torture_comment(tctx, "Subtest: T_WRITE\n"); + + status = smb2_util_unlink(tree, BASEDIR "\\file"); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_util_unlink failed\n"); + + status = torture_smb2_testfile_access(tree, BASEDIR "\\file", + &baseh, SEC_FILE_ALL); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "torture_smb2_testfile_access failed\n"); + + status = torture_smb2_testfile_access(tree, tcase->name, &streamh, + tcase->access); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "torture_smb2_testfile_access failed\n"); + + status = smb2_util_write(tree, streamh, tcase->write_data, 0, + tcase->write_size); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "torture_smb2_open failed\n"); + + ret = test_empty_stream_do_checks(tctx, tree, tree2, tcase, + mem_ctx, baseh, streamh, T_WRITE); + torture_assert_goto(tctx, ret, ret, done, "test failed\n"); + + /* + * Subtest: overwrite + */ + torture_comment(tctx, "Subtest: T_OVERWRITE\n"); + + status = smb2_util_unlink(tree, BASEDIR "\\file"); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_util_unlink failed\n"); + + status = torture_smb2_testfile_access(tree, BASEDIR "\\file", + &baseh, SEC_FILE_ALL); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "torture_smb2_testfile_access failed\n"); + + create = (struct smb2_create) { + .in.desired_access = SEC_FILE_ALL, + .in.share_access = NTCREATEX_SHARE_ACCESS_MASK, + .in.file_attributes = FILE_ATTRIBUTE_NORMAL, + .in.create_disposition = NTCREATEX_DISP_OVERWRITE_IF, + .in.fname = tcase->name, + }; + + status = smb2_create(tree, tctx, &create); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "torture_smb2_testfile failed\n"); + streamh = create.out.file.handle; + + ret = test_empty_stream_do_checks(tctx, tree, tree2, tcase, + mem_ctx, baseh, streamh, T_OVERWRITE); + torture_assert_goto(tctx, ret, ret, done, "test failed\n"); + + /* + * Subtest: setinfo EOF 0 + */ + torture_comment(tctx, "Subtest: T_EOF\n"); + + status = smb2_util_unlink(tree, BASEDIR "\\file"); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_util_unlink failed\n"); + + status = torture_smb2_testfile_access(tree, BASEDIR "\\file", + &baseh, SEC_FILE_ALL); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "torture_smb2_testfile_access failed\n"); + + status = torture_smb2_testfile_access(tree, tcase->name, &streamh, + tcase->access); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "torture_smb2_testfile_access failed\n"); + + status = smb2_util_write(tree, streamh, tcase->write_data, 0, + tcase->write_size); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "torture_smb2_open failed\n"); + + sfinfo = (union smb_setfileinfo) { + .end_of_file_info.level = RAW_SFILEINFO_END_OF_FILE_INFORMATION, + .end_of_file_info.in.file.handle = streamh, + .end_of_file_info.in.size = 0, + }; + status = smb2_setinfo_file(tree, &sfinfo); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "set eof 0 failed\n"); + + ret = test_empty_stream_do_checks(tctx, tree, tree2, tcase, + mem_ctx, baseh, streamh, T_EOF); + torture_assert_goto(tctx, ret, ret, done, "test failed\n"); + + /* + * Subtest: delete-on-close + */ + torture_comment(tctx, "Subtest: T_DOC\n"); + + status = smb2_util_unlink(tree, BASEDIR "\\file"); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_util_unlink failed\n"); + + status = torture_smb2_testfile_access(tree, BASEDIR "\\file", + &baseh, SEC_FILE_ALL); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "torture_smb2_testfile_access failed\n"); + + status = torture_smb2_testfile_access(tree, tcase->name, &streamh, + tcase->access); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "torture_smb2_testfile_access failed\n"); + + status = smb2_util_write(tree, streamh, tcase->write_data, 0, + tcase->write_size); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "torture_smb2_open failed\n"); + + sfinfo = (union smb_setfileinfo) { + .disposition_info.level = RAW_SFILEINFO_DISPOSITION_INFORMATION, + .disposition_info.in.file.handle = streamh, + .disposition_info.in.delete_on_close = true, + }; + status = smb2_setinfo_file(tree, &sfinfo); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "set eof 0 failed\n"); + + ret = test_empty_stream_do_checks(tctx, tree, tree2, tcase, + mem_ctx, baseh, streamh, + T_DOC); + torture_assert_goto(tctx, ret, ret, done, "test failed\n"); + + ret = true; + +done: + smb2_util_close(tree, baseh); + TALLOC_FREE(mem_ctx); + return ret; +} + +static bool test_empty_stream(struct torture_context *tctx, + struct smb2_tree *tree) +{ + struct smb2_tree *tree2 = NULL; + struct tcase *tcase = NULL; + const char *fname = BASEDIR "\\file"; + struct smb2_handle h1; + bool ret = true; + NTSTATUS status; + AfpInfo ai = (AfpInfo) { + .afpi_Signature = AFP_Signature, + .afpi_Version = AFP_Version, + .afpi_BackupTime = AFP_BackupTime, + .afpi_FinderInfo = "FOO BAR ", + }; + char *ai_blob = torture_afpinfo_pack(tctx, &ai); + struct tcase tcase_afpinfo_ro = (struct tcase) { + .name = BASEDIR "\\file" AFPINFO_STREAM, + .access = SEC_FILE_READ_DATA|SEC_FILE_READ_ATTRIBUTE, + .create.size = 60, + .create.initial_status = NT_STATUS_OBJECT_NAME_NOT_FOUND, + .create.final_status = NT_STATUS_OBJECT_NAME_NOT_FOUND, + .create.num_streams_open_handle = 1, + .create.num_streams_closed_handle = 1, + .create.streams_open_handle = {"::$DATA"}, + .create.streams_closed_handle = {"::$DATA"}, + }; + struct tcase tcase_afpinfo_rw = (struct tcase) { + .name = BASEDIR "\\file" AFPINFO_STREAM, + .access = SEC_FILE_READ_DATA|SEC_FILE_READ_ATTRIBUTE|SEC_FILE_WRITE_DATA|SEC_STD_DELETE, + .write_data = ai_blob, + .write_size = AFP_INFO_SIZE, + .create.size = 60, + .create.initial_status = NT_STATUS_OBJECT_NAME_NOT_FOUND, + .create.final_status = NT_STATUS_OBJECT_NAME_NOT_FOUND, + .create.num_streams_open_handle = 1, + .create.num_streams_closed_handle = 1, + .create.streams_open_handle = {"::$DATA"}, + .create.streams_closed_handle = {"::$DATA"}, + .write.size = 60, + .write.initial_status = NT_STATUS_OK, + .write.final_status = NT_STATUS_OK, + .write.num_streams_open_handle = 2, + .write.num_streams_closed_handle = 2, + .write.streams_open_handle = {"::$DATA", AFPINFO_STREAM}, + .write.streams_closed_handle = {"::$DATA", AFPINFO_STREAM}, + .overwrite.size = 60, + .overwrite.initial_status = NT_STATUS_OBJECT_NAME_NOT_FOUND, + .overwrite.final_status = NT_STATUS_OBJECT_NAME_NOT_FOUND, + .overwrite.num_streams_open_handle = 1, + .overwrite.num_streams_closed_handle = 1, + .overwrite.streams_open_handle = {"::$DATA"}, + .overwrite.streams_closed_handle = {"::$DATA"}, + .eof.size = 60, + .eof.initial_status = NT_STATUS_OK, + .eof.final_status = NT_STATUS_OK, + .eof.num_streams_open_handle = 2, + .eof.num_streams_closed_handle = 2, + .eof.streams_open_handle = {"::$DATA", AFPINFO_STREAM}, + .eof.streams_closed_handle = {"::$DATA", AFPINFO_STREAM}, + .doc.size = 60, + .doc.initial_status = NT_STATUS_DELETE_PENDING, + .doc.final_status = NT_STATUS_OBJECT_NAME_NOT_FOUND, + .doc.num_streams_open_handle = 2, + .doc.num_streams_closed_handle = 1, + .doc.streams_open_handle = {"::$DATA", AFPINFO_STREAM}, + .doc.streams_closed_handle = {"::$DATA"}, + }; + + struct tcase tcase_afpresource_ro = (struct tcase) { + .name = BASEDIR "\\file" AFPRESOURCE_STREAM, + .access = SEC_FILE_READ_DATA|SEC_FILE_READ_ATTRIBUTE, + .create.size = 0, + .create.initial_status = NT_STATUS_OBJECT_NAME_NOT_FOUND, + .create.final_status = NT_STATUS_OBJECT_NAME_NOT_FOUND, + .create.num_streams_open_handle = 1, + .create.num_streams_closed_handle = 1, + .create.streams_open_handle = {"::$DATA"}, + .create.streams_closed_handle = {"::$DATA"}, + }; + struct tcase tcase_afpresource_rw = (struct tcase) { + .name = BASEDIR "\\file" AFPRESOURCE_STREAM, + .access = SEC_FILE_READ_DATA|SEC_FILE_READ_ATTRIBUTE|SEC_FILE_WRITE_DATA|SEC_STD_DELETE, + .write_data = "foo", + .write_size = 3, + .create.size = 0, + .create.initial_status = NT_STATUS_OBJECT_NAME_NOT_FOUND, + .create.final_status = NT_STATUS_OBJECT_NAME_NOT_FOUND, + .create.num_streams_open_handle = 1, + .create.num_streams_closed_handle = 1, + .create.streams_open_handle = {"::$DATA"}, + .create.streams_closed_handle = {"::$DATA"}, + .write.size = 3, + .write.initial_status = NT_STATUS_OK, + .write.final_status = NT_STATUS_OK, + .write.num_streams_open_handle = 2, + .write.num_streams_closed_handle = 2, + .write.streams_open_handle = {"::$DATA", AFPRESOURCE_STREAM}, + .write.streams_closed_handle = {"::$DATA", AFPRESOURCE_STREAM}, + .overwrite.size = 0, + .overwrite.initial_status = NT_STATUS_OBJECT_NAME_NOT_FOUND, + .overwrite.final_status = NT_STATUS_OBJECT_NAME_NOT_FOUND, + .overwrite.num_streams_open_handle = 1, + .overwrite.num_streams_closed_handle = 1, + .overwrite.streams_open_handle = {"::$DATA"}, + .overwrite.streams_closed_handle = {"::$DATA"}, + .eof.size = 0, + .eof.initial_status = NT_STATUS_OBJECT_NAME_NOT_FOUND, + .eof.final_status = NT_STATUS_OBJECT_NAME_NOT_FOUND, + .eof.num_streams_open_handle = 1, + .eof.num_streams_closed_handle = 1, + .eof.streams_open_handle = {"::$DATA"}, + .eof.streams_closed_handle = {"::$DATA"}, + .doc.size = 3, + .doc.initial_status = NT_STATUS_DELETE_PENDING, + .doc.final_status = NT_STATUS_OK, + .doc.num_streams_open_handle = 2, + .doc.num_streams_closed_handle = 2, + .doc.streams_open_handle = {"::$DATA", AFPRESOURCE_STREAM}, + .doc.streams_closed_handle = {"::$DATA", AFPRESOURCE_STREAM}, + }; + + struct tcase tcase_foo_ro = (struct tcase) { + .name = BASEDIR "\\file:foo", + .access = SEC_FILE_READ_DATA|SEC_FILE_READ_ATTRIBUTE, + .write_data = "foo", + .write_size = 3, + .create.size = 0, + .create.initial_status = NT_STATUS_OBJECT_NAME_NOT_FOUND, + .create.final_status = NT_STATUS_OBJECT_NAME_NOT_FOUND, + .create.num_streams_open_handle = 1, + .create.num_streams_closed_handle = 1, + .create.streams_open_handle = {"::$DATA"}, + .create.streams_closed_handle = {"::$DATA"}, + }; + + struct tcase tcase_foo_rw = (struct tcase) { + .name = BASEDIR "\\file:foo", + .access = SEC_FILE_READ_DATA|SEC_FILE_READ_ATTRIBUTE|SEC_FILE_WRITE_DATA|SEC_STD_DELETE, + .write_data = "foo", + .write_size = 3, + .create.size = 0, + .create.initial_status = NT_STATUS_OBJECT_NAME_NOT_FOUND, + .create.final_status = NT_STATUS_OBJECT_NAME_NOT_FOUND, + .create.num_streams_open_handle = 1, + .create.num_streams_closed_handle = 1, + .create.streams_open_handle = {"::$DATA"}, + .create.streams_closed_handle = {"::$DATA"}, + .write.size = 3, + .write.initial_status = NT_STATUS_OK, + .write.final_status = NT_STATUS_OK, + .write.num_streams_open_handle = 2, + .write.num_streams_closed_handle = 2, + .write.streams_open_handle = {"::$DATA", ":foo:$DATA"}, + .write.streams_closed_handle = {"::$DATA", ":foo:$DATA"}, + .overwrite.size = 0, + .overwrite.initial_status = NT_STATUS_OBJECT_NAME_NOT_FOUND, + .overwrite.final_status = NT_STATUS_OBJECT_NAME_NOT_FOUND, + .overwrite.num_streams_open_handle = 1, + .overwrite.num_streams_closed_handle = 1, + .overwrite.streams_open_handle = {"::$DATA"}, + .overwrite.streams_closed_handle = {"::$DATA"}, + .eof.size = 0, + .eof.initial_status = NT_STATUS_OBJECT_NAME_NOT_FOUND, + .eof.final_status = NT_STATUS_OBJECT_NAME_NOT_FOUND, + .eof.num_streams_open_handle = 1, + .eof.num_streams_closed_handle = 1, + .eof.streams_open_handle = {"::$DATA"}, + .eof.streams_closed_handle = {"::$DATA"}, + .doc.size = 3, + .doc.initial_status = NT_STATUS_DELETE_PENDING, + .doc.final_status = NT_STATUS_OBJECT_NAME_NOT_FOUND, + .doc.num_streams_open_handle = 2, + .doc.num_streams_closed_handle = 1, + .doc.streams_open_handle = {"::$DATA", ":foo:$DATA"}, + .doc.streams_closed_handle = {"::$DATA"}, + }; + + struct tcase tcases[] = { + tcase_afpinfo_ro, + tcase_afpinfo_rw, + tcase_afpresource_ro, + tcase_afpresource_rw, + tcase_foo_ro, + tcase_foo_rw, + {NULL} + }; + + ret = torture_smb2_connection(tctx, &tree2); + torture_assert_goto(tctx, ret == true, ret, done, + "torture_smb2_connection failed\n"); + + ret = enable_aapl(tctx, tree); + torture_assert(tctx, ret == true, "enable_aapl failed\n"); + + ret = enable_aapl(tctx, tree2); + torture_assert(tctx, ret == true, "enable_aapl failed\n"); + + smb2_deltree(tree, BASEDIR); + + status = torture_smb2_testdir(tree, BASEDIR, &h1); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "torture_smb2_testdir\n"); + smb2_util_close(tree, h1); + + for (tcase = &tcases[0]; tcase->name != NULL; tcase++) { + ret = torture_setup_file(tctx, tree, fname, false); + torture_assert_goto(tctx, ret == true, ret, done, + "torture_setup_file failed\n"); + + ret = test_empty_stream_do_one(tctx, tree, tree2, tcase); + torture_assert_goto(tctx, ret == true, ret, done, + "subtest failed\n"); + + status = smb2_util_unlink(tree, fname); + torture_assert_ntstatus_ok_goto(tctx, status, ret, done, + "smb2_util_unlink failed\n"); + } + +done: + smb2_deltree(tree, BASEDIR); + TALLOC_FREE(tree2); + return ret; +} + /* * Note: This test depends on "vfs objects = catia fruit streams_xattr". For * some tests torture must be run on the host it tests and takes an additional @@ -5307,6 +5915,7 @@ struct torture_suite *torture_vfs_fruit(TALLOC_CTX *ctx) torture_suite_add_1smb2_test(suite, "OS X AppleDouble file conversion", test_adouble_conversion); torture_suite_add_1smb2_test(suite, "NFS ACE entries", test_nfs_aces); torture_suite_add_1smb2_test(suite, "OS X AppleDouble file conversion without embedded xattr", test_adouble_conversion_wo_xattr); + torture_suite_add_1smb2_test(suite, "empty_stream", test_empty_stream); return suite; } -- 2.17.2 From 740117e8cc7751debf5408667592481d26be61dd Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 17 Oct 2018 19:07:11 +0200 Subject: [PATCH 32/43] vfs_fruit: update and add some debugging of dev/ino Aids in debugging dev/ino mismatch failures in open_file_ntcreate. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- source3/modules/vfs_fruit.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c index 181038c9817..7af7c43755e 100644 --- a/source3/modules/vfs_fruit.c +++ b/source3/modules/vfs_fruit.c @@ -2374,6 +2374,10 @@ static SMB_INO_T fruit_inode(const SMB_STRUCT_STAT *sbuf, const char *sname) SMB_INO_T result; char *upper_sname; + DBG_DEBUG("fruit_inode called for 0x%jx/0x%jx [%s]\n", + (uintmax_t)sbuf->st_ex_dev, + (uintmax_t)sbuf->st_ex_ino, sname); + upper_sname = talloc_strdup_upper(talloc_tos(), sname); SMB_ASSERT(upper_sname != NULL); @@ -2391,8 +2395,8 @@ static SMB_INO_T fruit_inode(const SMB_STRUCT_STAT *sbuf, const char *sname) /* Hopefully all the variation is in the lower 4 (or 8) bytes! */ memcpy(&result, hash, sizeof(result)); - DEBUG(10, ("fruit_inode \"%s\": ino=0x%llu\n", - sname, (unsigned long long)result)); + DBG_DEBUG("fruit_inode \"%s\": ino=0x%jx\n", + sname, (uintmax_t)result); return result; } @@ -4815,6 +4819,11 @@ static int fruit_stat_base(vfs_handle_struct *handle, rc = SMB_VFS_NEXT_LSTAT(handle, smb_fname); } smb_fname->stream_name = tmp_stream_name; + + DBG_DEBUG("fruit_stat_base [%s] dev [0x%jx] ino [0x%jx]\n", + smb_fname->base_name, + (uintmax_t)smb_fname->st.st_ex_dev, + (uintmax_t)smb_fname->st.st_ex_ino); return rc; } -- 2.17.2 From cd88ba0bdf413751d394261b21367ca4da2be344 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Mon, 15 Oct 2018 18:38:33 +0200 Subject: [PATCH 33/43] vfs_fruit: remove resource fork special casing Directly unlinking a file with open handles is not good, don't do it. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- source3/modules/vfs_fruit.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c index 7af7c43755e..9a943452642 100644 --- a/source3/modules/vfs_fruit.c +++ b/source3/modules/vfs_fruit.c @@ -5736,10 +5736,6 @@ static int fruit_ftruncate_rsrc_xattr(struct vfs_handle_struct *handle, struct files_struct *fsp, off_t offset) { - if (offset == 0) { - return SMB_VFS_FREMOVEXATTR(fsp, AFPRESOURCE_EA_NETATALK); - } - #ifdef HAVE_ATTROPEN return SMB_VFS_NEXT_FTRUNCATE(handle, fsp, offset); #endif @@ -5787,10 +5783,6 @@ static int fruit_ftruncate_rsrc_stream(struct vfs_handle_struct *handle, struct files_struct *fsp, off_t offset) { - if (offset == 0) { - return SMB_VFS_NEXT_UNLINK(handle, fsp->fsp_name); - } - return SMB_VFS_NEXT_FTRUNCATE(handle, fsp, offset); } -- 2.17.2 From 5de35fbb1dd47526a545f19a009ddc1888fd1584 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Mon, 22 Oct 2018 16:56:46 +0200 Subject: [PATCH 34/43] vfs_fruit: add fio->created fio->created tracks whether a create created a stream. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- source3/modules/vfs_fruit.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c index 9a943452642..c3ad91b3986 100644 --- a/source3/modules/vfs_fruit.c +++ b/source3/modules/vfs_fruit.c @@ -514,6 +514,9 @@ struct fio { /* Denote stream type, meta or rsrc */ adouble_type_t type; + + /* Whether the create created the stream */ + bool created; }; /* @@ -5885,6 +5888,7 @@ static NTSTATUS fruit_create_file(vfs_handle_struct *handle, NTSTATUS status; struct fruit_config_data *config = NULL; files_struct *fsp = NULL; + struct fio *fio = NULL; status = check_aapl(handle, req, in_context_blobs, out_context_blobs); if (!NT_STATUS_IS_OK(status)) { @@ -5935,6 +5939,11 @@ static NTSTATUS fruit_create_file(vfs_handle_struct *handle, goto fail; } + fio = (struct fio *)VFS_FETCH_FSP_EXTENSION(handle, fsp); + if (fio != NULL && pinfo != NULL && *pinfo == FILE_WAS_CREATED) { + fio->created = true; + } + if (is_ntfs_stream_smb_fname(smb_fname) || fsp->is_directory) { return status; -- 2.17.2 From 100f52c4c9c74d770390edbd17e439a67053afde Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 22 Aug 2018 15:22:57 +0200 Subject: [PATCH 35/43] vfs_fruit: prepare struct fio for fake-fd and on-demand opening Not used for now, that comes in the subsequent commits. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- source3/modules/vfs_fruit.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c index c3ad91b3986..326976947a6 100644 --- a/source3/modules/vfs_fruit.c +++ b/source3/modules/vfs_fruit.c @@ -517,6 +517,17 @@ struct fio { /* Whether the create created the stream */ bool created; + + /* + * AFP_AfpInfo stream created, but not written yet, thus still a fake + * pipe fd. This is set to true in fruit_open_meta if there was no + * exisiting stream but the caller requested O_CREAT. It is later set to + * false when we get a write on the stream that then does open and + * create the stream. + */ + bool fake_fd; + int flags; + int mode; }; /* -- 2.17.2 From 25fa0b39d1210d267d915f36c55adacf47149cfb Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 22 Aug 2018 15:21:08 +0200 Subject: [PATCH 36/43] vfs_fruit: prepare fruit_pwrite_meta() for on-demand opening and writing This avoid creating files or blobs in our streams backend when a client creates a stream but hasn't written anything yet. This is the only sane way to implement the following semantics: * client 1: create stream "file:foo" * client 2: open stream "file:foo" The second operation of client 2 must fail with NT_STATUS_NOT_FOUND. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- source3/modules/vfs_fruit.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c index 326976947a6..e6d6a9df087 100644 --- a/source3/modules/vfs_fruit.c +++ b/source3/modules/vfs_fruit.c @@ -4495,10 +4495,44 @@ static ssize_t fruit_pwrite_meta_stream(vfs_handle_struct *handle, files_struct *fsp, const void *data, size_t n, off_t offset) { + struct fio *fio = (struct fio *)VFS_FETCH_FSP_EXTENSION(handle, fsp); AfpInfo *ai = NULL; size_t nwritten; + int ret; bool ok; + DBG_DEBUG("Path [%s] offset=%"PRIdMAX", size=%zd\n", + fsp_str_dbg(fsp), (intmax_t)offset, n); + + if (fio == NULL) { + return -1; + } + + if (fio->fake_fd) { + int fd; + + ret = SMB_VFS_NEXT_CLOSE(handle, fsp); + if (ret != 0) { + DBG_ERR("Close [%s] failed: %s\n", + fsp_str_dbg(fsp), strerror(errno)); + fsp->fh->fd = -1; + return -1; + } + + fd = SMB_VFS_NEXT_OPEN(handle, + fsp->fsp_name, + fsp, + fio->flags, + fio->mode); + if (fd == -1) { + DBG_ERR("On-demand create [%s] in write failed: %s\n", + fsp_str_dbg(fsp), strerror(errno)); + return -1; + } + fsp->fh->fd = fd; + fio->fake_fd = false; + } + ai = afpinfo_unpack(talloc_tos(), data); if (ai == NULL) { return -1; -- 2.17.2 From c921016a40379f8b473a74330db9b2a12a49e3dd Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 22 Aug 2018 15:22:08 +0200 Subject: [PATCH 37/43] vfs_fruit: prepare fruit_pread_meta() for reading on fake-fd If the read on the stream fails we may have hit a handle on a just created stream (fio->created=true) with no data written yet. If that's the case return an empty initialized FinderInfo blob. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- source3/modules/vfs_fruit.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c index e6d6a9df087..5dbdeedab89 100644 --- a/source3/modules/vfs_fruit.c +++ b/source3/modules/vfs_fruit.c @@ -4194,8 +4194,7 @@ static ssize_t fruit_pread_meta_stream(vfs_handle_struct *handle, int ret; nread = SMB_VFS_NEXT_PREAD(handle, fsp, data, n, offset); - - if (nread == n) { + if (nread == -1 || nread == n) { return nread; } @@ -4299,6 +4298,25 @@ static ssize_t fruit_pread_meta(vfs_handle_struct *handle, return -1; } + if (nread == -1 && fio->created) { + AfpInfo *ai = NULL; + char afpinfo_buf[AFP_INFO_SIZE]; + + ai = afpinfo_new(talloc_tos()); + if (ai == NULL) { + return -1; + } + + nread = afpinfo_pack(ai, afpinfo_buf); + TALLOC_FREE(ai); + if (nread != AFP_INFO_SIZE) { + return -1; + } + + memcpy(data, afpinfo_buf, to_return); + return nread; + } + return nread; } -- 2.17.2 From 932a4b841b5ed78d7037441e5471448f1546b084 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 22 Aug 2018 16:49:23 +0200 Subject: [PATCH 38/43] vfs_fruit: do ino calculation As we'll start returning fake fds in open shortly, we can't rely on the next module to calculat correct inode numbers for streams and must take over that responsibility. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- source3/modules/vfs_fruit.c | 50 +++++++++++++++++++++++++++++++++++-- 1 file changed, 48 insertions(+), 2 deletions(-) diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c index 5dbdeedab89..1e6ceb0e8f0 100644 --- a/source3/modules/vfs_fruit.c +++ b/source3/modules/vfs_fruit.c @@ -4898,6 +4898,14 @@ static int fruit_stat_meta_stream(vfs_handle_struct *handle, bool follow_links) { int ret; + ino_t ino; + + ret = fruit_stat_base(handle, smb_fname, false); + if (ret != 0) { + return -1; + } + + ino = fruit_inode(&smb_fname->st, smb_fname->stream_name); if (follow_links) { ret = SMB_VFS_NEXT_STAT(handle, smb_fname); @@ -4905,6 +4913,8 @@ static int fruit_stat_meta_stream(vfs_handle_struct *handle, ret = SMB_VFS_NEXT_LSTAT(handle, smb_fname); } + smb_fname->st.st_ex_ino = ino; + return ret; } @@ -5158,7 +5168,41 @@ static int fruit_fstat_meta_stream(vfs_handle_struct *handle, files_struct *fsp, SMB_STRUCT_STAT *sbuf) { - return SMB_VFS_NEXT_FSTAT(handle, fsp, sbuf); + struct fio *fio = (struct fio *)VFS_FETCH_FSP_EXTENSION(handle, fsp); + ino_t ino; + int ret; + + if (fio == NULL) { + return -1; + } + + if (fio->fake_fd) { + ret = fruit_stat_base(handle, fsp->base_fsp->fsp_name, false); + if (ret != 0) { + return -1; + } + + *sbuf = fsp->base_fsp->fsp_name->st; + sbuf->st_ex_size = AFP_INFO_SIZE; + sbuf->st_ex_ino = fruit_inode(sbuf, fsp->fsp_name->stream_name); + return 0; + } + + ret = fruit_stat_base(handle, fsp->base_fsp->fsp_name, false); + if (ret != 0) { + return -1; + } + *sbuf = fsp->base_fsp->fsp_name->st; + + ino = fruit_inode(sbuf, fsp->fsp_name->stream_name); + + ret = SMB_VFS_NEXT_FSTAT(handle, fsp, sbuf); + if (ret != 0) { + return -1; + } + + sbuf->st_ex_ino = ino; + return 0; } static int fruit_fstat_meta_netatalk(vfs_handle_struct *handle, @@ -5393,12 +5437,14 @@ static NTSTATUS fruit_streaminfo_meta_stream( goto out; } - ret = SMB_VFS_NEXT_STAT(handle, sname); + ret = fruit_stat_base(handle, sname, false); if (ret != 0) { status = map_nt_error_from_unix(errno); goto out; } + sname->st.st_ex_ino = fruit_inode(&sname->st, AFPINFO_STREAM); + id = SMB_VFS_NEXT_FILE_ID_CREATE(handle, &sname->st); lck = get_existing_share_mode_lock(talloc_tos(), id); -- 2.17.2 From f627dc0975d928b3d509681f4766415f69f6bf82 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 17 Oct 2018 16:51:34 +0200 Subject: [PATCH 39/43] vfs_fruit: let fruit handle all aio on the FinderInfo metadata stream This will be required to support using fake fds for the FinderInfo metadata stream. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- source3/modules/vfs_fruit.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c index 1e6ceb0e8f0..440be3ffe04 100644 --- a/source3/modules/vfs_fruit.c +++ b/source3/modules/vfs_fruit.c @@ -4416,9 +4416,7 @@ static bool fruit_must_handle_aio_stream(struct fio *fio) return false; }; - if ((fio->type == ADOUBLE_META) && - (fio->config->meta == FRUIT_META_NETATALK)) - { + if (fio->type == ADOUBLE_META) { return true; } -- 2.17.2 From 6a803a2345db9fc2ba1940954298e82a8b10fd4e Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sat, 20 Oct 2018 23:46:43 +0200 Subject: [PATCH 40/43] vfs_fruit: pass stream size to delete_invalid_meta_stream() delete_invalid_meta_stream() is meant to guard against random data being present in the FinderInfo stream. If the stream size is 0, it's likely a freshly created stream where no data has been written to yet, so don't delete it. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- source3/modules/vfs_fruit.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c index 440be3ffe04..f486a0589e7 100644 --- a/source3/modules/vfs_fruit.c +++ b/source3/modules/vfs_fruit.c @@ -5352,7 +5352,8 @@ static NTSTATUS delete_invalid_meta_stream( const struct smb_filename *smb_fname, TALLOC_CTX *mem_ctx, unsigned int *pnum_streams, - struct stream_struct **pstreams) + struct stream_struct **pstreams, + off_t size) { struct smb_filename *sname = NULL; int ret; @@ -5363,6 +5364,10 @@ static NTSTATUS delete_invalid_meta_stream( return NT_STATUS_INTERNAL_ERROR; } + if (size == 0) { + return NT_STATUS_OK; + } + sname = synthetic_smb_fname(talloc_tos(), smb_fname->base_name, AFPINFO_STREAM_NAME, @@ -5416,8 +5421,12 @@ static NTSTATUS fruit_streaminfo_meta_stream( DBG_ERR("Removing invalid AFPINFO_STREAM size [%jd] from [%s]\n", (intmax_t)stream[i].size, smb_fname_str_dbg(smb_fname)); - return delete_invalid_meta_stream(handle, smb_fname, mem_ctx, - pnum_streams, pstreams); + return delete_invalid_meta_stream(handle, + smb_fname, + mem_ctx, + pnum_streams, + pstreams, + stream[i].size); } /* -- 2.17.2 From 38ce75c2f85d874deeec25b8b5afd5442abf9c39 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sat, 20 Oct 2018 23:40:14 +0200 Subject: [PATCH 41/43] vfs_fruit: let fruit_pwrite_meta_stream also ftruncate empty FinderInfo fruit_streaminfo currently filters out the FinderInfo stream is delete-on-close is set. We set it here internally, but the client may also set it over SMB. Turns out that the macOS SMB server does NOT filter out FinderInfo stream with delete-on-close set, so we must change the way filtering is done in fruit_streaminfo. Filtering is now done based on the FinderInfo stream being 0-bytes large which is why I'm adding the ftruncate here. No idea why the tests that check the filtering passed the commits leading up to this one, but if you revert this commit after applying the whole patchset, the "delete AFP_AfpInfo by writing all 0" test will fail. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- source3/modules/vfs_fruit.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c index f486a0589e7..34b7f25c99b 100644 --- a/source3/modules/vfs_fruit.c +++ b/source3/modules/vfs_fruit.c @@ -4554,23 +4554,29 @@ static ssize_t fruit_pwrite_meta_stream(vfs_handle_struct *handle, return -1; } - nwritten = SMB_VFS_NEXT_PWRITE(handle, fsp, data, n, offset); - if (nwritten != n) { - return -1; - } - - if (!ai_empty_finderinfo(ai)) { - return n; - } + if (ai_empty_finderinfo(ai)) { + ret = SMB_VFS_NEXT_FTRUNCATE(handle, fsp, 0); + if (ret != 0) { + DBG_ERR("SMB_VFS_NEXT_FTRUNCATE on [%s] failed\n", + fsp_str_dbg(fsp)); + return -1; + } - ok = set_delete_on_close( + ok = set_delete_on_close( fsp, true, handle->conn->session_info->security_token, handle->conn->session_info->unix_token); - if (!ok) { - DBG_ERR("set_delete_on_close on [%s] failed\n", - fsp_str_dbg(fsp)); + if (!ok) { + DBG_ERR("set_delete_on_close on [%s] failed\n", + fsp_str_dbg(fsp)); + return -1; + } + return n; + } + + nwritten = SMB_VFS_NEXT_PWRITE(handle, fsp, data, n, offset); + if (nwritten != n) { return -1; } -- 2.17.2 From e4f7973f660aadfe000cd92f0c96ba99d813369e Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sat, 20 Oct 2018 23:50:32 +0200 Subject: [PATCH 42/43] vfs_fruit: don't check for delete-on-close on the FinderInfo stream macOS SMB server doesn't filter out the FinderInfo stream if it has delete-on-close set. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- source3/modules/vfs_fruit.c | 73 +------------------------------------ 1 file changed, 1 insertion(+), 72 deletions(-) diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c index 34b7f25c99b..4f3db64a681 100644 --- a/source3/modules/vfs_fruit.c +++ b/source3/modules/vfs_fruit.c @@ -5402,16 +5402,7 @@ static NTSTATUS fruit_streaminfo_meta_stream( { struct stream_struct *stream = *pstreams; unsigned int num_streams = *pnum_streams; - struct smb_filename *sname = NULL; - char *full_name = NULL; - uint32_t name_hash; - struct share_mode_lock *lck = NULL; - struct file_id id = {0}; - bool delete_on_close_set; int i; - int ret; - NTSTATUS status; - bool ok; for (i = 0; i < num_streams; i++) { if (strequal_m(stream[i].name, AFPINFO_STREAM)) { @@ -5435,70 +5426,8 @@ static NTSTATUS fruit_streaminfo_meta_stream( stream[i].size); } - /* - * Now check if there's a delete-on-close pending on the stream. If so, - * hide the stream. This behaviour was verified against a macOS 10.12 - * SMB server. - */ - sname = synthetic_smb_fname(talloc_tos(), - smb_fname->base_name, - AFPINFO_STREAM_NAME, - NULL, 0); - if (sname == NULL) { - status = NT_STATUS_NO_MEMORY; - goto out; - } - - ret = fruit_stat_base(handle, sname, false); - if (ret != 0) { - status = map_nt_error_from_unix(errno); - goto out; - } - - sname->st.st_ex_ino = fruit_inode(&sname->st, AFPINFO_STREAM); - - id = SMB_VFS_NEXT_FILE_ID_CREATE(handle, &sname->st); - - lck = get_existing_share_mode_lock(talloc_tos(), id); - if (lck == NULL) { - status = NT_STATUS_OK; - goto out; - } - - full_name = talloc_asprintf(talloc_tos(), - "%s%s", - sname->base_name, - AFPINFO_STREAM); - if (full_name == NULL) { - status = NT_STATUS_NO_MEMORY; - goto out; - } - - status = file_name_hash(handle->conn, full_name, &name_hash); - if (!NT_STATUS_IS_OK(status)) { - goto out; - } - - delete_on_close_set = is_delete_on_close_set(lck, name_hash); - if (delete_on_close_set) { - ok = del_fruit_stream(mem_ctx, - pnum_streams, - pstreams, - AFPINFO_STREAM); - if (!ok) { - status = NT_STATUS_INTERNAL_ERROR; - goto out; - } - } - - status = NT_STATUS_OK; - -out: - TALLOC_FREE(sname); - TALLOC_FREE(lck); - TALLOC_FREE(full_name); - return status; + return NT_STATUS_OK; } static NTSTATUS fruit_streaminfo_meta_netatalk( -- 2.17.2 From b95b733217fb63c105e90e6c5e984793ebf76a29 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 22 Aug 2018 15:25:26 +0200 Subject: [PATCH 43/43] vfs_fruit: let fruit_open_meta() with O_CREAT return a fake-fd This is the final step in implementing the needed macOS semantics on the FinderInfo stream: as long as the client hasn't written a non-zero FinderInfo blob to the stream, there mustn't be a visible filesystem entry for other openers. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13646 Signed-off-by: Ralph Boehme --- selftest/knownfail.d/samba3.vfs.fruit | 3 - source3/modules/vfs_fruit.c | 165 +++++++++++--------------- 2 files changed, 72 insertions(+), 96 deletions(-) diff --git a/selftest/knownfail.d/samba3.vfs.fruit b/selftest/knownfail.d/samba3.vfs.fruit index 2d3c21caf91..6307e2b3404 100644 --- a/selftest/knownfail.d/samba3.vfs.fruit +++ b/selftest/knownfail.d/samba3.vfs.fruit @@ -1,5 +1,2 @@ ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion$nt4_dc$ ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion without embedded xattr$nt4_dc$ -^samba3.vfs.fruit metadata_netatalk.empty_stream$nt4_dc$ -^samba3.vfs.fruit metadata_stream.empty_stream$nt4_dc$ -^samba3.vfs.fruit streams_depot.empty_stream$nt4_dc$ diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c index 4f3db64a681..dd1aa123104 100644 --- a/source3/modules/vfs_fruit.c +++ b/source3/modules/vfs_fruit.c @@ -3403,66 +3403,68 @@ static int fruit_connect(vfs_handle_struct *handle, return rc; } +static int fruit_fake_fd(void) +{ + int pipe_fds[2]; + int fd; + int ret; + + /* + * Return a valid fd, but ensure any attempt to use it returns + * an error (EPIPE). Once we get a write on the handle, we open + * the real fd. + */ + ret = pipe(pipe_fds); + if (ret != 0) { + return -1; + } + fd = pipe_fds[0]; + close(pipe_fds[1]); + + return fd; +} + static int fruit_open_meta_stream(vfs_handle_struct *handle, struct smb_filename *smb_fname, files_struct *fsp, int flags, mode_t mode) { - AfpInfo *ai = NULL; - char afpinfo_buf[AFP_INFO_SIZE]; - ssize_t len, written; - int hostfd = -1; - int rc = -1; + struct fruit_config_data *config = NULL; + struct fio *fio = NULL; + int open_flags = flags & ~O_CREAT; + int fd; - hostfd = SMB_VFS_NEXT_OPEN(handle, smb_fname, fsp, flags, mode); - if (hostfd == -1) { - return -1; - } + DBG_DEBUG("Path [%s]\n", smb_fname_str_dbg(smb_fname)); - if (!(flags & (O_CREAT | O_TRUNC))) { - return hostfd; - } + SMB_VFS_HANDLE_GET_DATA(handle, config, + struct fruit_config_data, return -1); - ai = afpinfo_new(talloc_tos()); - if (ai == NULL) { - rc = -1; - goto fail; - } + fio = VFS_ADD_FSP_EXTENSION(handle, fsp, struct fio, NULL); + fio->type = ADOUBLE_META; + fio->config = config; - len = afpinfo_pack(ai, afpinfo_buf); - if (len != AFP_INFO_SIZE) { - rc = -1; - goto fail; + fd = SMB_VFS_NEXT_OPEN(handle, smb_fname, fsp, open_flags, mode); + if (fd != -1) { + return fd; } - /* Set fd, needed in SMB_VFS_NEXT_PWRITE() */ - fsp->fh->fd = hostfd; - - written = SMB_VFS_NEXT_PWRITE(handle, fsp, afpinfo_buf, - AFP_INFO_SIZE, 0); - fsp->fh->fd = -1; - if (written != AFP_INFO_SIZE) { - DBG_ERR("bad write [%zd/%d]\n", written, AFP_INFO_SIZE); - rc = -1; - goto fail; + if (!(flags & O_CREAT)) { + VFS_REMOVE_FSP_EXTENSION(handle, fsp); + return -1; } - rc = 0; + fd = fruit_fake_fd(); + if (fd == -1) { + VFS_REMOVE_FSP_EXTENSION(handle, fsp); + return -1; + } -fail: - DBG_DEBUG("rc=%d, fd=%d\n", rc, hostfd); + fio->fake_fd = true; + fio->flags = flags; + fio->mode = mode; - if (rc != 0) { - int saved_errno = errno; - if (hostfd >= 0) { - fsp->fh->fd = hostfd; - SMB_VFS_NEXT_CLOSE(handle, fsp); - } - hostfd = -1; - errno = saved_errno; - } - return hostfd; + return fd; } static int fruit_open_meta_netatalk(vfs_handle_struct *handle, @@ -3471,56 +3473,42 @@ static int fruit_open_meta_netatalk(vfs_handle_struct *handle, int flags, mode_t mode) { - int rc; - int fakefd = -1; + struct fruit_config_data *config = NULL; + struct fio *fio = NULL; struct adouble *ad = NULL; - int fds[2]; + bool meta_exists = false; + int fd; DBG_DEBUG("Path [%s]\n", smb_fname_str_dbg(smb_fname)); - /* - * Return a valid fd, but ensure any attempt to use it returns an error - * (EPIPE). All operations on the smb_fname or the fsp will use path - * based syscalls. - */ - rc = pipe(fds); - if (rc != 0) { - goto exit; + ad = ad_get(talloc_tos(), handle, smb_fname, ADOUBLE_META); + if (ad != NULL) { + meta_exists = true; } - fakefd = fds[0]; - close(fds[1]); - - if (flags & (O_CREAT | O_TRUNC)) { - /* - * The attribute does not exist or needs to be truncated, - * create an AppleDouble EA - */ - ad = ad_init(fsp, handle, ADOUBLE_META); - if (ad == NULL) { - rc = -1; - goto exit; - } - rc = ad_set(ad, fsp->fsp_name); - if (rc != 0) { - rc = -1; - goto exit; - } + TALLOC_FREE(ad); - TALLOC_FREE(ad); + if (!meta_exists && !(flags & O_CREAT)) { + errno = ENOENT; + return -1; } -exit: - DEBUG(10, ("fruit_open meta rc=%d, fd=%d\n", rc, fakefd)); - if (rc != 0) { - int saved_errno = errno; - if (fakefd >= 0) { - close(fakefd); - } - fakefd = -1; - errno = saved_errno; + fd = fruit_fake_fd(); + if (fd == -1) { + return -1; } - return fakefd; + + SMB_VFS_HANDLE_GET_DATA(handle, config, + struct fruit_config_data, return -1); + + fio = VFS_ADD_FSP_EXTENSION(handle, fsp, struct fio, NULL); + fio->type = ADOUBLE_META; + fio->config = config; + fio->fake_fd = true; + fio->flags = flags; + fio->mode = mode; + + return fd; } static int fruit_open_meta(vfs_handle_struct *handle, @@ -3529,7 +3517,6 @@ static int fruit_open_meta(vfs_handle_struct *handle, { int fd; struct fruit_config_data *config = NULL; - struct fio *fio = NULL; DBG_DEBUG("path [%s]\n", smb_fname_str_dbg(smb_fname)); @@ -3554,14 +3541,6 @@ static int fruit_open_meta(vfs_handle_struct *handle, DBG_DEBUG("path [%s] fd [%d]\n", smb_fname_str_dbg(smb_fname), fd); - if (fd == -1) { - return -1; - } - - fio = VFS_ADD_FSP_EXTENSION(handle, fsp, struct fio, NULL); - fio->type = ADOUBLE_META; - fio->config = config; - return fd; } -- 2.17.2