[squid-dev] [PATCH] Ignore impossible SSL bumping actions, as intended and documented / bug 4237 fix

[Tsantilas Christos]

According to Squid wiki: "Some actions are not possible during  certain 
processing steps. During a given processing step, Squid ignores ssl_bump 
lines with impossible actions". The distributed squid.conf.documented 
has similar text.

Current Squid violates the above rule. Squid considers all actions, and 
if an impossible action matches first, Squid guesses what the true 
configuration intent was. Squid may guess wrong. For example, depending 
on the transaction, Squid may guess that a matching  stare or peek 
action during bumping step3 means "bump", breaking peeked connections 
that cannot be bumped.

This unintended but gross configuration semantics violation remained 
invisible until bug 4237, probably because most configurations in most 
environments either worked around the problem (where admins experimented 
to "make it work") or did not result in visible errors (where Squid 
guesses did not lead to terminated connections).

While configuration workarounds are possible, the current 
implementation is very wrong and leads to overly complex and, hence, 
often wrong configurations. It is also nearly impossible to document 
accurately because the guessing logic depends on too many factors.

To fix this, we add an action filtering/banning mechanism to Squid ACL 
code. This mechanism is then used to:
   - ban client-first and server-first on bumping steps 2 and 3.
   - ban peek and stare actions on bumping step 3.
   - ban splice on step3 if stare is selected on step2 and
     Squid cannot splice the SSL connection any more.
   - ban bump on step3 if peek is selected on step2 and
     Squid cannot bump the connection any more.

The same action filtering mechanism may be useful for other ACL-driven 
directives with state-dependent custom actions.

This change adds a runtime performance overhead of a single virtual 
method call to all ORed ACLs that do not use banned actions. That method 
itself just returns false unless the ACL represents a whole directive 
rule. In the latter case, an std::vector size() is also checked. It is 
possible to avoid this overhead by adding a boolean "I may ban actions" 
flag to Acl::OrNode, but we decided the small performance harm is not 
worth the extra code to set that flag.

This is a Measurement Factory project
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ignore-bumping-actions-t4.patch
Type: text/x-patch
Size: 24443 bytes
Desc: not available
URL: <http://lists.squid-cache.org/pipermail/squid-dev/attachments/20150810/64dd2753/attachment.bin>