From 7380bb5eaa230d45127a75d3472756faecc17f39 Mon Sep 17 00:00:00 2001 From: Uri Simchoni Date: Thu, 23 Mar 2017 14:08:26 +0200 Subject: [PATCH 1/5] smbd: add zero_file_id flag This flag instructs the SMB layer to report a zero on-disk file identifier. According to [MS-SMB2] 3.3.5.9.9, the reported on-disk file ID SHOULD be unique. However, macOS clients seem to expect it to be unique over time as well, like the HFS+ CNID. Reporting a file ID of 0 seems to instruct the Mac client not to trust the server-reported file ID. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12715 Signed-off-by: Uri Simchoni Reviewed-by: Ralph Boehme (cherry picked from commit 6711522e1e57980e50e245f43167d0daf5a705ad) --- source3/smbd/globals.h | 1 + source3/smbd/proto.h | 1 + source3/smbd/trans2.c | 14 ++++++++++++++ 3 files changed, 16 insertions(+) diff --git a/source3/smbd/globals.h b/source3/smbd/globals.h index 76ce0bb..67d3a89 100644 --- a/source3/smbd/globals.h +++ b/source3/smbd/globals.h @@ -864,6 +864,7 @@ struct smbd_server_connection { struct messaging_context *msg_ctx; struct notify_context *notify_ctx; bool using_smb2; + bool aapl_zero_file_id; /* Apple-specific */ int trans_num; size_t num_users; diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h index 9301c78..5505734 100644 --- a/source3/smbd/proto.h +++ b/source3/smbd/proto.h @@ -1134,6 +1134,7 @@ NTSTATUS check_access(connection_struct *conn, uint32_t access_mask); uint64_t smb_roundup(connection_struct *conn, uint64_t val); uint64_t get_FileIndex(connection_struct *conn, const SMB_STRUCT_STAT *psbuf); +void aapl_force_zero_file_id(struct smbd_server_connection *sconn); bool samba_private_attr_name(const char *unix_ea_name); NTSTATUS get_ea_value(TALLOC_CTX *mem_ctx, connection_struct *conn, files_struct *fsp, const char *fname, diff --git a/source3/smbd/trans2.c b/source3/smbd/trans2.c index 6999b2d..f0d8e7d 100644 --- a/source3/smbd/trans2.c +++ b/source3/smbd/trans2.c @@ -143,6 +143,9 @@ uint64_t smb_roundup(connection_struct *conn, uint64_t val) uint64_t get_FileIndex(connection_struct *conn, const SMB_STRUCT_STAT *psbuf) { uint64_t file_index; + if (conn->sconn->aapl_zero_file_id) { + return 0; + } if (conn->base_share_dev == psbuf->st_ex_dev) { return (uint64_t)psbuf->st_ex_ino; } @@ -151,6 +154,17 @@ uint64_t get_FileIndex(connection_struct *conn, const SMB_STRUCT_STAT *psbuf) return file_index; } + +/******************************************************************** + Globally (for this connection / multi-channel) disable file-ID + calculation. This is required to be global because it serves + Macs in AAPL mode, which is globally set. +********************************************************************/ +void aapl_force_zero_file_id(struct smbd_server_connection *sconn) +{ + sconn->aapl_zero_file_id = true; +} + /**************************************************************************** Utility functions for dealing with extended attributes. ****************************************************************************/ -- 2.9.3 From 66df6a7a53252b2da93d64361398eecfbbc0058a Mon Sep 17 00:00:00 2001 From: Uri Simchoni Date: Thu, 23 Mar 2017 14:08:45 +0200 Subject: [PATCH 2/5] vfs_fruit: enable zero file id Enable zero_file_id if both conditions are met: - AAPL negotiated - fruit:zero_file_id is set BUG: https://bugzilla.samba.org/show_bug.cgi?id=12715 Signed-off-by: Uri Simchoni Reviewed-by: Ralph Boehme (cherry picked from commit 245a325532c9a46ec3e459ceca38e903b203f691) --- source3/modules/vfs_fruit.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c index b296d74..c3d7535 100644 --- a/source3/modules/vfs_fruit.c +++ b/source3/modules/vfs_fruit.c @@ -135,6 +135,7 @@ struct fruit_config_data { bool copyfile_enabled; bool veto_appledouble; bool posix_rename; + bool aapl_zero_file_id; /* * Additional options, all enabled by default, @@ -1591,6 +1592,9 @@ static int init_fruit_config(vfs_handle_struct *handle) config->posix_rename = lp_parm_bool( SNUM(handle->conn), FRUIT_PARAM_TYPE_NAME, "posix_rename", true); + config->aapl_zero_file_id = + lp_parm_bool(-1, FRUIT_PARAM_TYPE_NAME, "zero_file_id", true); + config->readdir_attr_rsize = lp_parm_bool( SNUM(handle->conn), "readdir_attr", "aapl_rsize", true); @@ -2236,6 +2240,9 @@ static NTSTATUS check_aapl(vfs_handle_struct *handle, blob); if (NT_STATUS_IS_OK(status)) { global_fruit_config.nego_aapl = true; + if (config->aapl_zero_file_id) { + aapl_force_zero_file_id(handle->conn->sconn); + } } return status; -- 2.9.3 From 7ce8f137a5cb2786eabe3a807011498b5f3be570 Mon Sep 17 00:00:00 2001 From: Uri Simchoni Date: Thu, 23 Mar 2017 14:51:32 +0200 Subject: [PATCH 3/5] vfs_fruit: document added zero_file_id parameter BUG: https://bugzilla.samba.org/show_bug.cgi?id=12715 Signed-off-by: Uri Simchoni Reviewed-by: Ralph Boehme (cherry picked from commit e11d4eb4d5c6cfc6daa3dbdcc301a4fa83298f0e) --- docs-xml/manpages/vfs_fruit.8.xml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/docs-xml/manpages/vfs_fruit.8.xml b/docs-xml/manpages/vfs_fruit.8.xml index 9f999da..cbeb12c 100644 --- a/docs-xml/manpages/vfs_fruit.8.xml +++ b/docs-xml/manpages/vfs_fruit.8.xml @@ -145,6 +145,23 @@ + + fruit:zero_file_id = yes | no + + A global option whether to return + zero to queries of on-disk file identifier, if the client + has negotiated AAPL. + Mac applications and / or the Mac SMB + client code expect the on-disk file identifier to have the + semantics of HFS+ Catalog Node Identifier (CNID). Samba + doesn't provide those semantics, and that occasionally cause + usability issues or even data loss. Returning a file identifier + of zero causes the Mac client to stop using and trusting the + file id returned from the server. + The default is yes. + + + -- 2.9.3 From ae2bcb49197dc9522c1d4fa9ab274a209bcc63d0 Mon Sep 17 00:00:00 2001 From: Uri Simchoni Date: Thu, 23 Mar 2017 21:30:50 +0200 Subject: [PATCH 4/5] torture: add torture_assert_mem_not_equal_goto() BUG: https://bugzilla.samba.org/show_bug.cgi?id=12715 Signed-off-by: Uri Simchoni Reviewed-by: Ralph Boehme (cherry picked from commit f31fd41ca728d664ded940a7309ef1e32383bb66) --- lib/torture/torture.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/torture/torture.h b/lib/torture/torture.h index b6d1301..668458a 100644 --- a/lib/torture/torture.h +++ b/lib/torture/torture.h @@ -367,6 +367,16 @@ void torture_result(struct torture_context *test, } \ } while(0) +#define torture_assert_mem_not_equal_goto(torture_ctx,got,expected,len,ret,label,cmt) \ + do { const void *__got = (got), *__expected = (expected); \ + if (memcmp(__got, __expected, len) == 0) { \ + torture_result(torture_ctx, TORTURE_FAIL, \ + __location__": "#got" of len %d unexpectedly matches "#expected": %s", (int)len, cmt); \ + ret = false; \ + goto label; \ + } \ + } while(0) + static inline void torture_dump_data_str_cb(const char *buf, void *private_data) { char **dump = (char **)private_data; -- 2.9.3 From 42f95b4f42c0922816dfa5da3ed07066bca58c2d Mon Sep 17 00:00:00 2001 From: Uri Simchoni Date: Thu, 23 Mar 2017 21:32:04 +0200 Subject: [PATCH 5/5] selftest: tests for vfs_fruite file-id behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test is in its own suite because it validates our hackish workaround rather than some reference implementation behavior. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12715 Signed-off-by: Uri Simchoni Reviewed-by: Ralph Boehme Autobuild-User(master): Ralph Böhme Autobuild-Date(master): Sun Mar 26 23:31:08 CEST 2017 on sn-devel-144 (cherry picked from commit b6baf35ebde68db75515910ede26e74bb8313284) --- source3/selftest/tests.py | 4 ++- source4/torture/vfs/fruit.c | 72 +++++++++++++++++++++++++++++++++++++++++++++ source4/torture/vfs/vfs.c | 1 + 3 files changed, 76 insertions(+), 1 deletion(-) diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py index 9fd3d0a..4f1a667 100755 --- a/source3/selftest/tests.py +++ b/source3/selftest/tests.py @@ -335,7 +335,7 @@ nbt = ["nbt.dgram" ] libsmbclient = ["libsmbclient"] -vfs = ["vfs.fruit", "vfs.acl_xattr", "vfs.fruit_netatalk"] +vfs = ["vfs.fruit", "vfs.acl_xattr", "vfs.fruit_netatalk", "vfs.fruit_file_id"] tests= base + raw + smb2 + rpc + unix + local + rap + nbt + libsmbclient + idmap + vfs @@ -423,6 +423,8 @@ for t in tests: plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/vfs_fruit_stream_depot --option=torture:share2=vfs_wo_fruit_stream_depot -U$USERNAME%$PASSWORD', 'streams_depot') elif t == "vfs.fruit_netatalk": plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/vfs_fruit -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 == "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 d74a153..857e738 100644 --- a/source4/torture/vfs/fruit.c +++ b/source4/torture/vfs/fruit.c @@ -3914,6 +3914,63 @@ done: return ret; } +static bool test_zero_file_id(struct torture_context *tctx, + struct smb2_tree *tree) +{ + const char *fname = "filtest_file_id"; + struct smb2_create create = {0}; + NTSTATUS status; + bool ret = true; + uint8_t zero_file_id[8] = {0}; + + torture_comment(tctx, "Testing zero file id\n"); + + ret = torture_setup_file(tctx, tree, fname, false); + torture_assert_goto(tctx, ret == true, ret, done, "torture_setup_file"); + + 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 = fname; + create.in.query_on_disk_id = true; + + status = smb2_create(tree, tctx, &create); + torture_assert_ntstatus_equal_goto(tctx, status, NT_STATUS_OK, ret, + done, + "test file could not be opened"); + torture_assert_mem_not_equal_goto(tctx, create.out.on_disk_id, + zero_file_id, 8, ret, done, + "unexpected zero file id"); + + smb2_util_close(tree, create.out.file.handle); + + ret = enable_aapl(tctx, tree); + torture_assert(tctx, ret == true, "enable_aapl failed"); + + 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 = fname; + create.in.query_on_disk_id = true; + + status = smb2_create(tree, tctx, &create); + torture_assert_ntstatus_equal_goto( + tctx, status, NT_STATUS_OK, ret, done, + "test file could not be opened with AAPL"); + torture_assert_mem_equal_goto(tctx, create.out.on_disk_id, zero_file_id, + 8, ret, done, "non-zero file id"); + + smb2_util_close(tree, create.out.file.handle); + +done: + smb2_util_unlink(tree, fname); + 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 @@ -3968,3 +4025,18 @@ struct torture_suite *torture_vfs_fruit_netatalk(void) return suite; } + +struct torture_suite *torture_vfs_fruit_file_id(void) +{ + struct torture_suite *suite = + torture_suite_create(talloc_autofree_context(), "fruit_file_id"); + + suite->description = + talloc_strdup(suite, "vfs_fruit tests for on-disk file ID that " + "require fruit:zero_file_id=yes"); + + torture_suite_add_1smb2_test(suite, "zero file id if AAPL negotiated", + test_zero_file_id); + + return suite; +} diff --git a/source4/torture/vfs/vfs.c b/source4/torture/vfs/vfs.c index a4f8125..710e93b 100644 --- a/source4/torture/vfs/vfs.c +++ b/source4/torture/vfs/vfs.c @@ -111,6 +111,7 @@ NTSTATUS torture_vfs_init(void) torture_suite_add_suite(suite, torture_vfs_fruit()); torture_suite_add_suite(suite, torture_vfs_fruit_netatalk()); torture_suite_add_suite(suite, torture_acl_xattr()); + torture_suite_add_suite(suite, torture_vfs_fruit_file_id()); torture_register_suite(suite); -- 2.9.3