Opened 16 years ago
Closed 12 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)
Change History (18)
comment:1 Changed 16 years ago by
comment:2 Changed 16 years ago by
Cc: | kellen@… added |
---|---|
Component: | Uncategorized → HTTP handling |
Keywords: | redirect httpresponseredirect get_host reverse proxy squid apache added |
comment:3 Changed 16 years ago by
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 16 years ago by
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 15 years ago by
Triage Stage: | Unreviewed → Accepted |
---|
Changed 15 years ago by
Attachment: | 6880_patch.diff added |
---|
patch adding USE_X_FORWARDED_HOST setting in get_host, docs
comment:6 Changed 15 years ago by
Has patch: | set |
---|
comment:7 Changed 15 years ago by
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.
comment:8 Changed 14 years ago by
Triage Stage: | Accepted → 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 13 years ago by
#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, andget_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 13 years ago by
comment:10 Changed 13 years ago by
Needs tests: | set |
---|---|
Severity: | → Normal |
Type: | → Bug |
comment:11 follow-up: 13 Changed 12 years ago by
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 12 years ago by
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 Changed 12 years ago by
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 12 years ago by
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 12 years ago by
Resolution: | → fixed |
---|---|
Status: | new → 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).
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.
Debian, apache 2.2.3-4+etch4, django 0.97~svn7189-1.