Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly support HTTP/1.1 with non-http scheme absolute-uri and empty Host header #7278

Open
joakime opened this issue Dec 14, 2021 · 4 comments
Labels
Bug For general bugs on Jetty side Specification For all industry Specifications (IETF / Servlet / etc)

Comments

@joakime
Copy link
Contributor

joakime commented Dec 14, 2021

Jetty version(s)
9+

Java version/vendor (use: java -version)
All

OS type/version
All

Description
In the process of investigating the HostHeaderCustomizer in issue #7250 and the HostPort implementation in issue #7269 it was discovered
that our handling of empty Host header is wholly invalid and introduces a side effect on HTTP/1.1 requests that causes the exposure
of internal server ip to on requests forwarded through intermediaries as identified in issue #7277

Per the HTTP/1.1 spec section on Host
https://datatracker.ietf.org/doc/html/rfc7230#section-5.4

   The "Host" header field in a request provides the host and port
   information from the target URI, enabling the origin server to
   distinguish among resources while servicing requests for multiple
   host names on a single IP address.

     Host = uri-host [ ":" port ] ; Section 2.7.1

   A client MUST send a Host header field in all HTTP/1.1 request
   messages.  If the target URI includes an authority component, then a
   client MUST send a field-value for Host that is identical to that
   authority component, excluding any userinfo subcomponent and its "@"
   delimiter (Section 2.7.1).  If the authority component is missing or
   undefined for the target URI, then a client MUST send a Host header
   field with an empty field-value.

Basically, on HTTP/1.1 the rules for the Host header are ...

  1. a Host header field MUST always be present
  2. if the request uri path is absolute, then the Host header value MUST be the same as the absolute URI authority (even empty).
  3. if the request uri path is relative, then the Host header value MUST be a valid authority with a defined "host" portion.

If you don't satisfy those requirements, the request results in a 400 Bad Request.

We do a good job on rule 1 already.
We fail rule 2.
However the above mentioned issues show we fail on rule 3.
The same above mentioned Issues will address rules 1 and 3, but this issue needs to address rule 2 separately.

@joakime joakime added the Bug For general bugs on Jetty side label Dec 14, 2021
@joakime
Copy link
Contributor Author

joakime commented Dec 14, 2021

Note: that the http spec does not allow for a URI with scheme http or https to have no authority.

Also, the websocket spec's URI for schemes ws and wss defers to the http spec uri definition, so those too cannot have a no-authority scenario.

@Hexcles
Copy link

Hexcles commented Oct 14, 2022

One more point: Jetty doesn't seem to throw on multiple Host headers (nginx, for example, does). Even worse, it seems to take the last value, which can be surprising: e.g. CDNs like Cloudflare usually proxy based on the first Host, so I was puzzled to see an unexpected domain in our backend server only reachable from Cloudflare and I even thought it was a critical Cloudflare bug for a while.

@joakime
Copy link
Contributor Author

joakime commented Oct 14, 2022

One more point: Jetty doesn't seem to throw on multiple Host headers (nginx, for example, does). Even worse, it seems to take the last value, which can be surprising: e.g. CDNs like Cloudflare usually proxy based on the first Host, so I was puzzled to see an unexpected domain in our backend server only reachable from Cloudflare and I even thought it was a critical Cloudflare bug for a while.

I just opened Issue #8716 about this

@github-actions
Copy link

This issue has been automatically marked as stale because it has been a
full year without activity. It will be closed if no further activity occurs.
Thank you for your contributions.

@github-actions github-actions bot added the Stale For auto-closed stale issues and pull requests label Oct 15, 2023
@sbordet sbordet added Specification For all industry Specifications (IETF / Servlet / etc) and removed Stale For auto-closed stale issues and pull requests labels Oct 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side Specification For all industry Specifications (IETF / Servlet / etc)
Projects
None yet
Development

No branches or pull requests

3 participants