Re: svn commit: r1791192 - /httpd/httpd/branches/2.4.x/support/abs.mak

2017-04-12 Thread William A Rowe Jr
On Wed, Apr 12, 2017 at 5:31 PM,   wrote:
> Author: gsmith
> Date: Wed Apr 12 22:31:15 2017
> New Revision: 1791192
>
> URL: http://svn.apache.org/viewvc?rev=1791192=rev
> Log:
> Add another include since applink.c has been moved in
> the OpenSSL source. More info:
> http://marc.info/?t=14919286431=1=2
>
> Modified:
> httpd/httpd/branches/2.4.x/support/abs.mak
>
> Modified: httpd/httpd/branches/2.4.x/support/abs.mak
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/support/abs.mak?rev=1791192=1791191=1791192=diff
> ==
> --- httpd/httpd/branches/2.4.x/support/abs.mak (original)
> +++ httpd/httpd/branches/2.4.x/support/abs.mak Wed Apr 12 22:31:15 2017
> @@ -28,7 +28,7 @@ NULL=nul
>  !IF "$(_HAVE_OSSL110)" == "1"
>  SSLCRP=libcrypto
>  SSLLIB=libssl
> -SSLINC=/I ../srclib/openssl/include
> +SSLINC=/I ../srclib/openssl/include /I ../srclib/openssl/ms

Question (untested)... does this work for local builds, since ms is not
in the include  scope?


Re: blocking listener thread during SSL shutdown

2017-04-12 Thread Eric Covener
On Wed, Apr 12, 2017 at 3:30 PM, Ruediger Pluem  wrote:
> A debug build which states the line numbers would be more helpful here. What 
> I can conclude so far by looking at the
> code is that it does not wait on the client to sent something but that it 
> does wait on the socket to accept data for
> sending.
> So possibly the problem will go away if you fiddle around a little bit with 
> the SendBufferSize.

Any chance the logs also report write timeouts that precede this?  Are
there "similar" threads waiting for writability of actual response
data (e.g. ap_invoke_handler in the stack)


Re: blocking listener thread during SSL shutdown

2017-04-12 Thread Ruediger Pluem


On 04/12/2017 05:03 PM, Frank Meier wrote:
> Hi
> 
> I've submitted a bug report [1] about hanging listener threads in mod_event 
> during SSL shutdown. But since I did not
> hear anything I choose to discuss it on the Mailing List.
> 
> I was remotely debugging an Issue, where an Apache HTTPD did not serve 
> requests any more. In the stacks of the HTTPD
> processes I saw that a lot of listener threads are blocked in a poll() call, 
> triggered by modssl_smart_shutdown().
> 
> #0  0x7f996d44f283 in poll () from /lib64/libc.so.6
> #1  0x7f996df8764f in apr_poll () from /opt/apache/bin/libapr-1.so.0
> #2  0x7f996eacb485 in ap_core_output_filter ()
> #3  0x7f996cf46488 in bio_filter_out_pass () from 
> /opt/apache/bin/mod_ssl.so
> #4  0x7f996cf483bf in bio_filter_out_ctrl () from 
> /opt/apache/bin/mod_ssl.so
> #5  0x7f996cf5803b in modssl_smart_shutdown () from 
> /opt/apache/bin/mod_ssl.so
> #6  0x7f996cf4856e in ssl_filter_io_shutdown.isra.2 () from 
> /opt/apache/bin/mod_ssl.so
> #7  0x7f996cf49c10 in ssl_io_filter_output () from 
> /opt/apache/bin/mod_ssl.so
> #8  0x7f996cf46b4e in ssl_io_filter_coalesce () from 
> /opt/apache/bin/mod_ssl.so
> #9  0x7f996ead9f93 in ap_shutdown_conn ()
> #10 0x7f996a505702 in start_lingering_close_nonblocking () from 
> /opt/apache/bin/mod_mpm_event.so
> #11 0x7f996a5040ac in process_timeout_queue () from 
> /opt/apache/bin/mod_mpm_event.so
> #12 0x7f996a5063b0 in listener_thread () from 
> /opt/apache/bin/mod_mpm_event.so
> #13 0x7f996d90faa1 in start_thread () from /lib64/libpthread.so.0
> #14 0x7f996d458aad in clone () from /lib64/libc.so.6

A debug build which states the line numbers would be more helpful here. What I 
can conclude so far by looking at the
code is that it does not wait on the client to sent something but that it does 
wait on the socket to accept data for
sending.
So possibly the problem will go away if you fiddle around a little bit with the 
SendBufferSize.

Regards

RĂ¼diger



Re: Fixing more OpenSSL callback crashes

2017-04-12 Thread Jacob Champion

On 04/12/2017 07:53 AM, Rainer Jung wrote:

The good news:

I checked by compiling your test program standalone (outside of
configure) to be able to run it easily very often (1000 times). I
checked on Solaris 8 (32 Bit) and 10 Sparc (32 Bit and 64 Bit) compilation.

When I compile without any flags, the test fails with return code 5,
when I compile with -D_REENTRANT it succeeds (rc 0). Since apr always
adds -D_REENTRANT for Solaris in build/apr_hints.m4 (independent of
APR_HAS_THREADS), and your test runs in httpd configure after apr
configure and thus inherits the flags, the test would correctly detect a
thread-safe errno for Solaris during configure.


Excellent. Thanks for the test!


The bad news:

If building httpd with apr/apu sources in srclib and using
--with-included-apr (and not assuming some unrelated installed system
APR), the test can't run, because the binary needs to get linked against
libapr-1 during configure runtime (where the lib has not yet been
build). I don't know how easy a non-apr using test would be that still
runs on the relevant target platforms :(


I was worried about that. I don't think it makes much sense to 
reimplement the portability layer just to get threads and locks for a 
configure test, especially if this problem is going to slowly disappear 
as people upgrade OpenSSL. Let me know if you have any good ideas, though...


Worst case, we don't run the test, and fall back to assuming  is 
unsafe. Then affected users either risk the crashes like they do today, 
or they set the cachevar manually to override the test.


It's probably worth noting at this point that, even if  is unsafe:

- Windows and BeOS users are still handled explicitly by default in 1.0.x.
- If OpenSSL still provides the deprecated CRYPTO_set_id_callback(), we 
can use it instead. We're not making use of the pointer-based THREADID 
implementation like we should be, heh, so we're not really getting a 
benefit out of the new system.

- This whole problem goes away in 1.1.x.

So the users who will still be affected are those who use OpenSSL 1.0.x, 
compiled with OPENSSL_NO_DEPRECATED, on platforms that don't pass (or 
can't run) our errno test. The only way to fix them is to pin the 
callback address somehow, and I'm not entirely sure it's worth the 
effort, if we're not currently seeing other reports of problems.



Side note:

I did increase NUM_THREADS to 10. But actually your original number of 3
gave the same results. Nevertheless I would increase the number if it is
OK for you (I know, quadratic behavior, but we shouldn't care too much
for this test):

[patch]


Looks good to me, thanks! If you'd like, go ahead and check it into the 
trunk-openssl-threadid feature branch. Otherwise I'll plan to get around 
to it tomorrow.


--Jacob


blocking listener thread during SSL shutdown

2017-04-12 Thread Frank Meier

Hi

I've submitted a bug report [1] about hanging listener threads in 
mod_event during SSL shutdown. But since I did not hear anything I 
choose to discuss it on the Mailing List.


I was remotely debugging an Issue, where an Apache HTTPD did not serve 
requests any more. In the stacks of the HTTPD processes I saw that a lot 
of listener threads are blocked in a poll() call, triggered by 
modssl_smart_shutdown().


#0  0x7f996d44f283 in poll () from /lib64/libc.so.6
#1  0x7f996df8764f in apr_poll () from /opt/apache/bin/libapr-1.so.0
#2  0x7f996eacb485 in ap_core_output_filter ()
#3  0x7f996cf46488 in bio_filter_out_pass () from /opt/apache/bin/mod_ssl.so
#4  0x7f996cf483bf in bio_filter_out_ctrl () from /opt/apache/bin/mod_ssl.so
#5  0x7f996cf5803b in modssl_smart_shutdown () from 
/opt/apache/bin/mod_ssl.so
#6  0x7f996cf4856e in ssl_filter_io_shutdown.isra.2 () from 
/opt/apache/bin/mod_ssl.so
#7  0x7f996cf49c10 in ssl_io_filter_output () from 
/opt/apache/bin/mod_ssl.so
#8  0x7f996cf46b4e in ssl_io_filter_coalesce () from 
/opt/apache/bin/mod_ssl.so
#9  0x7f996ead9f93 in ap_shutdown_conn ()
#10 0x7f996a505702 in start_lingering_close_nonblocking () from 
/opt/apache/bin/mod_mpm_event.so
#11 0x7f996a5040ac in process_timeout_queue () from 
/opt/apache/bin/mod_mpm_event.so
#12 0x7f996a5063b0 in listener_thread () from 
/opt/apache/bin/mod_mpm_event.so
#13 0x7f996d90faa1 in start_thread () from /lib64/libpthread.so.0
#14 0x7f996d458aad in clone () from /lib64/libc.so.6

Unfortunately I'm not able to reproduce the problem locally. But just 
looking at this stack makes me think that this should never happen, 
since the listener thread is very important and should never block 
during a operation that is not waiting for new connections. Also the 
function name "start_lingering_close_nonblocking()" suggests, that this 
operation should not block, but obviously does.


I think it's quite serious, and can lead to problems where clients are 
not reacting "fast" to a SSL Shutdown alert. I also thought this 
behaviour could be exploited to be used as a DoS attack. But on the 
other hand it seems nobody else is experiencing this in the wild.


Am I misinterpreting something or does anyone else think this really is 
an issue?


cheers, Frank


[1] https://bz.apache.org/bugzilla/show_bug.cgi?id=60956



Re: Fixing more OpenSSL callback crashes

2017-04-12 Thread Rainer Jung

Hi Jacob,

Am 12.04.2017 um 02:16 schrieb Jacob Champion:

On 04/10/2017 03:59 PM, Jacob Champion wrote:

So it looks like my test program might still be a possible solution for
detecting whether we need a callback at configure time, unless anyone
knows of a platform where two thread-local errnos can have the same
address some of the time and different addresses at other times...


I've checked my first attempt into trunk. It writes the result into the
new AP_OPENSSL_USE_ERRNO_THREADID macro in ap_config_auto.h. On
platforms where every thread has its own errno address, that macro will
be #defined to 1.

Rainer, would you mind sanity-checking the result on your Solaris box
(sounds like the test should *not* pass there)? I've tested on Ubuntu
16.04, where the test passes.


The good news:

I checked by compiling your test program standalone (outside of 
configure) to be able to run it easily very often (1000 times). I 
checked on Solaris 8 (32 Bit) and 10 Sparc (32 Bit and 64 Bit) compilation.


When I compile without any flags, the test fails with return code 5, 
when I compile with -D_REENTRANT it succeeds (rc 0). Since apr always 
adds -D_REENTRANT for Solaris in build/apr_hints.m4 (independent of 
APR_HAS_THREADS), and your test runs in httpd configure after apr 
configure and thus inherits the flags, the test would correctly detect a 
thread-safe errno for Solaris during configure.


The bad news:

If building httpd with apr/apu sources in srclib and using 
--with-included-apr (and not assuming some unrelated installed system 
APR), the test can't run, because the binary needs to get linked against 
libapr-1 during configure runtime (where the lib has not yet been 
build). I don't know how easy a non-apr using test would be that still 
runs on the relevant target platforms :(


Side note:

I did increase NUM_THREADS to 10. But actually your original number of 3 
gave the same results. Nevertheless I would increase the number if it is 
OK for you (I know, quadratic behavior, but we shouldn't care too much 
for this test):


Index: trunk/acinclude.m4
===
--- trunk/acinclude.m4(revision 1791085)
+++ trunk/acinclude.m4(working copy)
@@ -653,7 +653,7 @@
   #include "apr_thread_cond.h"
   #include "apr_thread_proc.h"

-  #define NUM_THREADS 3
+  #define NUM_THREADS 10

   struct thread_data {
   apr_thread_mutex_t *mutex;
@@ -692,6 +692,7 @@
   int ret = 0;
   apr_status_t status;
   int i;
+  int j;

   apr_pool_t *pool;
   apr_thread_mutex_t *mutex;
@@ -738,10 +739,13 @@
   }

   /* Check that no addresses were duplicated. */
-  if ((tdata[0].errno_addr == tdata[1].errno_addr)
-  || (tdata[1].errno_addr == tdata[2].errno_addr)
-  || (tdata[0].errno_addr == tdata[2].errno_addr)) {
-  ret = 5;
+  for (i = 0; i < NUM_THREADS - 1; ++i) {
+  for (j = i + 1; j < NUM_THREADS; ++j) {
+  if (tdata[i].errno_addr == tdata[j].errno_addr) {
+  ret = 5;
+  goto out;
+  }
+  }
   }

   out:



If there's anyone else who wouldn't mind updating and posting their
platform and test result, that would really help me figure out how
widespread the "problem" is. Remember to rebuild configure first! :)


Regards,

Rainer