Keepalived occasionally fails SSL_CHECK about keepalived HOT 1 OPEN

The man page for SSL_get_error() list a number of functions to which it applies, which includes SSL_shutdown(), and so I think ERR_clear_error() should also be called before the two calls of SSL_shutdown(). keepalived does not use any of the other functions listed.

However, I think the patch will not work as a solution; in fact I think we have a severe problem.

The SSL_get_error(3) man page states:

In addition to ssl and ret, SSL_get_error() inspects the current thread's OpenSSL error queue.  Thus, SSL_get_error() must be used in the same thread that performed the TLS/SSL I/O operation, and no other OpenSSL function calls should appear in between.  The current thread's error queue must be empty before the TLS/SSL I/O operation is attempted, or SSL_get_error() will not work reliably.

The last sentence says we need to ensure that the thread's error queue is empty before we call SSL_connect(), SSL_read(), SSL_write() or SSL_shutdown(), so either we must call ERR_clear_error() before any of the SSL functions, or after an error we need to ensure we read all the errors, which is a solution I prefer since we ought to process all the errors and not just throw them away.

The problem is the second sentence above "... and no other OpenSSL function calls should appear in between.". Apart from it being unclear what between refers to, the real problem is that keepalived can have SSL connection open simultaneously for each SSL_CHECK. If an error is returned from any SSL_* call, SSL_get_error() will return errors not only for the particular connection, but also for errors that have occurred on any other SSL connection that are on the OpenSSL error queue. Calling ERR_clear_error() before all SSL_* function calls could mean that errors are discarded without being processed.

There are two possible solutions that I can see (but only 1 can work for keepalived):

1. Only run one SSL_GET checker at a time (this won't scale for large configurations with a large number of SSL_GET checkers)
2. Run each SSL_GET checker in a separate process thread (i.e. using pthread_create()).

The problem with option 2 is that (except for DBus) we do not use pthreads, and this would require a not insignificant architecture change for keepalived.

I will think on about this and see if I can work out the best solution. It might need reading the openSSL code to really understand how it works, but I would rather avoid that.

from keepalived.