Scenario: 1. Create a share, set "hide unreadable = Yes", "vfs objects = acl_xattr": [v1] path = /export/v1 read only = no browseable = Yes hide unreadable = Yes vfs objects = acl_xattr 2. Create a director and file inside the directory: V:\>dir Test Volume in drive V is v1 Volume Serial Number is C964-5443 Directory of V:\Test 03/20/2022 05:08 AM <DIR> . 03/20/2022 05:02 AM <DIR> .. 03/20/2022 05:08 AM 0 file.txt 1 File(s) 0 bytes 2 Dir(s) 107,121,086,464 bytes free 3. On file, disable inheritance, remove all inherited permissions. File becomes unreadable and is not shown any more: V:\>dir Test Volume in drive V is v1 Volume Serial Number is C964-5443 Directory of V:\Test 03/20/2022 05:08 AM <DIR> . 03/20/2022 05:02 AM <DIR> .. 0 File(s) 0 bytes 2 Dir(s) 107,121,086,464 bytes free 4. Try to delete directory. rmdir doesn't fail, however directory was not really deleted: V:\>rmdir Test V:\>dir Test Volume in drive V is v1 Volume Serial Number is C964-5443 Directory of V:\Test 03/20/2022 05:08 AM <DIR> . 03/20/2022 05:02 AM <DIR> .. 0 File(s) 0 bytes 2 Dir(s) 107,121,086,464 bytes free 5. If reset the flag "hide unreadable", rmdir fails, as expected: V:\>rmdir Test The directory is not empty. According to tcpdump, if "hide unreadable" is No, request SetInfo(0x11) with flag DeleteOnClose fails with STATUS_DIRECTORY_NOT_EMPTY. But if "hide unreadable" is Yes, SetInfo(0x11) succeeds, and only then Close(0x6) fails with STATUS_DIRECTORY_NOT_EMPTY. Looks like Windows client ignores failure of Close(0x6). I think the problem is in can_delete_directory_fsp() -> is_visible_fsp() - it honors "hide unreadable" option, and as a result can_delete_directory_fsp() returns NT_STATUS_OK.
Yes, Windows clients ignore errors on close (it's a big pain). They only look at the return from the set-delete-on-close operation.
The man page for smb.conf says of "hide unreadable": "This parameter prevents clients from seeing the existence of files that cannot be read." If we deny deletion of a directory containing files hidden by "hide unreadable" via can_delete_directory_fsp() -> is_visible_fsp(), then the client knows there are files in the directory. It's not a security issue though, as the client doesn't get any information about the files within. We could fix this by adding a check for lp_delete_veto_files(SNUM(conn)) inside can_delete_directory_fsp(), as that is the check that the rmdir_internals() code uses to decide if it should delete files that have been hidden from the client when deleting the containing directory. Something like: --------------------------------------------------------------------- diff --git a/source3/smbd/dir.c b/source3/smbd/dir.c index 6180eba2fac..059de584d17 100644 --- a/source3/smbd/dir.c +++ b/source3/smbd/dir.c @@ -2029,11 +2029,17 @@ NTSTATUS can_delete_directory_fsp(files_struct *fsp) } if (!is_visible_fsp(direntry_fname->fsp)) { - TALLOC_FREE(talloced); - TALLOC_FREE(fullname); - TALLOC_FREE(smb_dname_full); - TALLOC_FREE(direntry_fname); - continue; + /* + * Hidden file. + * Allow if "delete veto files = yes" + */ + if (lp_delete_veto_files(SNUM(conn))) { + TALLOC_FREE(talloced); + TALLOC_FREE(fullname); + TALLOC_FREE(smb_dname_full); + TALLOC_FREEcan_delete_directory_fsp()(direntry_fname); + continue; + } } TALLOC_FREE(talloced); --------------------------------------------------------------------- Actually, I think this is the correct fix, as this is exactly what we do in the same function can_delete_directory_fsp() if we run into a dangling symlink.
Created attachment 17229 [details] git-am fix for master. Patch I'm putting into CI so I don't lose it.
Created attachment 17230 [details] git-am fix for master. Fixed added nl.
This bug was referenced in samba master: 5fe341d2d67afb7088edcb772b058c747ab341b1 80503b46e7238d0796f5cc9eb6104958c3b3fcc7
Created attachment 17235 [details] git-am fix for 4.16.next, 4.15.next. Cherry-picked from master. Applies cleanly to 4.16.next, 4.15.next.
Comment on attachment 17235 [details] git-am fix for 4.16.next, 4.15.next. lgtm
reassign to Jule for inclusion in 4.15/4.16
Pushed to autobuild-v4-{16,15}-test.
This bug was referenced in samba v4-16-test: 5242660aa14a66d5cdecf2b2916b6c44999f58ab 7676cb51450c7559806fc20158dcd73925ccc0b9
This bug was referenced in samba v4-15-test: bbd8bdc14c8e6a76d93b6abde57d55b6a0e0dac9 f2e124e423c9dbe1f140320b88016be6f6ed84b1
Closing out bug report. Thanks!
This bug was referenced in samba v4-15-stable (Release samba-4.15.7): bbd8bdc14c8e6a76d93b6abde57d55b6a0e0dac9 f2e124e423c9dbe1f140320b88016be6f6ed84b1
This bug was referenced in samba v4-16-stable (Release samba-4.16.1): 5242660aa14a66d5cdecf2b2916b6c44999f58ab 7676cb51450c7559806fc20158dcd73925ccc0b9