Re: [reSIProcate] issues with mInWritable during TLS handshake
On 19/02/15 11:26, Daniel Pocock wrote:
> I've observed a fault with the way mInWritable is used.
>
> Normally, when a TLS handshake starts, SSL_do_handshake puts
> SSL_ERROR_WANT_READ on the error queue and all is OK.
>
> Very rarely, there is a situation where the TLS client hello came in
> very quickly (or was already buffered by the OS) and the call to
> SSL_do_handshake puts SSL_ERROR_WANT_WRITE on the error queue.
>
> In this scenario, there is a crash:
>
> - reSIProcate is acting as a server, receives a connection and calls
> SSL_do_handshake
>
> - SSL_do_handshake gives SSL_ERROR_WANT_WRITE
>
> - TlsConnection::checkState() calls Connection::ensureWritable()
>
> - Connection::ensureWritable() does
> assert(!mOutstandingSends.empty())
>
> - as it is in accepting an incoming connection, there are no outstanding
> sends from the application, the assert stops the process
>
> I notice there is a method called transportWrite() that returns true if
> TLS is handshaking and wants to read and otherwise it returns false.
> Can anybody clarify the exact intention of this method?
>
> I think we need to
>
> a) add some other method, maybe call it isTransportWriting() for
> transports like TLS that want to be in the ConnectionManager's write set
> at times even when there is no application layer data to write. This
> may also apply to situations such as renegotiation or keepalive traffic
> for some types of transport.
>
> b) in Connection.cxx, ensure that any attempt to use mOutstandingSends
> operates safely even if it is empty - this would probably mean that some
> of the other asserts come out of that class too
>
> Can anybody else comment on these issues?
>
> I'd like to find a way to resolve this that can be safely backported to
> the 1.9.x release branch without breaking the ABI.
I've made a commit to reduce the impact of this issue, it is not a
comprehensive solution though:
https://github.com/resiprocate/resiprocate/commit/be3c73610c275451c232d5535cfb9a5afa07bdbe
While looking at that, I also noticed another issue that can cause TLS
connections to drop:
https://www.resiprocate.org/bugzilla/show_bug.cgi?id=87