Re: [squid-dev] RFC: Reject repeated same-name annotations
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 On Behalf Of Alex Rousskov Sent: Thursday, 15 December 2022 23:30 To: Squid Developers 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 dea
Re: [squid-dev] RFC: Reject repeated same-name annotations
Hey Alex, I must admit that I didn't understand it enough to make sense on what specific scenario for example it will affect. We have a set of sources for a "note" ie: * "note" directive * adaptation_meta directive * annotate_transaction ACL [1] * annotate_client ACL [1] * adaptation services responses (eCAP and ICAP) * helper responses The only one I have used until today is the helpers and maybe once more with an ICAP service. I couldn't make sense the 1+2 appended comments with the rejection RFC. I assumed that what would happen is that if multiple helpers for example will use the same note or a single response will contain multiple notes with the same key these will be appended. The last time I have seen this I assumed it's in an array fashion ie multiple values compared to a single string. I do not remember the subject enough but what I do remember is that there was an issue with the comparison/check of a note ACL when two values were applied with the same name. >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 and I have mixed feelings about it. However, if multiple helpers will send both each in it's turn a note_1=v these will be appended. 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". I would say that since it's not a "sync" engine which the timing belt must not miss a "beat" or a "tooth" It's an async engine which is far more complex. I hope I understood the RFC and what's above it so my words will make sense of themselves. Eliezer Eliezer Croitoru NgTech, Tech Support Mobile: +972-5-28704261 Email: ngtech1...@gmail.com Web: https://ngtech.co.il/ My-Tube: https://tube.ngtech.co.il/ -Original Message- From: squid-dev On Behalf Of Alex Rousskov Sent: Thursday, 15 December 2022 23:30 To: Squid Developers 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] 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
