Okay, I see I have very misunderstood your proposals.
So snipping away my side-trails...
On 4/12/2015 7:51 p.m., Alex Rousskov wrote:
> On 12/03/2015 10:33 PM, Amos Jeffries wrote:
>> On 4/12/2015 4:03 p.m., Alex Rousskov wrote:
>>> On 12/03/2015 06:10 PM, Amos Jeffries wrote:
>>>> On 4/12/2015 1:02 p.m., Alex Rousskov wrote:
>>>>> In summary, this is a tough choice between:
>
>>>>> #1: Supporting what the admins thought they are configuring.
>>>>> Better configuration interface?
>>>>> More similar to how -i is handled today.
>
>>>>> #2: Backward compatible (but with "Is your config broken?" warnings).
>>>>> A lot less development work.
>
>>>>> How do you think this should be fixed?
>
>>>> I prefer #1, because it is already familiar to admins and does not force
>>>> us all to do a non-backward-compatible manual config update (those as
>>>> you may see below that is not mandatory). We have enough config updates
>>>> happening without this.
>
>
>>> If we do #1, do you think we should ignore the possibility that some
>>> Squids will be broken because Squid interpretation of the configuration
>>> file will change? We can add warnings, but then removing those warnings
>>> will require changing the config...
>
>
> This is the last open question on this thread AFAICT: What to do about
> backward compatibility in approach #1 that you favor?
>
#2 does sound like an attractve short-term fix rather than a good
long-term solution.
But as you state in that one-liner summary of #2 later it would also
involve warnings. And ones we cant get rid of while the -n behaviour
remains unchanged.
We would need to add a completely different flag for the no-rDNS and
"burn" -n for a very long time. Which is somewhat worse.
>
>>>> I know that has issues of its own as side effects on *access complexity.
>>>> Allowing one "name" to be shared between the dstdomain, dstdom_regex,
>>>> and dstdom_str types would simplify those.
>>>> OR, using a namespace sub-type ('_' maybe?) that implies the flagged
>>>> behaviour difference. A renaming dstdom_regex to dstdomain_regex etc.
>>>> under the new model.
>
>>> Sorry, you lost me here completely. Can you give an example?
>
>
>> acl foo dstdomain .example.com
>> acl foo dstdomain 192.0.2.1
>>
>> acl foo dstdomain_str 192.0.2.3
>>
>> http_access deny foo
>>
>> when looking up http://192.0.2.3/ the ACL only performs an exact match.
>> when looking up http://192.0.2.1/ the ACL performs exact-match, then
>> rDNS lookup and match.
>
> AFAICT, the above will perform an rDNS lookup (to match against
> .example.com) in both cases. We are still matching each acl data entry
> in order, right? Did you mean to place the dstdomain_str line first?
>
>
>> Same as the below would do today:
>>
>> acl foo dstdomain .example.com
>> acl foo dstdomain 192.0.2.1
>>
>> acl bar dstdomain -n 192.0.2.3
>>
>> http_access deny foo
>> http_access deny bar
>
>
>> (bar currently needing to exist due to -n's per-ACL not per-line nature).
>
>
> The -n approach is overall better than _str suffix approach because the
> former makes it easy to mix-and-match multiple flags and to support
> flags with values such as -m=delimiters.
>
Good point.
>
>>>> AFAICS it should not be much more difficult to
>>>> re-implement -n using #1 than to re-implement regex parsing and -i to
>>>> implement #2.
>>>
>>> True, but I do not think we should touch the -i interface. It works as
>>> expected.
>>>
>>
>> Then I must have completely misunderstood what your #2 proposal was about.
>
> #2 issues a warning when -n is used on some but not all lines of a given
> ACL foo. No other changes in #2.
>
Which makes that a permanent warning, and a permanent annoyance that
only forces admin to expand the number of named ACLs they use. Which
kind of defeats the purpose of having the flag vs a different ACL type name.
>
>> -i flag operates per-line, and resets at the EOL.
>
> Yes (which, BTW, also breaks the "multiple lines are the same as one
> combined line" design principle).
>
Nod.
>
>> If we remove per-line/value flags then a lot of configs are broken.
>
> Neither #1 nor #2 change how -i works.
>
> #1 makes -n work per line, as intended.
>
> #2 does not change how -n works, but issues a warning if -n is used
> inconsistently across multiple lines.
>
>
>
>> I am proposing option #3 be that we have the flag (or ACL type _suffix)
>> cause parser to generate a similar sub-tree of nodes to what anyof would
>> generate. BUT using only the specific named type of ACL that the line
>> was given.
>>
>>
>> So this:
>> acl foo dstdomain 192.0.2.1
>> acl foo dstdomain -n 192.0.2.3
>>
>> or this:
>> acl foo dstdomain 192.0.2.1
>> acl foo dstdomain_str 192.0.2.3
>>
>>
>> both generates internally a tree identical to:
>>
>> acl foo:1 dstdomain 192.0.2.1
>> acl foo:2 dstdomain -n 192.0.2.3
>> acl foo anyof foo:1 foo:2
>
>
> AFAICT this is the same as optimized #1 I have mentioned earlier (except
> for the _suffix part which I think we should avoid as discussed above).
> To illustrate this even better, you need to add more lines and values:
>
> acl foo dstdomain v1
> acl foo dstdomain -n v2a v2b
> acl foo dstdomain v3
>
> is interpreted after #1 changes as
>
> acl foo1 dstdomain v1
> acl foo2a dstdomain -n v2a
> acl foo2b dstdomain -n v2b
> acl foo3 dstdomain v3
> acl foo anyof foo1 foo2a foo2b foo3
>
> Does this match what you think we should do?
Yes.
> If yes, what to do about
> backward compatibility? Break it because -n is not that widely used?
Yes. With a +n flag (akin to the +i, re-enabling what -n disabled) we
can follow -i/+i pattern with an auto-reset at the end of each line but
add a warning if the +n is omitted on the next one.
The internal implementation of that could be a node listing values to
check pre-rDNS and a node listing values only checked post-rDNS.
The message I have been getting about back-compat seems to be that
admins are relatively okay with pushing out config changes after the
fact, so long as headless upgrades dont actually self-abort during the
upgrade process itself and leave the proxy shutdown.
Much as I hate having a proxy suddenly start behaving differently than
expected it seems to be what the vocal others are saying they prefer (ew).
From a security standpoint changing the behaviour of -n on line endings
is borderline. Depending on whether the ACL is used as whitelist or
blacklist it might have CVE-serious implications. But we avoid CVE
assignment by documenting the change as intentional.
Could this be something completed quickly enough for Squid-4?
Amos
_______________________________________________
squid-dev mailing list
squid-dev@lists.squid-cache.org
http://lists.squid-cache.org/listinfo/squid-dev
