On Fri, Nov 11, 2016 at 9:01 AM, Paul Spangler <paul.spang...@ni.com> wrote:

> On 10/17/2016 2:04 PM, Paul Spangler wrote:
>
>> Hello,
>>
>> Due to the way OpenSSL stores errors in a per-thread queue, functions
>> such as SSL_read followed by SSL_get_error may not produce the desired
>> result if the error queue is not empty prior to calling SSL_read[1]. For
>> example, a non-blocking read reports that no data is available by
>> setting up SSL_get_error to return SSL_ERROR_WANT_READ. But if an error
>> is already present in the queue, SSL_get_error sees that error instead
>> and returns SSL_ERROR_SSL.
>>
>> I found at least one case where the error queue may be non-empty prior
>> to a non-blocking read[2] that involves combining mod_session_crypto
>> (which leaves the error queue non-empty) and the third-party
>> mod_websocket (which uses non-blocking reads), resulting in the
>> connection being closed. I included a potential patch to mod_ssl for
>> consideration on the bug report that simply clears the error queue prior
>> to any of the three SSL_* calls that mod_ssl makes. An ideal fix might
>> be to keep the error queue empty at all times (i.e. patch the APR crypto
>> library), but I propose that this patch is more robust in a modular
>> environment.
>>
>> Thanks for your consideration.
>>
>> [1]
>> https://www.openssl.org/docs/manmaster/ssl/SSL_get_error.html#DESCRIPTION
>> [2] https://bz.apache.org/bugzilla/show_bug.cgi?id=60223
>>
>> Anyone have any thoughts on this small patch? It addresses an issue with
> OpenSSL's per-thread state causing connections to fail.
>
>
> Regards,
> Paul Spangler
> LabVIEW R&D
> National Instruments
>


The patch looks reasonable to me. As you point out, fixing this in APR
might be worthwhile, but doesn't protect mod_ssl from other OpenSSL
consuming logic such as OpenLDAP auth, database connections or
other plug-ins that may leave a lingering SSL_error state lying about.

Reply via email to