[Bug 59311] New: Do not send "Upgrade: h2" header to HTTP/1.1 clients when SSL/TLS is used

[b***@apache.org]

https://bz.apache.org/bugzilla/show_bug.cgi?id=59311

Bug ID: 59311
Summary: Do not send "Upgrade: h2" header to HTTP/1.1 clients
when SSL/TLS is used
Product: Apache httpd-2
Version: 2.4.18
Hardware: PC
OS: Linux
Status: NEW
Severity: normal
Priority: P2
Component: Core
Assignee: ***@httpd.apache.org
Reporter: apache-***@michael-kaufmann.ch

Apache httpd sends these response headers to HTTP/1.1 clients if the HTTP/2
protocol is enabled:

Upgrade: h2
Connection: Upgrade

These headers should NOT be sent when SSL is used, because support for HTTP/2
is advertised using ALPN. If the client does not advertise that it supports
HTTP/2 with ALPN, then Apache httpd knows that the client does not support
HTTP/2, so it should not send these headers.

This is a compatibility issue: Some clients behave strangely when they receive
these headers, e.g. old versions of NodeJS (
https://github.com/nodejs/node/issues/4334 )

I have tested some popular sites, and none of them sends these headers to
HTTP/1.1 clients:
- https://www.nghttp2.org/ (nghttpx nghttp2/1.10.0-DEV)
- https://www.google.ch/ (gws)
- https://twitter.com/ (tsa_b)
- https://www.nginx.com/ (nginx)
- https://h2o.examp1e.net/ (h2o/2.0.0-beta2)
- https://www.facebook.com

Note: This has previously been reported here:
https://github.com/icing/mod_h2/issues/73

[b***@apache.org]

https://bz.apache.org/bugzilla/show_bug.cgi?id=59311

--- Comment #1 from Michael Kaufmann <apache-***@michael-kaufmann.ch> ---
Created attachment 33755
--> https://bz.apache.org/bugzilla/attachment.cgi?id=33755&action=edit
Proposed patch: Do not send "Upgrade: h2"

[b***@apache.org]

https://bz.apache.org/bugzilla/show_bug.cgi?id=59311

Michael Kaufmann <apache-***@michael-kaufmann.ch> changed:

What |Removed |Added
----------------------------------------------------------------------------
Keywords| |PatchAvailable

[b***@apache.org]

https://bz.apache.org/bugzilla/show_bug.cgi?id=59311

A server MUST ignore an "h2" token in an Upgrade header field. Presence of a token with "h2" implies HTTP/2 over TLS, which is instead negotiated as described in Section 3.3.

This means that clients must not upgrade from HTTP/1.1 to HTTP/2 when using
TLS. It makes no sense that Apache sends an "Upgrade: h2" response header.

[b***@apache.org]

https://bz.apache.org/bugzilla/show_bug.cgi?id=59311

Michael Kaufmann <apache-***@michael-kaufmann.ch> changed:

What |Removed |Added
----------------------------------------------------------------------------
Keywords| |FixedInTrunk

--- Comment #3 from Michael Kaufmann <apache-***@michael-kaufmann.ch> ---
This has been fixed in trunk in r1740075. Proposal for backport to 2.4.x:
r1740079

[b***@apache.org]

https://bz.apache.org/bugzilla/show_bug.cgi?id=59311

Stefan Eissing <***@eissing.org> changed:

What |Removed |Added
----------------------------------------------------------------------------
Keywords|FixedInTrunk, |TryAgain
|PatchAvailable |

--- Comment #4 from Stefan Eissing <***@eissing.org> ---
Thinking about this again, I tested a simple solution using mod_headers. Just
add the following to your host config:

Header unset Upgrade

This will suppress any 'Upgrade' header in responses from the given host (or
all hosts if placed in the base server). Further conditional application is
possible using expressions. See mod_headers documentation for details.

Please let me know if this works for you.

[b***@apache.org]

https://bz.apache.org/bugzilla/show_bug.cgi?id=59311

--- Comment #5 from Michael Kaufmann <apache-***@michael-kaufmann.ch> ---
I don't need a workaround, I can compile Apache from the sources and apply
r1740075 + r1740119 ;-)

I think a good solution would be that the setting "H2Upgrade on/off" also
controls whether the "Upgrade: h2" or "Upgrade: h2c" is sent to the client
(depending on the type of the virtual host). I think wrowe would agree on this
(see
http://mail-archives.apache.org/mod_mbox/httpd-dev/201606.mbox/%3CCACsi251E_DoG-xoHL-Y5bKFWwNouDGZYbadyemB2TCfFbNUH6g%40mail.gmail.com%3E
)

If you disagree, then please go ahead and close this bug as "won't fix".

[b***@apache.org]

https://bz.apache.org/bugzilla/show_bug.cgi?id=59311

--- Comment #6 from Stefan Eissing <***@eissing.org> ---
It's not really HTTP/2 specific, although that is where you currently observer
it. So, a better directive would be placed in core, such as

ProtocolsHttpUpgradeAnnounce on|off

to suppress a response header and

ProtocolsHttpUpgrade on|off

to disable the whole mechanism (including announce).

[William A Rowe Jr]

The correct fix, in this particular case, is to use the Header unset
mechanism,
for the specific user-agent only.

What is the User-agent string of the offended client application?

Post by b***@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=59311
It's not really HTTP/2 specific, although that is where you currently observer
it. So, a better directive would be placed in core, such as
ProtocolsHttpUpgradeAnnounce on|off
to suppress a response header and
ProtocolsHttpUpgrade on|off
to disable the whole mechanism (including announce).
--
You are the assignee for the bug.
---------------------------------------------------------------------

[b***@apache.org]

https://bz.apache.org/bugzilla/show_bug.cgi?id=59311

--- Comment #7 from Laurence 'GreenReaper' Parry <***@hotmail.com> ---
One other thing this breaks: nginx cached responses. This caused us to rollback
our Apache HTTP/2 upgrade because iOS Safari clients were unable to load files
from our nginx caches which had been served from our Apache origin.

We worked around this at the time with proxy_hide_header Upgrade;
The Headers fix is better because it then doesn't even save to the cache.

[b***@apache.org]

https://bz.apache.org/bugzilla/show_bug.cgi?id=59311

Jim Jagielski <***@apache.org> changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |FIXED

[b***@apache.org]

https://bz.apache.org/bugzilla/show_bug.cgi?id=59311

Michael Kaufmann <apache-***@michael-kaufmann.ch> changed:

What |Removed |Added
----------------------------------------------------------------------------
Resolution|FIXED |---
Status|RESOLVED |REOPENED

--- Comment #8 from Michael Kaufmann <apache-***@michael-kaufmann.ch> ---
Unfortunately, the commit mentioned in comment 3 has been reverted in r1748591,
so this bug is still present :-(

See this thread:
http://mail-archives.apache.org/mod_mbox/httpd-dev/201606.mbox/%3CB388B626-2EC2-4F67-B418-98F8DF8D3968%40greenbytes.de%3E

[b***@apache.org]

https://bz.apache.org/bugzilla/show_bug.cgi?id=59311

--- Comment #9 from Stefan Eissing <***@eissing.org> ---
Michael, it is nice that you reference my mail. It would be even more helpful
if you could answer the question I asked you:

As per dev discussion on the list, we'd like to fix this (by default without
further config) for the UAs that have problems with it. I asked you for the UA
strings (or pattern) where the fix is needed. Unless I overlooked something,
you did not give an answer to that.

As stated in that mail discussion, Apache httpd will make workarounds for UAs
with faults in implementing the HTTP specs. It will however make use of all
features of the protocol and expect others to cope with that as well.

If you prefer your installations to behave in a non-default way, there are
plenty of directives that allow you to do so. In this case, an easy example to
suppress the header was provided.

Either you can make a case for UA strings where httpd should make a work-around
or I will close this ticket as wont-fix.

[b***@apache.org]

https://bz.apache.org/bugzilla/show_bug.cgi?id=59311

As per dev discussion on the list, we'd like to fix this (by default without further config) for the UAs that have problems with it. I asked you for the UA strings (or pattern) where the fix is needed. Unless I overlooked something, you did not give an answer to that.

There was a consensus on the mailing list that more discussion is needed about
how to address this issue. There was no consensus about what to implement.

I have no such list of affected user-agents. NodeJS is affected, but I don't
know the affected versions.

If you prefer your installations to behave in a non-default way, there are plenty of directives that allow you to do so. In this case, an easy example to suppress the header was provided.

It is disputed whether sending the "Upgrade: h2" header is a "default". Most
other HTTP/2 servers don't send it.

[b***@apache.org]

https://bz.apache.org/bugzilla/show_bug.cgi?id=59311

brian russell <***@yahoo.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@yahoo.com

[b***@apache.org]

https://bz.apache.org/bugzilla/show_bug.cgi?id=59311

Szőgyényi Gábor <***@freemail.hu> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@freemail.hu

[b***@apache.org]

https://bz.apache.org/bugzilla/show_bug.cgi?id=59311

***@vshosting.cz changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@vshosting.cz

[b***@apache.org]

https://bz.apache.org/bugzilla/show_bug.cgi?id=59311

Paul Menzel <pmenzel+***@molgen.mpg.de> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |pmenzel+***@molge
| |n.mpg.de