libunftp breaks PHP ftp client when it won't allow PROT and PBZS without authenticating first.

[jrouaix]

Hello, I have some troubles with a php FTP client connection.

Implemented a kind of ftp file to http api adapter thanks to you folks thank you,
authenticator and backend working very fine !

Deployed in production with a let's encrypt certificates, again piece of cake, thank you !
Perfect connection with filezilla, passive mode, perfect.

Now my customers try it with PHP ftp client and ... ACTIVE mode is not supported - use PASSIVE instead.
https://github.com/bolcom/libunftp/blob/master/src/server/controlchan/commands/port.rs

Interestingly, it does work fine when we try a non TLS connection, then we try ftp_ssl_connect and ...

 INFO  libunftp::server::controlchan::log            > Control channel event Command(Port), source: x.x.x.x:42058, trace-id: 0x186277850b1cba9d, seq: 11
 INFO  libunftp::server::controlchan::log            > Control channel reply CodeAndMsg { code: CommandNotImplemented, msg: "ACTIVE mode is not supported - use PASSIVE instead" }, source: x.x.x.x:42058, trace-id: 0x186277850b1cba9d, seq: 11

I can't imagine I find a bug in the ftp protocol handling of PHP ....
Would it exists some way I can allow the PORT command, without forking this repo ?

best regards ...

[jrouaix]

Perhaps it could just return a CommandOkayNotImplemented (202) instead of CommandNotImplemented (502) ... will try in a fork ...

[hannesdejager]

Hi :-). Its only a pleasure. We haven’t implemented PORT yet unfortunately as you’ve seen. It is interesting though that it works with plaintext. I think I might have seen this before. If you can possibly give some of the php code that cause this so we can reproduce then I can look into it.

[jrouaix]

yep :

<?php
ini_set('display_errors',1);
error_reporting(E_ALL|E_STRICT);

$ftp_server = "ftp.mydomain.com";

$conn_id = ftp_ssl_connect($ftp_server, 2123);
print("ftp_ssl_connect result: " . $conn_id . PHP_EOL);
// $conn_id = ftp_connect($ftp_server, 2123);
// print("ftp_connect result: " . $conn_id . PHP_EOL);

$login_result = ftp_login($conn_id, "some_user_name", "the_password");
print("ftp_login result: " . $login_result . PHP_EOL);

print("FTP_USEPASVADDRESS: " . ftp_get_option ($conn_id, FTP_USEPASVADDRESS) . PHP_EOL);
$passive = ftp_pasv($conn_id, true);
print("FTP_USEPASVADDRESS: " . ftp_get_option ($conn_id, FTP_USEPASVADDRESS) . PHP_EOL);

print("ftp_systype: " . ftp_systype($conn_id) . PHP_EOL);

$ok = ftp_put($conn_id, "a_file.zip", "a_file.zip", FTP_BINARY);
print("ftp_put result: " . $ok . PHP_EOL);

ftp_close($conn_id); 
?>

FYI, changing the return code on a fork didn't work at all !

[hannesdejager]

@jrouaix I've now tried your PHP script and it worked the first time with me so was unable to reproduce. I was wondering if you set the libunftp PassiveHost option in your server? Perhaps try that?

What version of libunftp are you using by the way?

[jrouaix]

Indeed, i'm using server.passive_host(Ipv4Addr).

Did you try ftps ? it's working well without that.

I'm using the latest version if libunftp.

My full configuration looks like that :

  let server = Server::with_authenticator(Box::new(move || MyBackend {}), Arc::new(MyAuthenticator {}))
    .greeting("Welcome to my FTP server")
    .passive_ports(FTP_OPTIONS.ftp_passive_ports_range_begin..FTP_OPTIONS.ftp_passive_ports_range_end)
    .passive_host(&"x.x.x.x".parse::<Ipv4Addr>().unwrap())
    .ftps("cert_file", "key_file")
    .ftps_required(FtpsRequired::All, FtpsRequired::All);

  server.listen("0.0.0.0:xxxx").await.expect("server should have started, but didn't");

[hannesdejager]

I indeed tested with ftps. Both the control and data channel connections updraded tot tls. Do you have the problem too when you run your server and connect on localhost?

[hannesdejager]

OK I've been able to reproduce it now.

[hannesdejager]

OK so this only happens if FTPS is required on the data channel. The second FtpsRequired::All in .ftps_required(FtpsRequired::All, FtpsRequired::All);

The problem is that the PHP FTP library wants to setup FTPS on the data channel (it issues PROT and PBZS) before it logs in (command USER and PASS). libunftp currently expects the FTP client to log in first so it returns an error. The FTP client then thinks libunftp doesn't support TLS on the data channel and tries to connect with plaintext. libunftp of course rejects this connection since it was configured to do so. The FTP client the tries to do an Active connection with PORT which we don't support.

The fix is probably in libunftp, to allow PROT and PBZS before a login was made.

[hannesdejager]

Might want to make this an option, not sure if allowing this before logging in poses any security risks. What do you think @robklg

[robklg]

Nice find! I cannot think of any risk with allowing the use of PBSZ / PROT commands to be used prior to login.

[hannesdejager]

Will probably be a release by end of Friday or over the week-end.

[jrouaix]

Well, it's in production since a few hours now, and working as expected 🎉
Thank you a lot !

[hannesdejager]

@jrouaix Ah, nice, great to hear. Thanks for the feedback!

[hannesdejager]

@jrouaix oh and feel free to share a bit more on your use case. Always interesting for us to hear and we’ll keep it in mind also when handling the roadmap