From 294e4676a3b775a7accb50eb8428f293c218b5e2 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Mon, 11 May 2015 18:30:33 +0200 Subject: [PATCH 2/2] BUG/MEDIUM: http: don't forward client shutdown without NOLINGER except for tunnels There's an issue related with shutting down POST transfers or closing the connection after the end of the upload : the shutdown is forwarded to the server regardless of the abortonclose option. The problem it causes is that during a scan, brute force or whatever, it becomes possible that all source ports are exhausted with all sockets in TIME_WAIT state. There are multiple issues at once in fact : - no action is done for the close, it automatically happens at the lower layers thanks for channel_auto_close(), so we cannot act on NOLINGER ; - we *do* want to continue to send a clean shutdown in tunnel mode because some protocols transported over HTTP may need this, regardless of option abortonclose, thus we can't set the option inconditionally - for all other modes, we do want to close the dirty way because we're certain whether we've sent everything or not, and we don't want to eat all source ports. The solution is a bit complex and applies to DONE/TUNNEL states : 1) disable automatic close for everything not a tunnel and not just keep-alive / server-close. Force-close is now covered, as is HTTP/1.0 which implicitly works in force-close mode ; 2) when processing option abortonclose, we know we can disable lingering if the client has closed and the connection is not in tunnel mode. Since the last case above leads to a situation where the client side reports an error, we know the connection will not be reused, so leaving the flag on the stream-interface is safe. A client closing in the middle of the data transmission already aborts the transaction so this case is not a problem. This fix must be backported to 1.5 where the problem was detected. (cherry picked from commit bbfb6c40854925367ae5f9e8b22c5c9a18dc69d5) --- src/proto_http.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/proto_http.c b/src/proto_http.c index 0ac3a47..5db64b5 100644 --- a/src/proto_http.c +++ b/src/proto_http.c @@ -5452,9 +5452,10 @@ int http_request_forward_body(struct session *s, struct channel *req, int an_bit msg->sov -= msg->next; msg->next = 0; - /* for keep-alive we don't want to forward closes on DONE */ - if ((txn->flags & TX_CON_WANT_MSK) == TX_CON_WANT_KAL || - (txn->flags & TX_CON_WANT_MSK) == TX_CON_WANT_SCL) + /* we don't want to forward closes on DONE except in + * tunnel mode. + */ + if ((txn->flags & TX_CON_WANT_MSK) != TX_CON_WANT_TUN) channel_dont_close(req); if (http_resync_states(s)) { /* some state changes occurred, maybe the analyser @@ -5478,10 +5479,15 @@ int http_request_forward_body(struct session *s, struct channel *req, int an_bit * want to monitor the client's connection and forward * any shutdown notification to the server, which will * decide whether to close or to go on processing the - * request. + * request. We only do that in tunnel mode, and not in + * other modes since it can be abused to exhaust source + * ports. */ if (s->be->options & PR_O_ABRT_CLOSE) { channel_auto_read(req); + if ((req->flags & (CF_SHUTR|CF_READ_NULL)) && + ((txn->flags & TX_CON_WANT_MSK) != TX_CON_WANT_TUN)) + s->si[1].flags |= SI_FL_NOLINGER; channel_auto_close(req); } else if (s->txn.meth == HTTP_METH_POST) { -- 2.0.5