You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Paul Spangler <pa...@ni.com> on 2016/10/17 19:04:48 UTC
[PATCH 60223] non-empty OpenSSL error queue preventing non-blocking
reads through mod_ssl
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
Regards,
Paul Spangler
LabVIEW R&D
National Instruments
Re: [PATCH 60223] non-empty OpenSSL error queue preventing
non-blocking reads through mod_ssl
Posted by Jacob Champion <ch...@gmail.com>.
On 11/11/2016 11:20 AM, Jacob Champion wrote:
> I suspect that these failures have nothing to do with this patch, but I
> just want to confirm that before it goes into trunk.
Yeah, the failures show up with or without the patch, so I added a
couple comments and committed.
The SSL failures in the trunk test suite are going to make this
difficult to test fully; has anyone seen them before or already have a
fix tucked away privately?
--Jacob
Re: [PATCH 60223] non-empty OpenSSL error queue preventing
non-blocking reads through mod_ssl
Posted by Jacob Champion <ch...@gmail.com>.
On 11/11/2016 10:15 AM, William A Rowe Jr wrote:
> 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.
Looks good to me too -- thanks for the bump, Paul.
I haven't committed this yet because of some SSL-related test failures
in our suite when running against httpd-trunk:
- mod_session_crypto appears to be segfaulting the server for some reason
- mod_proxy tests hang the suite when run in -ssl test mode, and httpd
complains about malformed request lines
I suspect that these failures have nothing to do with this patch, but I
just want to confirm that before it goes into trunk.
--Jacob
Re: [PATCH 60223] non-empty OpenSSL error queue preventing
non-blocking reads through mod_ssl
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Fri, Nov 11, 2016 at 9:01 AM, Paul Spangler <pa...@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.
Re: [PATCH 60223] non-empty OpenSSL error queue preventing
non-blocking reads through mod_ssl
Posted by Paul Spangler <pa...@ni.com>.
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