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

2017-02-18 Thread Amos Jeffries
On 17/02/2017 12:08 p.m., Alex Rousskov wrote: > On 02/16/2017 02:48 AM, Amos Jeffries wrote: >> +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. > > I am still OK with this

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

2017-02-16 Thread Alex Rousskov
On 02/16/2017 02:48 AM, Amos Jeffries wrote: > +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. I am still OK with this patch going in, but you should be the one committing it

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

2017-02-16 Thread 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

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

2017-02-14 Thread Alex
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

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

2017-02-14 Thread Alex Rousskov
On 02/14/2017 02:39 PM, Alex wrote: > Should be a bit better now. Thank you for covering all three cases and overcoming your conviction that one of them does not exist and that r12742.1.41 is bogus/useless. The IPv4 part still looks bad to me, but I do not think that _you_ have to be responsible

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

2017-02-14 Thread Alex
Also, r12742.1.41 works in proxy mode because data connection originates from local address, this case simply does not require COMM_TRANSPARENT. The only thing the patch may break in this case is when squid box has more than one suitable address. Probably this case should be addressed.14.02.2017,

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

2017-02-14 Thread Alex
The problem is that r12742.1.41 is bogus and useless at the same time due to the following reasons:1. To bind some non-local IP address one needs COMM_TRANSPARENT flag which will trigger IP_TRANSPARENT socket option. The flag was not set, therefore attempts to bind any non-local address are bound

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

2017-02-14 Thread Alex Rousskov
On 02/14/2017 12:25 PM, Alex wrote: > It seems that the patch doesn't make things worse (if I understood you > correctly). AFAICT, you are not testing the use case that prompted [trunk] revision 12742.1.41 changes. There are three possibilities: 1. r12742.1.41 changes were bogus/useless. 2.

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

2017-02-14 Thread Alex
  14.02.2017, 21:50, "Alex Rousskov" :When I am talking about the forwarding case, I am talking about a simpleconfiguration without any redirection or other TCP/IP level tricks oreven reverse proxing of any sort: The FTP client sends FTP requestsdirectly to

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

2017-02-14 Thread Alex
Added some clarifications.committer: Alexander Gozman timestamp: Tue 2017-02-14 21:46:28 +0300 message: Native FTP relay: NAT and TPROXY interception fixes. diff: === modified file 'src/servers/FtpServer.cc' --- src/servers/FtpServer.cc 2017-01-01 00:14:42 + +++

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

2017-02-14 Thread Alex Rousskov
On 02/14/2017 10:38 AM, Alex wrote: >>> + if (clientConnection->flags & COMM_TRANSPARENT) { >>> + conn->setAddrs(clientConnection->local, cltAddr); >>> + conn->flags |= COMM_TRANSPARENT; >>> + } else { >>> + // In case of NAT interception ... >> Are there really just two cases here (tproxy

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

2017-02-14 Thread Alex
14.02.2017, 19:06, "Alex Rousskov" : >>  + if (clientConnection->flags & COMM_TRANSPARENT) { >>  + conn->setAddrs(clientConnection->local, cltAddr); >>  + conn->flags |= COMM_TRANSPARENT; >>  + } else { >>  + // In case of NAT interception ... > > Are there

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

2017-02-14 Thread Alex Rousskov
On 02/14/2017 03:44 AM, Alex wrote: > The attached patch allows FTP relay to work in NAT interception mode > and also fixes IP address binding in TPROXY mode. Thank you for working on this bug. NAT/TPROXY address manipulation is not my area of expertise, but I have one higher level concern and a

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

2017-02-14 Thread Alex
Hello. Recently I've noticed some issues in native FTP relay that made it unusable with active FTP mode (data connection was not established): http://lists.squid-cache.org/pipermail/squid-users/2017-February/014404.html The attached patch allows FTP relay to work in NAT interception mode and