Tornado security vulnerabilities with respect to DNS Rebinding

[gripedthumbtacks]

Hello,

Can some language be added to the security page regarding DNS rebinding attacks on Tornado apps? There is no mention of this issue, and by default, any app running on Tornado is likely to be vulnerable to DNS rebinding since this issue is not mentioned and the framework does not actively encourage protections against it.

http://www.tornadoweb.org/en/stable/guide/security.html

https://en.wikipedia.org/wiki/DNS_rebinding

A good base fix is pretty simple. Simply ensure that the default Host handler is at least somewhat restrictive. Perhaps default to localhost. If a developer or configuration wants to override this, they can refer to the security risks with allowing any domain in the HTTP Host header for their application. The risk of allowing * means that the app is likely vulnerable to DNS rebinding. Encouraging developers to choose safe defaults here could protect millions of users. A default regex that is not * would go a long way :) But I know fixing thousands of apps already built is impossible, so adding the language to the docs and thinking about the next major release changes to protect this might be the best way forward to protect new apps being created atop Tornado.

[ploxiln]

Just as a side note: this does not apply to applications that run on a server and are accessible from other servers. This only applies to tornado http server applications run on end-user systems. Also possible: run on servers but listening only on localhost or giving special authority to requests from localhost, while another service on that server is crawling arbitrary URLs or something. I'm sure this exists out there in some number, but it's probably not nearly the most common case.

[gripedthumbtacks]

@ploxiln Even though the number of apps that run like this may be small, the actual number of users impacted may be many millions of users because of how widely those few apps are deployed by potentially large impact companies. Just something to consider :)

[bdarnell]

The easy embeddability of Tornado means that these localhost-only apps are a significant use case (see ipython/jupyter, bokeh, etc). We should definitely add some discussion of this issue to the docs at a minimum.

We might also want to formally deprecate reliance on the default .* host pattern and require hostnames to be passed in. However, it would be a very long time before we could remove or change that, if ever. (Deprecating and eventually removing default_host, which has similar issues, would be easier, since I think it is not so commonly used).

[gripedthumbtacks]

Sounds reasonable. As you stated, the default host deprecation does sound like the best short term solution.

To note, the more problematic issues are such apps running on an internal network like RFC1918 address ranges. Just wanted to clarify that point so the impact is considered in additional to the localhost or local machine apps.

[bdarnell]

@DtpEJsaYXDU4GDH8dE4MyI9VrieF0UZpPZ0K76K I've added some docs in #2297; could you take a look?

[gripedthumbtacks]

@bdarnell looks good! Nice work :)