[PATCHES] report zero on-disk file ID to Macs
Uri Simchoni
uri at samba.org
Thu Mar 23 20:15:32 UTC 2017
Hi,
The attached patch set allows reporting zero on-disk file ID (e.g. QFid
create context, FileInternalInformation) if the client has negotiated
AAPL Apple extensions.
The motive for this is explained in Ralph's SambaXP 2016 talk, and also
documented in the accompanied bug report -
https://bugzilla.samba.org/show_bug.cgi?id=12715 .
In the talk, Ralph mentioned a client-side workaround. This patch
hopefully achieves the same without tweaking the client.
I'd like to thank Ralph for all the pointers he provided on the road to
this workaround.
Review appreciated.
Thanks,
Uri.
-------------- next part --------------
From cf6165e00f2153fa61eba1795d36532b3496a088 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
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 <uri at samba.org>
---
source3/smbd/proto.h | 1 +
source3/smbd/trans2.c | 17 +++++++++++++++++
2 files changed, 18 insertions(+)
diff --git a/source3/smbd/proto.h b/source3/smbd/proto.h
index 5671a96..ecb23e2 100644
--- a/source3/smbd/proto.h
+++ b/source3/smbd/proto.h
@@ -1131,6 +1131,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 force_zero_file_id(void);
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 b6bf93f..c4081f3 100644
--- a/source3/smbd/trans2.c
+++ b/source3/smbd/trans2.c
@@ -54,6 +54,8 @@ static char *store_file_unix_basic_info2(connection_struct *conn,
files_struct *fsp,
const SMB_STRUCT_STAT *psbuf);
+static bool forced_zero_file_id;
+
/****************************************************************************
Check if an open file handle or smb_fname is a symlink.
****************************************************************************/
@@ -146,6 +148,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 (forced_zero_file_id) {
+ return 0;
+ }
if (conn->base_share_dev == psbuf->st_ex_dev) {
return (uint64_t)psbuf->st_ex_ino;
}
@@ -154,6 +159,18 @@ 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 force_zero_file_id(void)
+{
+ forced_zero_file_id = true;
+}
+
/****************************************************************************
Utility functions for dealing with extended attributes.
****************************************************************************/
--
2.9.3
From 506a03e1c4d2ae6f142cb233f3664986d4802d3d Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
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 <uri at samba.org>
---
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 fc80629..92e88c9 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) {
+ force_zero_file_id();
+ }
}
return status;
--
2.9.3
From 84b7bcc2a69b4eaeabc68243c1e851ee53fe697e Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
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 <uri at samba.org>
---
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 a00f6a9..0bddd4a 100644
--- a/docs-xml/manpages/vfs_fruit.8.xml
+++ b/docs-xml/manpages/vfs_fruit.8.xml
@@ -143,6 +143,23 @@
</listitem>
</varlistentry>
+ <varlistentry>
+ <term>fruit:zero_file_id = yes | no</term>
+ <listitem>
+ <para>A <emphasis>global</emphasis> option whether to return
+ zero to queries of on-disk file identifier, if the client
+ has negotiated AAPL.</para>
+ <para>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.</para>
+ <para>The default is <emphasis>yes</emphasis>.</para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</refsect1>
--
2.9.3
From 8c6beaef4f545f90ed80ead4e85ecdee70adccab Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
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 <uri at samba.org>
---
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 7477ba205eba907d70261625e12ee034e8722f1f Mon Sep 17 00:00:00 2001
From: Uri Simchoni <uri at samba.org>
Date: Thu, 23 Mar 2017 21:32:04 +0200
Subject: [PATCH 5/5] selftest: tests for vfs_fruite file-id behavior
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 <uri at samba.org>
---
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 54f5b7b..336ec92 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -340,7 +340,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
@@ -428,6 +428,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
More information about the samba-technical
mailing list