DOMjudge / domjudge

DOMjudge programming contest jury system
https://www.domjudge.org
GNU General Public License v2.0
737 stars 258 forks source link

How to set trust proxy #1368

Closed Dup4 closed 2 years ago

Dup4 commented 2 years ago

NOTE: If this is a discussion starter, you need any installation help or have a question on how to accomplish something, rather post at our discussion channel or send an email to our DOMjudge-devel mailinglist instead of filing an issue here.

Description of the problem

Replace this line with a short description.

I use docker to run domserver.

On the host outside the container, I use Nginx to forward traffic to domserver.

The forwarding configuration on my Nginx is below:

location ^~ /domjudge { return 301 /domjudge/; }

location ^~ /domjudge/ {
    proxy_set_header Host                $host:$server_port;
    proxy_set_header X-Forwarded-For     $proxy_add_x_forwarded_for;
    proxy_set_header X-Forwarded-Proto   $scheme;
    proxy_set_header X-Forwarded-Port    $server_port;
    proxy_pass http://127.0.0.1:8101/domjudge/;
}

But the X-Forwarded-For I set does not seem to take effect.

The user's login IP is always the IP of my proxy server.

Your environment

Include details about your installation here.

  • DOMjudge version (e.g. 7.0.0 or a github commit hash)
  • Operating system / Linux distribution and version (e.g. Ubuntu 18.04)
  • Webserver (e.g. Apache or nginx)

Steps to reproduce

Replace this with a description how we can reproduce your bug.

  • Step 1
  • Step 2
  • Step 3

Expected behaviour

Replace this line with what you would expect to happen.

Actual behaviour

Replace this line with what happens instead.

Any other information that you want to share?

Please include webserver, symfony and judgedaemon log snippets here as appropriate. Screenshots may help in case of UI bugs.

I read the [related documents of Symfony](https://symfony.com/doc/current/create_framework/http_foundation.html#:~:text=That%27s%20not%20the%20case%20with%20the%20getClientIp()%20method%20as%20you%20must%20explicitly%20trust%20your%20reverse%20proxies%20by%20calling%20setTrustedProxies()%3A) and found that trust proxy needs to be set.

image

But I can’t find a page to set the trust proxy on domserver, unless change the source code and recompile.

nickygerritsen commented 2 years ago

It seems in symfony 4.4 this is the only option: https://symfony.com/doc/current/deployment/proxies.html But I think I have some ideas how to make it work using a config file, which wouldn’t require changing code. Let me investigate

nickygerritsen commented 2 years ago

Seems we already do this: https://github.com/DOMjudge/domjudge/blob/main/webapp/public/index.php#L15

Can you try adding:

TRUSTED_PROXIES=1.2.3.4 # your IP here

to webapp/.env.local (create it if it doesn't exist) and run webapp/bin/console cache:clear?

If that ind eed works, I will add a section to the docs somewhere.

Dup4 commented 2 years ago

It works, thank you.

By the way, if there are multiple proxys, can separate multiple ips with commas.

Such as:

TRUSTED_PROXIES=127.0.0.1,172.10.0.1
nickygerritsen commented 2 years ago

I do wonder, we basically have three options now:

What do others think? I'd lean towards the first option or otherwise the third.

thijskh commented 2 years ago

Aren't the IPs of the trusted proxies typically env specific and therefore quite on-topic for .env files?

nickygerritsen commented 2 years ago

Aren't the IPs of the trusted proxies typically env specific and therefore quite on-topic for .env files?

Doesn't the same hold for many other things we currently have stored in other places? I'm fine with using the .env.local, but it would be the first use of it for non-developers (the only other use and text in the docs is for developers). And as said, in that case the IP's in the nginx/apache access logs do not match the IP's as reported by DOMjudge.

Dup4 commented 2 years ago

I do wonder, we basically have three options now:

  • Let the webserver handle it. For example nginx has set_real_ip_from. The advantage is that access logs also show the correct IP. If we want this we should maybe add an example snippet to the nginx and apache configurations and add an option for Docker.
  • Use TRUSTED_PROXIES in .env.local as described above. But this is a bit ugly IMHO.
  • We add a DB config option to set trusted proxies. I'm not sure if this is possible but I think it is. This means you don't need to write any config files.

What do others think? I'd lean towards the first option or otherwise the third.

If use docker to run domserver, when the container starts, the docker gateway ip will be added to TRUSTED_PROXIES by default.

https://github.com/DOMjudge/domjudge-packaging/blob/95ffd1053678454e89635bad42eb3530b9f71a46/docker/domserver/scripts/start.d/50-domjudge.sh#L82-L88

In this case, we may be able to allow the user to pass in TRUSTED_PROXIES through environment variables when starting the container, and we will append the TRUSTED_PROXIES passed in by the user together.

nickygerritsen commented 2 years ago

I do wonder, we basically have three options now:

  • Let the webserver handle it. For example nginx has set_real_ip_from. The advantage is that access logs also show the correct IP. If we want this we should maybe add an example snippet to the nginx and apache configurations and add an option for Docker.
  • Use TRUSTED_PROXIES in .env.local as described above. But this is a bit ugly IMHO.
  • We add a DB config option to set trusted proxies. I'm not sure if this is possible but I think it is. This means you don't need to write any config files.

What do others think? I'd lean towards the first option or otherwise the third.

If use docker to run domserver, when the container starts, the docker gateway ip will be added to TRUSTED_PROXIES by default.

https://github.com/DOMjudge/domjudge-packaging/blob/95ffd1053678454e89635bad42eb3530b9f71a46/docker/domserver/scripts/start.d/50-domjudge.sh#L82-L88

In this case, we may be able to allow the user to pass in TRUSTED_PROXIES through environment variables when starting the container, and we will append the TRUSTED_PROXIES passed in by the user together.

If we go for the .env.local option that could indeed work.

nickygerritsen commented 2 years ago

Maybe since the trusted proxies env already does something in wrqr we go with that for at least docker. In the docs I suggest we give both options (nginx and trusted proxy) with the reason when to use what. Expect two PR’s from me to update the docs and the docker

Dup4 commented 2 years ago

Maybe this issue can be closed?