SSL connection handling needs work

It is quite difficult to obtain comprehensive information on the use of OpenSSL in conjunction with epoll. There is a risk that the following is incomplete and it may need more research.

When a read or write operation is used with SSL, there is a possibility of receiving an “error” response of SSL_ERROR_WANT_READ or SSL_ERROR_WANT_WRITE. Either call can get either response. This is likely to be caused by renegotiation of the SSL connection, which can happen at any time. These returns are tantamount to a “would block” response, and thus it is wrong to either enter a tight loop or to sleep.

The operation must be retried when SSL is ready to progress. There is a further consideration, which is that no other SSL operation should be attempted on the connection until the retry has been completed and the “WANT” condition no longer obtains.

The simple matter of retry can be achieved by ending the operation and allowing the standard logic to cycle round the poll loop. But this does not provide for the interlock needed to prevent any other operation until the retry is completed, since DCBs are normally operated with EPOLLIN | EPOLLOUT. Additional flag(s) and logic are required.

Before changes are implemented, some clarification and optimisation should be done to reduce the complexity of the DCB handling code. At present there is effectively duplicate code between dcb_write and dcb_drain_writeq (and their SSL equivalents). This can be obviated by always adding the new buffer to the write queue in dcb_write. If the queue was previously empty, then dcb_drain_writeq should be called immediately to instigate output.

The SSL and non-SSL code throughout the system that relates to ordinary DCB processing should be merged – in many cases it is identical. The only point at which a distinction needs to be made (and this can be done by testing for the presence of an SSL structure in the DCB) is within dcb_drain_writeq. The code to be called needs to be a choice of gw_write or gw_write_SSL with slightly expanded functionality. The new gw_write should carry out the write operation and all error analysis. Its only parameter is the DCB and its return the number of bytes written.

There are faults in the SSL code in the develop and release-1.3.0 branches to do with the analysis of error conditions. The problems in the handling of the return from the call to write_SSL can be fixed by adopting the relatively standard and simplified code:

result = SSL_write(ssl, buf, nbytes);
switch (SSL_get_error(ssl, result))
{
case SSL_ERROR_NONE:
/* Successful write */

case SSL_ERROR_ZERO_RETURN:
/* react to the SSL connection being closed */

case SSL_ERROR_WANT_READ:
/* Prevent SSL I/O on connection until retried, return to poll loop */

case SSL_ERROR_WANT_WRITE:
/* Prevent SSL I/O on connection until retried, return to poll loop */

default:
/* Report error(s) and shutdown the connection */
}

The test for the file description being greater than zero in gw_write(_SSL) is almost certainly not needed and complicates the logic.

Taking all the above together, this seems too large a change to go into a release that is already more or less in beta, so the target has been set as the next release.