[squid-dev] RFC: Reject repeated same-name annotations

Thu Dec 15 23:56:06 UTC 2022

On 12/15/22 17:27, ngtech1ltd at gmail.com wrote:

> I must admit that I didn't understand it enough to make sense on what
> specific scenario for example it will affect.

Here is a configuration example:

   # Let's mark certain allowed transactions as green and hot:
   acl markCertain annotate_transaction color=green t=hot color=grn
   http_access allow certainSlowAclHere markAsGreen

   # And then use 10.0.0.1 for those certain transactions:
   acl markedAsGreen note color green
   tcp_outgoing_address 10.0.0.1 markedAsGreen

The above will not use 10.0.0.1 for transactions that should use that 
outgoing address because somebody accidentally overwritten the color on 
the annotate_transaction line. We are actually coloring matching 
transactions "grn" instead of "green" now, and the above 
tcp_outgoing_address rule does not match.

Such errors a more difficult to spot when the offending acl line is in a 
different configuration file than the correct acl line. They are even 
more difficult to spot when the problem is hidden in a helper response!

> I assumed that what would happen is that if ... a single response
> will contain multiple notes with the same key these will be
> appended.

No, they will not be appended. Or, to be more precise, they may not be 
appended in every case. The actual results vary depending on the 
annotation source, Squid version, etc. What happens is currently an 
undefined behavior.

> From what I understood until now a single helper that will respond
> with multiple note_1=v note_1=v Will trigger a fatal error

No, bad helper responses will not trigger fatal errors. They will 
trigger non-fatal ERROR messages.

Only misconfigurations (e.g., the above squid.conf example) will be fatal.

> However, if multiple helpers will send both each in it's turn a
> note_1=v these will be appended.

IIRC, annotations from earlier helper responses will be overwritten by 
annotations in the later ones. However, this RFC is _not_ about multiple 
responses, so let's not lose our focus, even if my recollection is 
wrong. :-)

> I agree that the result should be predictable however if logs can
> help to trace the issue I believe it's predicted enough to not say
> about the current situation "un-predictable".

Currently, ALL,1 cache.log is silent about same-name annotations in many 
cases. Debugging cache.log does not count, of course (and it would be 
rather difficult to triage these problems in a busy debugging cache.log 
anyway). As far as Squid administration is concerned, the biggest 
trouble is not in figuring out where the problem is. It is knowing that 
there is a problem (e.g., that users that should be blocked are actually 
allowed when everybody seems "happy").

Also, the value of this RFC is not just in making Squid safer. It also 
helps with enhancing Squid code, adding features. Right now, if Bob 
tries to add a feature involving annotations, Alice may shoot it down 
because the new code does not handle same-name annotations the same way 
as the code Alice happens to know about (while Bob is copying another 
piece of code that handles similar situation differently or inventing 
his own thing).

We need to fix this mess from both development and administration point 
of view.

Hope this clarifies,

Alex.

> -----Original Message-----
> From: squid-dev <squid-dev-bounces at lists.squid-cache.org> On Behalf Of Alex Rousskov
> Sent: Thursday, 15 December 2022 23:30
> To: Squid Developers <squid-dev at lists.squid-cache.org>
> Subject: [squid-dev] RFC: Reject repeated same-name annotations
> 
> Hello,
> 
>       I propose to adjust Squid code to reject repeated same-name
> annotations from each and every source that supplies annotations:
> 
> * "note" directive
> * adaptation_meta directive
> * annotate_transaction ACL [1]
> * annotate_client ACL [1]
> * adaptation services responses (eCAP and ICAP)
> * helper responses
> 
> If this RFC is approved: A configuration that contains a directive with
> repeated same-name annotations will be rejected with a fatal ERROR[2]. A
> helper or service response that contains repeated same-name annotations
> will trigger a non-fatal (to Squid or transaction) cache.log ERROR[2].
> 
> 
> Currently, Squid treats repeated same-name annotations inconsistently.
> Depending on the annotation source, Squid processing code may
> 
> * use the first same-name annotation and ignore repetitions
> * use the last same-name annotation and ignore repetitions
> * use all same-name annotations, honoring repetitions
> 
> These inconsistencies make it difficult to improve/enhance/optimize
> Squid code, while Squid ignorance hides misconfigurations and
> helper/service implementation bugs, including problems that may be
> related to access controls and other sensitive matters.
> 
> 
> Any objections or better ideas?
> 
> 
> Thank you,
> 
> Alex.
> 
> [1] In this context, we are talking about same-name annotations
> mentioned in the corresponding ACL _configuration_ (i.e. all "acl"
> directives with a given ACL name). A repeated _computation_ of
> annotate_foo ACL will continue to deal with same-name annotations as
> documented -- a "name+=value" configuration will continue to append
> values to the existing same-name annotation, while a "name=value"
> configuration will continue to overwrite any existing same-name annotation.
> 
> [2] Repeated same-name annotations that all have identical _values_ will
> be flagged with a WARNING instead. Some overly simplistic configuration
> generators, complicated configurations build from many include files,
> and dumb helpers/services might generate repeated same-everything
> annotations. Since such repetitions can be _safely_ ignored (honoring
> just one name=value pair among all the identical ones), we do not have
> to reject the configuration or log an ERROR because of them.
> _______________________________________________
> squid-dev mailing list
> squid-dev at lists.squid-cache.org
> http://lists.squid-cache.org/listinfo/squid-dev
> 
> _______________________________________________
> squid-dev mailing list
> squid-dev at lists.squid-cache.org
> http://lists.squid-cache.org/listinfo/squid-dev