Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
ebarnard opened this issue Dec 16, 2016 · 6 comments
Open

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

ebarnard opened this issue Dec 16, 2016 · 6 comments
Labels

Comments

@ebarnard
Copy link

ebarnard commented Dec 16, 2016

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.

@ebarnard ebarnard changed the title FtpS and FtpES connections do not necessarily secure data sockets. FtpS and FtpES connections do not necessarily secure data sockets Dec 16, 2016
@zharris6
Copy link

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

@ebarnard
Copy link
Author

ebarnard commented Dec 16, 2016

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
Copy link
Member

picrap commented Dec 16, 2016

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
Copy link
Member

picrap commented Dec 16, 2016

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).

@picrap picrap added the bug label Dec 16, 2016
@ebarnard
Copy link
Author

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
Copy link
Member

picrap commented Dec 18, 2016

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants