search

ArxOne / FTP

Simple FTP client
MIT License
37 stars 15 forks source link

FtpS and FtpES connections do not necessarily secure data sockets #35

Open ebarnard opened 7 years ago

ebarnard commented 7 years ago

If you set ChannelProtection to contain FtpProtection.DataChannel the library never explicitly informs the server of this.

Some servers do not by default use SSL on the data channel even if it is being used on the command channel.

This can result in the server sending unencrypted data which is picked up by System.Net.Security.SslStream.AuthenticateAsClient and causes the exception seen above as the data is not a valid SSL handshake.

FtpSession.CheckProtection should, if State["PROT"] does not equal the desired protection level, issue a PROT command and fail on a non 2xx response code. State["PROT"] should not initially be set on a new connection.

I'm currently using the below as a temporary fix:

if (client.SendSingleCommand("PROT", "P").Code.Code != 200)
    throw new Exception("Could not enable data channel encryption.");

It appears the library also doesn't issue a PBSZ command which is apparently required by https://tools.ietf.org/html/rfc2228.

Other libraries seem to use PBSZ 0 successfully.

Unrelated to #32, as it turns out.

zharris6 commented 7 years ago

So it sounds like we need to send these commands when SSL is required? PBSZ 0 PROT P

ebarnard commented 7 years ago

I think that's right, and much simpler.

PBSZ 0 seems a bit odd but the idea of the protected buffer doesn't make much sense with SSL.

picrap commented 7 years ago

There is a FtpSession.CheckProtection invoked every time a data channel is created. I don't understand where it fails. The method then uses FtpSession.CheckSessionParameter, which tests two thinkgs:

  1. The PROT command is said to be supported by server (returned by reply to FEAT command)
  2. The PROT is invoked only once per connection. And when reading this, I think that the point 2 may be the source of the problem.
picrap commented 7 years ago

The version 1.12 (just released) does PBSZ 0 before a PROT P. However I could not find documentation saying that these commands had to be sent everytime a protected data channel is created on the same connection (FtpSession).

ebarnard commented 7 years ago

They should only be sent once per session.

I didn't realise setting State had side effects.

I think the server I was using didn't report PROT as a capability so no command was sent. Unfortunately I no longer have access to that server to test.

It might be worth sending PBSZ and PROT even if servers don't report the capability but not failing is they return errors.

Feel free to close as I can no longer test this.

picrap commented 7 years ago

I keep the issue open, and I'll add a flag to force PBSZ/PROT commands on server which don't explicitly say they are available.