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

Multiple Host header values handled poorly #8716

Closed
joakime opened this issue Oct 14, 2022 · 14 comments
Closed

Multiple Host header values handled poorly #8716

joakime opened this issue Oct 14, 2022 · 14 comments
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@joakime
Copy link
Contributor

joakime commented Oct 14, 2022

Jetty version(s)
9.4.x thru 12.0.x

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

OS type/version
All

Description
When a request is received with multiple host headers it's handled poorly.

This was pointed out by @Hexcles at #7278 (comment)

Example using the jetty 11 demos.

$ curl -vvvv -H"Host: localhost, foo, bar, zed" http://localhost:8080/test/dump/info
*   Trying 127.0.0.1:8080...
* Connected to localhost (127.0.0.1) port 8080 (#0)
> GET /test/dump/info HTTP/1.1
> Host: localhost, foo, bar, zed
> User-Agent: curl/7.81.0
> Accept: */*
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Set-Cookie: visited=yes
< Expires: Thu, 01 Jan 1970 00:00:00 GMT
< Set-Cookie: JSESSIONID=node013ntsnazjuutz1gpikvehddsnk1.node0; Path=/test
< Content-Type: text/html;charset=utf-8
< Vary: Accept-Encoding
< Content-Length: 10477
< Server: Jetty(11.0.11)
< 
<html>
<body>
<h1>Dump Servlet</h1>
...(snip)...
<th align="right">getRequestURI:&nbsp;</th><td>/test/dump/info</td></tr><tr>
<th align="right">getRequestURL:&nbsp;</th><td>http://localhost, foo, bar, zed/test/dump/info</td></tr><tr>
<th align="right">getScheme:&nbsp;</th><td>http</td></tr><tr>
<th align="right">getServerName:&nbsp;</th><td>localhost, foo, bar, zed</td></tr><tr>
...(snip)...

So the key bits in the above example are ...

  • HTTP/1.1 was used
  • The request header Host: localhost, foo, bar, zed was sent
  • Jetty parsed it without error, and it resulted in odd API behavior
    • HttpServletRequest.getRequestURL() is http://localhost, foo, bar, zed/test/dump/info
    • HttpServletRequest.getServerName() is localhost, foo, bar, zed
@joakime
Copy link
Contributor Author

joakime commented Oct 14, 2022

Looks like Jetty interprets the all field values, including commas, as a single "Host" (or Server Name), complete with commas.

@joakime joakime changed the title Multiple Host headers values treated poorly Multiple Host header values handled poorly Oct 14, 2022
@Hexcles
Copy link

Hexcles commented Oct 14, 2022

Oh a minor correction: it's not multiple Host values that I encountered (I'm not sure if you're supposed to split Host value by commas like other headers), but multiple Host`` headers. You can't produce such a malformed request using curl, but you can use netcat: https://blog.shalvah.me/posts/fixing-the-host-header-vulnerability-with-nginx#:~:text=a%20custom%20middleware.-,Appendix,-If%20you%27re%20interested

https://httpwg.org/specs/rfc9112.html#request.target

A server MUST respond with a 400 (Bad Request) status code to any HTTP/1.1 request message that lacks a Host header field and to any request message that contains more than one Host header field line or a Host header field with an invalid field value.

@joakime
Copy link
Contributor Author

joakime commented Oct 14, 2022

We might want to revive the closed PR #7279 as it addressed many of the "bad host" and "bad authority" cases. (just not the multiple header cases)

joakime added a commit that referenced this issue Oct 14, 2022
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
@joakime
Copy link
Contributor Author

joakime commented Oct 14, 2022

Opened PR #8717 as a simple way to address this

@joakime joakime self-assigned this Oct 14, 2022
joakime added a commit that referenced this issue Oct 25, 2022
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Nov 9, 2022
* Issue #8716 - Handle bad host/authority headers better
* Remove extra `Host` header in testcase that doesn't deal with bad Host headers
* Create URIUtil.isRegName
* Correcting HostPortTest.testValidAuthority
* Correcting RequestTest.testInvalidHostHeader
* Remove clonable, set to final

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
@joakime
Copy link
Contributor Author

joakime commented Dec 1, 2022

Closing, as PR #8717 is merged.

@joakime joakime closed this as completed Dec 1, 2022
@bjorncs
Copy link
Contributor

bjorncs commented Jan 18, 2023

Will Jetty issue a security advisory for the previous behaviour for accepting multiple Host headers? Jetty was susceptible of Host header injection attacks when combined with a load balancer / reverse proxy accepting multiple Host headers, as outlined in #7278 (comment).

@joakime
Copy link
Contributor Author

joakime commented Jan 18, 2023

@bjorncs please explain how accepting the last Host header is a security advisory?

@joakime
Copy link
Contributor Author

joakime commented Jan 18, 2023

@bjorncs there's a few variations of this ...

Variation 1: Duplicate headers, same value

GET /foo HTTP/1.1
Host: machine.com
Host: machine.com
Connection: close

This variation is rejected by the HttpParser.

Variation 2: Duplicate headers, different values

GET /foo HTTP/1.1
Host: machine.com
Host: example.org
Connection: close

This variation is rejected by the HttpParser.

Variation 3: Single header, comma delimited, with space, same value

GET /foo HTTP/1.1
Host: machine.com, machine.com
Connection: close

As the parsing of the Host header is not ever delimited, this is viewed a single authority.
HostPort will reject this due to the space character.

Variation 4: Single header, comma delimited, with space, different values

GET /foo HTTP/1.1
Host: machine.com, example.org
Connection: close

As the parsing of the Host header is not ever delimited, this is viewed a single authority.
HostPort will reject this due to the space character.

Variation 5: Single Header, comma delimited, no space.

GET /foo HTTP/1.1
Host: machine.com,example.org
Connection: close

As the parsing of the Host header is not ever delimited, this is viewed a single authority.
HostPort will allow this due to the fact that comma is not an invalid character for an authority.

@joakime
Copy link
Contributor Author

joakime commented Jan 18, 2023

What does java support when it comes to the variations of multiple headers with a comma?

 jshell> new URI("http", "machine.com,example.org", "/", null, null)
$1 ==> http://machine.com,example.org/

jshell> new URI("http://machine.com,example.org/")
$2 ==> http://machine.com,example.org/

jshell> new URI("http://machine.com, example.org/")
|  Exception java.net.URISyntaxException: Illegal character in authority at index 7: http://machine.com, example.org/
|        at URI$Parser.fail (URI.java:2913)
|        at URI$Parser.parseAuthority (URI.java:3247)
|        at URI$Parser.parseHierarchical (URI.java:3158)
|        at URI$Parser.parse (URI.java:3114)
|        at URI.<init> (URI.java:600)
|        at (#4:1)

@gregw
Copy link
Contributor

gregw commented Jan 18, 2023

I tried to generate a CVSS score for this change, but the problem is that on it's own there is no affect - no change of scope, no loss of integrity, no loss of confidentiality, no loss of availability.... so the score is 0.0.

It's the kind of issue that needs another component in order to have consequences. I.e. if a front end policed some security policy based on the first host header and then jetty acted on the second host header, that would indeed be a security problem.
So perhaps that is a loss of integrity with high attack complexity? CVSS:3.1/AV:N/AC:H/PR:L/UI:N/S:U/C:N/I:L/A:N That's a score of 3.1, so at least something we could make an advisory around.

@bjorncs
Copy link
Contributor

bjorncs commented Jan 20, 2023

@joakime
I have reproduced variation 2 on 9.4.49 and 11.0.12 using nc/openssl s_client with handwritten HTTP/1.1 request. Both instances results in 200 OK in our configuration of Jetty embedded.

I'll try to outline the hypothetical scenario from my previous comment.
Let's say Jetty is configured with two virtual hosts, public.api and internal.api.
The public.api virtual host serves handlers that should be accessible publicy through a reverse proxy. The internal.api virtual host serves handlers that should not be accessible from the reverse proxy.

I interpret @Hexles's comment as CDNs such as Cloudflare would route based on the value of the first Host header, while including both Host headers in the forwarded request.

GET /my/internal/api HTTP/1.1
Host: public.api
Host: internal.api
Connection: close

Could the above request access an internal handler/rest-API in this scenario?

@gregw Agree, it's a fair assessment that it's not worthy an advisory. A reverse proxy that's non-compliant with RFC-9112 is required, and relying on virtual hosts configuration as a security measure is unlikely. I have not verified whether claims regarding the CDNs' handling of Host headers are true.

@Hexcles
Copy link

Hexcles commented Jan 22, 2023

It's easy to verify that scenario 2 does get through Cloudflare (routed by the first Host), but I'm not at work and can't reproduce/confirm that the second Host isn't dropped by Cloudflare. Nginx, on the other hand, rejects scenario 2 with 400.

There is another variant 6: Host inconsistent with the authority component in the target URI:

GET https://public.api/my/internal/api HTTP/1.1
Host: internal.api
Connection: close

This scenario gets past both Cloudflare and Nginx, and I'd be surprised if the Host header is dropped by either proxy in this case. I believe this Host header should be considered as an "invalid field value" and result in a 400, too, but that's a different story.

@bjorncs your idea about having a single Jetty with two virtual hosts is interesting, and might worth verifying. In my case, it was because of some application-level auth code that determines whether a request is internal by looking at the host (to see if it's using the private Kubernetes domain). In addition, our pentester used this trick to get past WAF to exploit the infamous log4j bug because of course host/URL is logged.

@gregw
Copy link
Contributor

gregw commented Jan 23, 2023

All,

I'm very happy for us to issue a security advisory on this. We were interpreting the host header contrary to the specification, which in some circumstances can definitely be a security issue. It's hard to characterize, but no harm at least advising people that if they are on an old jetty, they need to consider any checking/routing done by an intermediary that might interpret multiple host headers differently to jetty.

@joakime
Copy link
Contributor Author

joakime commented Feb 3, 2023

@bjorncs @Hexcles There's been 2 recent PRs in jetty-10.0.x (will be merged to jetty-11.0.x) that are related to these issues.

Default is to reject with 400 Bad Request.

These Compliance modes exist allow a server to operate in an unsafe mode while they work with their clients to address their bad usages of HTTP.

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
Projects
None yet
Development

No branches or pull requests

4 participants