Is there a bug in ngx_http_upstream_module's broken connection check code?

[Maxim Dounin]

Hello!

On Sat, Nov 26, 2011 at 07:41:24AM -0500, nviennot wrote:

> I am experiencing this bug in a production system. (SSL closes are not
> detected,  thus sockets stay in CLOSE_WAIT state forever -- nice DoS).

I believe I've already stated this several times, but let me 
repeat: this is not a bug, it's rather lack of optimization.

And if this makes DoS possible in you setup, this means your setup 
is broken (i.e. there is a bug in your setup).  Even if nginx will 
be able to detect connection close in simple SSL case, this 
doesn't mean all connection closes will be detectable.  

Consider connection with a pipelined request not yet read from 
client: it won't be possible to detect pending connection close 
with standard interfaces (without reading the request first, which 
isn't an option).  You must use some timeouts/keepalives to handle 
this.

> I was looking at nginx sources, and it seems that this bug has not been
> fixed.
> The alternative is to use stunnel with the X-Forwarded-For patch, but
> that's way too messy.
> 
> In ngx_http_upstream_check_broken_connection(), there seems to be a
> different path for kqueue. What about modifying the poll/epoll behavior
> to detect disconnections for other event modules ? In
> ngx_epoll_add_connection(), we can add the EPOLLHUP event, and mark the
> connection as disconnected when processing HUP events instead of using
> the buggy MSG_PEEK hack

Likely, EPOLLRDHUP is needed here.  Patches are welcome.

Maxim Dounin