Code

#19517 closed Bug (wontfix)

Add support for X-Forwarded-Port to HttpRequest.get_host when USE_X_FORWARDED_HOST is in use

Reported by: david_k_hess@… Owned by: nobody
Component: HTTP handling Version: 1.4
Severity: Normal Keywords: httprequest, get_host, X-Forwarded-Host, USE_X_FORWARDED_HOST
Cc: hirokiky@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

The use case is an app served on ports 80, 443 (SSL) and 444 (SSL) served by gunicorn behind nginx. I'm sending X-Forwarded-Host and X-Forwarded-Port from nginx to gunicorn. Port 80 is open solely to redirect to 443 and both are open to the public internet. 444 is firewalled and reserved for the admin interface which is protected by this middleware component:

class ProtectAdminMiddleware(object):
    def process_request(self, request):
        if request.path.startswith("/admin") and request.META["HTTP_X_FORWARDED_PORT"] != "444":
            raise Http404

When turning on USE_X_FORWARDED_HOST, the code in HttpRequest.get_host does not take port into account:

    [...]
    if settings.USE_X_FORWARDED_HOST and (
        'HTTP_X_FORWARDED_HOST' in self.META):
        host = self.META['HTTP_X_FORWARDED_HOST']
    elif 'HTTP_HOST' in self.META:
        host = self.META['HTTP_HOST']
    [...]

Because of this, at least two things in the admin interface on port 444 don't function correctly; the CSRF token breaks saying there is a problem between referrer on 444 and token on 443 and you can't login. Also, redirects from say "/admin" on port 444 redirect to "/admin/" on 443 instead of staying on 444. That's just what I was able to reach with logins broken.

I monkey-patched HttpRequest.get_host to add support for X-Forwarded-Port when USE_X_FORWARDED_HOST is specified and it appears to have corrected these issues:

    [...]
    if settings.USE_X_FORWARDED_HOST and (
        'HTTP_X_FORWARDED_HOST' in self.META):
        host = self.META['HTTP_X_FORWARDED_HOST']
        if 'HTTP_X_FORWARDED_PORT' in self.META:
            server_port = str(self.META['HTTP_X_FORWARDED_PORT'])
            if server_port != (self.is_secure() and '443' or '80'):
                host = '%s:%s' % (host, server_port)
    elif 'HTTP_HOST' in self.META:
        host = self.META['HTTP_HOST']
    [...]

Attachments (0)

Change History (5)

comment:1 Changed 16 months ago by cyphase

  • Easy pickings set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Bug

comment:2 Changed 15 months ago by hirokiky

I found pull-request for this ticket here https://github.com/django/django/pull/619 written by mattrobenolt.

comment:3 Changed 15 months ago by hirokiky

  • Has patch set

comment:4 Changed 15 months ago by hirokiky

  • Cc hirokiky@… added

comment:5 Changed 14 months ago by jezdez

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

I'm rejecting this after a discussion during the 2013 sprint:

  • The use case of handing a SSL request with a load balancer is handled already by the SECURE_PROXY_SSL_HEADER setting.
  • HTTP_X_FORWARDED_PORT is not a standard header and this ticket hasn't produced compelling evidence that Django is breaking any spec.
  • Adding a new setting pair of HTTP_X_FORWARDED_PORT and USE_X_FORWARDED_PORT just for this edge case is a no-go, too.

In other words, the issue is an edge case that can be solved by a custom middleware in your application (e.g. by setting HTTP_HOST).

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.