Trusted proxy cannot resolve hostname (docker), only IP or CIDR-ip

maanloper commented 3 years ago

The problem

A Nextcloud instance running behind an NGINX reverse-proxy (both in Docker) cannot reliably trust the reverse proxy due to the IP-address of the reverse-proxy changing every time the containers are restarted. Normally one would use the container_name as the hostname, but the relevant code in Nextcloud cannot resolve hostnames - only IP addresses or CIDR-ranges. I have pinpointed the source of the problem - but do not know how to proceed/solve it.

Problem example

Relevant Nextcloud config.php part that does not work:

'trusted_proxies' =>
   array (
      0 => 'nginx_container_name',
   ),

Relevant Nextcloud config.php part that does work:

'trusted_proxies' =>
   array (
      0 => '192.168.160.4',
   ),

(192.168.160.4 is the IP of the nginx-proxy container within Docker, this changes every time one starts the Docker containers)

Problem source

The problem originates from lib/private/AppFramework/Http/Request.php#L607:

protected function matchesTrustedProxy($trustedProxy, $remoteAddress) {
   $cidrre = '/^([0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3})\/([0-9]{1,2})$/';

   if (preg_match($cidrre, $trustedProxy, $match)) {
      $net = $match[1];
      $shiftbits = min(32, max(0, 32 - intval($match[2])));
      $netnum = ip2long($net) >> $shiftbits;
      $ipnum = ip2long($remoteAddress) >> $shiftbits;

      return $ipnum === $netnum;
   }

   return $trustedProxy === $remoteAddress;
}

It cannot resolve trusted_proxy host names, only CIDR comparison or literal IP-comparison.

Examples in config where 'container_name' does work

For example in the DB or Redis configuration one can use the host name within docker:

'dbhost' => 'db_container_name',

'redis' =>
  array (
    'host' => 'redis_container_name',
    ...
  ),

The big question

Is it possible to add host name resolving to the lib/private/AppFramework/Http/Request.php code? This would make Nextcloud in Docker work flawless without requiring 'overwritehost'/'overwriteprotocol'/etc.

Looking forward to anyone's thoughts on this.

kesselb commented 3 years ago

'trusted_proxies' =>
   array (
      0 => gethostbyname('nginx_container_name'),
   ),

maanloper commented 3 years ago

You're a hero! That worked like a charm :)

Now, I have seen the exact same problem in multiple places (help.nextcloud.com, github, reddit, etc..), but none of the problems were solved with this proper solution. So I'm led to believe this is not a trivial solution for most people (especially those running NC in Docker).

I would suggest to add the follow code snippet + text:

If both Nextcloud and reverse proxy are run in Docker OR if the IP of your reverse proxy can change but has a hostname, use the following config to have Nextcloud look up the IP of your reverse proxy:

'trusted_proxies' =>
   array (
      0 => gethostbyname('reverse_proxy_container_name'),
   ),

to these places so other people are shown this working example:

Most important: https://docs.nextcloud.com/server/latest/admin_manual/configuration_server/reverse_proxy_configuration.html#defining-trusted-proxies
https://docs.nextcloud.com/server/latest/admin_manual/configuration_server/config_sample_php_parameters.html?highlight=trusted%20proxies
config.sample.php
Most important: https://github.com/nextcloud/docker
Most important: https://hub.docker.com/_/nextcloud

How can these changes be realized?

emmaliddell commented 3 years ago

An important caveat: From a brief look over the high-performance backend, it looks like the rust config parser doesn't evaluate any PHP when parsing the config. As such, this won't work if, like me, you're trying to set the trusted_proxies value to a container hostname to work around the fact that the underlying IPs can change.

martingjohn commented 1 year ago

'trusted_proxies' =>
   array (
      0 => gethostbyname('nginx_container_name'),
   ),

Sadly, the TRUSTED_PROXIES environment variable also doesn't seem to handle hostnames either (either on its own or in that php wrapper), they seem to be stored literally rather than changed into IP addresses

# docker-compose exec --user www-data app php occ config:system:get trusted_proxies
192.168.10.3
caddy

or

# docker-compose exec --user www-data app php occ config:system:get trusted_proxies
192.168.10.3
gethostbyname('caddy')

You can quite easily set the values post start up with the occ command (although you do seem to have to disable the TRUSTED_PROXIES variable as that takes precedence) (index 1 being the second value)

# docker-compose exec --user www-data app php occ config:system:set trusted_proxies 1 --value=172.24.0.8
# docker-compose exec --user www-data app php occ config:system:get trusted_proxies
192.168.10.3
172.24.0.8

luzidd commented 4 months ago

Being able to use the container name in trusted_proxies is expected functionality. Since it's common for IP addresses to change in docker networks this is very much needed.

Resolving the hostname once on the startup of nextcloud would probably be enough to be usable but even then a restart of the proxy container (caddy, nginx, traefik) could change their IP again.

Since it would probably be too much to resolve the hostname on every call of matchesTrustedProxy() maybe the nextcloud-cron script could resolve the hostname and update the $trustedProxy variable?

nextcloud / documentation