New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
http_ntlm_wb: Fix the error handling to align with that of native/SSPI NTLM #3894
Conversation
824d24e
to
ede110f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be clear you are changing state here because you would like to cope with retries better? If there is just NTLM by itself isn't it better to be strict here and error?
ede110f
to
863776e
Compare
No - I appreciate my description was a bit misleading. As such I have reworded it. But in summary this is to align the error handling of the winbind NTLM to that of the native/SSPI behaviour that was missed in to the three commits that I mentioned above.
|
863776e
to
a7904ef
Compare
Currently when the server responds with 401 on NTLM authenticated connection (re-used) we consider it to have failed. However this is legitimate and may happen when for example IIS is set configured to 'authPersistSingleRequest' or when the request goes thru a proxy (with 'via' header). Implemented by imploying an additional state once a connection is re-used to indicate that if we receive 401 we need to restart authentication. Missed in fe6049f.
a7904ef
to
d3a95e1
Compare
Any commit sent upstream ideally should have points of reference,
We all have had to do follow-ups occasionally, it happens. |
The current code doesn't clean up properly on failed authentication attempts nor does it handle a 401 error properly.
These fixes were implemented in the native/SSPI NTLM code in b4d6db8, 50b87c4 and fe6049f but not im the winbind code. As such this patch brings the code up to date.