Provide a host application setting

[bhch]

The docs, and almost every Tornado code example on the internet adds handlers to an Application via the handlers argument to the constructor. That means the application accepts requests for any host.

But listening to wildcard hosts is vulnerable to HTTP Host header vulnerability.

Please provide a new app setting called host which, if set, will be used to match all incoming requests.

[bdarnell]

What exactly do you mean by "HTTP Host header vulnerability"? Is it the same as DNS rebinding, discussed in #2256, or is there something more? The docs added in https://github.com/tornadoweb/tornado/pull/2297/files show the two current methods for validating the Host header. I don't think we need a new way to specify this as much as we need more encouragement of the methods we already have.

[bhch]

When an app accepts requests for wildcard hosts, an attacker can send their own controlled domain in the Host header. This becomes a problem if the application relies on request.host to construct full urls for certain things such as password reset links or absolute URLs for static files (include_host option in RequestHandler.static_url method).

For example, an attacker can initiate a password reset for any user and pass their own controlled domain via the Host header:

curl -d "username=victim" -H "Host: attacker-website.com" -X POST http://example.com/forgot-password

Your app will construct the full url with the attacker's supplied domain name and send the link to the victim in an email.

When the victim visits that link, they are taken to the attackers website which can capture the token from the URL.

This only happens when the app accepts wildcard domains.

I don't think we need a new way to specify this as much as we need more encouragement of the methods we already have.

IDK, but adding handlers via the Application constructor has become the de-facto way. Literally hundreds of tutorials across the web show it that way.

A host setting will provide a shortcut for restricting the host. If this is not set, Tornado can print a warning in the console, or something to that effect.

If not, then either the examples in the docs should be changed, or some sort of warning must be added.

Fortunately, I've been running Tornado behind Nginx so it hasn't been a problem for me. But apps hosted on PaaS platforms will be vulnerable.

[bdarnell]

It seems to me that the vulnerability here is not in the fact that the server accepts requests with unknown host headers, but that the app generates password reset links (or other URLs) with unverified client-supplied data. And this is still tornado's fault, but I'd place the blame on the HTTPServerRequest.full_url method. This method needs a better way to determine what hostname it should be using; the Host header is clearly wrong unless it's been validated. But it's also not right to make this an application setting - it's perfectly legitimate (and common in my experience) for Tornado apps to serve multiple hostnames from one Application object.

I think what we need is some sort of flag that is set on HTTPServerRequest if the Host header was matched against a non-trivial regex (something other than .*), and full_url should fail if this is not set.

[bhch]

it's perfectly legitimate (and common in my experience) for Tornado apps to serve multiple hostnames from one Application object.

For allowing multiple hosts, we can take ideas from Django's ALLOWED_HOSTS setting.

But instead of a list, it could be a regex so that it readily works with the HostMatches matcher.

Something like: allowed_hosts = r'(example\.com|example\.org)'.

So:

1. No breaking changes if this value is not set. Tornado will keep matching wildcard hosts (with AnyMatches).
2. If this value is set, match the host with the given pattern (with HostMatches).

[bdarnell]

Hmm, an allowed_hosts setting that automatically creates an appropriate HostMatches is an interesting idea. It doesn't save that much code compared to using HostMatches directly, but is likely to avoid the need to import tornado.routing at all in most cases and perhaps most importantly it gives us a place to document the importance of the feature.

On the other hand, a multi-valued allowed_hosts setting might cause confusion for any user who actually needs multiple hosts, since those users will generally want to explicitly use HostMatches on at least some of their routes, so relying on the allowed_hosts shortcut might hinder learning about the more explicit form. And regular expressions are annoying for hostnames since . is significant in both syntaxes.

I'll think about this some more, and how it might relate to changes to full_url.

[xylo987]

Could use UFW. deny and allow you want.

[bdarnell]

I'm not very familiar with UFW but I don't see how it could be used here. Note that the problem here relates primarily to the HTTP Host header and not about TCP/IP traffic. If there's a way to solve this with UFW, please explain how it would work in more detail.