nextcloud / server

☁️ Nextcloud server, a safe home for all your data
https://nextcloud.com
GNU Affero General Public License v3.0
26.9k stars 4.01k forks source link

[Bug]: `trusted_proxies` handles '127.0.0.1' incorrectly #44439

Closed metorm closed 1 month ago

metorm commented 6 months ago

⚠️ This issue respects the following points: ⚠️

Bug description

I use a caddy reverse proxy running on the same machine with nextcloud to access it, and the admin/overview warns me about "The reverse proxy header configuration is incorrect", while I'd done everything provided by the manual.

Eventually, it turns of that 'trusted_proxies' => array('127.0.0.1') is not accepted by nextcloud, but setting 'trusted_proxies' => array('localhost') does the trick.out

I think we should add the logic to accept 127.0.0.1, or at least mention it in the mannual.

Steps to reproduce

  1. Install next cloud and access it from a local caddy reverse proxy, set 'trusted_proxies' to 127.0.0.1
  2. The admin/overview warns me that "The reverse proxy header configuration is incorrect", which is not supposed to be there
  3. Change from 'trusted_proxies' => array('127.0.0.1') to 'trusted_proxies' => array('localhost'), the warning is gone.

Expected behavior

By setting 'trusted_proxies' => array('127.0.0.1'), the server trusts the reverse proxy connection from localhost.

Installation method

Community Manual installation with Archive

Nextcloud Server version

28

Operating system

Debian/Ubuntu

PHP engine version

PHP 8.1

Web server

Apache (supported)

Database engine version

MySQL

Is this bug present after an update or on a fresh install?

Fresh Nextcloud Server install

Are you using the Nextcloud Server Encryption module?

None

What user-backends are you using?

Configuration report

No response

List of activated Apps

No response

Nextcloud Signing status

No response

Nextcloud Logs

No response

Additional info

No response

solracsf commented 6 months ago
'trusted_proxies' =>
  array (
    0 => '127.0.0.1',
  ),

?

susnux commented 6 months ago

Are you sure caddy sets the correct ip? I had this with docker where it is not 127.0.0.1 but e.g. 172.17.0.2.

metorm commented 6 months ago
'trusted_proxies' =>
  array (
    0 => '127.0.0.1',
  ),

?

I tried that and it didn't work. Maybe in some systems nextcloud gets 127.0.0.1 while in others it gets localhost?

metorm commented 6 months ago

Are you sure caddy sets the correct ip? I had this with docker where it is not 127.0.0.1 but e.g. 172.17.0.2.

It's not a docker installation. I have a caddy in another host, and used a TCP tunnel to transport the requests from caddy to nextcloud, so the request received by nextcloud is indeed made from 127.0.0.1.

I changed nothing but 127.0.0.1 => localhost and the warning disappeared magically.

solracsf commented 6 months ago

Well, in my installation (v27) 127.0.0.1 is accepted. For example, without it, notify_push don't work and once set, it can connect.

susnux commented 6 months ago

warns me about "The reverse proxy header configuration is incorrect"

What is the exact message you receive?

metorm commented 6 months ago

Well, in my installation (v27) 127.0.0.1 is accepted. For example, without it, notify_push don't work and once set, it can connect.

Do you think it's possible that php is receiving 127.0.0.1 on some systems and localhost on others due to host files or docker network, distro differences, etc.?

metorm commented 6 months ago

warns me about "The reverse proxy header configuration is incorrect"

What is the exact message you receive?

I've tried both v27 & v28, the messgae in v27 is:

The reverse proxy header configuration is incorrect, or you are accessing Nextcloud from a trusted proxy. If not, this is a security issue and can allow an attacker to spoof their IP address as visible to the Nextcloud. Further information can be found in the documentation.

and in v28 it's simpleer

The reverse proxy header configuration is incorrect. This is a security issue and can allow an attacker to spoof their IP address as visible to the Nextcloud. Further information can be found in the documentation.

and the solution is found here:

https://help.nextcloud.com/t/the-reverse-proxy-header-configuration-is-incorrect-or-you-are-accessing-nextcloud-from-a-trusted-proxy/139286

What I did more than the above post is that I tested 127.0.01 instead of localhost and it doesn't work on my v28 installtion.

The problem is solved on v28 and since v27 is already uninstalled, I cannot make sure v27 behaves alike.

susnux commented 6 months ago

You should probably not do that, trusted proxies is expected to be an array of IP addresses OR IP address with subnet.

When does this warning show up?

  1. When no trusted_proxies is setup, but X-Forwarded-Host header is set.
  2. When your remote address is in the trusted_proxies and Nextcloud could not resolve your real remote address

I think in your case the second one is the issue, if you have a reverse proxy you need to set it up to add the X-Forwarded-For header (or a custom header you can configure with forwarded_for_headers).

What it does to resolve your real IP address is going reverse through the X-Forwarded-For while it is an trusted proxy. The first (from reverse) IP in that header that is not a trusted proxy is expected to be your real IP address.

As you can see, there are no host names involved, it is all IP. So probably your reverse proxy is not setting the headers correctly.

metorm commented 6 months ago

Regarding X-Forwarded-For, I double-checked. I even wrote a php file specifically for this purpose to display all the headers, placed it in the root of Nextcloud, accessed it through a browser, and confirmed that it contained the key for X-Forwarded-For and pointed to the browser's public address.

So I guess the problem could be in the first step, where for some unknown reason the judgment function is supplied with localhost and fails to recognize the difference between localhost and 127.0.0.1 are equivalent.

BTW the PHP version is 8.2

susnux commented 6 months ago

There is nowhere a name resolve, meaning "localhost" is never resolved but simply invalid as it is not an IP address.

The reverse proxy header configuration is incorrect. This is a security issue and can allow an attacker to spoof their IP address as visible to the Nextcloud.

You get that message on 28.0.3? This message is only shown if trusted_proxies is empty and X-Forwarded-Host is set. Meaning you have a reverse proxy but did not configure Nextcloud for it.

metorm commented 6 months ago

So under what circumstances would it be reasonable to explain the fact that changing 'trusted_proxies' => array('127.0.0.1') to 'trusted_proxies' => array('localhost') would eliminate the warning?

susnux commented 6 months ago

It eliminates the warning because your remote address is no longer found in trusted_proxies thus the test is skipped.

You get that message on 28.0.3?

Asking this because as said this only happens with empty trusted_proxies. Or do you get:

The reverse proxy header configuration is incorrect, or you are accessing Nextcloud from a trusted proxy. If not, this is a security issue and can allow an attacker to spoof their IP address as visible to the Nextcloud.

metorm commented 6 months ago

Since I use a Chinese version, I am not sure the warning message was exactly like that. But I am sure about the "The reverse proxy header configuration is incorrect" part, and the message shown by v28 is shorter, since v28 message doesn't include the "or you are accessing Nextcloud from a trusted proxy" part.

Based on your explanation, we seem to come back to the very start: I'd configured both trusted_proxies = array('127.0.0.1') and the header is there (automatically added by caddy and I am sure its there), but there is still a waring "The reverse proxy header configuration is incorrect".

susnux commented 6 months ago

Could you apply this patch, set the log level to debug and report the log entry?

diff --git a/apps/settings/lib/SetupChecks/ForwardedForHeaders.php b/apps/settings/lib/SetupChecks/ForwardedForHeaders.php
index daf84e265af..2265d0d9ba1 100644
--- a/apps/settings/lib/SetupChecks/ForwardedForHeaders.php
+++ b/apps/settings/lib/SetupChecks/ForwardedForHeaders.php
@@ -32,6 +32,7 @@ use OCP\IRequest;
 use OCP\IURLGenerator;
 use OCP\SetupCheck\ISetupCheck;
 use OCP\SetupCheck\SetupResult;
+use Psr\Log\LoggerInterface;

 class ForwardedForHeaders implements ISetupCheck {
        public function __construct(
@@ -39,6 +40,7 @@ class ForwardedForHeaders implements ISetupCheck {
                private IConfig $config,
                private IURLGenerator $urlGenerator,
                private IRequest $request,
+               private LoggerInterface $logger,
        ) {
        }

@@ -69,6 +71,8 @@ class ForwardedForHeaders implements ISetupCheck {
                        }
                }

+               $this->logger->debug('Running header test', ['proxies' => $trustedProxies, 'remote' => $remoteAddress, 'detected' => $detectedRemoteAddress]);
+
                if (empty($trustedProxies) && $this->request->getHeader('X-Forwarded-Host') !== '') {
                        return SetupResult::error(
                                $this->l10n->t('The reverse proxy header configuration is incorrect. This is a security issue and can allow an attacker to spoof their IP address as visible to the Nextcloud.'),
metorm commented 6 months ago

I'll try that later, it's already too late to go to bed in my place ...

avuton commented 3 months ago

Whoops, I was conflating trusted_domains and trusted_proxies.

metorm commented 3 months ago

Well, I've upgraded 29.0.3 now, in this version, localhost is no longer allowed (an error message will be displayed saying "trusted_proxies must be IP"), I then tried my external IP 192.168.x.xx and all warning messages are gone.

nextcloud-command commented 2 months ago

This issue has been automatically marked as stale because it has not had recent activity and seems to be missing some essential information. It will be closed if no further activity occurs. Thank you for your contributions.