On 12/15/22 17:27, ngtech1...@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-boun...@lists.squid-cache.org> On Behalf Of Alex
Rousskov
Sent: Thursday, 15 December 2022 23:30
To: Squid Developers <squid-dev@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@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev
_______________________________________________
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev
_______________________________________________
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev
