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

[BUG] Wrong IP logged with several internal proxies or internal network IPs #5726

Closed
mossroy opened this issue Oct 24, 2023 · 11 comments · Fixed by #5740
Closed

[BUG] Wrong IP logged with several internal proxies or internal network IPs #5726

mossroy opened this issue Oct 24, 2023 · 11 comments · Fixed by #5740
Labels
Docker Everything related to Docker Security 🛡️
Milestone

Comments

@mossroy
Copy link
Contributor

mossroy commented Oct 24, 2023

Describe the bug
With #5549 and v1.22, reverse-proxies are trusted by default (if on internal network).
But, if you have at least 2 chained proxies on internal network, it does not work (wrong IP is logged).

It seems to me that RemoteIPInternalProxy directive should be used instead of RemoteIPTrustedProxy. See https://httpd.apache.org/docs/current/mod/mod_remoteip.html#remoteipinternalproxy

To Reproduce
Steps to reproduce the behavior:

  1. Deploy 2 reverse-proxies on your internal network (i.e. on 10.x, 192.168.x or 172.16.x networks)
  2. The first one must be configured to send requests to the second one, that must be configured to send requests to freshrss docker image (with default configuration)
  3. Access freshrss through the first reverse-proxy
  4. See the logged IP is the IP of the second reverse-proxy

Expected behavior
The logged IP should be the one of the real user

Environment information (please complete the following information):

  • Device: Desktop
  • OS: Ubuntu 22.04
  • Browser: Firefox 118.0.2
  • FreshRSS version: 1.22
  • Database version: PostgreSQL 14.7
  • PHP version: 8.2.7
  • Installation type: docker image 1.22.0-arm (but should be the same with amd)

Additional context
It's probable that the same issue occurs if you use a single reverse proxy, and call it from an internal network (but I did not check)

See the explanation of RemoteIPInternalProxy directive here: https://httpd.apache.org/docs/current/mod/mod_remoteip.html#remoteiptrustedproxy
It says that remote internal IPs are not trusted with this directive. In my steps to reproduce, the IP of the first reverse-proxy is an internal one, so it's not trusted.

It's probable that replacing RemoteIPTrustedProxy directive by RemoteIPInternalProxy (in Apache conf, and in the entrypoint that can modify it) would solve the issue

I can do a PR if necessary

@Alkarex
Copy link
Member

Alkarex commented Oct 24, 2023

Quick answer already: you need to make sure that both your proxy addresses are in the list of trusted proxies

@mossroy
Copy link
Contributor Author

mossroy commented Oct 24, 2023

you need to make sure that both your proxy addresses are in the list of trusted proxies

Sure.
First proxy is Apache, with an IP 192.168.x
Second proxy is a Traefik ingress inside a k3s cluster, and has an IP 10.x (from inside the cluster) and 192.168.x (from outside the cluster)

I tested without setting TRUSTED_PROXY env var, or with setting it to "10.0.0.0/8 192.168.0.0/16" for example

I have other pods in the same cluster that log the user IP (going through the same 2 reverse proxies), after switching (inside their docker image) from RemoteIPTrustedProxy to RemoteIPInternalProxy (wordpress, for example, see docker-library/wordpress#830)

@Alkarex
Copy link
Member

Alkarex commented Oct 24, 2023

I am confused by RemoteIPInternalProxy vs. RemoteIPTrustedProxy even after re-reading the definition (in English) several times. Additional explanations (and tests?) welcome

@mossroy
Copy link
Contributor Author

mossroy commented Oct 24, 2023

Here is how I understood the documentation: let's consider the following test-case (simpler than mine above, with a single reverse-proxy):

                                       X-Real-Ip: 192.168.x
Client  --------------> Reverse-proxy ---------------------> Apache remoteip  (trusting 10.x, and using X-Real-Ip header)
192.168.x                   10.x                    

If FreshRSS used RemoteIPInternalProxy directive, the remoteip apache module would receive the X-Real-IP header (containing 192.168.x), would accept it as the remote user IP, and fill this address in REMOTE_ADDR. Apache LogFormat %a would also display the right IP: 192.168.x

With the current configuration of FreshRSS (using RemoteIPTrustedProxy), the remoteip apache module receives the same X-Real-Ip HTTP header, containing 192.168.x, but it will ignore it, because 192.168.x is a local IP address, and this directive does not trust them. So REMOTE_ADDR var will have value 10.x, and Apache LogFormat %a will print 10.x too.

The difference between the 2 directives is about the client IP, not about the reverse-proxy IP.

@Alkarex
Copy link
Member

Alkarex commented Oct 24, 2023

and this directive does not trust them

even when the proxy's IP is given in the list of RemoteIPTrustedProxy?

What is the use-case of RemoteIPTrustedProxy, then? I am afraid that we might break another use-case

@mossroy
Copy link
Contributor Author

mossroy commented Oct 24, 2023

The IPs you give on the line RemoteIPTrustedProxy or RemoteIPInternalProxy directive are IPs trusted for the reverse-proxy, not for the client. They are useful to tell which reverse-proxies you trust. With current configuration of FreshRSS, it's the ones of local networks, but you could choose to trust a remote reverse-proxy.

The choice between RemoteIPTrustedProxy and RemoteIPInternalProxy gives you the choice to trust or not the local IPs that could be sent in the HTTP header sent by your trusted proxies.

@mossroy
Copy link
Contributor Author

mossroy commented Oct 24, 2023

You can check this behavior by injecting a phpinfo.php file inside the docker container (for example in /var/www/FreshRSS/p/ directory), call this phpinfo.php from a browser on local network, and see the value of REMOTE_ADDR.

I've tested that with FreshRSS, too. With standard 1.22, the docker logs display the IP of the reverse proxy (10.x)
If I replace RemoteIPTrustedProxy by RemoteIPInternalProxy inside /etc/apache2/sites-available/FreshRSS.Apache.conf, the docker logs display the good IP (192.168.x)

@mossroy
Copy link
Contributor Author

mossroy commented Oct 24, 2023

I don't know why Apache provides these 2 different directives, with this subtle difference.
The Apache documentation unfortunately does not give more detail on the reason.

@Alkarex
Copy link
Member

Alkarex commented Oct 24, 2023

You are welcome to send a PR.
I am still confused regarding the use-case for RemoteIPTrustedProxy

@mossroy
Copy link
Contributor Author

mossroy commented Oct 25, 2023

After digging in the history of remoteip, I found that this distinction has been introduced in this commit: https://svn.apache.org/viewvc?view=revision&revision=756323 . See the changes here: https://svn.apache.org/viewvc/httpd/sandbox/mod_remoteip/mod_remoteip.c?r1=756323&r2=755437&pathrev=756323&diff_format=h

So both RemoteIPTrustedProxy and RemoteIPInternalProxy directives have been introduced at the same time, in 2009.

Here is my understanding of the developer intention:

  • if you use a remote proxy (i.e a proxy that is not on the same private network as your Apache), and that remote proxy sends you a X-Forwarded-For or X-Real-Ip header that contains an internal IP (for example a 192.168.x), it's an IP that has no sense for the server that runs apache remoteip (because it's a different network). So the developer probably decided to not trust this kind of IPs in this case, because it might be a security risk. In this case, RemoteIPTrustedProxy should be used. Notice the example in https://httpd.apache.org/docs/2.4/mod/mod_remoteip.html#remoteiptrustedproxy : proxy.example.com
  • but if you use an internal proxy (i.e a proxy that is on the same private network as your Apache), there's no reason to not trust internal IPs. In this case, RemoteIPInternalProxy can be used. Notice the example in https://httpd.apache.org/docs/2.4/mod/mod_remoteip.html#remoteipinternalproxy : gateway.localdomain

mossroy added a commit to mossroy/FreshRSS that referenced this issue Oct 25, 2023
instead of RemoteIPTrustedProxy directive

To allow internal IPs to be trusted: for internal clients,
and also for the case of chained internal reverse-proxies

Fixes FreshRSS#5726
@Alkarex Alkarex added this to the 1.22.1 milestone Oct 25, 2023
@Alkarex Alkarex added the Docker Everything related to Docker label Oct 25, 2023
@Alkarex
Copy link
Member

Alkarex commented Oct 25, 2023

Thanks for the archaeology efforts, @mossroy 👍🏻
It looks like the relevant part is the following (internal being the sole apparent difference between RemoteIPInternalProxy and RemoteIPTrustedProxy):

image

Let's go for RemoteIPInternalProxy

Alkarex added a commit that referenced this issue Oct 25, 2023
* Use RemoteIPInternalProxy directive of remoteip Apache module

instead of RemoteIPTrustedProxy directive

To allow internal IPs to be trusted: for internal clients,
and also for the case of chained internal reverse-proxies

Fixes #5726

* One last reference forgotten

---------

Co-authored-by: Alexandre Alapetite <alexandre@alapetite.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docker Everything related to Docker Security 🛡️
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants