]> git.ipfire.org Git - thirdparty/squid.git/commit
Prohibit bad --enable-linux-netfilter combinations (#1893)
authorAlex Rousskov <rousskov@measurement-factory.com>
Sat, 5 Oct 2024 16:18:33 +0000 (16:18 +0000)
committerSquid Anubis <squid-anubis@squid-cache.org>
Sun, 6 Oct 2024 22:29:36 +0000 (22:29 +0000)
commit399990a71bf992654897404bed3ca8fdc217ceed
tree2be7659ec18add98901a858aa32d4ad4c6bc6109
parent620efbd5907753d01a1b764b9275ef4753b03399
Prohibit bad --enable-linux-netfilter combinations (#1893)

Since 2009 commit 51f4d36b, Squid declared that "all transparent build
options are independent, and may be used in any combination". That
declaration was accurate from a "successful build" point of view, but
Ip::Intercept::LookupNat() (and its precursors) started mishandling at
least two combinations of options as detailed below.

LookupNat() tries up to four lookup methods (until one succeeds):

1. NetfilterInterception(): --enable-linux-netfilter; SO_ORIGINAL_DST
2. IpfwInterception(): --enable-ipfw-transparent; getsockname()
3. PfInterception(): --enable-pf-transparent; getsockname() or /dev/pf
4. IpfInterception(newConn): --enable-ipf-transparent; ioctl(SIOCGNATL)

The first method -- NetfilterInterception() -- fails to look up the true
destination address of an intercepted connection when, for example, the
client goes away just before the lookup. In those (relatively common in
busy environments) cases, the intended destination address cannot be
obtained via getsockname(), but LookupNat() proceeds calling other
methods, including the two methods that may rely on getsockname().

Methods 2 and 3 may rely on a previous getsockname() call to provide
true destination address, but getsockname() answers are not compatible
with what NetfilterInterception() must provide -- the destination
address returned by getsockname() is Squid's own http(s)_port address.
When Squid reaches problematic code now encapsulated in a dedicated
UseInterceptionAddressesLookedUpEarlier() function, Squid may end up
connecting to its own http(s)_port! Such connections may cause
forwarding loops and other problems. In SslBump deployments, these loops
form _before_ Forwarded-For protection can detect and break them.

These problems can be prevented if an admin does not enable incompatible
combinations of interception lookup methods. The relevant code is
correctly documented as "Trust the user configured properly", but that
statement essentially invalidates our "may be used in any combination"
design assertion and leads to runtime failures when user configured
improperly. Those runtime failures are difficult to triage because they
lack signs pointing to a build misconfiguration.

This change bans incompatible NetfilterInterception()+getsockname()
combinations at compile time: Squid ./configured with
--enable-linux-netfilter cannot use --enable-ipfw-transparent or
(--enable-pf-transparent --without-nat-devpf).

TODO: Ban incompatible combinations at ./configure time as well!

We have considered and rejected an alternative solution where all
./configure option combinations are still allowed, but LookupNat()
returns immediately on NetfilterInterception()/SO_ORIGINAL_DST failures.
That solution is inferior to build-time bans because an admin may think
that their Squid uses other configured lookup method(s) if/as needed,
but Squid would never reach them in --enable-linux-netfilter builds.

The only viable alternative is to specify lookup methods in squid.conf,
similar to the existing "tproxy" vs. "intercept" http(s)_port modes. In
that case, squid.conf will be validated for incompatible method
combinations (if combinations are supported at all), and LookupNat()
will only use configured method(s).
src/ip/Intercept.cc
src/ip/Intercept.h
test-suite/buildtests/layer-02-maximus.opts
test-suite/buildtests/layer-04-noauth-everything.opts