Bug 14983 - NT_STATUS_ACCESS_DENIED translates into EPERM instead of EACCES in SMBC_server_internal
Summary: NT_STATUS_ACCESS_DENIED translates into EPERM instead of EACCES in SMBC_serve...
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: libsmbclient (show other bugs)
Version: 4.15.5
Hardware: All Linux
: P5 normal (vote)
Target Milestone: ---
Assignee: Jeremy Allison
QA Contact: Samba QA Contact
Depends on:
Reported: 2022-02-18 19:13 UTC by Elia Geretto
Modified: 2022-07-04 17:09 UTC (History)
7 users (show)

See Also:

git-am fix for 4.16.rcNext, 4.15.next, 4.14.next. (1.50 KB, patch)
2022-03-16 21:32 UTC, Jeremy Allison
metze: review+

Note You need to log in before you can comment on or make changes to this bug.
Description Elia Geretto 2022-02-18 19:13:32 UTC
In the code snippet at:


the value of the NT_STATUS is discarded and EPERM is always returned, even on NT_STATUS_ACCESS_DENIED. Given the error paths around it, it seems that it should instead be translated using map_errno_from_nt_status.

I have reached that code location while debugging the addition of a printer in an authenticated SMB share in GNOME Settings 41, which expects EACCES when credentials are required.
Comment 1 Jeremy Allison 2022-02-18 20:56:40 UTC
Just clarifying, you think that the errno return should be from the NTSTATUS returned from:

564	status = cli_session_setup_creds(c, creds);
565	if (!NT_STATUS_IS_OK(status)) {

not from the (unsaved) status returned in:

572		    !NT_STATUS_IS_OK(cli_session_setup_anon(c))) {

Correct ?
Comment 2 Elia Geretto 2022-02-18 21:01:04 UTC
To be honest, I do not know what would make sense in general case. In the case of an authenticated server, I think there would be no difference because both are ACCESS_DENIED, right?.
Comment 3 Elia Geretto 2022-02-22 09:07:15 UTC
Just to clarify, does that look like the cause of the wrong errno value to you, or should I debug some more?
Comment 4 Elia Geretto 2022-03-08 15:19:50 UTC
Comment 5 Elia Geretto 2022-03-12 20:25:58 UTC
I have created an MR [1] with the change discussed before. I have tested it with GNOME 41.3 on Fedora 35 and it fixes the bug I was experiencing. I can now properly list SMB print servers protected by authentication.

[1]: https://gitlab.com/samba-team/samba/-/merge_requests/2436
Comment 6 Samba QA Contact 2022-03-16 19:45:03 UTC
This bug was referenced in samba master:

Comment 7 Jeremy Allison 2022-03-16 21:32:10 UTC
Created attachment 17220 [details]
git-am fix for 4.16.rcNext, 4.15.next, 4.14.next.

Cherry-picks cleanly from master for 4.16.rcNext, 4.15.next, 4.14.next.
Comment 8 Jule Anger 2022-03-17 08:29:26 UTC
Pushed to autobuild-v4-{16,15,14}-test.
Comment 9 Samba QA Contact 2022-03-17 09:46:04 UTC
This bug was referenced in samba v4-14-test:

Comment 10 Samba QA Contact 2022-03-17 10:13:03 UTC
This bug was referenced in samba v4-16-test:

Comment 11 Samba QA Contact 2022-03-17 10:55:30 UTC
This bug was referenced in samba v4-15-test:

Comment 12 Jule Anger 2022-03-17 11:03:59 UTC
Closing out bug report.

Comment 13 Samba QA Contact 2022-03-21 12:17:29 UTC
This bug was referenced in samba v4-16-stable (Release samba-4.16.0):

Comment 14 Andreas Schneider 2022-04-04 07:46:47 UTC
This creates a regression in Samba 4.16 that users are not able to connect to Samba shares with Nautilus anymore!

Comment 15 Samba QA Contact 2022-04-04 12:49:55 UTC
This bug was referenced in samba v4-14-stable (Release samba-4.14.13):

Comment 16 Jeremy Allison 2022-04-04 16:01:06 UTC
Andreas, can you get me a packet trace showing what is happening here ?

We obviously don't have a test for what nautilus is doing here so we have no guarentees that we won't keep breaking this. We need to have a test that does *exactly* what nautilus wants so we can fix this.
Comment 17 Jeremy Allison 2022-04-04 16:02:09 UTC
Why is nautilus depending on exact error codes here ? Can you give me a link to the nautilus code that is using this ?
Comment 18 Guenther Deschner 2022-04-04 17:38:58 UTC
Hi Jeremy,

I believe this is the relevant gnome code:

Comment 19 Jeremy Allison 2022-04-04 17:47:33 UTC
OK, so the change was to map the error code returned from the server to errno rather than hard-coding it to EACCESS. Which (IMHO) is the correct thing to do.

errno = map_errno_from_nt_status(status);

can you get a packet trace from a failed anonymous logon, so we can see exactly what NTSTATUS we're getting here and work out what it's being mapped to in errno.

The gnome code is doing:

if (.. || (errsv != EACCES && errsv != EPERM))

so if it really is getting NT_STATUS_ACCESS_DENIED then it should work. We need to see what exactly we got back here. I think hard coding these error codes in gnome isn't the right thing to do to fall-back to anonymous logon.

Why is it checking them ? Why not just always try anonymous logon if the user doesn't give a username or password ?
Comment 20 Jeremy Allison 2022-04-04 17:50:08 UTC
Here is the error map we are using to map NT_STATUS -> errno.


Looks like it should work to me. We really need to see what the server is sending back.
Comment 21 Guenther Deschner 2022-04-04 17:53:31 UTC
Can't check right now but from the redhat bug it is clearly NT_STATUS_INVALID_PARAMETER:

Failed to start GENSEC client mech gse_krb5: NT_STATUS_INTERNAL_ERROR
gensec_spnego_client_negTokenInit_step: Could not find a suitable mechtype in NEG_TOKEN_INIT
gensec_update_send: spnego[0x7f7ed801c150]: subreq: 0x7f7ed80230c0
gensec_update_done: spnego[0x7f7ed801c150]: NT_STATUS_INVALID_PARAMETER tevent_req[0x7f7ed80230c0/../../auth/gensec/spnego.c:1631]: state[3] error[-7963671676338569203 (0x917B5ACDC000000D)]  state[struct gensec_spnego_update_state (0x7f7ed8023280)] timer[(nil)] finish[../../auth/gensec/spnego.c:1947]
SPNEGO login failed: An invalid parameter was passed to a service or function.
map_errno_from_nt_status: 32 bit codes: code=c000000d
smb: do_mount - [smb://fnuc.local/finance; 0] res = -1, cancelled = 0, errno = [22] 'Invalid argument' 
smb: do_mount - (errno != EPERM && errno != EACCES), cancelled = 0, breaking
Comment 22 Jeremy Allison 2022-04-04 18:01:07 UTC
So that looks like an internal return from libsmbclient when krb5 isn't selected or something. But returning this back to the caller is clearly the right fix instead of hiding all error returns as EACESS.

The client should want to know that the library couldn't use krb5 due to a mis-configure IMHO.

Can we get the GNOME code fixed here instead ? I think Samba is doing the right thing.
Comment 23 Jeremy Allison 2022-04-07 01:01:35 UTC
Ping. Gunther or Andreas, can we get an update on this ? libsmbclient returning the correct error code to the caller, rather than squashing everything into EACESS seems to be a no-brainer in terms of being the right thing to do. If the caller wants to fall-back to unauthenticated connections they should always do so unless we return ECONNREFUSED.

Hard-coding an allow list or error returns in Gnome seems to be the wrong thing to do.
Comment 24 Jeremy Allison 2022-04-07 01:02:37 UTC
Adding Pavel to this in the hope he has some bandwidth :-).
Comment 25 Andreas Schneider 2022-04-13 08:46:42 UTC
So this means comsumers of libsmbclient need to handle more return codes, including EINVAL.
Comment 26 Jeremy Allison 2022-04-13 16:54:54 UTC
It means consumers of libsmbclient have to be able to handle any POSIX error returned. Having a hard-coded list of "only EACCES" means retry with anonymous credentials is poor programming practice.

The correct logic here is "if you get anything other than ECONNREFUSED then retry with anonymous".
Comment 27 Ralph Gauer 2022-04-19 18:53:05 UTC
Is it possible that this change is breaking the ability to browse the list of shares on a samba server?
Since upgrading to 4.16 both smbclient and Windows are not able to list the shares anymore - smbclient returns an empty list and Windows returns "access denied" for anonymous and authenticated users.
Access to shares directly via UNC is working normally.
"//sambaserver" fails.
"//sambaserver/share" is working.
Comment 28 Guenther Deschner 2022-04-20 09:24:52 UTC
Upstream gnome has that issue now addressed via https://gitlab.gnome.org/GNOME/gvfs/-/issues/611. The freshly released Fedora 36 brings that update already as part of gvfs in gvfs-1.50.0-4.fc36.x86_64.

Closing this bug again.
Comment 29 Andreas Schneider 2022-04-20 10:21:26 UTC
FYI: KDE is not affected!
Comment 30 Jeremy Allison 2022-04-25 16:12:02 UTC
(In reply to Guenther Deschner from comment #28)

Thanks a lot Guenther for taking care of this !
Comment 31 Samba QA Contact 2022-04-26 14:44:50 UTC
This bug was referenced in samba v4-15-stable (Release samba-4.15.7):

Comment 32 Harald Sitter 2022-04-27 22:04:19 UTC
Seems KIO is affected after all https://bugs.kde.org/show_bug.cgi?id=453090

Should this change maybe get refined to not break with the two biggest consumers of libsmbclient?
Comment 33 Jeremy Allison 2022-04-27 22:53:11 UTC
(In reply to Harald Sitter from comment #32)

Nope. The 2 biggest users were depending on very undocumented behavior. It's much better to feel the pain now and not depend on specific error codes for this sort of thing.

Make the calling libraries flexible in coping with the error returns to do what they need.

Any other way creates complete ossification of the underlying code, where no improvements are allowed due to fear of breaking some unknown caller dependency.

We can't live like that !
Comment 34 Harald Sitter 2022-06-24 10:17:48 UTC
Whoops, sorry, I've missed your reply.

Right now smbclient is misbehaving though, surely. It doesn't return EPERM (which KIO always handled), it also doesn't return EACCES (which KIO also handled) and is the documented return value [1]. It now returns EINVAL which makes it uniquely hard to tell it apart from the other 3 error reasons already associated with EINVAL. 

From the KDE bug report:

kf.kio.slaves.smb: open "smb://laptop-jvsbr4b4.local/" url-type: 2 dirfd: -1 errNum: 22

Unless I'm mistaken 22 is EINVAL.

[1] https://github.com/samba-team/samba/blob/8458449ddf1a5c939784116aa3f9d21edaf93a05/source3/include/libsmbclient.h#L1578
Comment 35 Jeremy Allison 2022-06-24 17:01:56 UTC
Again, I point you at comment #26.

It means consumers of libsmbclient have to be able to handle any POSIX error returned. Having a hard-coded list of "only EACCES" means retry with anonymous credentials is poor programming practice.

The correct logic here is "if you get anything other than ECONNREFUSED then retry with anonymous".

Callers of libsmbclient have to be able to handle any POSIX error returned.

You can't call expect a never-changing truncated list of errno's. What would you do if we returned ENOMEM (which may happen due to memory pressure) ?

We're returning EINVAL due to an incorrect parameter setup for krb5, which is the correct error to return.
Comment 36 Harald Sitter 2022-07-01 06:16:14 UTC
(In reply to Jeremy Allison from comment #35)
> The correct logic here is "if you get anything other than ECONNREFUSED then retry with anonymous".

How are we to know this though? The API documentation outlines numerous errors with special meaning. And ECONNREFUSED isn't even in that list.

Don't get me wrong I fully appreciate your point here, but if the stance is "anything goes" then the documentation needs to reflect that *somehow* (e.g. by not listing errnos and saying that any error may be returned?). Right now the "words" of the API aren't reflected by its "actions", as it were.

Comment 37 Jeremy Allison 2022-07-01 15:25:39 UTC
Sure, this is a documentation bug. Can you open a new bug for this ? I'm happy to consider any documentation patch you'd like to see.
Comment 38 Anthony Hung 2022-07-04 17:09:56 UTC
(In reply to Jeremy Allison from comment #35)
> The correct logic here is "if you get anything other than ECONNREFUSED then retry with anonymous".

Hi Jeremy,

I don't know if I get it wrong.
Does it mean I should only handle the error ECONNREFUSED and all the other error should retry with anonymous?

I tried to implement the logic as you suggested, but I think that it is quite weird if I get the error EHOSTUNREACH but still retry with anonymous.

Did I misunderstand something?