Opened 16 years ago

Closed 13 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: dev
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 15 years ago.
patch adding USE_X_FORWARDED_HOST setting in get_host, docs
get_hostpatch.txt (711 bytes ) - added by Kellen 15 years ago.
removes usage of X_HTTP_FORWARDED HOST
6880.diff (3.2 KB ) - added by Aymeric Augustin 13 years ago.

Download all attachments as: .zip

Change History (18)

comment:1 by Kellen <kellen@…>, 16 years ago

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 by Kellen <kellen@…>, 16 years ago

Cc: kellen@… added
Component: UncategorizedHTTP handling
Keywords: redirect httpresponseredirect get_host reverse proxy squid apache added

comment:3 by Kellen <kellen@…>, 16 years ago

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

comment:4 by Kellen <kellen@…>, 16 years ago

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 by Simon Greenhill, 16 years ago

Triage Stage: UnreviewedAccepted

by Kellen, 15 years ago

Attachment: 6880_patch.diff added

patch adding USE_X_FORWARDED_HOST setting in get_host, docs

comment:6 by Kellen, 15 years ago

Has patch: set

comment:7 by Kellen, 15 years ago

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.

by Kellen, 15 years ago

Attachment: get_hostpatch.txt added

removes usage of X_HTTP_FORWARDED HOST

comment:8 by Kellen, 15 years ago

Triage Stage: AcceptedDesign 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 by Aymeric Augustin, 13 years ago

#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?

by Aymeric Augustin, 13 years ago

Attachment: 6880.diff added

comment:10 by Julien Phalip, 13 years ago

Needs tests: set
Severity: Normal
Type: Bug

comment:11 by bastiao, 13 years ago

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 by Kellen, 13 years ago

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?

in reply to:  11 comment:13 by Aymeric Augustin, 13 years ago

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 by Kellen, 13 years ago

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 by Carl Meyer, 13 years ago

Resolution: fixed
Status: newclosed

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