BUG/MAJOR: http-ana: Always abort the request when a tarpit is triggered

If an client error is reported on the request channel (CF_READ_ERROR) while a
session is tarpitted, no error is returned to the client. Concretly,
http_reply_and_close() function is not called. This function is reponsible to
forward the error to the client. But not only. It is also responsible to abort
the request. Because this function is not called when a read error is reported
on the request channel, and because the tarpit analyzer is the last one, there
is nothing preventing a connection attempt on a server while it is totally
unexpected.

So, a useless connexion on a backend server may be performed because of this
bug. If an HTTP load-balancing algorithm is used on the backend side, it leads
to a crash of HAProxy because the request was already erased.

If you have tarpit rules and if you use an HTTP load-balancing algorithm on your
backends, you must apply this patch. Otherwise a simple TCP reset on a tarpitted
connexion will most likely crash your HAProxy. A safe workaround is to use a
silent-drop rule or a deny rule instead of a tarpit.

This bug also affect the legacy code. It is in fact an very old hidden bug. But
the refactoring of process_stream() in the 1.9 makes it visible. And,
unfortunately, with the HTX, it is easier to hit it because many processing has
been moved in lower layers, in the muxes.

It must be backported as far as 1.9. For the 2.0 and the 1.9, the legacy HTTP
code must also be patched the same way. For older versions, it may be backported
but the bug seems to not impact them.

Thanks to Olivier D <webmaster@ajeux.com> to have reported the bug and provided
all the infos to analyze it.

diff --git a/src/http_ana.c b/src/http_ana.c
index d1b6e8872409eb666f59e25f585f227205192466..b55e1bc778c885e2f404f0804872ac5b25730338 100644 (file)
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -1001,8 +1001,7 @@ int http_process_tarpit(struct stream *s, struct channel *req, int an_bit)
         */
        s->logs.t_queue = tv_ms_elapsed(&s->logs.tv_accept, &now);
 
-       if (!(req->flags & CF_READ_ERROR))
-               http_reply_and_close(s, txn->status, http_error_message(s));
+       http_reply_and_close(s, txn->status, (!(req->flags & CF_READ_ERROR) ? http_error_message(s) : NULL));
 
        req->analysers &= AN_REQ_FLT_END;
        req->analyse_exp = TICK_ETERNITY;