[Libwebsockets] OpenSSL error queue handling issues
ahanins at gmail.com
Thu Jul 11 12:30:26 CEST 2019
On 7/11/19 12:41 PM, Andy Green wrote:
> I am not really clear where OpenSSL apis are going to refer to the error
> queue first before performing their operation.
That's not what I'm saying. The problem is not that OpenSSL API itself
may refer to some error from the queue and fail prematurely when it
shouldn't. The potential problem is only for the callers of OpenSSL API
which may get earliest error from the queue instead of the latest if
queue wasn't empty before the API call. Typical example is
SSL_get_error() which docs says:
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.
Note that it doesn't say that I/O operation itself may not work reliably.
> For example we've done the clear now before SSL_new(), but is it really
> credible SSL_new() would choose to fail because something like a cert
> verification failed before on the same thread on a different connection?
Definitely no. OpenSSL APIs don't fail just because error queue was not
empty. Clear before SSL_new() is necessary for the following
ERR_get_error() to return a real reason of SSL_new() failure, but not
some error code from a previous failure, like cert validation. Imagine
how it could confuse the troubleshooting of why SSL_new() fails.
> I can imagine SSL_read() thinking it might have already failed and try
> to make the fail sticky.
As I said before - no. It doesn't happen this way. Only new errors are
pushed to the queue, if necessary. Existing errors don't hurt OpenSSL
APIs and they don't stick this way.
> So I guess the right thing is to definitively
> clear it before these kind of apis as we have done.
Yes, it's safest, IMO.
> Another possibile approach is clear it on any api where we check the
> return code.
Or always use ERR_peek_last_error() which returns the _latest_ error
from the queue, this way it doesn't matter how many errors are in the
queue. But it doesn't work for SSL_get_error() which internally uses
ERR_peek_error() which returns the _earliest_ error, thus we need do
clear errors before calling the following APIs: SSL_connect(),
SSL_accept(), SSL_do_handshake(), SSL_read_ex(), SSL_read(),
SSL_peek_ex(), SSL_peek(), SSL_write_ex(), SSL_write().
> But again if you consider crypto apis like RSA or hashes,
> it doesn't seem credible they will check for a pre-existing error stack
> and fail out on entry. What's most likely is it will return 1 (OK) if
> no problems, and if problems, push on the error stack which may also
> then have some old entries afterwards and return 0 (bad)... it's OK I
More information about the Libwebsockets