[squid-dev] [PATCH] Native FTP relay for active FTP

[Amos Jeffries]

On 15/02/2017 7:04 p.m., Alex wrote:
> Thank you. Actually, r12742.1.41 was a bit strange because it covered
> only one case and caused errors for other two (despite the idea was
> right).
> 
> I understood why you said that FTP relay was reported to be working:
> forward mode was doing well and proxy-aware FTP client did the trick.
> However, I tried to use the feature in interception mode. Docs said
> that it should work too, but it appeared that it's not that easy.
> 

Sorry I was not able to reply earlier. You seem to have sorted out the
cases anyhow. This is just to clarify the actual reasons behind the cases:


* T(ransparent )PROXY

The ctrl->local IP should be used (transparency). It is a remote IP
which Squid is allowed to spoof bind() via the *_TRANSPARENT flags.


* FTP explicit/forward proxy

The ctrl->local IP should be used. The client is talking directly to
Squid and it is the IP which the client knows about. The
COMM_TRANSPARENT flag is *not* set, but as we are not spoofing the
bind() should work.


==> The else-condition handles both these cases. The COMM_TRANSPARENT
being passed only if set should handle the TPROXY vs explicit-proxy
differences cleanly in comm_bind().


* NAT intercept proxy

The ctrl->local IP must not be used. It is a remote IP and Squid is not
permitted to spoof/bind() it. That leaves no choice but to rely on
OS-assigned address. However, we can/should ensure the OS selects an IP
with v4/v6 type matching the ctrl->local type - since we know the client
can speak that IP version.

==> The if(INTERCEPTION)-condition handles this case.


Hope that clarifies the things.

IMO, NAT intercept only works reliably with passive-FTP. Active might
work _if_ the client is happy to use [very] different IPs for ctrl and
data channels. Since that is something which will vary by implementation
YMMV, but this patch gives it a better chance of success than status-quo.

We might be able to figure out something more complex for multi-IP
machines involving tcp_outgoing_address or a new similar directive. But
that is out of scope for this patch IMO.

+1. The latest patch looks like correct code to me. If you are happy
with it too Alex please apply. Please consider using the above text in
the new if-else comments.

Amos