From 38b5c8c6632324f6376ac582ddeedbae1ad9250c Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Fri, 4 Oct 2024 19:01:19 +0200 Subject: [PATCH] smbd: remove "fruit:posix_rename" This option of the vfs_fruit VFS module that could be used to enable POSIX directory rename behaviour for OS X clients has been removed as it could result in severe problems for Windows clients. As a possible workaround it is possible to prevent creation of .DS_Store files (a Finder thingy to store directory view settings) on network mounts by running $ defaults write com.apple.desktopservices DSDontWriteNetworkStores true on the Mac. Signed-off-by: Ralph Boehme Reviewed-by: Volker Lendecke --- WHATSNEW.txt | 16 +++- docs-xml/manpages/vfs_fruit.8.xml | 11 --- source3/include/vfs.h | 4 +- source3/modules/vfs_fruit.c | 13 --- source3/smbd/dir.c | 3 +- source3/smbd/smb2_reply.c | 4 - source4/torture/vfs/fruit.c | 132 ------------------------------ 7 files changed, 17 insertions(+), 166 deletions(-) diff --git a/WHATSNEW.txt b/WHATSNEW.txt index a8933e08ef0..c9db9360169 100644 --- a/WHATSNEW.txt +++ b/WHATSNEW.txt @@ -23,6 +23,20 @@ REMOVED FEATURES The "nmbd proxy logon" feature was removed. This was used before Samba4 acquired a NBT server. +fruit:posix_rename +------------------ + +This option of the vfs_fruit VFS module that could be used to enable POSIX +directory rename behaviour for OS X clients has been removed as it could result +in severe problems for Windows clients. + +As a possible workaround it is possible to prevent creation of .DS_Store files +(a Finder thingy to store directory view settings) on network mounts by running + + $ defaults write com.apple.desktopservices DSDontWriteNetworkStores true + +on the Mac. + smb.conf changes ================ @@ -30,7 +44,7 @@ smb.conf changes Parameter Name Description Default -------------- ----------- ------- vfs mkdir use tmp name New Auto - + fruit:posix_rename Removed KNOWN ISSUES ============ diff --git a/docs-xml/manpages/vfs_fruit.8.xml b/docs-xml/manpages/vfs_fruit.8.xml index 61051f90873..9e27030b660 100644 --- a/docs-xml/manpages/vfs_fruit.8.xml +++ b/docs-xml/manpages/vfs_fruit.8.xml @@ -328,17 +328,6 @@ - - fruit:posix_rename = yes | no - - Whether to enable POSIX directory rename behaviour - for OS X clients. Without this, directories can't be - renamed if any client has any file inside it - (recursive!) open. - The default is yes. - - - readdir_attr:aapl_rsize = yes | no diff --git a/source3/include/vfs.h b/source3/include/vfs.h index 75e4d8f816d..1d4f78e3733 100644 --- a/source3/include/vfs.h +++ b/source3/include/vfs.h @@ -685,13 +685,11 @@ typedef struct files_struct { */ #define FSP_POSIX_FLAGS_OPEN 0x01 -#define FSP_POSIX_FLAGS_RENAME 0x02 #define FSP_POSIX_FLAGS_PATHNAMES 0x04 #define FSP_POSIX_FLAGS_ALL \ (FSP_POSIX_FLAGS_OPEN | \ - FSP_POSIX_FLAGS_PATHNAMES | \ - FSP_POSIX_FLAGS_RENAME) + FSP_POSIX_FLAGS_PATHNAMES) struct vuid_cache_entry { struct auth_session_info *session_info; diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c index f8b12c09594..d8ad4d37a4f 100644 --- a/source3/modules/vfs_fruit.c +++ b/source3/modules/vfs_fruit.c @@ -128,7 +128,6 @@ struct fruit_config_data { bool unix_info_enabled; bool copyfile_enabled; bool veto_appledouble; - bool posix_rename; bool aapl_zero_file_id; const char *model; bool time_machine; @@ -342,9 +341,6 @@ static int init_fruit_config(vfs_handle_struct *handle) config->use_copyfile = lp_parm_bool(-1, FRUIT_PARAM_TYPE_NAME, "copyfile", false); - config->posix_rename = lp_parm_bool( - SNUM(handle->conn), FRUIT_PARAM_TYPE_NAME, "posix_rename", true); - config->aapl_zero_file_id = lp_parm_bool(SNUM(handle->conn), FRUIT_PARAM_TYPE_NAME, "zero_file_id", true); @@ -4349,15 +4345,6 @@ static NTSTATUS fruit_create_file(vfs_handle_struct *handle, fsp = *result; - if (global_fruit_config.nego_aapl) { - if (config->posix_rename && fsp->fsp_flags.is_directory) { - /* - * Enable POSIX directory rename behaviour - */ - fsp->posix_flags |= FSP_POSIX_FLAGS_RENAME; - } - } - /* * If this is a plain open for existing files, opening an 0 * byte size resource fork MUST fail with diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c index 406db604f8e..86cb2a49f91 100644 --- a/source3/smbd/dir.c +++ b/source3/smbd/dir.c @@ -1540,8 +1540,7 @@ NTSTATUS can_delete_directory_fsp(files_struct *fsp) return status; } - if (!(fsp->posix_flags & FSP_POSIX_FLAGS_RENAME) && - lp_strict_rename(SNUM(conn)) && + if (lp_strict_rename(SNUM(conn)) && have_file_open_below(fsp->conn, fsp->fsp_name)) { return NT_STATUS_ACCESS_DENIED; diff --git a/source3/smbd/smb2_reply.c b/source3/smbd/smb2_reply.c index ab8b989c0a5..98377547bb3 100644 --- a/source3/smbd/smb2_reply.c +++ b/source3/smbd/smb2_reply.c @@ -1166,10 +1166,6 @@ static NTSTATUS can_rename(connection_struct *conn, files_struct *fsp, } if (S_ISDIR(fsp->fsp_name->st.st_ex_mode)) { - if (fsp->posix_flags & FSP_POSIX_FLAGS_RENAME) { - return NT_STATUS_OK; - } - /* If no pathnames are open below this directory, allow the rename. */ diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c index b9cab0c5467..c748326483a 100644 --- a/source4/torture/vfs/fruit.c +++ b/source4/torture/vfs/fruit.c @@ -3984,137 +3984,6 @@ done: return ret; } -/* Renaming a directory with open file, should work for OS X AAPL clients */ -static bool test_rename_dir_openfile(struct torture_context *torture, - struct smb2_tree *tree) -{ - bool ret = true; - NTSTATUS status; - union smb_open io; - union smb_close cl; - union smb_setfileinfo sinfo; - struct smb2_handle d1, h1; - const char *renamedir = BASEDIR "-new"; - bool server_is_osx = torture_setting_bool(torture, "osx", false); - - smb2_deltree(tree, BASEDIR); - smb2_util_rmdir(tree, BASEDIR); - smb2_deltree(tree, renamedir); - - ZERO_STRUCT(io.smb2); - io.generic.level = RAW_OPEN_SMB2; - io.smb2.in.create_flags = 0; - 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.alloc_size = 0; - io.smb2.in.create_disposition = NTCREATEX_DISP_CREATE; - io.smb2.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS; - io.smb2.in.security_flags = 0; - io.smb2.in.fname = BASEDIR; - - status = smb2_create(tree, torture, &(io.smb2)); - torture_assert_ntstatus_ok(torture, status, "smb2_create dir"); - d1 = io.smb2.out.file.handle; - - ZERO_STRUCT(io.smb2); - io.generic.level = RAW_OPEN_SMB2; - io.smb2.in.create_flags = 0; - io.smb2.in.desired_access = 0x0017019f; - io.smb2.in.create_options = NTCREATEX_OPTIONS_NON_DIRECTORY_FILE; - io.smb2.in.file_attributes = FILE_ATTRIBUTE_NORMAL; - io.smb2.in.share_access = 0; - io.smb2.in.alloc_size = 0; - io.smb2.in.create_disposition = NTCREATEX_DISP_CREATE; - io.smb2.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS; - io.smb2.in.security_flags = 0; - io.smb2.in.fname = BASEDIR "\\file.txt"; - - status = smb2_create(tree, torture, &(io.smb2)); - torture_assert_ntstatus_ok(torture, status, "smb2_create file"); - h1 = io.smb2.out.file.handle; - - if (!server_is_osx) { - torture_comment(torture, "Renaming directory without AAPL, must fail\n"); - - ZERO_STRUCT(sinfo); - sinfo.rename_information.level = RAW_SFILEINFO_RENAME_INFORMATION; - sinfo.rename_information.in.file.handle = d1; - sinfo.rename_information.in.overwrite = 0; - sinfo.rename_information.in.root_fid = 0; - sinfo.rename_information.in.new_name = renamedir; - status = smb2_setinfo_file(tree, &sinfo); - - torture_assert_ntstatus_equal(torture, status, - NT_STATUS_ACCESS_DENIED, - "smb2_setinfo_file"); - } - - status = smb2_util_close(tree, d1); - torture_assert_ntstatus_ok(torture, status, "smb2_util_close\n"); - ZERO_STRUCT(d1); - - torture_comment(torture, "Enabling AAPL\n"); - - ret = enable_aapl(torture, tree); - torture_assert(torture, ret == true, "enable_aapl failed"); - - torture_comment(torture, "Renaming directory with AAPL\n"); - - ZERO_STRUCT(io.smb2); - 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.alloc_size = 0; - io.smb2.in.create_disposition = NTCREATEX_DISP_OPEN; - io.smb2.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS; - io.smb2.in.security_flags = 0; - io.smb2.in.fname = BASEDIR; - - status = smb2_create(tree, torture, &(io.smb2)); - torture_assert_ntstatus_ok(torture, status, "smb2_create dir"); - d1 = io.smb2.out.file.handle; - - ZERO_STRUCT(sinfo); - sinfo.rename_information.level = RAW_SFILEINFO_RENAME_INFORMATION; - sinfo.rename_information.in.file.handle = d1; - sinfo.rename_information.in.overwrite = 0; - sinfo.rename_information.in.root_fid = 0; - sinfo.rename_information.in.new_name = renamedir; - - status = smb2_setinfo_file(tree, &sinfo); - torture_assert_ntstatus_ok(torture, status, "smb2_setinfo_file"); - - ZERO_STRUCT(cl.smb2); - cl.smb2.level = RAW_CLOSE_SMB2; - cl.smb2.in.file.handle = d1; - status = smb2_close(tree, &(cl.smb2)); - torture_assert_ntstatus_ok(torture, status, "smb2_close"); - ZERO_STRUCT(d1); - - cl.smb2.in.file.handle = h1; - status = smb2_close(tree, &(cl.smb2)); - torture_assert_ntstatus_ok(torture, status, "smb2_close"); - ZERO_STRUCT(h1); - - torture_comment(torture, "Cleaning up\n"); - - if (h1.data[0] || h1.data[1]) { - ZERO_STRUCT(cl.smb2); - cl.smb2.level = RAW_CLOSE_SMB2; - cl.smb2.in.file.handle = h1; - status = smb2_close(tree, &(cl.smb2)); - } - - smb2_util_unlink(tree, BASEDIR "\\file.txt"); - smb2_util_unlink(tree, BASEDIR "-new\\file.txt"); - smb2_deltree(tree, renamedir); - smb2_deltree(tree, BASEDIR); - return ret; -} - static bool test_afpinfo_enoent(struct torture_context *tctx, struct smb2_tree *tree) { @@ -7994,7 +7863,6 @@ struct torture_suite *torture_vfs_fruit(TALLOC_CTX *ctx) torture_suite_add_1smb2_test(suite, "truncate resource fork to 0 bytes", test_rfork_truncate); torture_suite_add_1smb2_test(suite, "opening and creating resource fork", test_rfork_create); torture_suite_add_1smb2_test(suite, "fsync_resource_fork", test_rfork_fsync); - torture_suite_add_1smb2_test(suite, "rename_dir_openfile", test_rename_dir_openfile); torture_suite_add_1smb2_test(suite, "File without AFP_AfpInfo", test_afpinfo_enoent); torture_suite_add_1smb2_test(suite, "create delete-on-close AFP_AfpInfo", test_create_delete_on_close); torture_suite_add_1smb2_test(suite, "setinfo delete-on-close AFP_AfpInfo", test_setinfo_delete_on_close);