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

Open redirect when sending custom Host header to URL with no trailing forward-slash #5029

Closed
jeanbritz opened this issue Jul 7, 2020 · 15 comments · Fixed by #5038
Closed
Labels

Comments

@jeanbritz
Copy link

jeanbritz commented Jul 7, 2020

Jetty version
9.4.9.v20180320
Java version
Oracle JDK 1.8.0_191-b12
OS type/version
Windows 10
Description
I have found that if you send an HTTP POST request to a Jetty server instance and you don't end the URL with a trailing forward-slash (/) it performs a redirection to the URL with the forward-slash appended. With this behaviour I can send a custom Host header which will be included in the redirect response.

For example:
I create a socket connection to localhost:8081.
Request
POST /servlet HTTP/1.1
Host: www.google.com:443
Content-Type: application/json
Content-Length: 16

{"name":"value"}

Response
HTTP/1.1 302 Found
Date: Tue, 07 Jul 2020 21:04:38 GMT
Location: http://www.google.com:443/servlet/
Content-Length: 0

I have not configured any virtual hosts.
Is this the intented behaviour? Because this can be regarded as an open redirect

@joakime joakime added Bug For general bugs on Jetty side High Priority Security labels Jul 7, 2020
@joakime
Copy link
Contributor

joakime commented Jul 7, 2020

That is indeed an interesting behavior.

No virtual hosts is pretty normal.
Have to ponder what this means.

@gregw
Copy link
Contributor

gregw commented Jul 8, 2020

This is entirely normal behaviour and is exactly what the host header is intended for.

If you have a machine that is hosting multiple virtual domains on the same IP address, then the only way you can ever send a redirect to the correct URL is to use the Host header.
But you may also have a client with a private DNS so that it can refer to a server with a private name, again in that case the only way to correctly do redirects is to use the Host header.
Even without virtual hosts, you may have a client using IP addresses as a host name, and again they may be private to the client due to NAT, so any redirect needs to use the Host header else it will be meaningless to the client.

If we started using the IP address or server name for such redirects, then we'd have the problem of leaking potentially private information from within a server cluster, as the IP and/or address name.

@gregw
Copy link
Contributor

gregw commented Jul 8, 2020

Doing some further research, the Location header is defined by https://tools.ietf.org/html/rfc7231#section-7.1.2 and it says that absolute URIs should be resolved against the effective request URI.
The effective request URI is defined in https://tools.ietf.org/html/rfc7230#section-5.5 and allows for the Host header to be used.

So I think we are doing things correctly. However, the question remains.... can we do things even better?

  • Why are we using an absolute redirection when a relative one would work and avoid the problem?
  • Should we offer a configuration to have fixed server name used for all absolute redirections?
  • Should we offer a mode where only hosts configured as virtual hosts are acceptable for redirections? In this case what should we use when a non virtual host arrives, as we don't want to leak info?

@gregw gregw added Enhancement Question and removed Bug For general bugs on Jetty side High Priority Security labels Jul 8, 2020
@gregw
Copy link
Contributor

gregw commented Jul 8, 2020

Finally I don't think this is an open redirect because, whilst Host header is not really trusted data, it is different to a URI path or parameter which can be either entered by a user or passed in a link. A browser cannot be tricked into sending a request to a different host than the host header with typical phishing attacks. To make a Host header different to the network routing implies an attacker already has access to change browser behaviour and/or alter bytes on the wire, so such a connection is already vulnerable.

@joakime
Copy link
Contributor

joakime commented Jul 8, 2020

I think we could issue a redirect without the hostname portion, no?

POST /servlet HTTP/1.1
Host: www.google.com:443
Content-Type: application/json
Content-Length: 16

{"name":"value"}

HTTP/1.1 302 Found
Date: Tue, 07 Jul 2020 21:04:38 GMT
Location: /servlet/
Content-Length: 0

@joakime
Copy link
Contributor

joakime commented Jul 8, 2020

Yeah, the RFC2616 rules on Location are that it is an absolute-uri, but RFC7230 allows for a relative reference.

https://tools.ietf.org/html/rfc7231#section-7.1.2

But this leads to something interesting, as the resolution of a chain of Location references is based on the original request URI (by the User-Agent).

Example:

Original Request is against http://machine.com/servlet

POST /servlet HTTP/1.1
Host: machine.com:80
Content-Type: application/json
Content-Length: 16

{"name":"value"}

HTTP/1.1 302 Found
Date: Tue, 07 Jul 2020 21:04:38 GMT
Location: https://www.machine.com:443/servlet
Content-Length: 0

Which redirects to https://www.machine.com:443/servlet

POST /servlet HTTP/1.1
Host: www.machine.com:443
Content-Type: application/json
Content-Length: 16

{"name":"value"}

HTTP/1.1 302 Found
Date: Tue, 07 Jul 2020 21:04:38 GMT
Location: /login
Content-Length: 0

Which redirects for login, but this is where it gets interesting...
The User-Agent (Chrome) will use this relative Location: /login to issue the next request based on the original URI.
Which becomes http://machine.com/login that it requests.

@joakime
Copy link
Contributor

joakime commented Jul 8, 2020

I don't see that last example as a show stopper.

Perhaps when we know we are in RFC7230 Compliance mode for the connector, we only make the Location uri absolute under the following circumstances ...

  • Application code set the Location header to absolute. (either by setting the Location response header directly, or using the HttpServletResponse.sendRedirect() method)
  • Redirect is attempting to change the host-name
  • Redirect is attempting to change the port
  • Redirect is attempting to change the scheme

All other redirects are relative.
WDYT?

@gregw
Copy link
Contributor

gregw commented Jul 8, 2020

Sounds good. We could even possibly control this with a compliance setting?

@joakime
Copy link
Contributor

joakime commented Jul 8, 2020

I think it should work with a compliance setting, sure.

But I still have 2 scenarios I'm still thinking about.

  1. How do virtual-host configurations play into this?
  2. How does SNI resolution play into this?

Are either of those scenarios currently impacting the absolute-uri resolution of the Location header we currently have?

@sbordet
Copy link
Contributor

sbordet commented Jul 8, 2020

@srapizzi what do you think of this issue?

We don't think there is a security issue, perhaps just an enhancement to produce relative Location headers, etc.

Are there security tools that check for this particular case? Thanks!

@srapizzi
Copy link

srapizzi commented Jul 8, 2020

Hi @sbordet
technically, it is an open redirect but it is not exploitable unless we are in a man-in-the-middle situation where the attacker can change the header HOST, or in a web-cache poisoning condition if it still works in modern infrastracture.
We can consider it not exploitable with zero impact so far.

@gregw
Copy link
Contributor

gregw commented Jul 9, 2020

Note that if this is considered a vulnerability in jetty, then apache have the exact same "vulnerability":

[315] telnet apache.org 80
Trying 40.79.78.1...
Connected to apache.org.
Escape character is '^]'.

GET /images HTTP/1.0
Host: www.example.com:8080

HTTP/1.1 301 Moved Permanently
Date: Thu, 09 Jul 2020 09:30:08 GMT
Server: Apache/2.4.18 (Ubuntu)
Location: http://www.example.com:8080/images/
Cache-Control: max-age=3600
Expires: Thu, 09 Jul 2020 10:30:08 GMT
Content-Length: 239
Connection: close
Content-Type: text/html; charset=iso-8859-1

<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML 2.0//EN">
<html><head>
<title>301 Moved Permanently</title>
</head><body>
<h1>Moved Permanently</h1>
<p>The document has moved <a href="http://www.example.com:8080/images/">here</a>.</p>
</body></html>
Connection closed by foreign host.

I think this is just the spec being correctly implemented.

gregw added a commit that referenced this issue Jul 9, 2020
Provide option to allow relative redirection
@gregw gregw linked a pull request Jul 9, 2020 that will close this issue
@gregw
Copy link
Contributor

gregw commented Jul 9, 2020

Since relative redirects are now allowed by RFC 7230, I've created a PR to add an option to use them when appropriate.

@jeanbritz
Copy link
Author

Thank you @gregw for providing a quick fix

gregw added a commit that referenced this issue Jul 9, 2020
gregw added a commit that referenced this issue Jul 15, 2020
* Issue #5029 Relative Redirection

Provide option to allow relative redirection

* Issue #5029 Relative Redirection

Fixed checkstyle

* rename from review
@joakime
Copy link
Contributor

joakime commented Jul 22, 2020

PR #5038 merged

@joakime joakime closed this as completed Jul 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants