Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential security issue in reverse proxy configuration with X-Forwarded-For #96

Open
hsenot opened this issue Aug 15, 2017 · 3 comments

Comments

@hsenot
Copy link

hsenot commented Aug 15, 2017

Hi there,

Having spent the last few days reading about how to provide a (somewhat) reliable REMOTE_ADDR environment variable to Django and associated modules in a reverse proxy setup (nginx + gunicorn on socket + django), I have the feeling that there is a potential security issue with defender's default reverse proxy setup.

Just switching the BEHIND_REVERSE_PROXY to True will result in the first IP in the X-Forwarded-For header (default header for reverse proxy setups in defender and many other projects) to be used here:
ip_address = request.META.get(config.REVERSE_PROXY_HEADER, '') ip_address = ip_address.split(",", 1)[0].strip()
This header is subject to IP spoofing, and the first IP is not guaranteed to be the user's.

It seems to be a widespread issue. django-axes attempts at mitigating this issue by an additional configuration option: the number of trusted proxies in the X-Forwarded-For, and using it to unstack the IP addresses in the header from right to left. Other projects like gorouter are considering a list of trusted proxies that can be used to unstack the IPs in X-Forwarded-For from right to left.

While acknowledging finding a generic solution is hard as there are a variety of proxy setups and evolving security practices around the Forward headers, I thought I'd just flag it as an issue for a module whose purpose is to prevent brute force attacks (and blocking by IP).

For this reason, I ended up turning off the reverse proxy setup of defender and coding an "unproxy" decorator applied to the WSGI application, based on fluent comments' recommendation for IP detection.

From what I read, the discussion on finding an appropriately generic solution to getting a user IP is ongoing. It might have to be on the Django middleware or WSGI side (rather than in individual libraries - comments, geo-ip, defender, axes, ...) and for libraries to trust the REMOTE_ADDR has been setup properly upstream.

Any thoughts on this issue?
Cheers!

@kencochrane
Copy link
Collaborator

@hsenot thanks for the note, I agree we should also do something about this, but I'm not 100% sure what the correct way will be.

I'm on vacation right now with limited internet, but when I get back I'll see what options we have, and what makes the most sense for us.

If anyone has any ideas or votes on which one to implement, feel free to chime in.

@mergengerel
Copy link

Hi,
still does not work.

Screen Shot 2022-04-26 at 6 34 55 PM

@mergengerel
Copy link

x_forwarded_for = request.META.get('HTTP_X_FORWARDED_FOR')

        if x_forwarded_for:
            # pass
            ip = x_forwarded_for.split(',')[0]
        else:
            ip = request.META.get('REMOTE_ADDR')

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants