[PATCH] HTTP/2: per-iteration stream handling limit

[Maxim Dounin]

Hello!

On Tue, Oct 10, 2023 at 03:29:02PM +0300, Maxim Dounin wrote:

> # HG changeset patch
> # User Maxim Dounin <mdounin at mdounin.ru>
> # Date 1696940019 -10800
> #      Tue Oct 10 15:13:39 2023 +0300
> # Node ID cdda286c0f1b4b10f30d4eb6a63fefb9b8708ecc
> # Parent  3db945fda515014d220151046d02f3960bcfca0a
> HTTP/2: per-iteration stream handling limit.
> 
> To ensure that attempts to flood servers with many streams are detected
> early, a limit of no more than 2 * max_concurrent_streams new streams per one
> event loop iteration was introduced.  This limit is applied even if
> max_concurrent_streams is not yet reached - for example, if corresponding
> streams are handled synchronously or reset.
> 
> Further, refused streams are now limited to maximum of max_concurrent_streams
> and 100, similarly to priority_limit initial value, providing some tolerance
> to clients trying to open several streams at the connection start, yet
> low tolerance to flooding attempts.

To clarify:

There is FUD being spread that nginx is vulnerable to 
CVE-2023-44487.

We do not consider nginx to be affected by this issue.  In the 
default configuration, nginx is sufficiently protected by the 
limit of allowed requests per connection (see 
http://nginx.org/r/keepalive_requests for details), so an attacker 
will be required to reconnect very often, making the attack 
obvious and easy to stop at the network level.  And it is not 
possible to circumvent the max concurrent streams limit in nginx, 
as nginx only allows additional streams when previous streams are 
completely closed.

Further, additional protection can be implemented in nginx by 
using the "limit_req" directive, which limits the rate of requests 
and rejects excessive requests.

Overall, with the handling as implemented in nginx, impact of 
streams being reset does no seem to be significantly different 
from impacts from over workloads with large number of requests 
being sent by the client, such as handling of multiple HTTP/2 
requests or HTTP/1.x pipelined requests.

Nevertheless, we've decided to implemented some additional 
mitigations which will help nginx to detect such attacks and drop 
connections with misbehaving clients faster.  Hence the patch.

> 
> diff --git a/src/http/v2/ngx_http_v2.c b/src/http/v2/ngx_http_v2.c
> --- a/src/http/v2/ngx_http_v2.c
> +++ b/src/http/v2/ngx_http_v2.c
> @@ -347,6 +347,7 @@ ngx_http_v2_read_handler(ngx_event_t *re
>      ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, "http2 read handler");
>  
>      h2c->blocked = 1;
> +    h2c->new_streams = 0;
>  
>      if (c->close) {
>          c->close = 0;
> @@ -1284,6 +1285,14 @@ ngx_http_v2_state_headers(ngx_http_v2_co
>          goto rst_stream;
>      }
>  
> +    if (h2c->new_streams++ >= 2 * h2scf->concurrent_streams) {
> +        ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
> +                      "client sent too many streams at once");
> +
> +        status = NGX_HTTP_V2_REFUSED_STREAM;
> +        goto rst_stream;
> +    }
> +
>      if (!h2c->settings_ack
>          && !(h2c->state.flags & NGX_HTTP_V2_END_STREAM_FLAG)
>          && h2scf->preread_size < NGX_HTTP_V2_DEFAULT_WINDOW)
> @@ -1349,6 +1358,12 @@ ngx_http_v2_state_headers(ngx_http_v2_co
>  
>  rst_stream:
>  
> +    if (h2c->refused_streams++ > ngx_max(h2scf->concurrent_streams, 100)) {
> +        ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
> +                      "client sent too many refused streams");
> +        return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_NO_ERROR);
> +    }
> +
>      if (ngx_http_v2_send_rst_stream(h2c, h2c->state.sid, status) != NGX_OK) {
>          return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_INTERNAL_ERROR);
>      }
> diff --git a/src/http/v2/ngx_http_v2.h b/src/http/v2/ngx_http_v2.h
> --- a/src/http/v2/ngx_http_v2.h
> +++ b/src/http/v2/ngx_http_v2.h
> @@ -131,6 +131,8 @@ struct ngx_http_v2_connection_s {
>      ngx_uint_t                       processing;
>      ngx_uint_t                       frames;
>      ngx_uint_t                       idle;
> +    ngx_uint_t                       new_streams;
> +    ngx_uint_t                       refused_streams;
>      ngx_uint_t                       priority_limit;
>  
>      size_t                           send_window;
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> https://mailman.nginx.org/mailman/listinfo/nginx-devel

-- 
Maxim Dounin
http://mdounin.ru/