[SCM] Samba Shared Repository - branch master updated
Jeremy Allison
jra at samba.org
Tue Oct 24 22:31:01 UTC 2023
The branch, master has been updated
via 7c8dea14da6 smbtorture: add test for fruit:validate_afpinfo option
via 825a992a629 vfs_fruit: add option fruit:validate_afpinfo = yes|no (default: yes)
from 9f54b94b52d s4:torture: Produce more output to help debug smb2.multichannel.bugs.bug_15346
https://git.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit 7c8dea14da645d5e98eacd82b82b2a7c8c6b242f
Author: Ralph Boehme <slow at samba.org>
Date: Fri Oct 20 15:45:31 2023 +0200
smbtorture: add test for fruit:validate_afpinfo option
Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
Autobuild-User(master): Jeremy Allison <jra at samba.org>
Autobuild-Date(master): Tue Oct 24 22:30:06 UTC 2023 on atb-devel-224
commit 825a992a62920a807ac96c222e87c43ab0daa228
Author: Ralph Boehme <slow at samba.org>
Date: Tue Jul 4 17:46:40 2023 +0200
vfs_fruit: add option fruit:validate_afpinfo = yes|no (default: yes)
Allows disabling validation of AfpInfo stream data. It seems in data migration
scenarios from other SMB servers to Samba with fruit, somehow such invalid
streams are present on the source SMB server and can't be copied to Samba.
Signed-off-by: Ralph Boehme <slow at samba.org>
Reviewed-by: Jeremy Allison <jra at samba.org>
-----------------------------------------------------------------------
Summary of changes:
docs-xml/manpages/vfs_fruit.8.xml | 18 ++++++++++
selftest/target/Samba3.pm | 1 +
source3/lib/adouble.c | 16 ++++++---
source3/lib/adouble.h | 2 +-
source3/modules/vfs_fruit.c | 42 ++++++++++++++++++++----
source3/selftest/tests.py | 7 ++++
source4/torture/vfs/fruit.c | 69 +++++++++++++++++++++++++++++++++++++++
source4/torture/vfs/vfs.c | 1 +
8 files changed, 143 insertions(+), 13 deletions(-)
Changeset truncated at 500 lines:
diff --git a/docs-xml/manpages/vfs_fruit.8.xml b/docs-xml/manpages/vfs_fruit.8.xml
index 7ae17c65976..61051f90873 100644
--- a/docs-xml/manpages/vfs_fruit.8.xml
+++ b/docs-xml/manpages/vfs_fruit.8.xml
@@ -419,6 +419,24 @@
</listitem>
</varlistentry>
+ <varlistentry>
+ <term>fruit:validate_afpinfo = yes | no</term>
+ <listitem>
+ <para>Apple clients use the AFP_AfpInfo stream to store structured
+ file metadata. As part of the marshaled data stored in the stream
+ the first eight bytes contain some header information. Apple's SMB
+ server as well as Samba will validate this header bytes processing
+ a client write request on this stream, and, if the validation
+ fails, fail the write. While this validation is generally correct,
+ in some data migration scenarios clients may try to migrate data
+ from 3rd-party SMB servers to Samba servers where the header
+ information is broken for whatever reason. To allow migration and
+ header fix-up in these scenarios, the validation can be temporarily
+ disabled by setting this option to <emphasis>no</emphasis>.</para>
+ <para>The default is <emphasis>yes</emphasis>.</para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</refsect1>
diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index a28e2be0581..ef68f63e348 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -3246,6 +3246,7 @@ sub provision($$)
fruit:resource = file
fruit:metadata = stream
fruit:zero_file_id=yes
+ fruit:validate_afpinfo = no
[fruit_resource_stream]
path = $fruit_resource_stream_shrdir
diff --git a/source3/lib/adouble.c b/source3/lib/adouble.c
index f0fb3c3be27..ad1e5059cf6 100644
--- a/source3/lib/adouble.c
+++ b/source3/lib/adouble.c
@@ -2827,7 +2827,7 @@ ssize_t afpinfo_pack(const AfpInfo *ai, char *buf)
* Buffer size must be at least AFP_INFO_SIZE
* Returns allocated AfpInfo struct
**/
-AfpInfo *afpinfo_unpack(TALLOC_CTX *ctx, const void *data)
+AfpInfo *afpinfo_unpack(TALLOC_CTX *ctx, const void *data, bool validate)
{
AfpInfo *ai = talloc_zero(ctx, AfpInfo);
if (ai == NULL) {
@@ -2840,10 +2840,16 @@ AfpInfo *afpinfo_unpack(TALLOC_CTX *ctx, const void *data)
memcpy(ai->afpi_FinderInfo, (const char *)data + 16,
sizeof(ai->afpi_FinderInfo));
- if (ai->afpi_Signature != AFP_Signature
- || ai->afpi_Version != AFP_Version) {
- DEBUG(1, ("Bad AfpInfo signature or version\n"));
- TALLOC_FREE(ai);
+ if (validate) {
+ if (ai->afpi_Signature != AFP_Signature
+ || ai->afpi_Version != AFP_Version)
+ {
+ DEBUG(1, ("Bad AfpInfo signature or version\n"));
+ TALLOC_FREE(ai);
+ }
+ } else {
+ ai->afpi_Signature = AFP_Signature;
+ ai->afpi_Version = AFP_Version;
}
return ai;
diff --git a/source3/lib/adouble.h b/source3/lib/adouble.h
index de44f3f5fdc..d2c729305f9 100644
--- a/source3/lib/adouble.h
+++ b/source3/lib/adouble.h
@@ -187,6 +187,6 @@ int adouble_path(TALLOC_CTX *ctx,
AfpInfo *afpinfo_new(TALLOC_CTX *ctx);
ssize_t afpinfo_pack(const AfpInfo *ai, char *buf);
-AfpInfo *afpinfo_unpack(TALLOC_CTX *ctx, const void *data);
+AfpInfo *afpinfo_unpack(TALLOC_CTX *ctx, const void *data, bool validate);
#endif
diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 60bfecd8d35..b510b04aea6 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -134,6 +134,7 @@ struct fruit_config_data {
bool convert_adouble;
bool wipe_intentionally_left_blank_rfork;
bool delete_empty_adfiles;
+ bool validate_afpinfo;
/*
* Additional options, all enabled by default,
@@ -377,6 +378,10 @@ static int init_fruit_config(vfs_handle_struct *handle)
SNUM(handle->conn), FRUIT_PARAM_TYPE_NAME,
"delete_empty_adfiles", false);
+ config->validate_afpinfo = lp_parm_bool(
+ SNUM(handle->conn), FRUIT_PARAM_TYPE_NAME,
+ "validate_afpinfo", true);
+
SMB_VFS_HANDLE_SET_DATA(handle, config,
NULL, struct fruit_config_data,
return -1);
@@ -2637,10 +2642,12 @@ static ssize_t fruit_pread_recv(struct tevent_req *req,
}
static ssize_t fruit_pwrite_meta_stream(vfs_handle_struct *handle,
- files_struct *fsp, const void *data,
+ files_struct *fsp, const void *indata,
size_t n, off_t offset)
{
struct fio *fio = fruit_get_complete_fio(handle, fsp);
+ const void *data = indata;
+ char afpinfo_buf[AFP_INFO_SIZE];
AfpInfo *ai = NULL;
size_t nwritten;
int ret;
@@ -2681,7 +2688,7 @@ static ssize_t fruit_pwrite_meta_stream(vfs_handle_struct *handle,
fio->fake_fd = false;
}
- ai = afpinfo_unpack(talloc_tos(), data);
+ ai = afpinfo_unpack(talloc_tos(), data, fio->config->validate_afpinfo);
if (ai == NULL) {
return -1;
}
@@ -2713,6 +2720,21 @@ static ssize_t fruit_pwrite_meta_stream(vfs_handle_struct *handle,
return n;
}
+ if (!fio->config->validate_afpinfo) {
+ /*
+ * Ensure the buffer contains a valid header, so marshall
+ * the data from the afpinfo struck back into a buffer
+ * and write that instead of the possibly malformed data
+ * we got from the client.
+ */
+ nwritten = afpinfo_pack(ai, afpinfo_buf);
+ if (nwritten != AFP_INFO_SIZE) {
+ errno = EINVAL;
+ return -1;
+ }
+ data = afpinfo_buf;
+ }
+
nwritten = SMB_VFS_NEXT_PWRITE(handle, fsp, data, n, offset);
if (nwritten != n) {
return -1;
@@ -2725,13 +2747,17 @@ static ssize_t fruit_pwrite_meta_netatalk(vfs_handle_struct *handle,
files_struct *fsp, const void *data,
size_t n, off_t offset)
{
+ struct fruit_config_data *config = NULL;
struct adouble *ad = NULL;
AfpInfo *ai = NULL;
char *p = NULL;
int ret;
bool ok;
- ai = afpinfo_unpack(talloc_tos(), data);
+ SMB_VFS_HANDLE_GET_DATA(handle, config,
+ struct fruit_config_data, return -1);
+
+ ai = afpinfo_unpack(talloc_tos(), data, config->validate_afpinfo);
if (ai == NULL) {
return -1;
}
@@ -2811,10 +2837,12 @@ static ssize_t fruit_pwrite_meta(vfs_handle_struct *handle,
return -1;
}
- cmp = memcmp(data, "AFP", 3);
- if (cmp != 0) {
- errno = EINVAL;
- return -1;
+ if (fio->config->validate_afpinfo) {
+ cmp = memcmp(data, "AFP", 3);
+ if (cmp != 0) {
+ errno = EINVAL;
+ return -1;
+ }
}
if (n <= AFP_OFF_FinderInfo) {
diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
index 2877273b8e1..8141b7e59ad 100755
--- a/source3/selftest/tests.py
+++ b/source3/selftest/tests.py
@@ -1836,6 +1836,13 @@ plantestsuite("samba3.blackbox.force-user-unlink",
[os.path.join(samba3srcdir,
"script/tests/test_force_user_unlink.sh")])
+plansmbtorture4testsuite(
+ "vfs.fruit_validate_afpinfo", "fileserver",
+ '//$SERVER_IP/vfs_fruit -U$USERNAME%$PASSWORD --option=torture:validate_afpinfo=yes')
+plansmbtorture4testsuite(
+ "vfs.fruit_validate_afpinfo", "fileserver",
+ '//$SERVER_IP/vfs_fruit_zero_fileid -U$USERNAME%$PASSWORD --option=torture:validate_afpinfo=no')
+
def planclusteredmembertestsuite(tname, prefix):
'''Define a clustered test for the clusteredmember environment'''
diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c
index 99d63becbb4..b9cab0c5467 100644
--- a/source4/torture/vfs/fruit.c
+++ b/source4/torture/vfs/fruit.c
@@ -8768,3 +8768,72 @@ struct torture_suite *torture_vfs_fruit_unfruit(TALLOC_CTX *ctx)
return suite;
}
+
+/*
+ * Write an invalid AFP_AfpInfo stream header
+ */
+bool test_fruit_validate_afpinfo(struct torture_context *tctx,
+ struct smb2_tree *tree)
+{
+ bool expect_invalid_param = torture_setting_bool(tctx, "validate_afpinfo", true);
+ const char *fname = "test_fruit_validate_afpinfo";
+ const char *sname = "test_fruit_validate_afpinfo" AFPINFO_STREAM_NAME;
+ struct smb2_handle handle;
+ AfpInfo *afpinfo = NULL;
+ char *afpinfo_buf = NULL;
+ uint8_t valbuf[8];
+ NTSTATUS status;
+ bool ret = true;
+
+ torture_comment(tctx, "Checking create of AfpInfo stream\n");
+
+ smb2_util_unlink(tree, fname);
+
+ ret = torture_setup_file(tctx, tree, fname, false);
+ torture_assert_goto(tctx, ret == true, ret, done, "torture_setup_file failed");
+
+ afpinfo = torture_afpinfo_new(tctx);
+ torture_assert_not_null_goto(tctx, afpinfo, ret, done,
+ "torture_afpinfo_new failed\n");
+
+ memcpy(afpinfo->afpi_FinderInfo, "FOO BAR ", 8);
+
+ ret = torture_write_afpinfo(tree, tctx, tctx, fname, afpinfo);
+ torture_assert_goto(tctx, ret == true, ret, done,
+ "torture_write_afpinfo failed\n");
+
+ afpinfo_buf = talloc_zero_size(tctx, 60);
+ torture_assert_goto(tctx, afpinfo_buf != NULL, ret, done,
+ "torture_afpinfo_new failed");
+ memcpy(afpinfo_buf + 16, "FOO ", 4);
+
+ status = torture_smb2_testfile_access(
+ tree, sname, &handle, SEC_FILE_ALL);
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "smb2_create failed\n");
+
+ status = smb2_util_write(tree, handle, afpinfo_buf, 0, AFP_INFO_SIZE);
+ if (expect_invalid_param) {
+ torture_assert_ntstatus_equal_goto(
+ tctx, status, NT_STATUS_INVALID_PARAMETER, ret, done,
+ "write didn't fail as expected\n");
+ } else {
+ torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+ "smb2_util_write failed");
+ }
+
+ smb2_util_close(tree, handle);
+
+ /*
+ * Verify the server fixed the header
+ */
+ PUSH_BE_U32(valbuf, 0, AFP_Signature);
+ PUSH_BE_U32(valbuf + 4, 0, AFP_Version);
+ ret = check_stream(tree, __location__, tctx, tctx, fname,
+ AFPINFO_STREAM, 0, 60, 0, 8, (char *)valbuf);
+ torture_assert_goto(tctx, ret == true, ret, done, "check_stream failed");
+
+done:
+ smb2_util_unlink(tree, fname);
+ return ret;
+}
diff --git a/source4/torture/vfs/vfs.c b/source4/torture/vfs/vfs.c
index 69da13f6d28..3d402eeee0d 100644
--- a/source4/torture/vfs/vfs.c
+++ b/source4/torture/vfs/vfs.c
@@ -115,6 +115,7 @@ NTSTATUS torture_vfs_init(TALLOC_CTX *ctx)
torture_suite_add_suite(suite, torture_vfs_fruit_timemachine(suite));
torture_suite_add_suite(suite, torture_vfs_fruit_conversion(suite));
torture_suite_add_suite(suite, torture_vfs_fruit_unfruit(suite));
+ torture_suite_add_1smb2_test(suite, "fruit_validate_afpinfo", test_fruit_validate_afpinfo);
torture_register_suite(ctx, suite);
--
Samba Shared Repository
More information about the samba-cvs
mailing list