Opened 16 years ago

Closed 11 years ago

#9064 closed Bug (wontfix)

Redirect is broken when HTTP_X_FORWARDED_HOST contains multiple hosts

Reported by: Artur Owned by: nobody
Component: HTTP handling Version: 1.0
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

HttpRequest.getHost uses HTTP_X_FORWARDED_HOST as a primary method of acquiring the host name of the server but does not take into account that it can be a list of multiple hosts separated by comma. Only the first host should be used if the header contains several.

Attached a patch for separating at the first comma if multiple hosts are listed.

Attachments (1)

multiple_proxies.patch (547 bytes ) - added by Artur 16 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 by tommycli@…, 16 years ago

Patch needs improvement: set

The current patch seems to work. It's meant to patch against trunk/django/http/__init__.py .

However, the patch doesn't specify which file it's applied to. Please resubmit with the proper directory hierarchy information.

comment:2 by Ivan Giuliani, 16 years ago

Component: UncategorizedHTTP handling
Has patch: set

by Artur, 16 years ago

Attachment: multiple_proxies.patch added

comment:3 by Artur, 16 years ago

Fixed directory hierarchy information in patch

comment:4 by Eric Holscher, 15 years ago

Triage Stage: UnreviewedAccepted

I remember being bitten by this as well. Marking it as accepted because if some agents are forwarding these headers and it's breaking, we should probably take it into consideration.

When searching for similar tickets, I found a ruby on rails ticket dealing with the same issues: http://dev.rubyonrails.org/ticket/3397

It appears they are honoring the last host. It also seems to be happening when there are multiple proxies in front of Django.

comment:5 by Malcolm Tredinnick, 15 years ago

We really shouldn't be using this header to compute redirects at all. if the hostname has been changed as part of the transmission process, it's up to the servers doing the changing to do rewrites on the way out as well. The best solution here is to ditch it altogether. Systems relying on this are arguably broken. Not holding my breath, though, since some people seem to like.

Note, also, there was a huge thread about the complete lack of standardisation and variance in behaviour for the X-FORWARDED-HOST header two or three years ago on django-dev. Here's one thread, although I have a memory there was another as well at some point. Unsurprising that it's confusing, since it's an "X-*" header (totally non-standard).

comment:6 by louis, 14 years ago

Ran into problem myself. Using django (1.1.1 final), mod_python, apache2 and nginx (as proxy).
The previous patch didn't work in my case but below did:

LINE 45:
    def get_host(self):
        """Returns the HTTP host using the environment or request headers."""
        # We try three options, in order of decreasing preference.
        if 'HTTP_X_FORWARDED_HOST' in self.META:
            host = self.META['HTTP_X_FORWARDED_HOST']
            host = host.split(',')[-1].strip()
        elif 'HTTP_HOST' in self.META:
            host = self.META['HTTP_HOST']
            host = host.split(',')[-1].strip()
        else:
            # Reconstruct the host using the algorithm from PEP 333.
            host = self.META['SERVER_NAME']
            host = host.split(',')[-1].strip()
            server_port = str(self.META['SERVER_PORT'])
            if server_port != (self.is_secure() and '443' or '80'):
                host = '%s:%s' % (host, server_port)
        return host

comment:7 by Aymeric Augustin, 13 years ago

The question here is "how to use X-FORWARDED-HOST?", but #6880 asks "should we use X-FORWARDED-HOST at all?" and contains a patch to not use it anymore. If this patch is applied, the problem discussed here vanishes.

get_host is the only place in Django that uses the non-standard X-FORWARDED-HOST header. I think removing it is the correct way to go:

  • Like mtredinnick said, reverse proxies are supposed to do the rewrites both on the request and the response.
  • Developers with a weird proxy setup may write their own middleware to do tweak headers.
  • People using the remote address for geolocation are facing very difficult problem, for which Django can not provide a generic solution. Consider the setup below: you would probably want the address of the Internet-facing proxy; how is Django supposed to find it in the list of 5 IPs in X-FORWARDED-HOST?
    user on private network -> internal proxy -> Internet-facing proxy -----> load-balancer -> reverse proxy -> Django
    

Finally, even if a decent solution was found for this problem, there would still be a parallel issue with is_secure (see also #6548) as proxies can convert from HTTP to HTTPS and vice-versa. I do not know how that one could be resolved.

comment:8 by Luke Plant, 13 years ago

Severity: Normal
Type: Bug

comment:9 by Aymeric Augustin, 12 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:10 by Aymeric Augustin, 12 years ago

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:11 by Aymeric Augustin, 11 years ago

Resolution: wontfix
Status: newclosed

Django no longer interprets X-Forwarded-Host headers by default, for security reasons. See also #6880.

Generally speaking this header isn't necessary if proxies under the owner's control are properly configured.

So I'm going to close this ticket, essentially for the reasons given by Malcolm in comment 5.

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