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 _______________________________________________ squid-dev mailing list [email protected] http://lists.squid-cache.org/listinfo/squid-dev
