[squid-dev] Extremely questionable code in Basic authentication module

[Amos Jeffries]

On 25/03/21 10:18 am, Joshua Rogers wrote:
> Hi there,
> 
> I was looking at the file src/auth/basic/UserRequest.cc, in 
> function Auth::Basic::UserRequest::module_direction:
> 
> 
>      case Auth::Ok:
>          if (user()->expiretime + 
> static_cast<Auth::Basic::Config*>(Auth::SchemeConfig::Find("basic"))->credentialsTTL 
> <= squid_curtime)
>              return Auth::CRED_LOOKUP;
>          return Auth::CRED_VALID;
> 
>      case Auth::Failed:
>          return Auth::CRED_VALID;
> 
> I was a bit alarmed that if an auth fails, it returns Auth::CRED_VALID.
> Why is CRED_ERROR or CRED_CHALLENGE not used here?
> 

CRED_VALID is because "Login Failed" is a valid state for a users 
credentials to have. It is also a final state (thus no CRED_CHALLENGE). 
No error has occured in Squid or the helper to reach that state (thus no 
CRED_ERROR).

These CRED_* values are what state of processing the HTTP authentication 
sequence is up to.


> In negotiate and NTLM code, there is a note:
> "XXX: really? not VALID or CHALLENGE?" when CRED_ERROR is returned.
> 

Those auth schemes are tied to the clients TCP connection and cannot be 
re-authenticated with different values on any pipeline messages. That 
causes some surprising/nasty effects on HTTP features including the auth 
internals.


> Thankfully Squid doesn't really rely on this return value to determine 
> whether a login is correct or not

Good. That return value is not saying anything about the user login 
success/failure. It is about the HTTP auth negotiation message(s) and 
any related helper processing all having reached a final decision.


> as it 
> calls authenticateUserAuthenticated() which eventually 
> checks credentials() == Auth::Ok. It all seems like quite a round-about 
> method, however.
> 
> According to 
> <http://www.squid-cache.org/Doc/code/namespaceAuth.html?#afd721f7bc874e61ad0111999abf22a19a2d0cf49d6f94b0664c99dffb68cb4d5d> 
> each of these calls should return CRED_CHALLENGE.
> 

The XXX is kind of outdated now. Part of the questions answer is known - 
CHALLENGE cannot be sent because Failed is a final state. But still 
unknown what the effects of CRED_VALID vs CRED_ERROR would be. I suspect 
it may be that re-authenticate situation of NTLM/Negotiate causing 
complications.


Amos