No way to implement ssl session reuse since #92

[Elrendio]

Hello !

Context

We were working on a fork of your repo to allow ssl session reuse and planning to offer a PR once stabilized, however there's no way to implement this since #92 as native_tls doesn't offer an interface to use sessions reuse. This is actually quite problematique as some servers require to use session reuse when in passive mode.

Thoughts around native_tls

Apparently the change to native_tls was made in #92 to make it easier to run on Windows and MacOs machine. As I use ArchLinux I have no idea how much easier it is, however I would like to point out that more than 90% of cloud based webservers run on linux. Since native_tls uses three different crates to implement ssl the configuration available is actually minimal when openssl provides a large set of parameters. This can be easily seen in the number of functions in the builders, 61 for SslConnectorBuilder versus 9 for TlsConnectorBuilder. As an example, I've added at the end how we can implement session reuse with openssl. Therefore I'm not sure the flexibility loss is worth it and would love to read more arguments againts it.

Next Steps

I see three possible solutions:

1. Revert to openssl (I can provide a PR)
2. Use two different feature flags secure-openssl and secure-native-tls that would basically use openssl or native_tls (I can provide a PR)
3. This crate stays on native_tls and we work on a fork.

Solution 1

Pros

• Easy to maintain
• Allows more flexibility
• Always use the same implementation of SSL (easier to debug)
• We'll provide updates to add features when required as FIX - Impl Error for FtpError #95

Cons

• Running on Microsoft & MacOs would be more complicated

Solution 2

Pros

• Keep it easy on Microsoft & MacOs
• Allows more flexibility
• We'll provide updates to add features when required as FIX - Impl Error for FtpError #95

Cons

• Maintainance is twice as hard

Solution 3

Pros

• Easy to maintain
• Keep it easy on Microsoft & MacOs

Cons

• Lot of flexibility lost
• We'll probably maintain only our fork

@mattnenterprise Please tell me which which solution you prefer 🙂

Thanks and have a nice day !

Implementing Session Reuse

Basically it ends up in replacing the function data_command (taken from version 3.0.1) from:

    /// Execute command which send data back in a separate stream
    #[cfg(feature = "secure")]
    fn data_command(&mut self, cmd: &str) -> Result<DataStream> {
        self.pasv()
            .and_then(|addr| self.write_str(cmd).map(|_| addr))
            .and_then(|addr| TcpStream::connect(addr).map_err(|e| FtpError::ConnectionError(e)))
            .and_then(|stream| {
                match self.ssl_cfg {
                    Some(ref ssl) => {
                        Ssl::new(ssl).unwrap().connect(stream)
                            .map(|stream| DataStream::Ssl(stream))
                            .map_err(|e| FtpError::SecureError(e.description().to_owned()))
                    },
                    None => Ok(DataStream::Tcp(stream))
                }
            })
    }

To:

    /// Execute command which send data back in a separate stream
    #[cfg(feature = "secure")]
    fn data_command(&mut self, cmd: &str) -> Result<DataStream> {
        self.pasv()
            .and_then(|addr| self.write_str(cmd).map(|_| addr))
            .and_then(|addr| TcpStream::connect(addr).map_err(|e| FtpError::ConnectionError(e)))
            .and_then(|stream| match self.ssl_cfg {
                Some(ref ssl) => {
                    let mut ssl = Ssl::new(ssl).unwrap();
                    match self.reader.get_ref() {
                        DataStream::Tcp(_) => {}
                        DataStream::Ssl(ssl_stream) => unsafe {
                            // SAFETY: ssl_stream was also using the context from self.ssl_cfg
                            ssl.set_session(ssl_stream.ssl().session().unwrap())
                                .map_err(|e| FtpError::SecureError(format!("{}", e)))?
                        },
                    }
                    ssl.connect(stream)
                        .map(|stream| DataStream::Ssl(stream))
                        .map_err(|e| FtpError::SecureError(format!("{}", e)))
                }
                None => Ok(DataStream::Tcp(stream)),
            })
    }

[mattnenterprise]

Since the initial implementation of this library the choice of TLS/SSL library used for FTPS has been a mixed decision for me. This is mainly because no library offers the same flexibility for users across all systems.

I have looked further into the main issue of some servers require to use session reuse when in passive mode. This is the first I have heard of it, but after some research it seems to be a common thing FTP servers do.

Looks like it could be a security concern if session reuse isn't used as stated in https://superuser.com/questions/989048/why-is-session-reuse-useful-in-ftps. Here is a quote from the answer:

Reusing TLS session protects you from a theoretical possibility that an attacker hijacks an FTP data connection.

When you initiate a data transfer, the server opens a data connection port on the server (in a passive mode). A possible attacker might guess the port and connect before your FTP client does, stealing your data.

If the server requires that the same TLS session is used for the data connection, the attacker will not be able to start its own TLS session, preventing him/her from decoding the data.

The fact that this issue makes the library completely unusable for some users make me inclined to switch back to openssl. While that would make the library harder to use for Windows and MacOS users as they would have to have openssl setup. It would still be possible for them to use it.

It does look like native-tls has a pull request for session reuse. Though it hasn't had any progress since May 12, 2020.

@ChristianVisintin you created the pull request for the use of native-tls. I was wondering if you had any further thoughts on this ?

[veeso]

Hi,

I didn't know about anything about this. Personally I would keep both, splitting the secure feature into openssl and native-tls. I think both features have their advantages and I wouldn't remove any of them.

In case session reuse were implemented in native-tls, it would make sense to merge this feature into the native-tls version and then deprecate the openssl feature.