On 08/29/2018 09:37 AM, Christophe Jaillet wrote:
> Pure speculation:
>
> Actually we ERR_clear_error unconditionally so that in case of error, we can
> safely call SSL_get_error.
>
> Doc says:
> <<<<<<<<<< >>>>>>>>>>
> ERR_get_error() returns the earliest error code from the thread's error queue
> and removes the entry. This function can be called repeatedly until there are
> no more error codes to return.
>
> ERR_peek_error() returns the earliest error code from the thread's error
> queue without modifying it.
>
> ERR_peek_last_error() returns the latest error code from the thread's error
> queue without modifying it.
> <<<<<<<<<< >>>>>>>>>>
>
> Couldn't we avoid the ERR_clear_error call (which looks expensive according
> to the thread), and:
> - loop on SSL_get_error to empty the error queue, in case an error
> occurred and we want la latest one?
> or
> - do a ERR_peek_last_error() / ERR_clear_error() in the error handling
> path (if we really care about clearing the error queue)
>
> IMHO, these 2 solutions would do the same as the current code, without
> requiring ERR_clear_error in the normal case.
Unfortunately it isn't that easy, because SSL_get_error does not only look at
the error stack but also at the return
code from a read / write call passed to it. It quickly breaks out with two
possible error return values if something
is on the error stack and this what caused the addition of the ERR_clear_error
call. We had the situation that another
consumer of openssl in the code base left an error unhandled on the stack and
thus caused SSL_get_error to draw the
wrong conclusions. While we could argue the error is in the other part of the
code and the other consumer of openssl
should clear up the error stack correctly I don't think this is the complete
solution, because
1. We should not fail if a another consumer is bogus. We should be prepared for
others to have bugs regarding this.
2. As the error handling is a stack I think it is a valid use case for another
consumer of openssl to check for the
error later and not directly like e.g. with errno.
I am not sure yet if it is not SSL_get_error that makes wrong assumptions about
the error stack, but that would be
something hard to solve for us.
Regards
Rüdiger
