Bug 10587 - Opening directories on SLES's cifsd and Apple's smbx succeeds
Summary: Opening directories on SLES's cifsd and Apple's smbx succeeds
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: libsmbclient (show other bugs)
Version: 4.1.6
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-05-04 20:51 UTC by Ross Lagerwall
Modified: 2021-02-11 14:07 UTC (History)
2 users (show)

See Also:


Attachments
Trace when running smbclient (7.29 KB, application/octet-stream)
2014-05-05 21:40 UTC, Ross Lagerwall
no flags Details
Test patch for 4.1.x (4.76 KB, patch)
2014-05-06 00:01 UTC, Jeremy Allison
no flags Details
Trace when running smbclient against smbx (8.72 KB, application/octet-stream)
2014-05-06 07:59 UTC, Ross Lagerwall
no flags Details
Trace when using smb2 (8.59 KB, application/octet-stream)
2014-05-06 19:56 UTC, Ross Lagerwall
no flags Details
Trace when using smb3 (9.59 KB, application/octet-stream)
2014-05-06 19:57 UTC, Ross Lagerwall
no flags Details
Trace when using smb2 (forced) (2.15 KB, application/octet-stream)
2014-05-07 08:01 UTC, Ross Lagerwall
no flags Details
Trace when using smbclient4 and smb2 (forced) (12.88 KB, application/octet-stream)
2014-05-07 08:02 UTC, Ross Lagerwall
no flags Details
git-am fix for master (5.98 KB, patch)
2014-05-08 21:08 UTC, Jeremy Allison
no flags Details
git-am fix for master. (57.70 KB, patch)
2014-05-09 04:41 UTC, Jeremy Allison
jra: review? (metze)
Details
Patches for v4-1-test (58.21 KB, patch)
2014-05-10 08:27 UTC, Stefan Metzmacher
jra: review+
Details
Fixed patch for 4.1.next (58.76 KB, patch)
2014-05-28 19:11 UTC, Jeremy Allison
jra: review? (ddiss)
vl: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ross Lagerwall 2014-05-04 20:51:00 UTC
SLES's cifsd and Apple's smbx do not correctly handle FILE_NON_DIRECTORY_FILE which prevents recursive copies in gvfs from working correctly [1] since GVFS tries to open the directory, expecting ENOTDIR, but it suceeds and appears as a zero byte file.

This can be trivially reproduced by executing "get some_dir" in smbclient against one of the broken servers. Instead of getting NT_STATUS_FILE_IS_A_DIRECTORY, you get a zero byte file.

Would it be possible for a workaround to be put in libsmbclient if these buggy servers are detected? I could implement a racy workaround in gvfs by stat()ing before every open but AFAIK libsmbclient doesn't expose the type of the server so this would have to be done for every open against every type of server...

Thanks!

[1] https://bugzilla.gnome.org/show_bug.cgi?id=599354
Comment 1 Jeremy Allison 2014-05-05 17:06:36 UTC
Can you get me a trace on this ? The workaround would be to detect the open coming back with an attribute FILE_DIRECTORY, and then having the libsmbclient close the returned handle and return -1,ENOTDIR in that that. But I really need to see the wire traces first.

Thanks,

Jeremy.
Comment 2 Ross Lagerwall 2014-05-05 21:40:42 UTC
Created attachment 9903 [details]
Trace when running smbclient

Attached is the trace running the following commands against a SLES 11 SP2 cifsd server:
"""
[ross@twerp ~]$ smbclient -U ubuntu //192.168.0.1/myvol
Enter ubuntu's password: 
Domain=[WORKGROUP] OS=[SUSE LINUX 10.1] Server=[SUSE LINUX 10.1]
smb: \> cd ubuntu
smb: \ubuntu\> ls
  .                                   D        0  Sun Apr 13 13:55:37 2014
  ..                                  D        0  Sun Apr 13 13:49:52 2014
  poo                                DA        0  Sun Apr 13 13:55:35 2014
  test                               DA        0  Sun Apr 13 13:53:20 2014

		48310 blocks of size 4096. 48167 blocks available
smb: \ubuntu\> get test
getting file \ubuntu\test of size 0 as test (0.0 KiloBytes/sec) (average 0.0 KiloBytes/sec)
smb: \ubuntu\> exit
"""

The trace was captured with: sudo tcpdump -i ens3 -s 0 -p -n -w smbclient.trace

Thanks for looking into this.
Comment 3 Jeremy Allison 2014-05-06 00:01:01 UTC
Created attachment 9904 [details]
Test patch for 4.1.x

Can you test this fix to see if it addresses the problem ?

Thanks,

Jeremy.
Comment 4 Jeremy Allison 2014-05-06 04:09:31 UTC
Oh yeah, can you also get me a trace against Apple's server too, as I thought that server only supported SMB2, not SMB1 - so I need to know if I need to put the same fix into the SMB2 client codepath as well as the SMB1 client codepath.

Jeremy.
Comment 5 Ross Lagerwall 2014-05-06 07:58:40 UTC
I tested the patch and it worked successfully against cifsd and smbx. Cool!

Nevertheless, I captured a trace when running unpatched samba against smbx:
"""
[liveuser@localhost ~]$ smbclient -U john //192.168.0.1/john
Enter john's password: 
Domain=[JOHNS-MAC-PRO] OS=[Darwin] Server=[@(#)PROGRAM:smbd  PROJECT:smbx-136.18]
smb: \> ls
  .                                  DA        0  Thu Apr  3 18:35:27 2014
  ..                                 DA        0  Thu Apr  3 18:35:27 2014
  .bash_history                      AH      860  Sun Apr 13 13:25:19 2014
  .CFUserTextEncoding                AH        3  Thu Apr  3 17:39:15 2014
  .DS_Store                          AH    15364  Tue May  6 03:42:53 2014
  .Trash                            DAH        0  Fri Apr  4 16:15:20 2014
  Desktop                            DA        0  Fri Apr  4 16:22:33 2014
  Documents                          DA        0  Fri Apr  4 16:42:43 2014
  Downloads                          DA        0  Tue May  6 03:41:59 2014
  Library                           DAH        0  Thu Apr  3 18:30:21 2014
  Movies                             DA        0  Thu Apr  3 17:57:58 2014
  Music                              DA        0  Thu Apr  3 17:39:15 2014
  Pictures                           DA        0  Thu Apr  3 17:39:15 2014
  Public                             DA        0  Thu Apr  3 17:39:15 2014

		60783 blocks of size 524288. 29155 blocks available
smb: \> get Library
getting file \Library of size 0 as Library (0.0 KiloBytes/sec) (average 0.0 KiloBytes/sec)
smb: \> exit
[liveuser@localhost ~]$ stat Library
  File: ‘Library’
  Size: 0         	Blocks: 0          IO Block: 4096   regular empty file
Device: fd00h/64768d	Inode: 45008       Links: 1
Access: (0644/-rw-r--r--)  Uid: ( 1000/liveuser)   Gid: ( 1000/liveuser)
Context: unconfined_u:object_r:user_home_t:s0
Access: 2014-05-06 03:51:02.772665541 -0400
Modify: 2014-05-06 03:51:02.772665541 -0400
Change: 2014-05-06 03:51:02.772665541 -0400
 Birth: -
"""

Captured with: sudo tcpdump -i p2p1 -s 0 -p -n -w smbclient-apple.trace
Comment 6 Ross Lagerwall 2014-05-06 07:59:14 UTC
Created attachment 9906 [details]
Trace when running smbclient against smbx
Comment 7 Jeremy Allison 2014-05-06 17:40:41 UTC
Oh interesting, I didn't think the Apple server supported SMB1. Oh well, I'm guessing that their SMB2 server does the right thing.

I'll see if I can get these fixes into master.

Jeremy.
Comment 8 Jeremy Allison 2014-05-06 18:20:08 UTC
Actually, can you check if their SMB2 server does the right thing by connecting to it using smbclient -mSMB3 (or -mSMB2 if it doesn't support SMB3) and seeing if you can open a directory in the same way ?

Thanks,

Jeremy.
Comment 9 Ross Lagerwall 2014-05-06 19:56:19 UTC
I tested with -mSMB2 and -mSMB3 and both gave the same result as not specifying the protocol, i.e. successfully retrieving the directory as a file.

I have attached traces for each of those attempts.

I then tried with the patch and -mSMB2 and -mSMB3 and and it returned NT_STATUS_FILE_IS_A_DIRECTORY for both.
Comment 10 Ross Lagerwall 2014-05-06 19:56:50 UTC
Created attachment 9908 [details]
Trace when using smb2
Comment 11 Ross Lagerwall 2014-05-06 19:57:05 UTC
Created attachment 9909 [details]
Trace when using smb3
Comment 12 Jeremy Allison 2014-05-06 21:39:57 UTC
Ah, I took a look - the server is still selecting SMB1, that's why it's still working (using the patch path).

Try using: -mSMB2 and the add "client min protocol = SMB2" in your smb.conf to force the client to only offer SMB2. See if the server accepts..
Comment 13 Ross Lagerwall 2014-05-07 08:01:13 UTC
No, the server did not accept it, nor SMB3. Note that this was testing against OS X 10.8.2, I don't know if anything would have changed with other versions. The wire trace is attached.

"""
[liveuser@localhost ~]$ smbclient -mSMB2 -U john //192.168.1.72/john
Enter john's password: 
protocol negotiation failed: NT_STATUS_IO_TIMEOUT
"""

For completeness, I tried with smbclient4 and SMB2 (what's the difference?). This allowed connecting but didn't work to get files or directories. Wire trace attached.

"""
[liveuser@localhost ~]$ smbclient4 -mSMB2 -U john //192.168.1.72/john
Password for [MYGROUP\john]:
smb: \> ls
  .                                  DA        0  Wed May  7 03:39:03 2014
  ..                                 DA        0  Wed May  7 03:39:03 2014
  .bash_history                      AH      900  Wed May  7 03:39:11 2014
  .CFUserTextEncoding                AH        3  Thu Apr  3 17:39:15 2014
  .DS_Store                          AH    15364  Tue May  6 03:42:53 2014
  .Trash                            DAH        0  Fri Apr  4 16:15:20 2014
  .viminfo                           AH      710  Wed May  7 03:39:03 2014
  Desktop                            DA        0  Fri Apr  4 16:22:33 2014
  Documents                          DA        0  Fri Apr  4 16:42:43 2014
  Downloads                          DA        0  Tue May  6 03:41:59 2014
  Library                           DAH        0  Thu Apr  3 18:30:21 2014
  Movies                             DA        0  Thu Apr  3 17:57:58 2014
  Music                              DA        0  Thu Apr  3 17:39:15 2014
  Pictures                           DA        0  Thu Apr  3 17:39:15 2014
  Public                             DA        0  Thu Apr  3 17:39:15 2014

		7780342 blocks of size 4096. 3726627 blocks available
smb: \> get Library
NT_STATUS_NOT_SUPPORTED opening remote file \Library
smb: \> cd Desktop
smb: \Desktop\> ls
  .                                  DA        0  Fri Apr  4 16:22:33 2014
  ..                                 DA        0  Wed May  7 03:39:03 2014
  .DS_Store                          AH    12292  Wed May  7 03:40:06 2014
  .localized                         AH        0  Thu Apr  3 17:39:15 2014
  d1                                 DA        0  Fri Apr  4 16:07:32 2014
  dude                               DA        0  Fri Apr  4 16:42:11 2014
  TotalFinder-1-1.5.22ddd copy.dmg      A 10061824  Thu Apr  3 18:39:33 2014
  TotalFinder-1-1.5.22ddd.dmg         A 10061824  Thu Apr  3 18:39:33 2014

		7780342 blocks of size 4096. 3726626 blocks available
smb: \Desktop\> get TotalFinder-1-1.5.22ddd.dmg
NT_STATUS_NOT_SUPPORTED opening remote file \Desktop\TotalFinder-1-1.5.22ddd.dmg
smb: \Desktop\> exit
"""
Comment 14 Ross Lagerwall 2014-05-07 08:01:44 UTC
Created attachment 9914 [details]
Trace when using smb2 (forced)
Comment 15 Ross Lagerwall 2014-05-07 08:02:03 UTC
Created attachment 9915 [details]
Trace when using smbclient4 and smb2 (forced)
Comment 16 Jeremy Allison 2014-05-08 21:08:50 UTC
Created attachment 9928 [details]
git-am fix for master

After some push back from metze about the right layer to fix this, here is an updated patch that should work identically to the original, but moves the fix up one later to cli_open().

As cli_open() has no asynchronous variants, we can treat it as a special case.

This version leaves the behavior of the synchronous and asynchronous versions of cli_ntcreate() and cli_nttrans_create() unmodified.

If you could test this out please I'd appreciate it !

Thanks,

Jeremy.
Comment 17 Jeremy Allison 2014-05-08 21:09:35 UTC
(In reply to comment #16)
> Created attachment 9928 [details]
> git-am fix for master
> 
> After some push back from metze about the right layer to fix this, here is an
> updated patch that should work identically to the original, but moves the fix
> up one later to cli_open().

Oh yeah, in addition it also fixes the bug if it happens with any SMB2 servers too :-).
Comment 18 Jeremy Allison 2014-05-09 04:41:12 UTC
Created attachment 9929 [details]
git-am fix for master.

All right Metze, here's the goddam rewrite
you insisted upon :-).

Add a return parameter of struct smb_create_returns *cr to
cli_ntcreate()
cli_ntcreate_recv()
cli_nttrans_create()
cli_nttrans_create_recv()

and fixes everything up... It's now a 4-patch set.
Comment 19 Ross Lagerwall 2014-05-10 07:11:41 UTC
(In reply to comment #18)
> Created attachment 9929 [details]
> git-am fix for master.
> 
> All right Metze, here's the goddam rewrite
> you insisted upon :-).
> 
> Add a return parameter of struct smb_create_returns *cr to
> cli_ntcreate()
> cli_ntcreate_recv()
> cli_nttrans_create()
> cli_nttrans_create_recv()
> 
> and fixes everything up... It's now a 4-patch set.

I applied this to 4.1 and tested against SLES's cifsd and Apple's smbx and it worked successfully. Thanks!
Comment 20 Stefan Metzmacher 2014-05-10 08:27:54 UTC
Created attachment 9932 [details]
Patches for v4-1-test
Comment 21 Stefan Metzmacher 2014-05-10 08:28:25 UTC
Jeremy, 4.0 seems to need different patches.
Comment 22 Lars Müller 2014-05-13 15:22:55 UTC
@Ross: When you talk about SLES cifsd the actual product in use is a Novell Open Enterprise Server powered by SUSE Linux Enterprise Server?
Comment 23 Ross Lagerwall 2014-05-13 21:53:48 UTC
(In reply to comment #22)
> @Ross: When you talk about SLES cifsd the actual product in use is a Novell
> Open Enterprise Server powered by SUSE Linux Enterprise Server?

Yes. Confusingly, SLES provides Samba and SLES+OES provides a custom server which shows up as cifsd.
Comment 24 Jeremy Allison 2014-05-14 08:26:55 UTC
Comment on attachment 9932 [details]
Patches for v4-1-test

LGTM. Karolin please push to 4.1`.next, and I'll work on finishing up a 4.0.next patchset.
Comment 25 Jeremy Allison 2014-05-14 08:27:32 UTC
Karolin, reassign back to me once the 4.1.next patch has gone in and I'll back port to 4.0.next.
Comment 26 Karolin Seeger 2014-05-19 10:24:40 UTC
(In reply to comment #25)
> Karolin, reassign back to me once the 4.1.next patch has gone in and I'll back
> port to 4.0.next.

Pushed to autobuild-4-1-test.
Comment 27 Karolin Seeger 2014-05-19 11:00:50 UTC
(In reply to comment #26)
> (In reply to comment #25)
> > Karolin, reassign back to me once the 4.1.next patch has gone in and I'll back
> > port to 4.0.next.
> 
> Pushed to autobuild-4-1-test.

Patchset breaks the build:

[2258/4124] Compiling source3/passdb/pdb_tdb.c
../source3/libsmb/cli_np_tstream.c: In function 'tstream_cli_np_open_done':
../source3/libsmb/cli_np_tstream.c:207: error: too few arguments to function 'cli_ntcreate_recv'
[2259/4124] Compiling source3/lib/server_mutex.c
../source3/libads/kerberos.c: In function 'smb_krb5_get_ntstatus_from_krb5_error_init_creds_opt':
../source3/libads/kerberos.c:126: warning: 'krb5_get_init_creds_opt_get_error' is deprecated (declared at default/source4/heimdal/lib/krb5/krb5-protos.h:2004)
Waf: Leaving directory `/memdisk/kseeger/a41/b932397/samba/bin'
Build failed:  -> task failed (err #1): 
	{task: cc cli_np_tstream.c -> cli_np_tstream_65.o}
make: *** [all] Error 1

Re-assigning to Jeremy.
Comment 28 Jeremy Allison 2014-05-28 19:11:34 UTC
Created attachment 9992 [details]
Fixed patch for 4.1.next

Fixed the patch (was missing one extra NULL argument to cli_ntcreate_recv() in source3/libsmb/cli_np_tstream.c). Sorry for the breakage.

Thanks,

Jeremy.
Comment 29 Jeremy Allison 2014-05-28 20:15:34 UTC
Re-assigning to Karolin for inclusion in 4.1.next.

Thanks Volker !

Jeremy.
Comment 30 Karolin Seeger 2014-06-10 08:59:47 UTC
(In reply to comment #29)
> Re-assigning to Karolin for inclusion in 4.1.next.
> 
> Thanks Volker !
> 
> Jeremy.

Pushed to autobuild-v4-1-test.
Comment 31 Karolin Seeger 2014-06-21 19:59:01 UTC
(In reply to comment #30)
> (In reply to comment #29)
> > Re-assigning to Karolin for inclusion in 4.1.next.
> > 
> > Thanks Volker !
> > 
> > Jeremy.
> 
> Pushed to autobuild-v4-1-test.

Pushed to v4-1-test.
Closing out bug report.

Thanks!