Opened 8 years ago

Closed 5 years ago

#6880 closed Bug (fixed)

django.http.get_host() breaks reverse proxying on apache

Reported by: Kellen Owned by: nobody
Component: HTTP handling Version: master
Severity: Normal Keywords: redirect httpresponseredirect get_host reverse proxy squid apache
Cc: kellen@… Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

django.http.HttpResponseRedirect uses django.http.get_host() to get the hostname for relative redirects, but generates incorrect redirects when an app is behind a reverse proxy. In particular, get_host() uses the HTTP_X_FORWARDED_HOST, which will be the proxying domain rather than the proxied-to domain. This is bad when a proxy is being used to avoid part of /the/path/to/an/app/, since the whole path will be appended to the proxying domain, which will then rewrite it for the proxied-to domain and most often 404.

As an example:
I have a non-fancy django app, which uses HttpResponseRedirect to forward from various pages to others (say, on successful form submission). This lives on one domain, called backend. On another domain, frontend, I am using apache2 to reverse proxy to part of the application on backend.

When my app successfully adds a new item, it redirects from the add view at /app/object/add/ to the object view at /app/object/id/. This works when one adds an object while accessing backend directly. When one adds an object through frontend, however, a redirect is issued to the correct path on backend, but on the frontend domain. Backend should issue a redirect to: backend.com/app/object/id/, which apache should rewrite to frontend.com/object/id/ with ProxyPassReverse, which the browser should then attempt to access, and /object/id/ should be rewritten to /app/object/id/ on backend. What happens instead is a redirect to frontend.com/app/object/id/ is issued, the browser accesses the path, and it is rewritten to /app/app/object/id/ on backend, and a 404 results.

On frontend:

<VirtualHost xxx.xxx.xxx.xxx>
    ServerAdmin admin@frontend
    DocumentRoot /var/www/
    ServerName www.frontend.com
    ServerAlias frontend.com

    ProxyPass / http://frontend.com/app/
    ProxyPassReverse / http://frontend.com/app/
    SetOutputFilter proxy-html
    ProxyHTMLURLMap http://frontend.com/app/ /
</VirtualHost>

Attachments (3)

6880_patch.diff (2.7 KB) - added by Kellen 8 years ago.
patch adding USE_X_FORWARDED_HOST setting in get_host, docs
get_hostpatch.txt (711 bytes) - added by Kellen 7 years ago.
removes usage of X_HTTP_FORWARDED HOST
6880.diff (3.2 KB) - added by aaugustin 5 years ago.

Download all attachments as: .zip

Change History (18)

comment:1 Changed 8 years ago by Kellen <kellen@…>

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Yay for accidentally hitting submit.

The relative redirects inside an application can be avoided by using the full host, but this (a) is ugly and (b) doesn't work for, e.g., views which use @login_required. My temporary solution was to comment out the offending lines in get_host(), which solves my specific problem, but I'm unsure of what sort of impact this could have on other setups.

        #if 'HTTP_X_FORWARDED_HOST' in self.META:
        #    host = self.META['HTTP_X_FORWARDED_HOST']
        #elif 'HTTP_HOST' in self.META:
        if 'HTTP_HOST' in self.META:

Debian, apache 2.2.3-4+etch4, django 0.97~svn7189-1.

comment:2 Changed 8 years ago by Kellen <kellen@…>

  • Cc kellen@… added
  • Component changed from Uncategorized to HTTP handling
  • Keywords redirect httpresponseredirect get_host reverse proxy squid apache added

comment:3 Changed 8 years ago by Kellen <kellen@…>

This is related to: #4986 in which SmileyChris cites http://www.python.org/dev/peps/pep-0333/#url-reconstruction as his inspiration.

comment:4 Changed 8 years ago by Kellen <kellen@…>

Wow, sorry that apache snippet should be:

<VirtualHost xxx.xxx.xxx.xxx>
    ServerAdmin admin@frontend.com
    DocumentRoot /var/www/
    ServerName www.frontend.com
    ServerAlias frontend.com

    ProxyPass / http://backend.com/app/
    ProxyPassReverse / http://backend.com/app/
    SetOutputFilter proxy-html
    ProxyHTMLURLMap http://backend.com/app/ /
</VirtualHost>

comment:5 Changed 8 years ago by Simon Greenhill

  • Triage Stage changed from Unreviewed to Accepted

Changed 8 years ago by Kellen

patch adding USE_X_FORWARDED_HOST setting in get_host, docs

comment:6 Changed 8 years ago by Kellen

  • Has patch set

comment:7 Changed 8 years ago by Kellen

Patch adds a USE_X_FORWARDED_HOST setting for people who are in (what seem to me to be) non-standard virtual hosting configurations where they need the HTTP_X_FORWARDED_HOST as the hostname. So the default behavior if the patch is applied will be to ignore the header and use HTTP_HOST instead.

Changed 7 years ago by Kellen

removes usage of X_HTTP_FORWARDED HOST

comment:8 Changed 7 years ago by Kellen

  • Triage Stage changed from Accepted to Design decision needed

Patch attached, per a conversation with SmileyChris in IRC, and per http://code.djangoproject.com/ticket/9064#comment:5

(00:26:53) SmileyChris: Kellen`: i think the "right way" to do it would be to not use X_FORWARDED_HOST at all
(00:27:06) SmileyChris: Kellen`: and use middleware to set it
(00:27:12) SmileyChris: if it's needed
...
(00:27:40) Kellen`: maybe it doesn't matter in the future if django will recommend wsgi only
(00:28:10) SmileyChris: doesn't mean other cases are void
(00:28:33) SmileyChris: my suggestion currently would be if someone needs that behaviour to write middleware which clears their FORWARDED_HOST var
...
(00:33:32) Kellen`: shall i just submit a patch taking that line out, then?
(00:33:55) SmileyChris: yeah, why not
(00:34:04) SmileyChris: and move the ticket back to design decision
(00:34:24) SmileyChris: also mention that someone could work around the current issue by using some middleware (in case anyone stumbles across it)
(00:35:31) SmileyChris: Kellen`: note that the x meta var was there before i played with it, so it's not really my fault ;)

comment:9 Changed 5 years ago by aaugustin

#9064 shows that it is complicated to interpret X-FORWARDED-HOST when there is more than one proxy. It is also unnecessary when the proxies are properly configured. So it may be a good idea to drop it. That would close both this bug and #9064.

Attached patch:

  • no longer interprets the X-FORWARDED-HOST header, as suggested in #6548, #9064, and get_hostpatch.txt posted by Kellen above,
  • removes a complicated warning in the docs about that header.

This will certainly not make it in 1.3 so versionchanged must be changed to something other than 1.3 when applying the patch.

Obviously, that would be slightly backwards incompatible, but get_host is already broken anyway for people who expect it to handle proxies. Is it necessary to mention it in the release notes?

Changed 5 years ago by aaugustin

comment:10 Changed 5 years ago by julien

  • Needs tests set
  • Severity set to Normal
  • Type set to Bug

comment:11 follow-up: Changed 5 years ago by bastiao

  • Easy pickings unset
  • UI/UX unset

The patch does not works with two apache (rewrite rules)
Apache -> Another Apache -> django server

It can happen in enterprises with different levels of security. Is there any way to extract the exact address?
For people with the same problem I suggest to write a function that import the site url from settings for production environment.

comment:12 Changed 5 years ago by Kellen

Wouldn't having two levels of apache proxies be pretty uncommon? Wouldn't it be better for these users to face up to having an odd setup rather than forcing everyone doing a single level of proxying to adjust for these uncommon use cases?

comment:13 in reply to: ↑ 11 Changed 5 years ago by aaugustin

Replying to bastiao:

The patch does not works with two apache (rewrite rules)
Apache -> Another Apache -> django server

As explained in the docs (after applying the patch), the patch fixes redirects with any number of properly configured proxies, because each proxy rewrites the Location header appropriately.

Of course, I may have missed something. If so, please provide some evidence :)

comment:14 Changed 5 years ago by Kellen

Actually, I'm not sure why the patch wouldn't work for a double layer of proxies.

front.com/ is rewritten to middle.com/app/ is rewritten to back.com/back/app/

A redirect from back.com will point to back.com/back/app/redir/ which should be rewritten to middle.com/app/redir/ and then rewritten to front.com/redir/ by ProxyPassReverse on middle.com and front.com, respectively. That is to say: it would work like a normal apache instance, where the host to which a redirect is issued is the current host.

comment:15 Changed 5 years ago by carljm

  • Resolution set to fixed
  • Status changed from new to closed

This was just fixed in trunk in r16758 as a potential security issue (and backported in 1.2.7 and 1.3.1); the fix is the same as that proposed here (USE_X_FORWARDED_HOST setting, defaults to False).

Note: See TracTickets for help on using tickets.
Back to Top