Bug 13050 - smbc_opendir returns EEXIST with invalid login credentials
Summary: smbc_opendir returns EEXIST with invalid login credentials
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: libsmbclient (show other bugs)
Version: 4.7.0
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-09-22 22:56 UTC by Michal Malý
Modified: 2018-04-10 07:04 UTC (History)
6 users (show)

See Also:


Attachments
potential fix (1.37 KB, patch)
2017-11-02 14:26 UTC, David Mulder
no flags Details
git-am fix for 4.7.next. (1.72 KB, patch)
2018-04-04 00:23 UTC, Jeremy Allison
asn: review+
jmcd: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michal Malý 2017-09-22 22:56:32 UTC
Greetings,

I have just updated my laptop to Samba 4.7 and immediately found out that I 
cannot access any shared data on our departmental server. The server (running 
CentOS 7.4 and Samba 4.6) requires valid login credentials which are checked 
against a DC.

I use KDE's Dolphin file manager to access the shares. Dolphin's SMB plugin 
expects libsmbclient to return either EPERM or EACCES if the server rejects 
the login credentials. The libsmbclient that came with Samba 4.7 update 
returns EEXIST instead. I have verified that this is really happening with 
"smbbrowse2.c" example code supplied with Samba sources. Note that EEXIST is 
returned only when I try to access a "top-level" directory (a share) on the 
server. If I to access a directory within a share that I do not have access 
to, EPERM is returned. Here is a sample output from the slightly modified 
"testbrowse2.c"

/(unknown_group)/ect228-srv/root
Cannot open remote directory, errno: 17 (File exists)
/(unknown_group)/ect228-srv/guests
/(unknown_group)/ect228-srv/guests/ecguest1
/(unknown_group)/ect228-srv/instruments
/(unknown_group)/ect228-srv/instruments/308_CE1600-N
/(unknown_group)/ect228-srv/instruments/308_CE1600-A
/(unknown_group)/ect228-srv/home
/(unknown_group)/ect228-srv/home/kalusm
Cannot open remote directory, errno: 13 (Permission denied)
/(unknown_group)/ect228-srv/home/a6-22
Cannot open remote directory, errno: 13 (Permission denied)
/(unknown_group)/ect228-srv/home/kucerog1
Cannot open remote directory, errno: 13 (Permission denied)
/(unknown_group)/ect228-srv/home/ordogovm
Cannot open remote directory, errno: 13 (Permission denied)
/(unknown_group)/ect228-srv/home/boublikm
Cannot open remote directory, errno: 13 (Permission denied)
/(unknown_group)/ect228-srv/home/zuskova
Cannot open remote directory, errno: 13 (Permission denied)
/(unknown_group)/ect228-srv/home/vcelakov
Cannot open remote directory, errno: 13 (Permission denied)
/(unknown_group)/ect228-srv/home/kalikova
Cannot open remote directory, errno: 13 (Permission denied)
/(unknown_group)/ect228-srv/home/tesarove
Cannot open remote directory, errno: 13 (Permission denied)
/(unknown_group)/ect228-srv/home/ansorgem
Cannot open remote directory, errno: 13 (Permission denied)
/(unknown_group)/ect228-srv/home/dubsky
Cannot open remote directory, errno: 13 (Permission denied)
/(unknown_group)/ect228-srv/home/malymi
/(unknown_group)/ect228-srv/home/malymi/some_file.txt

Downgrading back to Samba 4.6 or making Dolphin's SMB plugin recognize the 
EEXIST retcode as a case of invalid credentials fixes the issue for me.

Is this behavior expected with Samba 4.7?
Comment 1 Donna 2017-10-24 14:10:03 UTC
I too have seen this issue. 

When the credentials are invalid when accessing a samba share we don't get the expected error number with samba 4.7.0; EEXIST is returned instead.

Downgrading to 4.6.8 fixes the issue.

After investigating the issue seems to be happening the the cli_shutdown code called from source3/libsmb/libsmc_server.c:

563    status = cli_tree_connect_creds(c, share, "?????", creds);
564    if (!NT_STATUS_IS_OK(status)) {
565        errno = map_errno_from_nt_status(status);
566	   cli_shutdown(c);
567	   return NULL;
568    }

In this instance, the cli_shutdown is called after setting the errno, however throughout this file the errno is set after the cli_shutdown is called. So my suggested patch for this is to do the same here and set the errno after cli_shutdown:

563    status = cli_tree_connect_creds(c, share, "?????", creds);
564    if (!NT_STATUS_IS_OK(status)) {
565        cli_shutdown(c);
566        errno = map_errno_from_nt_status(status);
567	   return NULL;
568    }
Comment 2 David Mulder 2017-11-02 14:26:58 UTC
Created attachment 13751 [details]
potential fix
Comment 3 David Mulder 2017-11-02 14:28:46 UTC
Tested the recommendation by Donna and this does resolve the issue in Dolphin. It returns the behavior to 4.6x.
Comment 4 Andrew Bartlett 2017-11-16 22:13:53 UTC
(In reply to David Mulder from comment #3)
Any chance we can get a test for this in our automated test suite, so we don't regress here again?
Comment 5 Harsh Shukla 2018-04-02 11:56:46 UTC
Is this bug open or resolved ?
Comment 6 Nathaniel Graham 2018-04-02 13:10:22 UTC
Dolphin has worked around the issue so users aren't affected, but the underlying issue in Samba remains.
Comment 7 Harsh Shukla 2018-04-02 16:46:47 UTC
(In reply to Nathaniel Graham from comment #6)

Okay. Thanks.
Comment 8 Jeremy Allison 2018-04-04 00:21:03 UTC
This is fixed with commit 7470b9b18af282a742929d3fc90f4be5520428a1 in Samba 4.8.0 and above. Do you need a back-port for 4.7.next also ? CC:ing Jim on this one as he +1'ed the original patch from SuSE.
Comment 9 Jeremy Allison 2018-04-04 00:23:36 UTC
Created attachment 14100 [details]
git-am fix for 4.7.next.

Cherry-pick from master.
Comment 10 Nathaniel Graham 2018-04-04 03:53:51 UTC
Let me know if this gets backported to 4.7.x (and what version it makes it into) so I can know what versions we can revert our workaround for.
Comment 11 Andreas Schneider 2018-04-04 09:19:23 UTC
Comment on attachment 14100 [details]
git-am fix for 4.7.next.

LGTM
Comment 12 Andreas Schneider 2018-04-04 09:19:59 UTC
Karolin, please add the fix to 4.7. Thanks!
Comment 13 Karolin Seeger 2018-04-04 10:11:46 UTC
(In reply to Andreas Schneider from comment #12)
Pushed to autobuild-v4-7-test.
Comment 14 Karolin Seeger 2018-04-10 07:04:37 UTC
(In reply to Karolin Seeger from comment #13)
Pushed to v4-7-test.
Closing out bug report.

Thanks!