From 7acdd018a1555c9bc5dcc1702074a10f862bb170 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Tue, 26 Aug 2014 23:58:13 +0200 Subject: [PATCH] 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 --- config/config.sample.php | 6 ++++++ lib/private/allconfig.php | 2 +- lib/private/request.php | 28 ++++++++++++++++++++++++++++ lib/public/config.php | 2 +- lib/public/iconfig.php | 2 +- tests/lib/request.php | 38 +++++++++++++++++++++++++++++++++++--- 6 files changed, 72 insertions(+), 6 deletions(-) diff --git a/config/config.sample.php b/config/config.sample.php index 9656555691..d232e18ab0 100755 --- 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 eb11454601..ef8673af23 100644 --- 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 5fd5b3a719..b063c1f596 100755 --- 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 ea3e0c1372..65dde39cdc 100644 --- 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 d4a8cdc738..4865f8bc85 100644 --- 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 bff84e1b03..b89bf92ece 100644 --- 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