[squid-dev] [RFC] annotate_transaction ACL

[Alex Rousskov]

On 07/12/2016 12:59 AM, Amos Jeffries wrote:
> On 11/07/2016 5:27 p.m., Alex Rousskov wrote:
>>>> * acl aclname annotate_client key value [fast]

>> One [documented] problem is that !foo will still annotate, which is a
>> little counter-intuitive, but I cannot think of a better approach.


> Well, that issue is partially resolved in the design I was mistakenly
> thinking you were suggesting.

I do not think your sub-tree design solves that problem, even partially,
because !foo annotates in your design as well (assuming foo itself
annotates). Moreover, I doubt this is a problem that can be solved! When
foo annotates,

  * if !foo annotates, some admins will be confused
  * if !foo does not annotate, some admins will be confused

My always-annotates design is the simplest among the various
confusing-for-some options: If Squid reaches your annotating ACL, it
_will_ annotate, no matter what happens afterwards.


> If this ACL is implmented as a Node with a sub-tree, like any-of/all-of.
> Which annotates whenever the sub-tree produces a match. Then the '!' on
> it wont matter so much.

I do not think it would make a difference because the "!" is applied
_after_ the new ACL, regardless of whether the new ACL is a leaf or a
sub-tree. In other words, the ACL itself does not know about its
subsequent negation. We used to _implement_ negation as an ACL flag, but
that [inferior for other reasons] design is long gone IIRC.

* In your proposal, the new ACL X matches/annotates when both the
previous ACLs in the same rule and X subtree match.

* In my proposal, the new ACL X matches when the previous ACLs in the
same rule match.

The latter is simpler.

Please note that one can still achieve your functionality with the
simpler approach by using an all-of ACL:

  acl foo_ annotate_client key=value
  acl foo  all-of sub-tree foo_

Also, if we add a sub-tree to the new ACL, then there is a question of
whether that sub-tree should be an "all-of" or "any-of" tree. Folks will
want both, and some will be confused about the actual support,
regardless of which one we actually support. In the simpler design,
there is no such question/confusion (but folks can still create all-of
and any-of subtrees if needed as the example above illustrates).


> I was thinking to use it like this to annotate:
> 
>  # without annotation
>  http_access allow localhost
> 
>  # with annotation "localhost" when its allowed:
>  acl foo annotate_client key="localhost" localhost
>  http_access allow foo
> 
>  # with annotation "localhost" when its denied
>  http_access deny foo
> 
> 
> Or both of these being equivalent to "deny !localhost" with different
> annotation when it denies:
> 
>  # annotates localhost, denies non-localhost stuff
>  http_access deny !foo
> 
>  # annotates non-localhost and denies non-localhost stuff
>  acl foo2 annotate_client key="non-localhost" !localhost
>  http_access deny foo2

Yes, that would work, but I do not see any _advantages_ this added
complexity buys us, and I see some disadvantages (besides complexity).
One of the disadvantages is that to clearly see what traffic is denied
while reading http_access rules, one would have to use long ACL names,
re-telling the annotating ACL construction:

  # sub-tree approach where it is unclear what is denied
  http_access deny mark

  # sub-tree approach where it is unclear that denied traffic is marked
  http_access deny myLocalhost

  # clear sub-tree approach that duplicates information
  http_access deny localhost_and_mark

  # clear leaf approach
  http_access deny localhost mark


I suggest that we "divide and conquer" by using simplest,
dedicated-to-one-task ACLs. If my arguments above did not convince you,
please detail the advantages of the more complex sub-tree scheme.


Thank you,

Alex.