On 26/02/2013 11:30 a.m., Amos Jeffries wrote:
On 26/02/2013 5:34 a.m., Steve Hill wrote:
Ok, I've had time to clean this patch up... I'm not sure how half my
patch went missing the last time I sent it - I was obviously having a
bad day. :)
The attached patch adds a "spoof_client_ip" fast ACL to control
whether TPROXY
requests have their source IP address spoofed by Squid. The ACL
defaults to allow (i.e. the current normal behaviour), but using an ACL
that results in a deny result will disable spoofing for that request.
Example config (disables spoofing for all requests):
spoof_client_ip deny all
I've implemented the changes suggested by both Alex and Amos.
The patch also does a bit of code-cleanup:
1. The flags.spoofClientIp flag was a general "this is a TPROXY request"
flag, which was a bit confusing given the name of the flag. So the
flags.spoofClientIp flag now only indicates whether we want to spoof the
source IP or not.
2. TPROXY requests now all set flags.interceptTproxy, irrespective of
whether there is going to be any address spoofing.
Hi Steve,
I still have the question about whether it is necessary to have this
as a full-blown ACL test, or if a flag on the http_port is sufficient.
- ACLs are more flexible, but a Fast-ACL lookup only goes halfway
towards supporting all the Slow-ACL things people will be tempted to
use there.
- The main use-cases supporting this as to noted will simply be "deny
all" or "allow all".
- Using this directive makes TPROXY into an equivalent of NAT, and
since there is now NAT in both IPv4 and IPv6 firewalls no benefits
over just using NAT rules in the firewall to begin with.
I'm on the fence for this, but definitely want to see a clear use-case
need before we add yet another ACL lookup to Squid.
Back to the audit:
1) 3.3 is closed to new features now. This will have to be targeted at
3.HEAD for merge.
2) Kinkie and I have since been doing some more code polishing towards
3.4 release. Amongst this was a small rationalization of the PortCfg
mode flags including the spoofClientIp one which is now
PortCfg::flags::tproxyIntercept. Several of the chunks of your patch
doing the re-naming can be removed when targeting 3.HEAD.
3) in src/client_side.cc
- the spoofing decision should probably be made inside
tcpOutgoingAddress() where a lot more transaction state has been
determined.
** this opens up a third implementation posibility - that we extend
tcp_outgoing_address directive with a flag to signal that the IP
listed may be selected for use even when TPROXY is used.
Err. Forget the above request, it won't work.
- for the sslBump fakeRequest please use HttpRequest::Pointer instead
of HttpRequest*X= HTTPMSGLOCK(...); and HTTPMSGUNLOCK(X).
Amos
