Report timeouts as Errors, with original connection (AsyncAccept::Connection)

[ahcodedthat]

Currently, when a TLS handshake times out, the connection is closed silently. It would probably be better to report these events to the application as a new variant of tls_listener::Error, containing the original connection object (AsyncAccept::Connection), so that the application can fetch the client's IP address (using TcpStream::peer_addr) and emit a log message.

That way, if an attacker attempts to DoS the application by opening a lot of dummy connections, thus exceeding max_handshakes, the log messages will tell the administrator what's going on and what the attacker's IP addresses are.

[tmccombs]

It would probably be better to report these events to the application as a new variant of tls_listener::Error

this would be pretty straightforward to do, although it would break backwards compatibility.

containing the original connection object (AsyncAccept::Connection)

This is more difficult. By the time we get a timeout the connection is owned by the future that is performing the handshake, and I no longer have any reference to it. Furthermore, I don't even have access to the actual Future, since I pull from a FuturesUnordered stream.

As a possible workaround, you could do the following:

1. Create a your own wrapper type that implements AsyncTls and wraps one of the existing implementations
2. In the accept method, capture the peer_addr or any other information of interest, and return a new Future that includes that data alongside the underlying Future, and have some way to track if the handshake completed. (such as a boolean, using an enum, calling a method on the inner future, etc.)
3. impl Drop on your type, check if the inner future has completed, and if not log the associated information.

If you have any good ideas on how I could include the connection or information about the connection, I would be happy to hear them.

[ahcodedthat]

Ah, you're right. Although tokio_openssl does allow aborting the handshake and taking back the SslStream, the other two TLS libraries take ownership of the socket during the handshake, making it impossible to abort the handshake without also closing the socket.

I've submitted #38 as an alternative. Instead of giving back the socket upon error, that PR changes AsyncAccept and TlsListener to report the remote address, both on error and on success. What do you think of that idea?