Reported by Andrew Walker <awalker@ixsystems.com> On Tue, Apr 10, 2018 at 1:49 PM, Jeremy Allison <jra@samba.org> wrote: > On Tue, Apr 10, 2018 at 05:58:58AM -0400, Andrew Walker wrote: > > > > Hi Jeremy, > > > > Sorry for delay in responding. The issue was observed on FreeBSD, Samba > Version > > 4.7.6. > > > > Config details: > > > > ### smb.conf ### > > [global] > > idmap config * : range = 3000-7999 > > idmap config * : backend = tdb > > ea support = Yes > > store dos attributes = Yes > > > > > > [SHARE] > > map readonly = no > > path = "/usr/home/SAMBA" > > read only = No > > vfs objects = zfsacl streams_xattr > > > > ### lsextattr output ### > > root@DC00:/usr/home/SAMBA # lsextattr user Test_Dir/ > > Test_Dir/ DOSATTRIB DosStream.Cats:$DATA > > > > _both_ DOSATTRIB and DosStream are required to reproduce bug. > > > > b64encoded DOSATTRIB > > MHgxMgAAAwADAAAAEQAAABIAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAJwlXM2f > zNMBAAAAAAAAAAA= > > > > b64encoded DosStream.Cats:$DATA > > QUZQAAAAAQAAAAAAgAAAAFBMQVBsdGFwBBAAAAAAAAAAAAAAAAAAAAAAAAAA > AAAAAAAAAAAAAAAAAAAA > > > > ### client ### > > windows 7 enterprise, service pack 1, all patches applied > > > > ### workflow ### > > 1) create directory on server. > > 2) write ADS to directory. > > 3) copy directory to client desktop (note dir must not exist on client). > > Problem is I can't reproduce this locally on Linux. I believe you, > and the fix seems obvious and non-intrusive, but I need > a reproducer first. Here's what I have: > > smb.conf share definition: > > [tmp2] > path = /tmp/testdir > store dos attributes = yes > ea support = yes > vfs objects = streams_xattr > read only = no > guest ok = yes > > bin/smbclient //127.0.0.1/tmp2 -Uuser%pass > smb: \> mkdir foo > smb: \> put look foo:bar > putting file look as \foo:bar (0.0 kb/s) (average 0.0 kb/s) > smb: \> allinfo foo:bar > altname: foo > create_time: Tue Apr 10 10:42:35 AM 2018 PDT > access_time: Tue Apr 10 10:42:35 AM 2018 PDT > write_time: Tue Apr 10 10:42:35 AM 2018 PDT > change_time: Tue Apr 10 10:42:35 AM 2018 PDT > attributes: A (20) > NT_STATUS_INVALID_PARAMETER getting streams for \foo:bar > > (the NT_STATUS_INVALID_PARAMETER is expected as you > can't enumerate streams on a stream handle). > > Note this is returning only A, not DA which > is what you are seeing. > > If I look at the xattrs directly I do see the xattrs: > > $ getfattr /tmp/testdir/foo > getfattr: Removing leading '/' from absolute path names > # file: tmp/testdir/foo > user.DOSATTRIB > user.DosStream.bar:$DATA > > Can you reproduce using smbclient somehow so I can > figure out what is different between your system > and mine ? > > Thanks, > > Jeremy. > Great! This is turning out to be more interesting than I thought :) I had same result as you when I first generated the ADS and output allinfo: smb: \> allinfo test35:ADS altname: test35 create_time: Tue Apr 10 13:45:00 2018 EDT access_time: Tue Apr 10 13:45:00 2018 EDT write_time: Tue Apr 10 13:45:00 2018 EDT change_time: Tue Apr 10 13:45:00 2018 EDT attributes: A (20) Then I removed the "archive" bit from the directory. setmode test35 -a Then I repeat the test smb: \> allinfo test35:ADS altname: test35 create_time: Tue Apr 10 13:45:00 2018 EDT access_time: Tue Apr 10 13:45:00 2018 EDT write_time: Tue Apr 10 13:45:00 2018 EDT change_time: Tue Apr 10 13:45:00 2018 EDT attributes: D (10) NT_STATUS_INVALID_PARAMETER getting streams for \test35:ADS The stream is now reported as a directory. It looks like FILE_ATTRIBUTE_DIRECTORY gets written to the dir's DOSATTRIB whenever we modify it. Patch to follow.
Created attachment 14124 [details] git-am fix for 4.8.next, 4.7.next. Cherry-picked from master.
I discovered this by running the: mkdir foo put "localfile" foo:bar setmode foo -a allinfo foo:bar tests against the streams_depot as well as streams_xattr modules. streams_depot doesn't have the bug in not removing the S_ISDIR bit, but still displayed the 'D' attribute in error. Digging deeper and reading the MS-FSA document gave me the fix. Samba 4.7.x and above use a new VFS function get_dos_attributes_fn() to return DOS attributes, and both streams_depot and streams_xattr don't implment these, meaning we fallback to the default call that gets the user.DOSATTRIB from the base file. MS-FSA states that file streams do *NOT* have their own DOS attribute metadata, this only exists on the file object. However, the client error discovered by Andrew shows that Windows filters out the 'FILE_ATTRIBUTE_DIRECTORY' attribute when base file attributes are returned on non-default name stream objects stored on a directory (as far as I know this isn't mentioned in the MS-docs, I should probably raise a dochelp issue on that). So I could have added the [f]get_dos_attributes() functions to our current streams modules which then filters the returned attributes to return the base file attribute minus the FILE_ATTRIBUTE_DIRECTORY attribute, and anyone with their own out-of-tree modules implementing streams would need to implement the identical code. This is needless code duplication IMHO. As logic must be identical for all stream-implementing modules, it seemed clearer to me to implement this in the calling code, dos_mode(), which is the central function for returning all dos attributes. This is close to Andrew's original fix but in a more central place, plus we have to ensure it's a non-default NTFS stream name (and not a POSIX name of course). Included are the regression tests that allowed me to find this and will make sure we don't break again in future :-).
Added note explaining the history of the decisions in the fix.
Reassigning to Karolin for inclusion in 4.7 and 4.8.
Pushed to autobuild-v4-[8,7]-test.
(In reply to Karolin Seeger from comment #5) Pushed to both branches. Closing out bug report. Thanks"