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

[stable28] fix(settings): Also verify that trusted_proxies only contains IP addresses (with range) #44495

Merged
merged 1 commit into from
Apr 17, 2024

Conversation

backportbot[bot]
Copy link

@backportbot backportbot bot commented Mar 26, 2024

Backport of PR #44483

…dresses (with range)

Co-authored-by: Côme Chilliet <[email protected]>
Signed-off-by: Ferdinand Thiessen <[email protected]>
@backportbot backportbot bot added bug 3. to review Waiting for reviews labels Mar 26, 2024
@backportbot backportbot bot added this to the Nextcloud 28.0.4 milestone Mar 26, 2024
@skjnldsv skjnldsv mentioned this pull request Mar 28, 2024
@Altahrim Altahrim mentioned this pull request Apr 17, 2024
5 tasks
@Altahrim Altahrim merged commit c4370b6 into stable28 Apr 17, 2024
52 of 53 checks passed
@Altahrim Altahrim deleted the backport/44483/stable28 branch April 17, 2024 09:18
@whisperdancer
Copy link

whisperdancer commented Apr 25, 2024

Just updated and got the error:
There are some errors regarding your setup.
Your "trusted_proxies" setting is not correctly set, it should be an array of IP addresses - optionally with range in CIDR notation.

I'm using docker and my proxy (container name swag) and nextcloud (container name nextcloud) containers are in the same docker compose file on same docker network so setting the trusted proxy in config.php to 'swag' has always worked perfectly without requiring the trusted proxy to be an IP address.

  'trusted_domains' =>
  array (
    0 => 'mydomain.com',
  ),
  'trusted_proxies' =>
  array (
    0 => 'swag',
  ),

Can you please confirm if the check does not actually perform a function check, but only looks at the values in config.php and then complains if it finds anything other than IP addresses? If this is the case then I assume I can leave my trusted proxy as 'swag' and safely ignore the new error?

Much appreciate a timely response. Thank you.

@susnux
Copy link
Contributor

susnux commented Apr 25, 2024

@whisperdancer The code worked because the trusted_proxies were never evaluated correctly there.
It is not possible to have domains in the trusted proxies, as this is an IP only value (the value of the header field is also only a list of IPs there are never host names involved).

Meaning your proxy was considered the end user IP, so brute-force protection etc would block your proxy instead of bad user.

@szaimen
Copy link
Contributor

szaimen commented Apr 25, 2024

However you could do this:

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

@whisperdancer
Copy link

Thanks so much for clearing this up. I've been running this incorrectly for a long time. Your new check along with your suggestion has my nextcloud running as it should now. The use of gethostbyname function when setting trusted_proxies eliminated the error and nextcloud is working great..

However, unsure what you mean by this statement? ..."(the value of the header field is also only a list of IPs there are never host names involved)...
Are you referring to the trusted domains? Should I be using gethostbyname function for both trusted_domains and trusted_proxies like this?

  'trusted_domains' =>
  array (
    0 => gethostbyname('mydomain.com'),
  ),
  'trusted_proxies' =>
  array (
    0 => gethostbyname('swag'),
  ),

@susnux
Copy link
Contributor

susnux commented Apr 25, 2024

No, it is just about the trusted_proxies. The proxy field does not accept hostnames but IPs.

For trusted_domains you of course need to set hostnames.

@whisperdancer
Copy link

Wonderful, much appreciate the clarification! :)

@akolodk
Copy link

akolodk commented Apr 26, 2024

hi, I just pulled the latest version of the 29.0.0.19 and noticed that my trusted_proxies stopped working.
I tried using the gethostbyname('proxy') directive in my docker compose file but it seems that config.php is not generated using the new directive...

so I used to have

- TRUSTED_PROXIES=proxy

in my docker compose file which translated to

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

now I wanted to have
- TRUSTED_PROXIES=gethostbyname('proxy')
but config.php doesnt change and I can still see the old proxy entry....

any thoughts?
A

edit:
it seems that within the container I can see updated value:

www-data@244256b6c22e:~/html$ php occ config:system:get trusted_proxies
gethostbyname('proxy')

but it still reports an error "Your "trusted_proxies" setting is not correctly set, it should be an array of IP addresses"

edit 2:
there must be some sort of periodic update because the config.php got updated eventually (is there? I have no idea what triggers an update to this file, it doesn't seem to be restarts).
however the generated text is:

  'trusted_proxies' =>
  array (
    0 => 'gethostbyname(\'proxy\')',
  ),

and still getting the same error...

@susnux
Copy link
Contributor

susnux commented Apr 26, 2024

still getting the same error.

Yes because it is a string, you need:

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

Do this change manually or report this issue at the docker repo: https://github.com/nextcloud/docker

@S0ulf3re
Copy link

I'm also having issues with this error message as well. I'm assuming that the swag name is meant to be the hostname of your reverse proxy (traefik in my case). I also noticed with this though with notify_push that trying to do this resulted in:

Error: php_literal_parser::unexpected_token

  × Error while parsing nextcloud config.php
  ╰─▶ Error while parsing '/config.php':
      No valid token found, expected one of boolean literal, integer literal,
      float literal, string literal, 'null', 'array' or '['
    ╭─[65:1]
 65 │   array (
 66 │     0 => gethostbyname('traefik'),
    ·          ┬
    ·          ╰── Expected boolean literal, integer literal, float literal, string literal, 'null', 'array' or '['
 67 │   ),
    ╰────

@techgeeksvk
Copy link

What if we are using Cloudflare tunnel? Do i need to add all Cloudflare IP's? Solution to add 0 => 'gethostbyname('proxy')', fails. THX

@susnux
Copy link
Contributor

susnux commented Apr 30, 2024

@techgeeksvk it was never working, as the header only uses IPs not host names.
Meaning if you added host names in the past it did also not work (meaning e.g. if someone run DDOS then not that user will by blocked by bruteforce protection but your proxy because the proxy IP was not listed and thus considered the enduser IP).

Solution to add 0 => 'gethostbyname('proxy')', fails

Do not use quotation marks there, gethostbyname is a function. Also ONLY do this if you are sure that your DNS will correctly resolve the IP of that host. If not do it manually and add the correct IP.

So it should look like:

// config.php
// ...
  'trusted_proxies' => array(
    0 => gethostbyname('my-proxy'),
  ),
// ...

@techgeeksvk
Copy link

Thanks but what proxy do I use when I am connected via tunnel u see. Do I need to use Cloudflare ip's? Docker with Nextcloud is directly connected via tunnel. Maybe I am forgetting something.

@TheMrCV
Copy link

TheMrCV commented May 2, 2024

My proxy server has a static IP and I have the following line in my config.php:

'trusted_proxies' => ['10.0.3.6'],

However I still get the trusted_proxies error. When I run:

php occ config:system:get trusted_proxies

It returns the hostname, and not the IP address that I specified in the config.php file. Could that configuration directive have been specified somewhere else? Is it reverse DNS messing me up? I do have a PTR record for that IP on my DNS server.

I was able to resolve my problem with the following command:

php occ config:system:set trusted_proxies 0 --value=10.0.3.6

It appears to persist through container reboots, but I do not see where that value was set.

@techgeeksvk
Copy link

Ok but what dns provider are you using?

@susnux
Copy link
Contributor

susnux commented May 2, 2024

It returns the hostname, and not the IP address that I specified in the config.php file.

The IP should never be resolved to a hostname, are you sure you have no other config that injects trusted_proxies?

@TheMrCV
Copy link

TheMrCV commented May 2, 2024

Ok but what dns provider are you using?

Builtin router DNS resolver, I believe it's unbound.

@TheMrCV
Copy link

TheMrCV commented May 2, 2024

It returns the hostname, and not the IP address that I specified in the config.php file.

The IP should never be resolved to a hostname, are you sure you have no other config that injects trusted_proxies?

I do not think so, I grepped all the files in the config directory and that yields no results for the hostname of the proxy server. Interestingly neither does grepping for the ip address that I set via the php occ config:system:set command. It looks like the setting is saved, it is persisting somehow, but it's not in the config directory.

@wetterbrienz
Copy link

Hello everyone,
So the tips above don't work for me.
I still get the same error.
My Nextcloud runs in Docker on a Synology and NPM is installed on a PI 5 with the IP 192.168.1.80.
Am I doing something wrong?
Greeting
Daniel

@susnux
Copy link
Contributor

susnux commented May 4, 2024

Please use the forums for help: https://help.nextcloud.com/

@jwilleke
Copy link

jwilleke commented May 7, 2024

No Idea where this error is emerging from and obviously the #44495 did not fix the issue as I installed App Version: 29.0.0
Here I only see workarounds and no fixes.

@come-nc
Copy link
Contributor

come-nc commented May 7, 2024

No Idea where this error is emerging from and obviously the #44495 did not fix the issue as I installed App Version: 29.0.0 Here I only see workarounds and no fixes.

What error are you talking about? You are commenting on #44495 which you are referring, did you post in the wrong tab?

@jwilleke
Copy link

jwilleke commented May 7, 2024

Sorry, came from another thread.

Constant repeating of:

[php:notice] [pid 162] [client 172.16.1.81:33092] Nextcloud trustedProxies has malformed entries

None of the tips above work for me.

@Nini150
Copy link

Nini150 commented May 8, 2024

Hello,

I checked in my config.php and I have this:
trusted_proxies' =>
array (
0 => 'nextcloud.mydomainname',
1 => 'x.x.x.x', #IP of the server
),

Can you help me to solve the problem ?

@susnux
Copy link
Contributor

susnux commented May 8, 2024

0 => 'nextcloud.mydomainname',

Remove that line, if you need that as a trusted proxy either use the static IP address of that server or try:

'trusted_proxies' => [
    gethostbyname('your.domain'),
    'ip.of.your.server'
]

This field only supports IP addresses, no hostnames so you need to resolve any hostname previously.

@Nini150
Copy link

Nini150 commented May 9, 2024

Thanks I found a solution.
But now I have another problem :(
Nextcloud

Have you ever encountered this problem ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.