Maxim:

If NGINX is unaffected, then F5 is distributing the FUD.

Refer to https://my.f5.com/manage/s/article/K000137106 and the NGINX category, 
specifically NGINX OSS.  Right now, unless this is updated by F5 (who owns 
NGINX now) there is conflicting information here.  May I suggest you go 
internally to F5 and have them actually *revise* their update, or further 
indicate that "There are already mitigations in place"?  Ultimately, though, I 
would assume that keepalive at 100 is also still not enough to *fully* protect 
against this, therefore at the core "NGINX is still vulnerable to this CVE 
despite mitigations already being in place for default configuration values and 
options" is the interpretation, which means yes NGINX is in fact affected by 
this *in non-default configurations*, which means that it *is* still vulnerable 
to the CVE.  Mitigations don't mean "fixed" or "not affected" in the strictest 
interpretation of languages and what means what.


Thomas

-----Original Message-----
From: nginx-devel <nginx-devel-boun...@nginx.org> On Behalf Of Maxim Dounin
Sent: Tuesday, October 10, 2023 8:53 AM
To: nginx-devel@nginx.org
Subject: Re: [PATCH] HTTP/2: per-iteration stream handling limit

Hello!

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

> # HG changeset patch
> # User Maxim Dounin <mdou...@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@nginx.org
> https://mailman.nginx.org/mailman/listinfo/nginx-devel

--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

Reply via email to