[squid-dev] [RFC] annotate_transaction ACL

[Amos Jeffries]

On 10/07/2016 7:14 a.m., Alex Rousskov wrote:
> 
> B. Add general ACL options to be able to force any existing ACL to add
> an annotation:
> 
>     acl myOldAcl dst --annotate foo=bar 127.0.0.1/32
> 
> Please let me know if you consider any of the above alternatives more
> attractive (than adding new ACLs) or need me to detail their drawbacks.
> 

I do like the below better. But for the record what was the drawback on
(B) that you saw as serious?
 This used to be what we were planning to add one day.

> 
> Please note that the documentation below does not currently detail what
> "adding" a foo=bar annotation means when there is already an annotation
> named "foo". During implementation, we will decide whether the new ACLs
> should always append to the existing matching annotation and/or
> overwrite it. The documentation will be adjusted accordingly, of course.
> 
> 
> * acl aclname annotate_transaction key value [fast]
> 
> Always matches. Used for its side effect: This ACL immediately adds
> a key=value annotation to the current master transaction. The added
> annotation can then be tested using note ACL and logged (or sent to
> helpers) using %note format code. This ACL is especially useful for
> recording complex multi-step ACL-driven decisions. For example:
> 
>     # Do not log transaction accepted after aclX matched
> 
>     # First, mark transactions accepted after aclX matched
>     acl markSpecial annotate_transaction special true
>     http_access allow acl001
>     ...
>     http_access deny acl100
>     http_access allow aclX markSpecial
> 
>     # Second, do not log marked transactions:
>     acl markedSpecial note special true
>     access_log ... deny markedSpecial
> 
>     # Note that the following would not have worked because aclX
>     # alone does not determine whether the transaction was allowed:
>     # access_log ... deny markedSpecial # Wrong
> 
> 
> Warning: This ACL annotates the transaction even when negated and
> even if subsequent ACLs fail to match. For example, the following
> three rules will have exactly the same effect as far as annotations
> set by the "mark" ACL are concerned:
> 
>     some_directive acl1 acl2 ... mark # rule matches if mark is reached
>     some_directive acl1 acl2 ... !mark     # rule never matches
>     some_directive acl1 acl2 ... mark !all # rule never matches
> 
> 
> * acl aclname annotate_client key value [fast]
> 
> Always matches. Used for its side effect: This ACL immediately adds
> a key=value annotation to the current client-to-Squid connection.
> Connection annotations are propagated to the current and all future
> master transactions on the annotated connection. All caveats for the
> annotate_transaction ACL apply to this ACL.

I think the format should use explicitly the ' key="value" ' syntax to
make it simpler to identify where the value ends and ACL sub-tree begin.

That will also allow us to define the above format as adding a new key
only if not already present. We could use key+="value" syntax for
appending a value to an possibly existing key (or adding if none).

"Always matches" seems incorrect definition for this ACL. When 'fast' is
provided it should pass-thru the match or non-match result of the [fast]
ACL sub-tree, and only annotate when that result is a match.
 I assume [fast] default would be 'all' to always annotate and be true
if there was no sub-tree.

We will also have to be careful to blacklist the reserved key names that
require special processing to be correct:
 user, group, password, ttl, url, url-rewrite, etc.

Otherwise it seems good. Thanks.

Amos