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

TRUSTED_PROXIES doesn't evaluate gethostbyname #2218

Open
Andreas02-dev opened this issue May 13, 2024 · 6 comments
Open

TRUSTED_PROXIES doesn't evaluate gethostbyname #2218

Andreas02-dev opened this issue May 13, 2024 · 6 comments
Labels
enhancement feature: auto config (environment variables) Auto configuring via environment variables needs review Needs confirmation this is still happening or relevant

Comments

@Andreas02-dev
Copy link

When setting the TRUSTED_PROXIES environment variable as shown in nextcloud/server#44495 (comment) (TRUSTED_PROXIES=gethostbyname('nginx-proxy'), I get the warning Your "trusted_proxies" setting is not correctly set, it should be an array of IP addresses - optionally with range in CIDR notation. and when executing php occ config:system:get trusted_proxies, the result is gethostbyname('proxy'). When manually editing config.php, the result of php occ config:system:get trusted_proxies will be the IP of the proxy as the gethostbyname function will get evaluated correctly.

Since it's best practice to not assign static IP's in a compose file, being able to evaluate gethostbyname when using the TRUSTED_PROXIES environment variable is quite important especially for the Nextcloud Docker image.

As @xeluior mentioned in this comment, we might be able to naively apply gethostbyname to all trusted proxies when the container starts, however container restarts would likely break this, and I'm not sure if Nextcloud's config works across container restarts with gethostbyname either.

Furthermore, it might be good to also discuss if we want to only evaluate gethostbyname or if we should evaluate the exact same way as config.php does for consistency.

@xeluior
Copy link

xeluior commented May 13, 2024

There's an open issue, nextcloud/server#7005, about the same issue. Not sure if the docker image should implement a workaround or just try to get upstream support for hostnames in the trusted_proxies config option. I was experimenting with some Nextcloud docker configuration setup which implements the naive fix (link), but given the issues mentioned in the nextcloud/server issue I'm not using it on my live server just yet.

@joshtrichards joshtrichards added enhancement feature: auto config (environment variables) Auto configuring via environment variables labels May 21, 2024
@joshtrichards
Copy link
Member

When manually editing config.php, the result of php occ config:system:get trusted_proxies will be the IP of the proxy as the gethostbyname function will get evaluated correctly.

Have you tried using an auto config hook script to set it entirely via occ config:system:set?

https://github.com/nextcloud/docker#auto-configuration-via-hook-folders

The point of the the hook script support is to permit any occ command - and thus any Nextcloud configuration directive - to be set in an automated fashion regardless of environment variable support.

@isdnfan
Copy link

isdnfan commented Jun 8, 2024

In majority of cases you can simply add the docker subnet range (172.16.0.0/12) - this will cover all reverse proxies running on the same host. The most "open" approach is to add all private non-routable IP ranges

TRUSTED_PROXIES=172.16.0.0/12 192.168.0.0/16 10.0.0.0/8 fc00::/7 fe80::/10 2001:db8::/32

this will accept all reverse proxies from internal network but still reject headers from public IPs.

Definitely little less secure than exact assignment should be good for most installations.

@Andreas02-dev
Copy link
Author

When manually editing config.php, the result of php occ config:system:get trusted_proxies will be the IP of the proxy as the gethostbyname function will get evaluated correctly.

Have you tried using an auto config hook script to set it entirely via occ config:system:set?

https://github.com/nextcloud/docker#auto-configuration-via-hook-folders

The point of the the hook script support is to permit any occ command - and thus any Nextcloud configuration directive - to be set in an automated fashion regardless of environment variable support.

Thanks for the advice. I've currently migrated away from the Docker installation, but I might migrate back and test this when I have some more time this summer, due to an issue with OnlyOffice & Nextcloud not being able to save a file in a bare-metal installation.

Would it be best to keep this open for now?

@Barborica-Alexandru
Copy link

I would leave this open since the solution above is more of a bandaid fix to make the warning disappear. An alternative to that would simply be to give the reverse proxy a fixed IP in docker and just add the IP. But none of these solutions are completely portable as just gethostbyname('') working.

@prairietree
Copy link

If I am reading this correctly TRUSTED_PROXIES is still not fixed to evaluate host names. Looking at the linked thread from nextcloud, it does not look like they are going to do anything about it. Is there any possibility some one will fix this or are we just stuck wit the workaround? Thanks

@joshtrichards joshtrichards added the needs review Needs confirmation this is still happening or relevant label Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement feature: auto config (environment variables) Auto configuring via environment variables needs review Needs confirmation this is still happening or relevant
Projects
None yet
Development

No branches or pull requests

6 participants