Opened 8 years ago

Last modified 3 years ago

#915 new enhancement

"Upgrade" header should not be proxied over h2

Reported by: Guillaume Rossolini Owned by:
Priority: minor Milestone:
Component: nginx-module Version: 1.9.x
Keywords: Cc:
uname -a: Linux XYZ 3.2.0-4-amd64 #1 SMP Debian 3.2.57-3 x86_64 GNU/Linux
nginx -V: nginx version: nginx/1.9.12
built by gcc 4.7.2 (Debian 4.7.2-5)
built with OpenSSL 1.0.2f 28 Jan 2016
TLS SNI support enabled
configure arguments: --with-cc-opt=-I/usr/local/include --with-ld-opt=-L/usr/local/lib --with-cpu-opt=amd64 --sbin-path=/usr/local/sbin --with-http_ssl_module --without-mail_pop3_module --without-mail_imap_module --without-mail_smtp_module --with-http_gunzip_module --with-http_stub_status_module --with-http_gzip_static_module --with-http_v2_module --add-module=/usr/local/src/ngx_brotli-557486 --conf-path=/etc/nginx/nginx.conf

Description

When proxying an HTTP/2-enabled webserver with nginx, nginx fetches resources using HTTP/1.1, which the backend server tries to upgrade to HTTP/2 by sending the "Upgrade: h2" header.
In the default configuration, this header is then forwarded to the client, which is incorrect.
In the case of nghttp, this is interpreted as an error:

inflatehd: header emission: upgrade: h2
recv: proclen=10
recv: HTTP error: type=1, id=13, header upgrade: h2
[  0.008] [INVALID; error=Invalid HTTP header field was received] recv HEADERS frame <length=798, flags=0x04, stream_id=13>
          ; END_HEADERS
          (padlen=0)
          ; First response header
recv: [IB_IGN_HEADER_BLOCK]

cf. https://github.com/curl/curl/issues/674

An example setup would be using httpd-2.4 as a backend.

This is easily remedied with the following nginx proxy config:

proxy_hide_header      Upgrade;

But maybe the default behaviour could be improved?

Change History (6)

comment:1 by Maxim Dounin, 8 years ago

The Upgrade header is expected to be present in response headers if a backend indeed upgrades the connection to a different protocol using "101 Switching Protocols". And it's currently used, e.g., by WebSocket proxying, so removing the Upgrade header from responses by default doesn't looks like a good idea.

Note well that the Upgrade header is removed by default from requests, and it previously worked well as the Upgrade header was expected to be initially used by clients, not servers. RFC7230 changed this though, and servers now can use the Upgrade header to indicate preferred protocols for future requests:

A server MAY send an Upgrade header field in any other response to
advertise that it implements support for upgrading to the listed
protocols, in order of descending preference, when appropriate for a
future request.

Not sure if we can do something with this.

comment:2 by Guillaume Rossolini, 8 years ago

To be clear, this might be specific to proxying when the backend responds with "upgrade: h2". Possibly only when the original request was already negotiated over http2.

In this case, the upgrade never makes sense. Either the original request is served over http2 already, and in this case the Upgrade shouldn't happen; or the original request is not served over http2, and in this case the proxy might not be able to upgrade on behalf of the backend. Or if it can, it should/could send its own headers, not those of the backend.
I might not be the best person to talk about this, but it seems nghttp's author has thought a great deal about this. See the issue on Gitub.

Their feedback to me was:
"This is because "upgrade" is disallowed in HTTP/2."

Version 0, edited 8 years ago by Guillaume Rossolini (next)

comment:3 by al.j.kinder@…, 8 years ago

I would like to add after experiencing this same issue it is still a problem with using nginx 1.9.15 and 1.10.0 as a reverse proxy for Apache 2.4.20. It seems to be the nginx should not even get a upgrade header from Apache since the communication between the servers were all done in http2 to start. On top of the previous posters mention of the error report in the nghttp2 client tool when used for http2 testing, this header specifically will make Safari 9/9.1 (http2 compatible) never load a page with the invalid (due to it being a http2 connection) upgrade header. The page in this example will never load and not supply any error on Safari 9/9.1

comment:4 by Laurence 'GreenReaper' Parry, 8 years ago

This was a very confusing blocker for us on multiple attempts to update our origin to h2; Safari clients (especially on mobile devices) could not access our new files. This results in massive connection churn and increased bandwidth use as the clients retry the download, impacting us for weeks afterwards, as the files in question were still cached and being served with the Upgrade header.

"proxy_hide_header Upgrade;" should be a default at least for cases where it includes h2 (possibly h2c?) as without it and with a HTTP/2 upgrading back-end, nginx essentially breaks for these clients.

The ticket says it applies to 1.9.x, but I'm using 1.11.5; it is a general issue. Fix should be backported to stable, as people are still migrating onto versions supporting HTTP/2 and will be using them for a while.

Last edited 8 years ago by Laurence 'GreenReaper' Parry (previous) (diff)

comment:5 by jan.reges.siteone.cz@…, 7 years ago

Hi,

this issue with "Upgrade" header in Nginx vs. Safari took me about 10-12 hours to debug :( Safari on MacOS/iOS retried a lot of requests on our website and did DoS attack. Retry-logic in Safari is very aggresive (for example, 500 requests to same URL in 10-15 seconds)

Hide Upgrade header by default for clients, please. It's very difficult to discover reason of this dangerous behavior.

Thank you.

comment:6 by Maxim Dounin, 3 years ago

See also #2078.

Note: See TracTickets for help on using tickets.