Add support for getting the real client IP behind proxies

Fixes https://github.com/owncloud/core/issues/10624

Fix copy paste fail

Add unittest for comma separated headers

Revert 3rdparty

diff --git a/config/config.sample.php b/config/config.sample.php
index 965655569104640b89000bd98c6c5165a9dfc889..d232e18ab08e6c7d28b91471d4f5efa6b745b054 100755 (executable)
--- a/config/config.sample.php
+++ b/config/config.sample.php
@@ -62,6 +62,12 @@ $CONFIG = array(
 /* List of trusted domains, to prevent host header poisoning ownCloud is only using these Host headers */
 'trusted_domains' => array('demo.owncloud.org', 'otherdomain.owncloud.org:8080'),
 
+/* List of trusted proxy servers */
+'trusted_proxies' => array('203.0.113.45', '198.51.100.128'),
+
+/* Headers that should be trusted as client IP address in combination with `trusted_proxies` */
+'forwarded_for_headers' => array('HTTP_X_FORWARDED', 'HTTP_FORWARDED_FOR'),
+
 /* Theme to use for ownCloud */
 "theme" => "",
 

diff --git a/lib/private/allconfig.php b/lib/private/allconfig.php
index eb1145460109ed4d61b0ff9c90ad03a3644b889e..ef8673af2310111eef0ba679f44ba55d46d80d85 100644 (file)
--- a/lib/private/allconfig.php
+++ b/lib/private/allconfig.php
@@ -28,7 +28,7 @@ class AllConfig implements \OCP\IConfig {
         *
         * @param string $key the key of the value, under which it was saved
         * @param mixed $default the default value to be returned if the value isn't set
-        * @return string the saved value
+        * @return mixed the value or $default
         */
        public function getSystemValue($key, $default = '') {
                return \OCP\Config::getSystemValue($key, $default);

diff --git a/lib/private/request.php b/lib/private/request.php
index 5fd5b3a7197d2cd5fb336d48ca7ed5a29a5273a2..b063c1f5967c378aca66fb38aca7c304131371a5 100755 (executable)
--- a/lib/private/request.php
+++ b/lib/private/request.php
@@ -15,6 +15,34 @@ class OC_Request {
 
        const REGEX_LOCALHOST = '/^(127\.0\.0\.1|localhost)(:[0-9]+|)$/';
 
+       /**
+        * Returns the remote address, if the connection came from a trusted proxy and `forwarded_for_headers` has been configured
+        * then the IP address specified in this header will be returned instead.
+        * Do always use this instead of $_SERVER['REMOTE_ADDR']
+        * @return string IP address
+        */
+       public static function getRemoteAddress() {
+               $remoteAddress = $_SERVER['REMOTE_ADDR'];
+               $trustedProxies = \OC::$server->getConfig()->getSystemValue('trusted_proxies', array());
+
+               if(is_array($trustedProxies) && in_array($remoteAddress, $trustedProxies)) {
+                       $forwardedForHeaders = \OC::$server->getConfig()->getSystemValue('forwarded_for_headers', array());
+
+                       foreach($forwardedForHeaders as $header) {
+                               if (array_key_exists($header, $_SERVER) === true) {
+                                       foreach (explode(',', $_SERVER[$header]) as $IP) {
+                                               $IP = trim($IP);
+                                               if (filter_var($IP, FILTER_VALIDATE_IP) !== false) {
+                                                       return $IP;
+                                               }
+                                       }
+                               }
+                       }
+               }
+
+               return $remoteAddress;
+       }
+
        /**
         * Check overwrite condition
         * @param string $type

diff --git a/lib/public/config.php b/lib/public/config.php
index ea3e0c1372a52242f1e0a390cf26f99c6f33f842..65dde39cdce25d797c46f24bc0867f977b63d650 100644 (file)
--- a/lib/public/config.php
+++ b/lib/public/config.php
@@ -43,7 +43,7 @@ class Config {
         * Gets a value from config.php
         * @param string $key key
         * @param mixed $default = null default value
-        * @return string the value or $default
+        * @return mixed the value or $default
         *
         * This function gets the value from config.php. If it does not exist,
         * $default will be returned.

diff --git a/lib/public/iconfig.php b/lib/public/iconfig.php
index d4a8cdc7381271ec4bd7c23f535b2fe0b43a6326..4865f8bc85b7c73708db960240da98bb302615da 100644 (file)
--- a/lib/public/iconfig.php
+++ b/lib/public/iconfig.php
@@ -47,7 +47,7 @@ interface IConfig {
         *
         * @param string $key the key of the value, under which it was saved
         * @param string $default the default value to be returned if the value isn't set
-        * @return string the saved value
+        * @return mixed the value or $default
         */
        public function getSystemValue($key, $default = '');
 

diff --git a/tests/lib/request.php b/tests/lib/request.php
index bff84e1b03f73200af6b0d24bed882ae2dfca715..b89bf92ece7246c1eee51683c4205d6a712468d0 100644 (file)
--- a/tests/lib/request.php
+++ b/tests/lib/request.php
@@ -9,21 +9,53 @@
 class Test_Request extends PHPUnit_Framework_TestCase {
 
        public function setUp() {
-               OC_Config::setValue('overwritewebroot', '/domain.tld/ownCloud');
+               OC::$server->getConfig()->setSystemValue('overwritewebroot', '/domain.tld/ownCloud');
+
+               OC::$server->getConfig()->setSystemValue('trusted_proxies', array());
+               OC::$server->getConfig()->setSystemValue('forwarded_for_headers', array());
        }
 
        public function tearDown() {
-               OC_Config::setValue('overwritewebroot', '');
+               OC::$server->getConfig()->setSystemValue('overwritewebroot', '');
+               OC::$server->getConfig()->setSystemValue('trusted_proxies', array());
+               OC::$server->getConfig()->setSystemValue('forwarded_for_headers', array());
        }
 
        public function testScriptNameOverWrite() {
                $_SERVER['REMOTE_ADDR'] = '10.0.0.1';
-               $_SERVER["SCRIPT_FILENAME"] = __FILE__;
+               $_SERVER['SCRIPT_FILENAME'] = __FILE__;
 
                $scriptName = OC_Request::scriptName();
                $this->assertEquals('/domain.tld/ownCloud/tests/lib/request.php', $scriptName);
        }
 
+       public function testGetRemoteAddress() {
+               $_SERVER['REMOTE_ADDR'] = '10.0.0.2';
+               $_SERVER['HTTP_X_FORWARDED'] = '10.4.0.5, 10.4.0.4';
+               $_SERVER['HTTP_X_FORWARDED_FOR'] = '192.168.0.233';
+
+               // Without having specified a trusted remote address
+               $this->assertEquals('10.0.0.2', OC_Request::getRemoteAddress());
+
+               // With specifying a trusted remote address but no trusted header
+               OC::$server->getConfig()->setSystemValue('trusted_proxies', array('10.0.0.2'));
+               $this->assertEquals('10.0.0.2', OC_Request::getRemoteAddress());
+
+               // With specifying a trusted remote address and trusted headers
+               OC::$server->getConfig()->setSystemValue('trusted_proxies', array('10.0.0.2'));
+               OC::$server->getConfig()->setSystemValue('forwarded_for_headers', array('HTTP_X_FORWARDED'));
+               $this->assertEquals('10.4.0.5', OC_Request::getRemoteAddress());
+               OC::$server->getConfig()->setSystemValue('forwarded_for_headers', array('HTTP_CLIENT_IP', 'HTTP_X_FORWARDED_FOR', 'HTTP_X_FORWARDED'));
+               $this->assertEquals('192.168.0.233', OC_Request::getRemoteAddress());
+
+               // With specifying multiple trusted remote addresses and trusted headers
+               OC::$server->getConfig()->setSystemValue('trusted_proxies', array('10.3.4.2', '10.0.0.2', '127.0.3.3'));
+               OC::$server->getConfig()->setSystemValue('forwarded_for_headers', array('HTTP_X_FORWARDED'));
+               $this->assertEquals('10.4.0.5', OC_Request::getRemoteAddress());
+               OC::$server->getConfig()->setSystemValue('forwarded_for_headers', array('HTTP_CLIENT_IP', 'HTTP_X_FORWARDED_FOR', 'HTTP_X_FORWARDED'));
+               $this->assertEquals('192.168.0.233', OC_Request::getRemoteAddress());
+       }
+
        /**
         * @dataProvider rawPathInfoProvider
         * @param $expected