Re: FTPS with passive mode is slow

2009-08-09 Thread Sai Pullabhotla
I did notice anywhere from 20 to 50 milliseconds improvement for each
data connection creation. The fix is checked in now to the trunk.

Regards,
Sai Pullabhotla
www.jMethods.com




On Wed, Aug 5, 2009 at 3:15 PM, Niklas Gustavssonnik...@protocol7.com wrote:
 On Wed, Aug 5, 2009 at 10:01 PM, Sai
 Pullabhotlasai.pullabho...@jmethods.com wrote:
 [snip]

 I know we may not notice much time difference with this change, but
 every little things add up, especially under high loads.

 Did you see any performance improvements in your local copy? Anyways,
 I think this makes sense, again for 1.1.X.

 /niklas



Re: FTPS with passive mode is slow

2009-08-09 Thread Niklas Gustavsson
On Sun, Aug 9, 2009 at 8:10 PM, Sai
Pullabhotlasai.pullabho...@jmethods.com wrote:
 I did notice anywhere from 20 to 50 milliseconds improvement for each
 data connection creation. The fix is checked in now to the trunk.

Great work, thanks!

/niklas


Re: FTPS with passive mode is slow

2009-08-05 Thread Sai Pullabhotla
Okay, done!

I also have another question around the same code...Should we be
checking the remote address and make sure it matches with the IP
address of the remote host on the control connection. If we do not do
this check, it is possible for a hacker to connect to this port before
the original client and may gain access to the data? I know it is not
very easy to do this, but just in case. What do you think?


Sai Pullabhotla
www.jMethods.com




On Tue, Aug 4, 2009 at 3:48 PM, Niklas Gustavssonnik...@protocol7.com wrote:
 I believe this problem has been reported multiple times. Please open a
 JIRA and apply the patch, it makes perfect sense.

 /niklas

 On Tue, Aug 4, 2009 at 10:02 PM, Sai
 Pullabhotlasai.pullabho...@jmethods.com wrote:
 I've been noticing that the passive data connections are taking quite
 some time when using SSL. I finally got some time to look into this
 and noticed the following while debugging through the code. This issue
 might have been introduced with the fix we put in for FTPSERVER-241.

 The code that wraps the plain socket into an SSL socket uses the
 following line:

                    SSLSocket sslSocket = (SSLSocket) ssocketFactory
                            .createSocket(serverSocket,
 serverSocket.getInetAddress().getHostName(),
                                    serverSocket.getPort(), true);

 Based on the JavaDocs, the InetAddress.getHostName() performs a
 reverse name look up, which was taking about 1.5 seconds on every
 system on our network. I'm not sure if this is an issue with the way
 our network is setup. Some one please let me know if this in fact an
 issue with our network.

 We are not seeing this lag when client and server are running on the
 same system. Things work too fast in this case, probably because the
 system knows very well about itself.

 Just to try it out, I changed the code to simply use the IP address
 rather than the host name, and I was able to get rid of the lag and
 things seem to be working much faster. Below is the change to the
 above line:

                    SSLSocket sslSocket = (SSLSocket) ssocketFactory
                            .createSocket(serverSocket,
 serverSocket.getInetAddress().getHostAddress(),
                                    serverSocket.getPort(), true);

 Could some one test the current code base with client and server
 running on different systems and tell me if they notice the lag when
 creating the passive data connection. If this can be reproduced on one
 of your environments, we should probably put the above fix. I don't
 think this suggested fix should cause any other issues, do you?

 Regards,

 Sai Pullabhotla
 www.jMethods.com




Re: FTPS with passive mode is slow

2009-08-05 Thread Niklas Gustavsson
On Wed, Aug 5, 2009 at 2:41 PM, Sai
Pullabhotlasai.pullabho...@jmethods.com wrote:
 I also have another question around the same code...Should we be
 checking the remote address and make sure it matches with the IP
 address of the remote host on the control connection. If we do not do
 this check, it is possible for a hacker to connect to this port before
 the original client and may gain access to the data? I know it is not
 very easy to do this, but just in case. What do you think?

I think this makes sense. We already do the logically same for active
connections. Probably only should apply this to the 1.1.X (trunk)
code, right?

/niklas


Re: FTPS with passive mode is slow

2009-08-05 Thread Sai Pullabhotla
I'm not sure, I think it should go to the main trunk as well.

Sai Pullabhotla
www.jMethods.com




On Wed, Aug 5, 2009 at 9:31 AM, Niklas Gustavssonnik...@protocol7.com wrote:
 On Wed, Aug 5, 2009 at 2:41 PM, Sai
 Pullabhotlasai.pullabho...@jmethods.com wrote:
 I also have another question around the same code...Should we be
 checking the remote address and make sure it matches with the IP
 address of the remote host on the control connection. If we do not do
 this check, it is possible for a hacker to connect to this port before
 the original client and may gain access to the data? I know it is not
 very easy to do this, but just in case. What do you think?

 I think this makes sense. We already do the logically same for active
 connections. Probably only should apply this to the 1.1.X (trunk)
 code, right?

 /niklas



Re: FTPS with passive mode is slow

2009-08-05 Thread Sai Pullabhotla
I'm also wondering if we could perform better with SSL, if we cache
the SSLContext and SSLSocketFactory. Currently, every data connection
creates the SSLContext (even though the context parameters are the
same) and gets the SocketFacotry from the context. Instead, we should
create the SSLContext and SSLSocketFactory once per each
SslConfiguration and reuse it. Obviously, this reduces the unnecessary
object creation and eliminates the unnecessary method calls and
context initialization. I've made some changes locally and things seem
to be working fine. If you think we should apply this suggested
change, I can post the patch for review. Basically, the changes we
would need are:

Add the following new method to the SslConfiguration interface -
SSLSocketFactory getSocketFactory() throws GeneralSecurityException;

Implement the above method method in the DefaultSslConfiguration.

When constructing the DefaultSslConfiguration, create the SSLContext
and an SSLSocketFactory and save them as members.

Then when we need a SSLSocket, we simply call
SSLConfiguration.getSocketFactory() which returns the pre-created
socket factory.

I know we may not notice much time difference with this change, but
every little things add up, especially under high loads.

Let me know what you think.

Regards,

Sai Pullabhotla
www.jMethods.com




On Wed, Aug 5, 2009 at 10:05 AM, Sai
Pullabhotlasai.pullabho...@jmethods.com wrote:
 I'm not sure, I think it should go to the main trunk as well.

 Sai Pullabhotla
 www.jMethods.com




 On Wed, Aug 5, 2009 at 9:31 AM, Niklas Gustavssonnik...@protocol7.com wrote:
 On Wed, Aug 5, 2009 at 2:41 PM, Sai
 Pullabhotlasai.pullabho...@jmethods.com wrote:
 I also have another question around the same code...Should we be
 checking the remote address and make sure it matches with the IP
 address of the remote host on the control connection. If we do not do
 this check, it is possible for a hacker to connect to this port before
 the original client and may gain access to the data? I know it is not
 very easy to do this, but just in case. What do you think?

 I think this makes sense. We already do the logically same for active
 connections. Probably only should apply this to the 1.1.X (trunk)
 code, right?

 /niklas




Re: FTPS with passive mode is slow

2009-08-05 Thread Niklas Gustavsson
On Wed, Aug 5, 2009 at 5:05 PM, Sai
Pullabhotlasai.pullabho...@jmethods.com wrote:
 I'm not sure, I think it should go to the main trunk as well.

Trunk is 1.1.X so we're saying the same thing :-). I don't think it
should go into 1.0.x.

/niklas


Re: FTPS with passive mode is slow

2009-08-05 Thread Niklas Gustavsson
On Wed, Aug 5, 2009 at 10:01 PM, Sai
Pullabhotlasai.pullabho...@jmethods.com wrote:
[snip]

 I know we may not notice much time difference with this change, but
 every little things add up, especially under high loads.

Did you see any performance improvements in your local copy? Anyways,
I think this makes sense, again for 1.1.X.

/niklas


FTPS with passive mode is slow

2009-08-04 Thread Sai Pullabhotla
I've been noticing that the passive data connections are taking quite
some time when using SSL. I finally got some time to look into this
and noticed the following while debugging through the code. This issue
might have been introduced with the fix we put in for FTPSERVER-241.

The code that wraps the plain socket into an SSL socket uses the
following line:

SSLSocket sslSocket = (SSLSocket) ssocketFactory
.createSocket(serverSocket,
serverSocket.getInetAddress().getHostName(),
serverSocket.getPort(), true);

Based on the JavaDocs, the InetAddress.getHostName() performs a
reverse name look up, which was taking about 1.5 seconds on every
system on our network. I'm not sure if this is an issue with the way
our network is setup. Some one please let me know if this in fact an
issue with our network.

We are not seeing this lag when client and server are running on the
same system. Things work too fast in this case, probably because the
system knows very well about itself.

Just to try it out, I changed the code to simply use the IP address
rather than the host name, and I was able to get rid of the lag and
things seem to be working much faster. Below is the change to the
above line:

SSLSocket sslSocket = (SSLSocket) ssocketFactory
.createSocket(serverSocket,
serverSocket.getInetAddress().getHostAddress(),
serverSocket.getPort(), true);

Could some one test the current code base with client and server
running on different systems and tell me if they notice the lag when
creating the passive data connection. If this can be reproduced on one
of your environments, we should probably put the above fix. I don't
think this suggested fix should cause any other issues, do you?

Regards,

Sai Pullabhotla
www.jMethods.com


Re: FTPS with passive mode is slow

2009-08-04 Thread Niklas Gustavsson
I believe this problem has been reported multiple times. Please open a
JIRA and apply the patch, it makes perfect sense.

/niklas

On Tue, Aug 4, 2009 at 10:02 PM, Sai
Pullabhotlasai.pullabho...@jmethods.com wrote:
 I've been noticing that the passive data connections are taking quite
 some time when using SSL. I finally got some time to look into this
 and noticed the following while debugging through the code. This issue
 might have been introduced with the fix we put in for FTPSERVER-241.

 The code that wraps the plain socket into an SSL socket uses the
 following line:

                    SSLSocket sslSocket = (SSLSocket) ssocketFactory
                            .createSocket(serverSocket,
 serverSocket.getInetAddress().getHostName(),
                                    serverSocket.getPort(), true);

 Based on the JavaDocs, the InetAddress.getHostName() performs a
 reverse name look up, which was taking about 1.5 seconds on every
 system on our network. I'm not sure if this is an issue with the way
 our network is setup. Some one please let me know if this in fact an
 issue with our network.

 We are not seeing this lag when client and server are running on the
 same system. Things work too fast in this case, probably because the
 system knows very well about itself.

 Just to try it out, I changed the code to simply use the IP address
 rather than the host name, and I was able to get rid of the lag and
 things seem to be working much faster. Below is the change to the
 above line:

                    SSLSocket sslSocket = (SSLSocket) ssocketFactory
                            .createSocket(serverSocket,
 serverSocket.getInetAddress().getHostAddress(),
                                    serverSocket.getPort(), true);

 Could some one test the current code base with client and server
 running on different systems and tell me if they notice the lag when
 creating the passive data connection. If this can be reproduced on one
 of your environments, we should probably put the above fix. I don't
 think this suggested fix should cause any other issues, do you?

 Regards,

 Sai Pullabhotla
 www.jMethods.com