External acl on delay_access directive

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

External acl on delay_access directive

Luca Savarino
Hello,


     Having recently upgraded from squid 3.4.8 to squid 4.4, I stumbled
into an issue that I tried to simplify the most I could through the
attached configuration files and the explanation below.

     I would like to use an external acl to set bandwidth limitations
for my different users. So in squid 3.4.8, I would do something like
(that's just a very simple example) :


             delay_pools 1

             delay_class 1 3

             external_acl_type ip_user_helper %SRC
/usr/lib/squid3/ext_file_userip_acl -f /etc/squid/ips.conf
             acl ip_list external ip_user_helper test


             delay_access 1 allow ip_list
             delay_access 1 deny all

             delay_parameters 1 80000/80000 80000/80000 80000/80000


     with /tmp/ips.conf containing something like :


             10.1.0.55 ALL


     If the ip I want to limit the bandwidth of is 10.1.0.55. In squid
4.4 however, I can't get it to work properly : the user can access her
page but she is not limited as expected and I get the following message
multiple times in my cache.log file :


             WARNING: ip_list ACL is used in context without an ALE
state. Assuming mismatch.


     I believe it is related but I am not sure (or maybe I just did
something wrong). You can find a minimal configuration file attached to
reproduce.


     Thanks in advance for your help,


Regards,


Luca


_______________________________________________
squid-users mailing list
[hidden email]
http://lists.squid-cache.org/listinfo/squid-users

squid.conf (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: External acl on delay_access directive

Alex Rousskov
On 1/17/19 9:13 AM, Luca Savarino wrote:

> WARNING: ip_list ACL is used in context without an ALE
> state. Assuming mismatch.

> delay_access 1 allow ip_list

Looks like a Squid bug to me -- Squid should supply ALE (a blob
containing various transaction details) to the delay_access code but
evidently does not.

If you are a developer or can hire a developer to fix this bug, a good
starting point could be the missing ACLFilledChecklist::al
initialization in DelayId::DelayClient().


HTH,

Alex.
_______________________________________________
squid-users mailing list
[hidden email]
http://lists.squid-cache.org/listinfo/squid-users
Reply | Threaded
Open this post in threaded view
|

Re: External acl on delay_access directive

Luca Savarino
Hello Alex,


         Thank you for your help. Attached is a patch which seems to fix
the issue. Does it seem correct to you ?


Regards,


Luca

On 1/17/19 5:39 PM, Alex Rousskov wrote:

> On 1/17/19 9:13 AM, Luca Savarino wrote:
>
>> WARNING: ip_list ACL is used in context without an ALE
>> state. Assuming mismatch.
>> delay_access 1 allow ip_list
> Looks like a Squid bug to me -- Squid should supply ALE (a blob
> containing various transaction details) to the delay_access code but
> evidently does not.
>
> If you are a developer or can hire a developer to fix this bug, a good
> starting point could be the missing ACLFilledChecklist::al
> initialization in DelayId::DelayClient().
>
>
> HTH,
>
> Alex.
> _______________________________________________
> squid-users mailing list
> [hidden email]
> http://lists.squid-cache.org/listinfo/squid-users

_______________________________________________
squid-users mailing list
[hidden email]
http://lists.squid-cache.org/listinfo/squid-users

0001-Initialize-ALE-for-delay_pools.patch (864 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: External acl on delay_access directive

Alex Rousskov
On 1/21/19 5:22 AM, Luca Savarino wrote:

> Attached is a patch which seems to fix the issue.

Glad you have a fix that works for you, but this mailing list is not the
right place for patch reviews. If you want to submit your changes to the
Squid project, I suggest creating a GitHub pull request. The procedure
is outlined at https://wiki.squid-cache.org/MergeProcedure


> Does it seem correct to you ?

The patch is buggy: Setting AccessLogEntry::reply field like that may
lead to memory leaks and/or crashes. Ideally, the reply field should be
set when the reply becomes known. I do not know whether that is already
done (elsewhere in the code). If it is done, than the reply setting line
in the patch can be removed. You can answer that question for your
particular use case by checking (e.g., in a debugger or by adding a
debugs() message) whether http->al->reply is nil before the assignment.


HTH,

Alex.


> On 1/17/19 5:39 PM, Alex Rousskov wrote:
>> On 1/17/19 9:13 AM, Luca Savarino wrote:
>>
>>> WARNING: ip_list ACL is used in context without an ALE
>>> state. Assuming mismatch.
>>> delay_access 1 allow ip_list
>> Looks like a Squid bug to me -- Squid should supply ALE (a blob
>> containing various transaction details) to the delay_access code but
>> evidently does not.
>>
>> If you are a developer or can hire a developer to fix this bug, a good
>> starting point could be the missing ACLFilledChecklist::al
>> initialization in DelayId::DelayClient().
>>
>>
>> HTH,
>>
>> Alex.
>> _______________________________________________
>> squid-users mailing list
>> [hidden email]
>> http://lists.squid-cache.org/listinfo/squid-users

_______________________________________________
squid-users mailing list
[hidden email]
http://lists.squid-cache.org/listinfo/squid-users