Config Param 'overwritecondaddr' not working

[ghmax]

Steps to reproduce

1. clean install nextcloud
2. add conditional overwrite settings to config.php
   'overwritehost' => 'my.domain.com',
   'overwritecondaddr' => '^192.168.0.1$',

Expected behaviour

Hostname should be overwritten when request comes in via 192.168.0.1, otherwise NOT.

Actual behaviour

Hostname is always overwritten

Server configuration

Operating system:
official Docker Image on different Docker Hosts

Nextcloud version: (see Nextcloud admin page)
12.0.3.3

Updated from an older Nextcloud/ownCloud or fresh install:
fresh

Where did you install Nextcloud from:
docker

Nextcloud configuration:

Config report

<?php
$CONFIG = array (
  'htaccess.RewriteBase' => '/',
  'memcache.local' => '\\OC\\Memcache\\APCu',
  'apps_paths' => 
  array (
    0 => 
    array (
      'path' => '/var/www/html/apps',
      'url' => '/apps',
      'writable' => false,
    ),
    1 => 
    array (
      'path' => '/var/www/html/custom_apps',
      'url' => '/custom_apps',
      'writable' => true,
    ),
  ),
  'instanceid' => 'xxx',
  'passwordsalt' => 'xxx',
  'secret' => 'xxx',
  'trusted_domains' => 
  array (
    0 => '192.168.99.100:9000',
  ),
  'datadirectory' => '/var/www/html/data',
  'overwrite.cli.url' => 'http://192.168.99.100:9000',
  'dbtype' => 'sqlite3',
  'version' => '12.0.3.3',
  'installed' => true,
  
   'overwritecondaddr' => '^192\.168\.99\.11$',
   'overwritehost' => 'my.domain.com',
  
);

Code causing Troubles...

Problem is in File .../lib/private/AppFramework/Http/Request.php in Method 'private function isOverwriteCondition($type = '')' ... type is empty when checking for e.g. Host-Override in Method 'getOverwriteHost' ... so with empty type check override method (isOverwriteCondition) always returns true cause of:

...
return $regex === '//' || preg_match($regex, $remoteAddr) === 1 || $type !== 'protocol';
...

-> type === '' ... so $type !== 'protocol' is always true ... so method always returns true in this case.

[blizzz]

'overwritecondaddr' => '^192.168.0.1$',

dots are not escaped. Please have a second look at the doc

/**
 * This option allows you to define a manual override condition as a regular
 * expression for the remote IP address. For example, defining a range of IP
 * addresses starting with ``10.0.0.`` and ending with 1 to 3:
 * ``^10\.0\.0\.[1-3]$``

Does it help?

[ghmax]

Hello,

no - that does not help. I already 'dotted' the 'overwritecondaddr' => '^192\.168\.99\.11$',

Look at the code in https://github.com/nextcloud/server/blob/master/lib/private/AppFramework/Http/Request.php -- there is the expression: $this->isOverwriteCondition() with empty params within getOverwriteHost() -- which always returns true - what is not what we should expect... (in my understanding)

/**
 * Check overwrite condition
 * @param string $type
 * @return bool
 */
private function isOverwriteCondition($type = '') {
	$regex = '/' . $this->config->getSystemValue('overwritecondaddr', '')  . '/';
	$remoteAddr = isset($this->server['REMOTE_ADDR']) ? $this->server['REMOTE_ADDR'] : '';
	return $regex === '//' || preg_match($regex, $remoteAddr) === 1
	|| $type !== 'protocol';
}

so a call to:

/**
 * Returns the overwritehost setting from the config if set and
 * if the overwrite condition is met
 * @return string|null overwritehost value or null if not defined or the defined condition
 * isn't met
 */
private function getOverwriteHost() {
	if($this->config->getSystemValue('overwritehost') !== '' && $this->isOverwriteCondition()) {
		return $this->config->getSystemValue('overwritehost');
	}
	return null;
}

always returns the configured value and so hostname replacement always takes place. Even if it should not be the case.

[blizzz]

@LukasReschke ^

[mxvin]

Alright then, can anybody make some hotfixes according this bugs? ;)

[danielwbn]

I think two things need to be fixed here:

1. what @ghmax described, $this->isOverwriteCondition() will ALWAYS return True
2. at least in my configuration (very similar to the above, using the docker container nextcloud:latest (currently version 13.0.5, and an nginx reverse proxy) $this->server['REMOTE_ADDR'] always seems to contain the IP of the client, even if it was routed via the proxy. This means that preg_match($regex, $remoteAddr) === 1 will always be False for me

I could try to come up with a patch, but I'm not sure how this could be fixed. The easiest solution could be to change how overwritecondaddr works (or introduce a new config parameter), in a sense that the new/changed parameter should be a regex for the ips that either should or should not be overwritten (i.e. e.g. LAN (do not overwrite) or WAN (do overwrite)), but this would require everybody who uses this to reconfigure their nextcloud installation -- probably something that is not appreciated :-)

[blizzz]

@danielwbn you can simply drop || $type !== 'protocol'. It was a left over when the forcessl option was removed, which was an essential part of that check. It should all have gone.

[kylemart]

I'm surprised this hasn't been addressed already. Is anyone working on this, or should I take a swing at it?

[blizzz]

I'm surprised this hasn't been addressed already. Is anyone working on this, or should I take a swing at it?

Please :)

[Petersoj]

+1
I sometimes want to access my Nextcloud instance on my LAN instead of through my reverse proxy!

[pavelkryl]

Could I get access to your repo to be able to push a branch/pull request with a fix?

Note: why was my yesterday comment marked as 'spam'

[blizzz]

Could I get access to your repo to be able to push a branch/pull request with a fix?

You don't need to have push access to create a pull request. You can do the changes in your fork and open a PR against this repo. You'll be invited nevertheless.

Note: why was my yesterday comment marked as 'spam'

The content of the comment was not exactly providing value to this issue. The commandeering question (as i perceived it) was rude. Open source software means to make the software work for you needs and contribute back the enhancements or fixed, but not demanding work done.

[pavelkryl]

You don't need to have push access to create a pull request. You can do the changes in your fork and open a PR against this repo. You'll be invited nevertheless.

Thanks for the instructions, I've just created the PR, so I would be pleased if somebody can have a look at it.

The content of the comment was not exactly providing value to this issue. The commandeering question (as i perceived it) was rude. Open source software means to make the software work for you needs and contribute back the enhancements or fixed, but not demanding work done.

Well, I don't see much difference between @Nero10578's last comment and mine. But that's not important, I guess.

[blizzz]

Thank your for the PR!

Well, I don't see much difference between @Nero10578's last comment and mine. But that's not important, I guess.

Fixed ;)

[gulyo]

Hey All,
I confirm the issue exists in 26.0.1.1

[IngLP]

+1. The issue still exists also in NC 27!

[IngLP]

I still have this issue!