[squid-dev] A new 'has' ACL

[Alex Rousskov]

N.B. The discussion from here until the "----" marker is probably
unimportant because you later seem to confirm that "has ALE" is needed.
I am preserving this discussion in case I misjudged your confirmation
and/or in case this discussion will help resolve the remaining confusion
(after the marker).

On 05/02/2017 04:18 PM, Eduard Bagdasaryan wrote:
> On 02.05.2017 23:53, Alex Rousskov wrote:
>> AFAICT, r14752 deals with transactions originated by client requests and
>> broken client connections. Even if all those transactions are fully
>> covered, there are other transactions (e.g., ESI and Downloader
>> requests) that may or may not be covered (and seem to be completely
>> unrelated to r14752 changes!). Is there some kind of common interface
>> that essentially requires all transactions to have ALE now?

> I have not found any ALE manipulations inside ESI and Downloader
> code, so that is probably unrelated.

Not sure I understand. Are you implying that Squid-initiated
transactions can never evaluate an ACL that requires ALE? Consider a
Downloader job that initiates a transaction which attempts to access
Squid cache, evaluating various caching directives, some of which may
use an external ACL that needs an ALE. If that code does not give
FilledChecklist an ALE, the admin will see warnings that "has ALE" is
meant to work around.

(The Downloader is used above as an example of a Squid-initiated
transaction -- I am not claiming that Downloader lacks ALE; based on
your research it does have ALE. This example illustrates that
transaction initiator code does not need to explicitly manipulate ALE to
trigger ALE-related ACL checks down the road).


> ClientHttpRequest is the object used in transactions,

Do all transactions (that may use ACLs) create a corresponding
ClientHttpRequest object? Downloader does. I do not think ESI does; it
seems to keep a link to the HTTP ClientHttpRequest object but I do not
know how that is used when it comes to ESI-triggered ACL checks. There
are also ICP/HTCP transactions, eCAP/ICAP transactions, netdb exchanges,
and Cache Digest transactions. There may be others. At least some of
them use ACLs or trigger ACLs use elsewhere.


> (including
> Downloader and ESI code). This object owns its ALE, which is
> created in the constructor. I have not found 'common interface'
> you are talking about, but if all Squid transactions require
> ClientHttpRequest object (which is probably true)

Why do you think it is probably true? What makes ClientHttpRequest
required for any code that might use ACLs?


> then they surely will have the corresponding ALE.

Even if that is true, "having" ALE and "supplying every FilledChecklist
with ALE" are two different things, right? Can we be sure that all that
code configures FilledChecklist with ALE?

To avoid misunderstanding, these are genuine questions -- I really do
not know the answers. I want to drop "has ALE" support, but you have not
convinced me that we can do that right now.


>> "The current ACL-driven code may feed an ACLFilledChecklist object
>> without ALE to an (ACLExternal) ACL that requires ALE."

> Yes.

OK, so you confirm that there is a problem and the remaining question is
whether we can fix it quickly (and drop "has ALE").


>> The question is whether we can "fix this" (i.e., guarantee correct ALE
>> presence when ALE is needed) _quickly_.

> I agree that it is possible large project, because I am not
> sure that every ACL checking context have its ALE near at hand.

OK, so we keep "has ALE".

Marker: --------


> ... and have our new 'has' ACL broken from the very beginning too.
> As I tried to explain earlier, how can we rely on
> ACLFilledChecklist::hasAle(), if the ALE itself have not been provided to
> ACLFilledChecklist before?

I do not understand your concern. If ALE has not been provided to
ACLFilledChecklist, then ACLFilledChecklist::hasAle() returns false and
our "has ALE" ACL does not match, allowing the admin to avoid the
warning about the missing ALE while making sure her ACL-driven
directives do what they are supposed to do, even though there is no ALE.
Is not that exactly what we want? What "breakage" do you anticipate with
"has ALE"?

For example, before/without "has ALE", the following directive might
produce numerous warnings and wrong Squid behavior in environments where
Squid evaluates store_miss for transactions that did not supply ALE to
FilledChecklist:

    store_miss deny foo

With "has ALE" in her toolbox, the admin can work around the problem:

    # option 1: store misses when ALE is missing
    store_miss allow !hasAle
    store_miss deny foo

    # option 2: do not store misses when ALE is missing
    store_miss deny !hasAle
    store_miss deny foo

(the choice is up to the admin -- we do not know what is the right
workaround in her environment)


Thank you,

Alex.