Re: [PATCH] AND and OR ACLs

On 12/21/2012 02:05 AM, Amos Jeffries wrote:

> I really have a big issue with designing it this way as well. The
> overheads of setting up many Checklists are way out of proportion
> compared to the problem avoided...

> If the value has changed the effect of running the whole line again is
> no worse than processing the request slightly later. I think this is a
> very minor issue and one we can optimize out later as a pure performance
> issue.

Hi Amos,

    IMO, this re-checking is a serious problem because it introduces
subtle bugs into rules checking logic when the decision during a
re-check changes. I realize that such bugs must be rare, and that Squid
has been "working fine" with arguably half-broken ACLs (for other
reasons) for years anyway, but it still bothers me.

What do you think is the right way to remember the last check position
so that the code can avoid rechecking previous rules? Skip N nodes? Skip
all nodes up to and including the node pointed to by P? I am worried
about effects of reconfiguration on both of these algorithms. Do we have
some kind of locking/cloning in place to prevent ongoing checks from
being affected by reconfigure?

> IMO design these ACLs to operate as nodes in the ACL test tree
> which operate on the one Checklist same as all other ACLs.

Christos,

    I wonder if we can avoid code duplication by moving existing AND and
OR logic from Checklist into a new ACL node type and then _always_ using
that node type to wrap _all_ ACL rules? It would be kind of the opposite
of what you have done: You are wrapping existing nodes into Checklist
rule when an AND/OR ACL is found. This solution would wrap ALL rules
into an AND or OR ACL node while Checklist will always check just _one_
ACL node (usually AND or OR).

In other words, this solution would automatically transfer

    http_access allow a1 a2 a3
    http_access allow b1 b2 b3

into

    acl autoA all-of a1 a2 a3
    acl autoB all-of b1 b2 b3
    acl autoAB any-of autoA autoB
    http_access allow autoAB

We would need to store the allow/deny keyword with the and/or node to
make this work, but I did not show that detail in the above example in
hope to avoid further confusion.

Do you see what I am getting at?

Thank you,

Alex.
Received on Fri Dec 21 2012 - 18:58:43 MST