limiting number of HTTP request headers
Maxim Dounin
mdounin at mdounin.ru
Thu May 23 15:41:40 UTC 2024
Hello!
On Tue, May 21, 2024 at 10:37:01AM -0700, Maksim Yevmenkin wrote:
> hello!
>
> [...]
>
> > Below is an attempt to address the above comments, please take a
> > look.
> >
> >
> > # HG changeset patch
> > # User Maxim Dounin <mdounin at mdounin.ru>
> > # Date 1716296209 -10800
> > # Tue May 21 15:56:49 2024 +0300
> > # Node ID 2850868eb5e8c0dff91f86f75cb36886d54af001
> > # Parent 429d40e8275d2606da7cb710de5bc40a905fe52f
> > Added max_client_headers directive.
> >
> > The directive limits the number of request headers accepted from clients.
> > While the total amount of headers is believed to be sufficiently limited
> > by the existing buffer size limits (client_header_buffer_size and
> > large_client_header_buffers), the additional limit on the number of headers
> > might be beneficial to better protect backend servers.
> >
> > Requested by Maksim Yevmenkin.
>
> this looks good to me. thank you!
Looking more into it while writing tests and docs, I tend to think
that just "max_headers" would be enough, similarly to
"ignore_invalid_headers" and "underscores_in_headers".
Mostly identical patch below (s/max_client_headers/max_headers/),
as well as tests and docs. I'm going to commit these shortly
unless there are objections.
Thanks for prodding this!
# HG changeset patch
# User Maxim Dounin <mdounin at mdounin.ru>
# Date 1716476544 -10800
# Thu May 23 18:02:24 2024 +0300
# Node ID 98d6497ab365f1ae6b44d7e0ab403a1dcf060124
# Parent 46ecad404a296042c0088e699f275a92758e5ab9
Added max_headers directive.
The directive limits the number of request headers accepted from clients.
While the total amount of headers is believed to be sufficiently limited
by the existing buffer size limits (client_header_buffer_size and
large_client_header_buffers), the additional limit on the number of headers
might be beneficial to better protect backend servers.
Requested by Maksim Yevmenkin.
diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c
--- a/src/http/ngx_http_core_module.c
+++ b/src/http/ngx_http_core_module.c
@@ -252,6 +252,13 @@ static ngx_command_t ngx_http_core_comm
offsetof(ngx_http_core_srv_conf_t, large_client_header_buffers),
NULL },
+ { ngx_string("max_headers"),
+ NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_TAKE1,
+ ngx_conf_set_num_slot,
+ NGX_HTTP_SRV_CONF_OFFSET,
+ offsetof(ngx_http_core_srv_conf_t, max_headers),
+ NULL },
+
{ ngx_string("ignore_invalid_headers"),
NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_FLAG,
ngx_conf_set_flag_slot,
@@ -3463,6 +3470,7 @@ ngx_http_core_create_srv_conf(ngx_conf_t
cscf->request_pool_size = NGX_CONF_UNSET_SIZE;
cscf->client_header_timeout = NGX_CONF_UNSET_MSEC;
cscf->client_header_buffer_size = NGX_CONF_UNSET_SIZE;
+ cscf->max_headers = NGX_CONF_UNSET_UINT;
cscf->ignore_invalid_headers = NGX_CONF_UNSET;
cscf->merge_slashes = NGX_CONF_UNSET;
cscf->underscores_in_headers = NGX_CONF_UNSET;
@@ -3504,6 +3512,8 @@ ngx_http_core_merge_srv_conf(ngx_conf_t
return NGX_CONF_ERROR;
}
+ ngx_conf_merge_uint_value(conf->max_headers, prev->max_headers, 1000);
+
ngx_conf_merge_value(conf->ignore_invalid_headers,
prev->ignore_invalid_headers, 1);
diff --git a/src/http/ngx_http_core_module.h b/src/http/ngx_http_core_module.h
--- a/src/http/ngx_http_core_module.h
+++ b/src/http/ngx_http_core_module.h
@@ -198,6 +198,8 @@ typedef struct {
ngx_msec_t client_header_timeout;
+ ngx_uint_t max_headers;
+
ngx_flag_t ignore_invalid_headers;
ngx_flag_t merge_slashes;
ngx_flag_t underscores_in_headers;
diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c
--- a/src/http/ngx_http_request.c
+++ b/src/http/ngx_http_request.c
@@ -1466,6 +1466,15 @@ ngx_http_process_request_headers(ngx_eve
/* a header line has been parsed successfully */
+ if (r->headers_in.count++ >= cscf->max_headers) {
+ r->lingering_close = 1;
+ ngx_log_error(NGX_LOG_INFO, c->log, 0,
+ "client sent too many header lines");
+ ngx_http_finalize_request(r,
+ NGX_HTTP_REQUEST_HEADER_TOO_LARGE);
+ break;
+ }
+
h = ngx_list_push(&r->headers_in.headers);
if (h == NULL) {
ngx_http_close_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR);
diff --git a/src/http/ngx_http_request.h b/src/http/ngx_http_request.h
--- a/src/http/ngx_http_request.h
+++ b/src/http/ngx_http_request.h
@@ -182,6 +182,7 @@ typedef struct {
typedef struct {
ngx_list_t headers;
+ ngx_uint_t count;
ngx_table_elt_t *host;
ngx_table_elt_t *connection;
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
@@ -1814,6 +1814,15 @@ ngx_http_v2_state_process_header(ngx_htt
}
} else {
+ cscf = ngx_http_get_module_srv_conf(r, ngx_http_core_module);
+
+ if (r->headers_in.count++ >= cscf->max_headers) {
+ ngx_log_error(NGX_LOG_INFO, r->connection->log, 0,
+ "client sent too many header lines");
+ ngx_http_finalize_request(r, NGX_HTTP_REQUEST_HEADER_TOO_LARGE);
+ goto error;
+ }
+
h = ngx_list_push(&r->headers_in.headers);
if (h == NULL) {
return ngx_http_v2_connection_error(h2c,
diff --git a/src/http/v3/ngx_http_v3_request.c b/src/http/v3/ngx_http_v3_request.c
--- a/src/http/v3/ngx_http_v3_request.c
+++ b/src/http/v3/ngx_http_v3_request.c
@@ -657,6 +657,15 @@ ngx_http_v3_process_header(ngx_http_requ
}
} else {
+ cscf = ngx_http_get_module_srv_conf(r, ngx_http_core_module);
+
+ if (r->headers_in.count++ >= cscf->max_headers) {
+ ngx_log_error(NGX_LOG_INFO, r->connection->log, 0,
+ "client sent too many header lines");
+ ngx_http_finalize_request(r, NGX_HTTP_REQUEST_HEADER_TOO_LARGE);
+ return NGX_ERROR;
+ }
+
h = ngx_list_push(&r->headers_in.headers);
if (h == NULL) {
ngx_http_close_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR);
Tests:
# HG changeset patch
# User Maxim Dounin <mdounin at mdounin.ru>
# Date 1716478510 -10800
# Thu May 23 18:35:10 2024 +0300
# Node ID 8350c65afbd64bef22dc90689c0c315f82977f0d
# Parent 79753dd514e60b47375e36c54739ea434e04a5b6
Tests: max_client_headers test.
diff --git a/h2_max_headers.t b/h2_max_headers.t
new file mode 100644
--- /dev/null
+++ b/h2_max_headers.t
@@ -0,0 +1,92 @@
+#!/usr/bin/perl
+
+# (C) Maxim Dounin
+
+# Tests for max_headers directive, HTTP/2.
+
+###############################################################################
+
+use warnings;
+use strict;
+
+use Test::More;
+use Socket qw/ CRLF /;
+
+BEGIN { use FindBin; chdir($FindBin::Bin); }
+
+use lib 'lib';
+use Test::Nginx;
+use Test::Nginx::HTTP2;
+
+###############################################################################
+
+select STDERR; $| = 1;
+select STDOUT; $| = 1;
+
+my $t = Test::Nginx->new()->has(qw/http http_v2 rewrite/);
+
+$t->write_file_expand('nginx.conf', <<'EOF');
+
+%%TEST_GLOBALS%%
+
+daemon off;
+
+events {
+}
+
+http {
+ %%TEST_GLOBALS_HTTP%%
+
+ server {
+ listen 127.0.0.1:8080;
+ server_name localhost;
+
+ http2 on;
+ max_headers 5;
+
+ location / {
+ return 204;
+ }
+ }
+}
+
+EOF
+
+$t->try_run('no max_headers')->plan(3);
+
+###############################################################################
+
+like(get('/'), qr/ 204/, 'two headers');
+like(get('/', ('Foo: bar') x 3), qr/ 204/, 'five headers');
+like(get('/', ('Foo: bar') x 4), qr/ 400/, 'six headers rejected');
+
+###############################################################################
+
+sub get {
+ my ($url, @headers) = @_;
+
+ my $s = Test::Nginx::HTTP2->new();
+ my $sid = $s->new_stream({
+ headers => [
+ { name => ':method', value => 'GET' },
+ { name => ':scheme', value => 'http' },
+ { name => ':path', value => $url },
+ { name => ':authority', value => 'localhost' },
+ { name => 'foo', value => 'bar', mode => 2 },
+ { name => 'foo', value => 'bar', mode => 2 },
+ map {
+ my ($n, $v) = split /:/;
+ { name => lc $n, value => $v, mode => 2 };
+ } @headers
+ ]
+ });
+
+ my $frames = $s->read(all => [{ sid => $sid, fin => 1 }]);
+
+ my ($frame) = grep { $_->{type} eq "HEADERS" } @$frames;
+
+ return join("\n", map { "$_: " . $frame->{headers}->{$_}; }
+ keys %{$frame->{headers}});
+}
+
+###############################################################################
diff --git a/h3_max_headers.t b/h3_max_headers.t
new file mode 100644
--- /dev/null
+++ b/h3_max_headers.t
@@ -0,0 +1,112 @@
+#!/usr/bin/perl
+
+# (C) Maxim Dounin
+
+# Tests for max_headers directive, HTTP/3.
+
+###############################################################################
+
+use warnings;
+use strict;
+
+use Test::More;
+use Socket qw/ CRLF /;
+
+BEGIN { use FindBin; chdir($FindBin::Bin); }
+
+use lib 'lib';
+use Test::Nginx;
+use Test::Nginx::HTTP3;
+
+###############################################################################
+
+select STDERR; $| = 1;
+select STDOUT; $| = 1;
+
+my $t = Test::Nginx->new()->has(qw/http http_v3 rewrite cryptx/);
+
+$t->write_file_expand('nginx.conf', <<'EOF');
+
+%%TEST_GLOBALS%%
+
+daemon off;
+
+events {
+}
+
+http {
+ %%TEST_GLOBALS_HTTP%%
+
+ ssl_certificate localhost.crt;
+ ssl_certificate_key localhost.key;
+
+ server {
+ listen 127.0.0.1:%%PORT_8980_UDP%% quic;
+ server_name localhost;
+
+ max_headers 5;
+
+ location / {
+ return 204;
+ }
+ }
+}
+
+EOF
+
+$t->write_file('openssl.conf', <<EOF);
+[ req ]
+default_bits = 2048
+encrypt_key = no
+distinguished_name = req_distinguished_name
+[ req_distinguished_name ]
+EOF
+
+my $d = $t->testdir();
+
+foreach my $name ('localhost') {
+ system('openssl req -x509 -new '
+ . "-config $d/openssl.conf -subj /CN=$name/ "
+ . "-out $d/$name.crt -keyout $d/$name.key "
+ . ">>$d/openssl.out 2>&1") == 0
+ or die "Can't create certificate for $name: $!\n";
+}
+
+$t->try_run('no max_headers')->plan(3);
+
+###############################################################################
+
+like(get('/'), qr/ 204/, 'two headers');
+like(get('/', ('Foo: bar') x 3), qr/ 204/, 'five headers');
+like(get('/', ('Foo: bar') x 4), qr/ 400/, 'six headers rejected');
+
+###############################################################################
+
+sub get {
+ my ($url, @headers) = @_;
+
+ my $s = Test::Nginx::HTTP3->new();
+ my $sid = $s->new_stream({
+ headers => [
+ { name => ':method', value => 'GET' },
+ { name => ':scheme', value => 'http' },
+ { name => ':path', value => $url },
+ { name => ':authority', value => 'localhost' },
+ { name => 'foo', value => 'bar' },
+ { name => 'foo', value => 'bar' },
+ map {
+ my ($n, $v) = split /:/;
+ { name => lc $n, value => $v };
+ } @headers
+ ]
+ });
+
+ my $frames = $s->read(all => [{ sid => $sid, fin => 1 }]);
+
+ my ($frame) = grep { $_->{type} eq "HEADERS" } @$frames;
+
+ return join("\n", map { "$_: " . $frame->{headers}->{$_}; }
+ keys %{$frame->{headers}});
+}
+
+###############################################################################
diff --git a/http_max_headers.t b/http_max_headers.t
new file mode 100644
--- /dev/null
+++ b/http_max_headers.t
@@ -0,0 +1,73 @@
+#!/usr/bin/perl
+
+# (C) Maxim Dounin
+
+# Tests for max_headers directive.
+
+###############################################################################
+
+use warnings;
+use strict;
+
+use Test::More;
+use Socket qw/ CRLF /;
+
+BEGIN { use FindBin; chdir($FindBin::Bin); }
+
+use lib 'lib';
+use Test::Nginx;
+
+###############################################################################
+
+select STDERR; $| = 1;
+select STDOUT; $| = 1;
+
+my $t = Test::Nginx->new()->has(qw/http rewrite/);
+
+$t->write_file_expand('nginx.conf', <<'EOF');
+
+%%TEST_GLOBALS%%
+
+daemon off;
+
+events {
+}
+
+http {
+ %%TEST_GLOBALS_HTTP%%
+
+ server {
+ listen 127.0.0.1:8080;
+ server_name localhost;
+
+ max_headers 5;
+
+ location / {
+ return 204;
+ }
+ }
+}
+
+EOF
+
+$t->try_run('no max_headers')->plan(3);
+
+###############################################################################
+
+like(get('/'), qr/ 204/, 'two headers');
+like(get('/', ('Foo: bar') x 3), qr/ 204/, 'five headers');
+like(get('/', ('Foo: bar') x 4), qr/ 400/, 'six headers rejected');
+
+###############################################################################
+
+sub get {
+ my ($url, @headers) = @_;
+ return http(
+ "GET $url HTTP/1.1" . CRLF .
+ 'Host: localhost' . CRLF .
+ 'Connection: close' . CRLF .
+ join(CRLF, @headers) . CRLF . CRLF
+ );
+}
+
+###############################################################################
Docs:
# HG changeset patch
# User Maxim Dounin <mdounin at mdounin.ru>
# Date 1716478618 -10800
# Thu May 23 18:36:58 2024 +0300
# Node ID 89939549eebd6fdaab76b1d0c55450bbcdc4cc0f
# Parent 796fbd13ae3662f808365ad8cf5410b53f7dce68
Documented max_headers directive.
diff --git a/xml/en/docs/http/ngx_http_core_module.xml b/xml/en/docs/http/ngx_http_core_module.xml
--- a/xml/en/docs/http/ngx_http_core_module.xml
+++ b/xml/en/docs/http/ngx_http_core_module.xml
@@ -10,7 +10,7 @@
<module name="Module ngx_http_core_module"
link="/en/docs/http/ngx_http_core_module.html"
lang="en"
- rev="107">
+ rev="108">
<section id="directives" name="Directives">
@@ -1778,6 +1778,31 @@ Enables or disables logging of subreques
</directive>
+<directive name="max_headers">
+<syntax><value>number</value></syntax>
+<default>1000</default>
+<context>http</context>
+<context>server</context>
+<appeared-in>1.27.1</appeared-in>
+
+<para>
+Limits the maximum allowed number of client request header fields.
+If the limit is exceeded, the
+<http-status code="400" text="Bad Request"/>
+error is returned to the client.
+</para>
+
+<para>
+If the directive is specified on the <link id="server"/> level,
+the value from the default server can be used.
+Details are provided in the
+“<link doc="server_names.xml" id="virtual_server_selection">Virtual
+server selection</link>” section.
+</para>
+
+</directive>
+
+
<directive name="max_ranges">
<syntax><value>number</value></syntax>
<default/>
diff --git a/xml/en/docs/http/server_names.xml b/xml/en/docs/http/server_names.xml
--- a/xml/en/docs/http/server_names.xml
+++ b/xml/en/docs/http/server_names.xml
@@ -8,7 +8,7 @@
<article name="Server names"
link="/en/docs/http/server_names.html"
lang="en"
- rev="4"
+ rev="5"
author="Igor Sysoev"
editor="Brian Mercer">
@@ -404,6 +404,7 @@ the server configuration chosen by SNI;
in case of the
<link doc="ngx_http_core_module.xml" id="ignore_invalid_headers"/>,
<link doc="ngx_http_core_module.xml" id="large_client_header_buffers"/>,
+<link doc="ngx_http_core_module.xml" id="max_headers"/>,
and
<link doc="ngx_http_core_module.xml" id="underscores_in_headers"/> directives
involved in processing request header fields,
diff --git a/xml/ru/docs/http/ngx_http_core_module.xml b/xml/ru/docs/http/ngx_http_core_module.xml
--- a/xml/ru/docs/http/ngx_http_core_module.xml
+++ b/xml/ru/docs/http/ngx_http_core_module.xml
@@ -10,7 +10,7 @@
<module name="Модуль ngx_http_core_module"
link="/ru/docs/http/ngx_http_core_module.html"
lang="ru"
- rev="107">
+ rev="108">
<section id="directives" name="Директивы">
@@ -1774,6 +1774,30 @@ location = /user {
</directive>
+<directive name="max_headers">
+<syntax><value>число</value></syntax>
+<default>1000</default>
+<context>http</context>
+<context>server</context>
+<appeared-in>1.27.1</appeared-in>
+
+<para>
+Ограничивает максимальное допустимое число полей заголовка запроса клиента.
+При превышении указанного ограничения клиенту возвращается ошибка
+<http-status code="400" text="Bad Request"/>.
+</para>
+
+<para>
+Если директива указана на уровне <link id="server"/>,
+то может использоваться значение из сервера по умолчанию.
+Подробнее см. в разделе
+“<link doc="server_names.xml" id="virtual_server_selection">Выбор
+виртуального сервера</link>”.
+</para>
+
+</directive>
+
+
<directive name="max_ranges">
<syntax><value>число</value></syntax>
<default/>
diff --git a/xml/ru/docs/http/server_names.xml b/xml/ru/docs/http/server_names.xml
--- a/xml/ru/docs/http/server_names.xml
+++ b/xml/ru/docs/http/server_names.xml
@@ -8,7 +8,7 @@
<article name="Имена сервера"
link="/ru/docs/http/server_names.html"
lang="ru"
- rev="4"
+ rev="5"
author="Игорь Сысоев"
editor="Brian Mercer">
@@ -408,7 +408,8 @@ server {
<listitem>
в случае использования директив
<link doc="ngx_http_core_module.xml" id="ignore_invalid_headers"/>,
-<link doc="ngx_http_core_module.xml" id="large_client_header_buffers"/>
+<link doc="ngx_http_core_module.xml" id="large_client_header_buffers"/>,
+<link doc="ngx_http_core_module.xml" id="max_headers"/>
и
<link doc="ngx_http_core_module.xml" id="underscores_in_headers"/>,
которые участвуют в обработке полей заголовка запроса,
--
Maxim Dounin
http://mdounin.ru/
More information about the nginx-devel
mailing list