Bug 11249 - Mangled names do not work with acl_xattr
Mangled names do not work with acl_xattr
Status: RESOLVED FIXED
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services
4.2.1
All All
: P5 normal
: ---
Assigned To: Karolin Seeger
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2015-05-01 18:55 UTC by Justin Maggard
Modified: 2015-05-21 08:05 UTC (History)
1 user (show)

See Also:


Attachments
git-am patchset for master. (10.50 KB, patch)
2015-05-01 21:10 UTC, Jeremy Allison
no flags Details
Full git-am patchset for master. (16.36 KB, patch)
2015-05-02 04:12 UTC, Jeremy Allison
no flags Details
Reviewed fix that is going into master. (16.37 KB, patch)
2015-05-04 20:54 UTC, Jeremy Allison
no flags Details
Updated version with the vfs_fake_acl fix. (16.39 KB, patch)
2015-05-05 03:50 UTC, Jeremy Allison
no flags Details
git-am fix for 4.2.next. (16.25 KB, patch)
2015-05-05 19:43 UTC, Jeremy Allison
slow: review+
Details
git-am fix for 4.1.next. (12.74 KB, patch)
2015-05-05 19:44 UTC, Jeremy Allison
slow: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Justin Maggard 2015-05-01 18:55:53 UTC
Trying to read a file whose name contains, for example, a colon (:) with mangled names enabled returns NT_STATUS_OBJECT_NAME_NOT_FOUND when acl_xattr is in the vfs objects list.  This is reproducible using both Windows 7/8 and smbclient.

To reproduce:

On a machine running Samba, in a share folder,
$ touch "Colon: A filename with a colon.txt"
$ smbclient //localhost/share
Enter jmaggard's password: 
Domain=[WORKGROUP] OS=[Windows 6.1] Server=[Samba 4.2.1]
smb: \> dir
  .                                   D        0  Fri May  1 11:52:57 2015
  ..                                  D        0  Tue Apr  7 18:08:21 2015
  C94IDU~S.TXT                        N        0  Fri May  1 11:12:52 2015

		288184384 blocks of size 1024. 151662596 blocks available
smb: \> get C94IDU~S.TXT 
NT_STATUS_OBJECT_NAME_NOT_FOUND opening remote file \C94IDU~S.TXT
smb: \> 

The same test works fine without acl_xattr.
Comment 1 Jeremy Allison 2015-05-01 19:06:25 UTC
Oh, I see the problem. Thanks - need to unmangle the name before calling into the xattr VFS object...
Comment 2 Jeremy Allison 2015-05-01 20:27:47 UTC
OK, got a fix for this in testing...

Will upload shortly.
Comment 3 Jeremy Allison 2015-05-01 21:10:04 UTC
Created attachment 11010 [details]
git-am patchset for master.

Justin please check this. Works for me.

NB. This is a temp patch. The real patch will have a regression test case (I'm writing that now) plus the removal of the functions in source3/smbd/vfs.c will not be in the 4.2 and 4.1 patchset, only master (in case people have out-of-tree VFS modules that depend on these functions).

Let me know if it fixes it for you.

Cheers,

Jeremy.
Comment 4 Justin Maggard 2015-05-01 21:57:28 UTC
It works!  Thank you Jeremy.
Comment 5 Jeremy Allison 2015-05-02 00:12:34 UTC
Ah, I've figured out the problem with the make test regression.

Updated patch will follow - there are a few other places where we're splitting the unmangled name post passing into the EA and ACL calls (which are only called POST-streams path checks).
Comment 6 Jeremy Allison 2015-05-02 04:12:09 UTC
Created attachment 11011 [details]
Full git-am patchset for master.

Contains fixes for problems with EA and ACL pathnames when streams_depot is being used. Also contains regression test.

Justin and you confirm this works for you, then I'll get into master and create the back-ports for 4.2.next and 4.1.next (which will be the same patch but minus the function removal, in case of out-of-tree VFS modules).
Comment 7 Jeremy Allison 2015-05-02 04:12:59 UTC
s/Justin and you confirm this works for you/Justin *can* you confirm this works for you/

:-).
Comment 8 Justin Maggard 2015-05-04 18:36:29 UTC
I have tested the latest patchset against Windows 7/8 and smbclient, and all are working now. :-)
Comment 9 Jeremy Allison 2015-05-04 20:54:11 UTC
Created attachment 11018 [details]
Reviewed fix that is going into master.
Comment 10 Jeremy Allison 2015-05-05 03:50:36 UTC
Created attachment 11019 [details]
Updated version with the vfs_fake_acl fix.
Comment 11 Jeremy Allison 2015-05-05 03:51:21 UTC
Comment on attachment 11019 [details]
Updated version with the vfs_fake_acl fix.

Andreas,

Here is the updated version of the patchset that now passes
the raw.streams tests. Sorry for the error, it turned out
to be a code path in the vfs_fake_acl module that was passing
stream pathnames to XATTR calls (what I've been removing in
the other code :-) and once the XATTR code was changed to stop
it from filtering out the stream name it caused OBJECT_NOT_FOUND
errors. vfs_fake_acl is not used in production, only in the
test rig so that's why I missed it :-(.

I've added the 'BUG: ' in the bugzilla URLs, and
the only other change from the previous patchset was the
addition of the patch:

-----------------------------------------------------
s3: smbd: VFS: fake_acl module called get_full_smb_filename()
with a stream path, then used the result to call XATTR functions directly.

Ensure when pulling XATTR values, we don't allow a stream filename.
-----------------------------------------------------

which fixed the streams tests.
Comment 12 Jeremy Allison 2015-05-05 19:43:18 UTC
Created attachment 11021 [details]
git-am fix for 4.2.next.

Backport from master of the fix that went in - minus the hunk that removes the vfs_stat_smb_fname() and vfs_lstat_smb_fname() calls to ensure ABI stability.
Comment 13 Jeremy Allison 2015-05-05 19:44:28 UTC
Created attachment 11022 [details]
git-am fix for 4.1.next.

Backport from master of the fix minus the hunk that removes the vfs_stat_smb_fname() and vfs_lstat_smb_fname() calls to ensure ABI stability and minus the regression test (4.1 test suite is less complete).
Comment 14 Jeremy Allison 2015-05-05 20:42:56 UTC
Re-assigning to Karolin for inclusion in 4.2.next, 4.1.next.
Comment 15 Karolin Seeger 2015-05-20 11:32:45 UTC
Pushed to autobuild-v4-[1|2]-test.
Comment 16 Karolin Seeger 2015-05-21 08:05:49 UTC
(In reply to Karolin Seeger from comment #15)
Pushed to both branches.
Closing out bug report.

Thanks!