[Samba] ntlm_auth not returning "STATUS_OK"

[Andrew Bartlett]

On Tue, 2024-01-23 at 17:07 -0500, Mark Foley via samba wrote:
> On Mon Jan 22 11:00:59 2024 Mark Foley via samba <
> samba at lists.samba.org
> > wrote:
> > I have scripts that runs ntlm_auth. Before upgrading my DC to
> > 4.18.9 I would
> > get text string output from the ntlm_auth command. For example:
> > 
> > STATUS_NO_SUCH_USER
> > NT_STATUS_WRONG_PASSWORD
> > STATUS_OK
> > 
> > My script(s) look for these strings. 
> > 
> > Now with the new Samba, the first two strings are output as usual
> > in the case of
> > non-existant user and invalid password, respectively, but if the
> > user/pw is OK
> > it now returns the string: ":  (0x0)", which, I suppose, is the
> > exit status of
> > the ntlm_auth command meaning OK.
> > 
> > Is there an option to change this back to the string "STATUS_OK"?
> > If not, I'll
> > change my programs, but I'd rather not do that.
> 
> After more investigation, I find that on another system running Samba
> 4.15.13 and
> ntlm_auth version 4.15.13 it continues to print "NT_STATUS_OK: The
> operation
> completed successfully. (0x0)" when 'ntlm_auth --username user --
> password pw' 
> is run. 
> 
> So, Samba/ntlm_auth version 4.18.9 changes that to print ": (0x0)".

I can see a code change that would have left this zeroed out, being 

https://git.samba.org/?p=samba.git;a=commitdiff;h=ddc551f4477bfb8bc7ec636c89af01a028190d35

https://git.samba.org/?p=samba.git;a=commitdiff;h=c68f21f26f10b60ca1ac294b7294bfbf37c9bb86


> I have never been in favor of developers changing the behavoir of
> programs when
> "new features" come out, expecially programs that might be used in
> scripts that
> rely on responses.  I think it's naughty when developers do that.  If
> behavoir
> is different from a previous version, then the new version ought to
> have a
> different name or a switch enabling the new/changed feature. 
> 
> Since the 4.18.9 ntlm_auth output has an oddly placed colon (:) in
> the string,
> as if some text was supposed to come before that, I'll assume this
> was an
> inadvertant omission and not a deliberate change to the output
> response of this
> program, espcially given that the other responses (full text):

There is a call, set_auth_errors() that sets this, but is in the
current code only called in the error case, it was previously always
set, but when the code was changed (per the above) not to handle this
output detail in the implementation routine, this was lost.

Sadly this aspect must not be covered in our integration test of
ntlm_auth.

> NT_STATUS_NO_SUCH_USER: The specified account does not exist.
> (0xc0000064)
> NT_STATUS_WRONG_PASSWORD: When trying to update a password, this
> return status indicates that the value provided as the current
> password is not correct. (0xc000006a)
> 
> are unchanged. The Samba developers are certainly too seasoned to do
> that
> deliberately.
> 
> Meanwhile, I'll change my programs to look for "(0x0)" as both
> versions have
> that, and maybe I'll just look for the 0x codes for all.

Please file a bug and if you can, open a merge request to fix this with
a test. 

winbindd_pam_auth_recv()

simply needs to call 

set_auth_errors(response, status);

In the success case also at the function end.

Andrew Bartlett

-- 
Andrew Bartlett (he/him)       https://samba.org/~abartlet/
Samba Team Member (since 2001) https://samba.org
Samba Team Lead                https://catalyst.net.nz/services/samba
Catalyst.Net Ltd

Proudly developing Samba for Catalyst.Net Ltd - a Catalyst IT group
company

Samba Development and Support: https://catalyst.net.nz/services/samba

Catalyst IT - Expert Open Source Solutions