Server: 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 :)