In the code snippet at: https://gitlab.com/samba-team/samba/-/blob/master/source3/libsmb/libsmb_server.c#L575 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.
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 ?
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?.
Just to clarify, does that look like the cause of the wrong errno value to you, or should I debug some more?
Ping.
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
This bug was referenced in samba master: 70b9977a46e5242174b4461a7f49d5f640c1db62
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.
Pushed to autobuild-v4-{16,15,14}-test.
This bug was referenced in samba v4-14-test: 3ae7ead5fd53e5ca590cb6bee82afc92b35264f6
This bug was referenced in samba v4-16-test: 34771e1931587807d0395c7ac7f4be18654997f4
This bug was referenced in samba v4-15-test: 9b6e8ae65e24788c82022e761bfb2c78b59260d9
Closing out bug report. Thanks!
This bug was referenced in samba v4-16-stable (Release samba-4.16.0): 34771e1931587807d0395c7ac7f4be18654997f4
This creates a regression in Samba 4.16 that users are not able to connect to Samba shares with Nautilus anymore! See https://bugzilla.redhat.com/show_bug.cgi?id=2068976 https://gitlab.gnome.org/GNOME/gvfs/-/issues/611
This bug was referenced in samba v4-14-stable (Release samba-4.14.13): 3ae7ead5fd53e5ca590cb6bee82afc92b35264f6
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.
Why is nautilus depending on exact error codes here ? Can you give me a link to the nautilus code that is using this ?
Hi Jeremy, I believe this is the relevant gnome code: https://gitlab.gnome.org/GNOME/gvfs/-/blob/master/daemon/gvfsbackendsmb.c#L516
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 ?
Here is the error map we are using to map NT_STATUS -> errno. {NT_STATUS_ACCESS_VIOLATION, EACCES}, .. {NT_STATUS_ACCESS_DENIED, EACCES}, .. {NT_STATUS_WRONG_PASSWORD, EACCES}, {NT_STATUS_LOGON_FAILURE, EACCES}, Looks like it should work to me. We really need to see what the server is sending back.
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 ...
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.
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.
Adding Pavel to this in the hope he has some bandwidth :-).
So this means comsumers of libsmbclient need to handle more return codes, including EINVAL.
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".
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.
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.
FYI: KDE is not affected!
(In reply to Guenther Deschner from comment #28) Thanks a lot Guenther for taking care of this !
This bug was referenced in samba v4-15-stable (Release samba-4.15.7): 9b6e8ae65e24788c82022e761bfb2c78b59260d9
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?
(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 !
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
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.
(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. https://github.com/samba-team/samba/blob/8458449ddf1a5c939784116aa3f9d21edaf93a05/source3/include/libsmbclient.h#L1578
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.
(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?