[SECURITY] - SSL Stream is not properly closed

[veeso]

Description

Severity:

• [ ] critical
• [ ] high
• [x] medium
• [ ] low

When using SSL data stream, the channel is not properly closed as specified in RFC2246 7.2.1 https://www.ietf.org/rfc/rfc2246.txt. This may lead to a truncation attack.

Additional information

It may be enough to implement the stream shut down as already done by @devbydav.

if let DataStream::Ssl(ssl_stream) = stream.get_mut() {
            ssl_stream
                .shutdown()
                .map_err(FtpError::ConnectionError)?;
}

[devbydav]

The fix was in the parent repo.

It looks like it's enough, but I might have missed something.

I guess it makes sense to have a function doing the shutdown/drop as it's needed in different places (finalize_put/retr_stream, stream_lines).

I changed finalize_*_stream signatures, but it's not ideal as it would break some users code:

pub fn finalize_put_stream(&mut self, mut stream: BufWriter<DataStream>) -> Result<()>

Do you want to do it yourself or should I start a draft PR ?

[veeso]

I think we'll need to change the signature here for sure, so you can start drafting the PR 👍🏻

I totally agree with making a re-usable function.

[devbydav]

VsFTPd

I found the problem testing with vsftpd server, getting this error when trying to put files :

DEBUG [...] "DATA connection terminated without SSL shutdown. Buggy client! Integrity of upload cannot be asserted."
FTP response: [...] "426 Failure reading network stream."

But retrieving files or getting NLST doesn't cause this error. Maybe the server does the SSL shutdown itself.

The options strict_ssl_read_eof and strict_ssl_write_shutdown don't seem to change anything for me (man here)