Opened 5 years ago

Closed 5 years ago

#831 closed defect (fixed)

Possible incorrect handling of invalid headers with HTTP/2.0 and POST/PUT requests

Reported by: Brian Stanback Owned by:
Priority: major Milestone:
Component: nginx-core Version: 1.9.x
Keywords: http2 Cc:
uname -a: Linux fc5d36d05eda 3.19.0-32-generic #37-Ubuntu SMP Wed Oct 21 10:23:06 UTC 2015 x86_64 Linux
nginx -V: nginx version: nginx/1.9.6
built by gcc 5.2.0 (Alpine 5.2.0)
built with OpenSSL 1.0.2d 9 Jul 2015
TLS SNI support enabled
configure arguments: --prefix=/etc/nginx --sbin-path=/usr/sbin/nginx --conf-path=/etc/nginx/nginx.conf --error-log-path=/var/log/nginx/error.log --http-log-path=/var/log/nginx/access.log --pid-path=/var/run/nginx.pid --lock-path=/var/run/nginx.lock --http-client-body-temp-path=/var/cache/nginx/client_temp --http-proxy-temp-path=/var/cache/nginx/proxy_temp --http-fastcgi-temp-path=/var/cache/nginx/fastcgi_temp --http-uwsgi-temp-path=/var/cache/nginx/uwsgi_temp --http-scgi-temp-path=/var/cache/nginx/scgi_temp --user=nginx --group=nginx --with-http_ssl_module --with-http_realip_module --with-http_addition_module --with-http_sub_module --with-http_dav_module --with-http_flv_module --with-http_mp4_module --with-http_gunzip_module --with-http_gzip_static_module --with-http_random_index_module --with-http_secure_link_module --with-http_stub_status_module --with-http_auth_request_module --with-threads --with-stream --with-stream_ssl_module --with-mail --with-mail_ssl_module --with-file-aio --with-http_v2_module --with-ipv6

Description

When an invalid header is encountered, it appears that all subsequent headers are also marked as invalid (whether they are or not). I specifically noticed this happen when doing POST requests (GET requests appeared to be unaffected). Additionally, the issue does not show up with SPDY 3.1 or HTTP/1.x.

I was testing an http2 rollout with nginx v1.9.6 and noticed that when users had a particular extension enabled in Chrome (Alexa Traffic Rank), it would append a custom header containing illegal characters. Rather than ignore the individual header, nginx was stripping out valid headers including the POST body.

When setting error_log to debug, Nginx was logging the request as follows:

nginx.1    | 2015/11/11 21:45:40 [info] 21#21: *175 client sent invalid header: "alexatoolbar-alx_ns_ph" while processing HTTP/2 connection, client: 10.9.1.21, server: _, host: "_"
nginx.1    | *175 client sent invalid header: "cookie" while processing HTTP/2 connection, client: 10.9.1.21, server: _, host: "_"
nginx.1    | 2015/11/11 21:45:40 [info] 21#21: *175 client sent invalid header: "x-requested-with" while processing HTTP/2 connection, client: 10.9.1.21, server: _, host: "_"
nginx.1    | 2015/11/11 21:45:40 [warn] 21#21: *175 a client request body is buffered to a temporary file /var/cache/nginx/client_temp/0000000001, client: 10.9.1.21, server: _, request: "POST /api HTTP/2.0", host: "_"

An interesting side-effect - for some reason, It seems that when an invalid header is encountered, Nginx reports "a client request body is buffered to a temporary file" regardless of how large I set the buffer in client_body_buffer_size.)

I was able to avoid this behavior by adding the following configuration directive:

ignore_invalid_headers off;

Change History (6)

comment:1 by Brian Stanback, 5 years ago

For what it's worth, the issue was actually happening to me on trac.nginx.org. I wasn't able to login with a combination of Chrome and the Alexa Traffic Rank extension, disabling the extension let me in without any issue.

comment:2 by Valentin V. Bartenev, 5 years ago

Status: newaccepted

Thank you for the report. Please try the following patch:

diff -r 909b5b191f25 src/http/v2/ngx_http_v2.c
--- a/src/http/v2/ngx_http_v2.c Thu Nov 05 15:01:09 2015 +0300
+++ b/src/http/v2/ngx_http_v2.c Thu Nov 12 18:01:06 2015 +0300
@@ -1630,6 +1630,8 @@ ngx_http_v2_state_process_header(ngx_htt
     }
 
     if (r->invalid_header) {
+        r->invalid_header = 0;
+
         cscf = ngx_http_get_module_srv_conf(r, ngx_http_core_module);
 
         if (cscf->ignore_invalid_headers) {

comment:3 by Valentin V. Bartenev, 5 years ago

The side-effect is cased by ignoring Content-Length in that case.

comment:4 by Brian Stanback, 5 years ago

I've applied your patch to my copy of the 1.9.6 source and did some testing - I can confirm that it resolves the issue.

Thanks for addressing this so quickly! The Content-Length explanation makes sense.

comment:5 by Valentin Bartenev <vbart@…>, 5 years ago

In 6291:932a465537ef/nginx:

HTTP/2: fixed invalid headers handling (ticket #831).

The r->invalid_header flag wasn't reset once an invalid header appeared in a
request, resulting in all subsequent headers in the request were also marked
as invalid.

comment:6 by Valentin V. Bartenev, 5 years ago

Resolution: fixed
Status: acceptedclosed

A different version of the patch has been committed after review.
It will be released with nginx 1.9.7.

Thanks.

Note: See TracTickets for help on using tickets.