Re: [squid-users] external_acl_type requests authentication

ian j hart wrote:
> On Friday 18 January 2008 20:10:07 ian j hart wrote:
>> On Friday 18 January 2008 00:51:40 ian j hart wrote:
>>> The external_acl_type requests authentication since bug 1278 was fixed.
>>>
>>> I have something like this (cut down and edited).
>>>
>>> external_acl_type logger ttl=0 negative_ttl=0 children=1 %LOGIN ...
>>>
>>> acl password proxy_auth REQUIRED
>>>
>>> acl proxylist dstdomain .some.site
>>> acl logproxy external logger 8
>>>
>>> http_access deny proxylist logproxy
>>> ###deny_info ERR_ACCESS_DENIED.proxy logproxy
>>>
>>> http_access allow password
>>> http_access deny all
>>>
>>> A hit on the proxylist causes authentication and the (bogus) error
>>> message only appears when the user selects cancel. Not intuitive.
>>>
>>> The bogus error message (you must authenticate) is easily fixed up with
>>> the deny_info line.
>>>
>>> Yes, I realise I could work around this with a dummy acl, but that's just
>>> nasty. In any case I'd rather add a feature than jump thru' hoops.
>>>
>>> Judging by the size of the patch to implement this is should be simple
>>> enough to fix up (famous last words).
>>>
>>> I had hoped I could just not set the flag, e.g.
>>>
>>> --- src/external_acl.c.orig Mon Jan 1 23:32:13 2007
>>> +++ src/external_acl.c Thu Jan 17 21:17:31 2008
>>> @@ -275,6 +275,8 @@
>>> format->type = EXT_ACL_LOGIN;
>>> a->require_auth = 1;
>>> }
>>> + else if (strcmp(token, "%NOAUTH") == 0)
>>> + format->type = EXT_ACL_LOGIN;
>>> #if USE_IDENT
>>> else if (strcmp(token, "%IDENT") == 0)
>>> format->type = EXT_ACL_IDENT;
>>>
>>> Unfortunately this breaks an assert in authenticate.c near line 648.
>>>
>>> At which point I need help.
>>>
>>> authenticateUserRequestUsername(auth_user_request_t * auth_user_request)
>>> {
>>> assert(auth_user_request != NULL);
>>>
>>> NULL seems to be a valid return value, that's one option. Dangerous?
>>>
>>> Fixing the call would be another. It appears to be called from
>>> external_acl.c makeExternalAclKey
>>>
>>> switch (format->type) {
>>> case EXT_ACL_LOGIN:
>>> str = authenticateUserRequestUsername(request->auth_user_request);
>>>
>>> Check the flag and set str=NULL?
>>>
>>> Maybe there's a patch for this already? Or a wish list where I could post
>>> it. Or is it near enough that someone could help me out?
>>>
>>> Thanks
>> This appears to work (tested for a whole 10 mins :)
>>
>> --- src/external_acl.c.orig Mon Jan 1 23:32:13 2007
>> +++ src/external_acl.c Fri Jan 18 19:29:15 2008
>> @@ -275,6 +275,8 @@
>> format->type = EXT_ACL_LOGIN;
>> a->require_auth = 1;
>> }
>> + else if (strcmp(token, "%NOAUTH") == 0)
>> + format->type = EXT_ACL_LOGIN;
>> #if USE_IDENT
>> else if (strcmp(token, "%IDENT") == 0)
>> format->type = EXT_ACL_IDENT;
>> @@ -627,7 +629,8 @@
>> const char *str = NULL;
>> switch (format->type) {
>> case EXT_ACL_LOGIN:
>> - str =
>> authenticateUserRequestUsername(request->auth_user_request); + if
>> (externalAclRequiresAuth(acl_data))
>> + str =
>> authenticateUserRequestUsername(request->auth_user_request); break;
>> #if USE_IDENT
>> case EXT_ACL_IDENT:
>
> No, that's not it. Username passed to external program is always -.
>
> Back to the drawing board.
>

I suspect the problem you are facing with that '-' is that the login
delay occurs during processing of the EXT_ACL_LOGIN state not the %LOGIN
parsing. So a new state EXT_ACL_NOLOGIN will be needed to skip the
credential remote-retrieval without skipping the local credential lookup.

You are testing with a non-zero auth_ttl right? (that is TTL on the
local auth details cache).

PS. could you move patches to squid-dev or bugzilla please.

Thanks
Amos

-- 
Please use Squid 2.6STABLE17+ or 3.0STABLE1+
There are serious security advisories out on all earlier releases.