8000 Always trust 127.0.0.1 as a proxy · Issue #9292 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

Always trust 127.0.0.1 as a proxy #9292

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

Closed
samuelvogel opened this issue Oct 14, 2013 · 9 comments
Closed

Always trust 127.0.0.1 as a proxy #9292

samuelvogel opened this issue Oct 14, 2013 · 9 comments
Labels
Good first issue Ideal for your first contribution! (some Symfony experience may be required) HttpFoundation

Comments

@samuelvogel
Copy link

The remote address in a fragment sub-request is always set to 127.0.0.1 and the original remote address is pushed into the X-FORWARDED_FOR header. This is the expected behavior implemented by @fabpot in 2f3b33a and explained for the HttpCache component here.

But when retrieving the client IP via Request::getClientIp(), one will now always get 127.0.0.1 when it's not added as a trusted proxy. This seems strange, as it will most likely be incorrect and might confuse users which now have to mangle with configuring a trusted proxy, even though they don't actually have a real proxy in place.

Also in the commit message of 2f3b33a it says:

also making getClientIp smarter by removing possible local IP addresses from being considered as the client IP address

But no according change is part of that commit, getClientIp does actually consider 127.0.0.1 as the client IP.

@ureimers
Copy link
Contributor

I support this. IMHO 127.0.0.1 is a trusted proxy. It's "under the same roof" as your server.

It also feels a little bit inconsistent with Symfony's reverse proxy as it's FragmentListener automatically merges 127.0.0.1 to the list of trusted IPs
(https://github.com/symfony/symfony/blob/2.3/src/Symfony/Component/HttpKernel/EventListener/FragmentListener.php#L81). So when the FragmentListener trusts 127.0.0.1 why shouldn't it be a trusted proxy IP too?

In the documentation, this would affect: symfony/symfony-docs#3926 (when I initially wrote that doc PR I wasn't aware of Request::getClientIp() also using the trusted proxy list so until this ticket here isn't solved better refer to it as "Change note about adding 127.0.0.1").

@stof
Copy link
Member
stof commented Jun 11, 2014

it is a trusted proxy only if there is a proxy running there. If you don't have a proxy on 127.0.0.1, registering it all the time open a security hole for the check on the forwarded host, forwarded port and forwarded proto

@ureimers
Copy link
Contributor

@stof I think you're right. But still it's inconvenient having to add 127.0.0.1 to the list only if you need to get the client IP from within a sub-request.

What about AppCache.php (or the HttpCache itself) automatically adding 127.0.0.1 to the list of trusted proxies? It runs on 127.0.0.1 and it is by definition trusted. As Symfony states that it's reverse proxy should not be used in a production environment this is no real security issue either.

It just feels more "Symfony-like" to keep the developer out of such cumbersome tasks.

@stof
Copy link
Member
stof commented Jun 11, 2014

the HttpCache could indeed register itself as a trusted proxy

@ureimers
Copy link
Contributor

Ok. I'll work out a small PR for that.

@weaverryan
Copy link
Member

This does indeed seem to be a confusing issue, so I'm happy there's some attention on it :). Thanks @ureimers

@ureimers
Copy link
Contributor

Sorry, I didn't have the time yet because of a very high project load (crunch, crunch, crunch).

fabpot added a commit that referenced this issue Sep 22, 2014
…lkybarkid)

This PR was merged into the 2.3 branch.

Discussion
----------

[HttpKernel] Make sure HttpCache is a trusted proxy

| Q             | A
| ------------- | ---
| Bug fix?      | yes (of sorts)
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #9292
| License       | MIT
| Doc PR        | symfony/symfony-docs#4239

Fixes #9292 by adding `127.0.0.1` as a trusted proxy when using `HttpCache` (assuming it hasn't been already).

Commits
-------

ca65362 Make sure HttpCache is a trusted proxy
@fabpot fabpot closed this as completed Sep 22, 2014
@praditha-hidayat
Copy link

Hi @ureimers

I've got the same issue here and I'm using Symfony 5.2.14.
The Request::getClientIp() is always return 127.0.0.1. But, if I put trusted_proxies: '127.0.0.1,REMOTE_ADDR' in my config/packages/framework.yml, it's working.

The way I reproduce it is by using ngrok to serve my localhost project and then I access it through another computer.

Is it recommended way or like @stof said, this will make a security hole in my website?

@stof
Copy link
Member
stof commented Oct 22, 2021

Well, always trusting 127.0.0.1 can cause issues if you have a proxy running on 127.0.0.1 but handling a different set of headers than the one you marked as trusted by default (what would they be), as the non-handled headers would then reflect what the untrusted client sent for them.

When you use ngrok, you are deploying your website behind a trusted proxy, and you need to configure it accordingly (based on the headers than ngrok sets)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good first issue Ideal for your first contribution! (some Symfony experience may be required) HttpFoundation
Projects
None yet
Development

No branches or pull requests

6 participants
0