Respect Forwarded-For headers

This commit is contained in:
Philipp 2022-06-23 22:42:35 +02:00
parent dbc1ebbb5c
commit d441b90bda
No known key found for this signature in database
GPG key ID: 24A7501396EB5432
11 changed files with 355 additions and 43 deletions

116
src/App/Request.php Normal file
View file

@ -0,0 +1,116 @@
<?php
namespace Friendica\App;
use Friendica\Core\Config\Capability\IManageConfigValues;
/**
* Container for the whole request
*
* @see https://www.php-fig.org/psr/psr-7/#321-psrhttpmessageserverrequestinterface
*
* @todo future container class for whole requests, currently it's not :-)
*/
class Request
{
/** @var string the default possible headers, which could contain the client IP */
const ORDERED_FORWARD_FOR_HEADER = 'HTTP_X_FORWARDED_FOR';
/** @var string The remote IP address of the current request */
protected $remoteAddress;
/**
* @return string The remote IP address of the current request
*/
public function getRemoteAddress(): string
{
return $this->remoteAddress;
}
public function __construct(IManageConfigValues $config, array $server = [])
{
$this->remoteAddress = $this->determineRemoteAddress($config, $server);
}
/**
* Checks if given $remoteAddress matches given $trustedProxy.
* If $trustedProxy is an IPv4 IP range given in CIDR notation, true will be returned if
* $remoteAddress is an IPv4 address within that IP range.
* Otherwise, $remoteAddress will be compared to $trustedProxy literally and the result
* will be returned.
*
* @return boolean true if $remoteAddress matches $trustedProxy, false otherwise
*/
protected function matchesTrustedProxy(string $trustedProxy, string $remoteAddress): bool
{
$cidrre = '/^([0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3})\/([0-9]{1,2})$/';
if (preg_match($cidrre, $trustedProxy, $match)) {
$net = $match[1];
$shiftbits = min(32, max(0, 32 - intval($match[2])));
$netnum = ip2long($net) >> $shiftbits;
$ipnum = ip2long($remoteAddress) >> $shiftbits;
return $ipnum === $netnum;
}
return $trustedProxy === $remoteAddress;
}
/**
* Checks if given $remoteAddress matches any entry in the given array $trustedProxies.
* For details regarding what "match" means, refer to `matchesTrustedProxy`.
*
* @return boolean true if $remoteAddress matches any entry in $trustedProxies, false otherwise
*/
protected function isTrustedProxy(array $trustedProxies, string $remoteAddress): bool
{
foreach ($trustedProxies as $tp) {
if ($this->matchesTrustedProxy($tp, $remoteAddress)) {
return true;
}
}
return false;
}
/**
* @param IManageConfigValues $config
* @param array $server
*
* @return string
*/
protected function determineRemoteAddress(IManageConfigValues $config, array $server): string
{
$remoteAddress = $server['REMOTE_ADDR'] ?? '0.0.0.0';
$trustedProxies = preg_split('/(\s*,*\s*)*,+(\s*,*\s*)*/', $config->get('proxy', 'trusted_proxies', ''));
if (\is_array($trustedProxies) && $this->isTrustedProxy($trustedProxies, $remoteAddress)) {
$forwardedForHeaders = preg_split('/(\s*,*\s*)*,+(\s*,*\s*)*/', $config->get('proxy', 'forwarded_for_headers')) ?? static::ORDERED_FORWARD_FOR_HEADER;
foreach ($forwardedForHeaders as $header) {
if (isset($server[$header])) {
foreach (explode(',', $server[$header]) as $IP) {
$IP = trim($IP);
// remove brackets from IPv6 addresses
if (strpos($IP, '[') === 0 && substr($IP, -1) === ']') {
$IP = substr($IP, 1, -1);
}
// skip trusted proxies in the list itself
if ($this->isTrustedProxy($trustedProxies, $IP)) {
continue;
}
if (filter_var($IP, FILTER_VALIDATE_IP) !== false) {
return $IP;
}
}
}
}
}
return $remoteAddress;
}
}

View file

@ -21,6 +21,7 @@
namespace Friendica\Core\Logger\Type\Monolog;
use Friendica\App\Request;
use Monolog\Handler;
use Monolog\Logger;
@ -38,15 +39,22 @@ class DevelopHandler extends Handler\AbstractHandler
private $developerIp;
/**
* @var string The IP of the current request
*/
private $remoteAddress;
/**
* @param Request $request The current http request
* @param string $developerIp The IP of the developer who wants to debug
* @param int $level The minimum logging level at which this handler will be triggered
* @param bool $bubble Whether the messages that are handled can bubble up the stack or not
*/
public function __construct($developerIp, $level = Logger::DEBUG, bool $bubble = true)
public function __construct(Request $request, $developerIp, int $level = Logger::DEBUG, bool $bubble = true)
{
parent::__construct($level, $bubble);
$this->developerIp = $developerIp;
$this->remoteAddress = $request->getRemoteAddress();
}
/**
@ -59,7 +67,7 @@ class DevelopHandler extends Handler\AbstractHandler
}
/// Just in case the remote IP is the same as the developer IP log the output
if (!is_null($this->developerIp) && $_SERVER['REMOTE_ADDR'] != $this->developerIp) {
if (!is_null($this->developerIp) && $this->remoteAddress != $this->developerIp) {
return false;
}

View file

@ -52,12 +52,12 @@ class Cookie
private $data;
/**
* @param App\Request $request The current http request
* @param IManageConfigValues $config
* @param App\BaseURL $baseURL
* @param array $SERVER The $_SERVER array
* @param array $COOKIE The $_COOKIE array
*/
public function __construct(IManageConfigValues $config, App\BaseURL $baseURL, array $SERVER = [], array $COOKIE = [])
public function __construct(App\Request $request, IManageConfigValues $config, App\BaseURL $baseURL, array $COOKIE = [])
{
$this->sslEnabled = $baseURL->getSSLPolicy() === App\BaseURL::SSL_POLICY_FULL;
$this->sitePrivateKey = $config->get('system', 'site_prvkey');
@ -66,7 +66,7 @@ class Cookie
self::DEFAULT_EXPIRE);
$this->lifetime = $authCookieDays * 24 * 60 * 60;
$this->remoteAddr = ($SERVER['REMOTE_ADDR'] ?? null) ?: '0.0.0.0';
$this->remoteAddr = $request->getRemoteAddress();
$this->data = json_decode($COOKIE[self::NAME] ?? '[]', true) ?: [];
}

View file

@ -21,14 +21,29 @@
namespace Friendica\Module\HTTPException;
use Friendica\App;
use Friendica\BaseModule;
use Friendica\Core\L10n;
use Friendica\Core\System;
use Friendica\DI;
use Friendica\Module\Response;
use Friendica\Network\HTTPException;
use Friendica\Util\Profiler;
use Psr\Http\Message\ResponseInterface;
use Psr\Log\LoggerInterface;
class PageNotFound extends BaseModule
{
/** @var string */
private $remoteAddress;
public function __construct(L10n $l10n, App\BaseURL $baseUrl, App\Arguments $args, LoggerInterface $logger, Profiler $profiler, Response $response, App\Request $request, array $server, array $parameters = [])
{
parent::__construct($l10n, $baseUrl, $args, $logger, $profiler, $response, $server, $parameters);
$this->remoteAddress = $request->getRemoteAddress();
}
protected function content(array $request = []): string
{
throw new HTTPException\NotFoundException(DI::l10n()->t('Page not found.'));
@ -58,7 +73,7 @@ class PageNotFound extends BaseModule
$this->logger->debug('index.php: page not found.', [
'request_uri' => $this->server['REQUEST_URI'],
'address' => $this->server['REMOTE_ADDR'],
'address' => $this->remoteAddress,
'query' => $this->server['QUERY_STRING']
]);

View file

@ -21,11 +21,13 @@
namespace Friendica\Module\Search;
use Friendica\App;
use Friendica\Content\Nav;
use Friendica\Content\Pager;
use Friendica\Content\Text\HTML;
use Friendica\Content\Widget;
use Friendica\Core\Cache\Enum\Duration;
use Friendica\Core\L10n;
use Friendica\Core\Logger;
use Friendica\Core\Renderer;
use Friendica\Core\Search;
@ -37,11 +39,24 @@ use Friendica\Model\Item;
use Friendica\Model\Post;
use Friendica\Model\Tag;
use Friendica\Module\BaseSearch;
use Friendica\Module\Response;
use Friendica\Network\HTTPException;
use Friendica\Util\Network;
use Friendica\Util\Profiler;
use Psr\Log\LoggerInterface;
class Index extends BaseSearch
{
/** @var string */
private $remoteAddress;
public function __construct(L10n $l10n, App\BaseURL $baseUrl, App\Arguments $args, LoggerInterface $logger, Profiler $profiler, Response $response, App\Request $request, array $server, array $parameters = [])
{
parent::__construct($l10n, $baseUrl, $args, $logger, $profiler, $response, $server, $parameters);
$this->remoteAddress = $request->getRemoteAddress();
}
protected function content(array $request = []): string
{
$search = (!empty($_GET['q']) ? trim(rawurldecode($_GET['q'])) : '');
@ -66,7 +81,7 @@ class Index extends BaseSearch
if ($crawl_permit_period == 0)
$crawl_permit_period = 10;
$remote = $_SERVER['REMOTE_ADDR'];
$remote = $this->remoteAddress;
$result = DI::cache()->get('remote_search:' . $remote);
if (!is_null($result)) {
$resultdata = json_decode($result);

View file

@ -64,6 +64,8 @@ class Authentication
private $session;
/** @var IManagePersonalConfigValues */
private $pConfig;
/** @var string */
private $remoteAddress;
/**
* Sets the X-Account-Management-Status header
@ -89,8 +91,9 @@ class Authentication
* @param User\Cookie $cookie
* @param IHandleSessions $session
* @param IManagePersonalConfigValues $pConfig
* @param App\Request $request
*/
public function __construct(IManageConfigValues $config, App\Mode $mode, App\BaseURL $baseUrl, L10n $l10n, Database $dba, LoggerInterface $logger, User\Cookie $cookie, IHandleSessions $session, IManagePersonalConfigValues $pConfig)
public function __construct(IManageConfigValues $config, App\Mode $mode, App\BaseURL $baseUrl, L10n $l10n, Database $dba, LoggerInterface $logger, User\Cookie $cookie, IHandleSessions $session, IManagePersonalConfigValues $pConfig, App\Request $request)
{
$this->config = $config;
$this->mode = $mode;
@ -101,6 +104,7 @@ class Authentication
$this->cookie = $cookie;
$this->session = $session;
$this->pConfig = $pConfig;
$this->remoteAddress = $request->getRemoteAddress();
}
/**
@ -163,10 +167,11 @@ class Authentication
// already logged in user returning
$check = $this->config->get('system', 'paranoia');
// extra paranoia - if the IP changed, log them out
if ($check && ($this->session->get('addr') != $_SERVER['REMOTE_ADDR'])) {
if ($check && ($this->session->get('addr') != $this->remoteAddress)) {
$this->logger->notice('Session address changed. Paranoid setting in effect, blocking session. ', [
'addr' => $this->session->get('addr'),
'remote_addr' => $_SERVER['REMOTE_ADDR']]
'remote_addr' => $this->remoteAddress
]
);
$this->session->clear();
$this->baseUrl->redirect();
@ -258,7 +263,7 @@ class Authentication
['uid' => User::getIdFromPasswordAuthentication($username, $password)]
);
} catch (Exception $e) {
$this->logger->warning('authenticate: failed login attempt', ['action' => 'login', 'username' => $username, 'ip' => $_SERVER['REMOTE_ADDR']]);
$this->logger->warning('authenticate: failed login attempt', ['action' => 'login', 'username' => $username, 'ip' => $this->remoteAddress]);
notice($this->l10n->t('Login failed. Please check your credentials.'));
$this->baseUrl->redirect();
}
@ -308,7 +313,7 @@ class Authentication
'page_flags' => $user_record['page-flags'],
'my_url' => $this->baseUrl->get() . '/profile/' . $user_record['nickname'],
'my_address' => $user_record['nickname'] . '@' . substr($this->baseUrl->get(), strpos($this->baseUrl->get(), '://') + 3),
'addr' => ($_SERVER['REMOTE_ADDR'] ?? '') ?: '0.0.0.0'
'addr' => $this->remoteAddress,
]);
Session::setVisitorsContacts();

View file

@ -632,6 +632,16 @@ return [
// Timeout in seconds for fetching the XRD links and other requests with an expected shorter timeout
'xrd_timeout' => 20,
],
'proxy' => [
// forwarded_for_headers (String)
// A comma separated list of all allowed header values to retrieve the real client IP
'forwarded_for_headers' => 'HTTP_X_FORWARDED_FOR',
// trusted_proxies (String)
// A comma separated list of all trusted proxies, which will get skipped during client IP retrieval
// IP ranges and CIDR notations are allowed
'trusted_proxies' => '',
],
'experimental' => [
// exp_themes (Boolean)
// Show experimental themes in user settings.

View file

@ -208,7 +208,7 @@ return [
],
Cookie::class => [
'constructParams' => [
$_SERVER, $_COOKIE
$_COOKIE
],
],
ICanWriteToStorage::class => [
@ -238,4 +238,9 @@ return [
$_SERVER
],
],
App\Request::class => [
'constructParams' => [
$_SERVER
],
]
];

View file

@ -61,4 +61,8 @@ return [
'REDIS_PORT' => ['system', 'redis_port'],
'REDIS_PW' => ['system', 'redis_password'],
'REDIS_DB' => ['system', 'redis_db'],
// Proxy Config
'FRIENDICA_FORWARDED_HEADERS' => ['proxy', 'forwarded_for_headers'],
'FRIENDICA_TRUSTED_PROXIES' => ['proxy', 'trusted_proxies'],
];

View file

@ -0,0 +1,110 @@
<?php
namespace Friendica\Test\src\App;
use Friendica\App\Request;
use Friendica\Core\Config\Capability\IManageConfigValues;
use Friendica\Test\MockedTest;
class RequestTest extends MockedTest
{
public function dataServerArray(): array
{
return [
'default' => [
'server' => ['REMOTE_ADDR' => '1.2.3.4'],
'config' => [
'trusted_proxies' => '',
'forwarded_for_headers' => '',
],
'assertion' => '1.2.3.4',
],
'proxy_1' => [
'server' => ['HTTP_X_FORWARDED_FOR' => '1.2.3.4, 4.5.6.7', 'REMOTE_ADDR' => '1.2.3.4'],
'config' => [
'trusted_proxies' => '1.2.3.4',
'forwarded_for_headers' => 'HTTP_X_FORWARDED_FOR',
],
'assertion' => '4.5.6.7',
],
'proxy_2' => [
'server' => ['HTTP_X_FORWARDED_FOR' => '4.5.6.7, 1.2.3.4', 'REMOTE_ADDR' => '1.2.3.4'],
'config' => [
'trusted_proxies' => '1.2.3.4',
'forwarded_for_headers' => 'HTTP_X_FORWARDED_FOR',
],
'assertion' => '4.5.6.7',
],
'proxy_CIDR_multiple_proxies' => [
'server' => ['HTTP_X_FORWARDED_FOR' => '4.5.6.7, 1.2.3.4', 'REMOTE_ADDR' => '10.0.1.1'],
'config' => [
'trusted_proxies' => '10.0.0.0/16, 1.2.3.4',
'forwarded_for_headers' => 'HTTP_X_FORWARDED_FOR',
],
'assertion' => '4.5.6.7',
],
'proxy_wrong_CIDR' => [
'server' => ['HTTP_X_FORWARDED_FOR' => '4.5.6.7, 1.2.3.4', 'REMOTE_ADDR' => '10.1.0.1'],
'config' => [
'trusted_proxies' => '10.0.0.0/24, 1.2.3.4',
'forwarded_for_headers' => 'HTTP_X_FORWARDED_FOR',
],
'assertion' => '10.1.0.1',
],
'proxy_3' => [
'server' => ['HTTP_X_FORWARDED_FOR' => '1.2.3.4, 4.5.6.7', 'REMOTE_ADDR' => '1.2.3.4'],
'config' => [
'trusted_proxies' => '1.2.3.4',
'forwarded_for_headers' => 'HTTP_X_FORWARDED_FOR',
],
'assertion' => '4.5.6.7',
],
'proxy_multiple_header_1' => [
'server' => ['HTTP_X_FORWARDED' => '1.2.3.4, 4.5.6.7', 'REMOTE_ADDR' => '1.2.3.4'],
'config' => [
'trusted_proxies' => '1.2.3.4',
'forwarded_for_headers' => 'HTTP_X_FORWARDED_FOR, HTTP_X_FORWARDED',
],
'assertion' => '4.5.6.7',
],
'proxy_multiple_header_2' => [
'server' => ['HTTP_X_FORWARDED_FOR' => '1.2.3.4', 'HTTP_X_FORWARDED' => '1.2.3.4, 4.5.6.7', 'REMOTE_ADDR' => '1.2.3.4'],
'config' => [
'trusted_proxies' => '1.2.3.4',
'forwarded_for_headers' => 'HTTP_X_FORWARDED_FOR, HTTP_X_FORWARDED',
],
'assertion' => '4.5.6.7',
],
'proxy_multiple_header_wrong' => [
'server' => ['HTTP_X_FORWARDED_FOR' => '1.2.3.4', 'HTTP_X_FORWARDED' => '1.2.3.4, 4.5.6.7', 'REMOTE_ADDR' => '1.2.3.4'],
'config' => [
'trusted_proxies' => '1.2.3.4',
'forwarded_for_headers' => '',
],
'assertion' => '1.2.3.4',
],
'no_remote_addr' => [
'server' => [],
'config' => [
'trusted_proxies' => '1.2.3.4',
'forwarded_for_headers' => '',
],
'assertion' => '0.0.0.0',
],
];
}
/**
* @dataProvider dataServerArray
*/
public function testRemoteAddress(array $server, array $config, string $assertion)
{
$configClass = \Mockery::mock(IManageConfigValues::class);
$configClass->shouldReceive('get')->with('proxy', 'trusted_proxies', '')->andReturn($config['trusted_proxies']);
$configClass->shouldReceive('get')->with('proxy', 'forwarded_for_headers')->andReturn($config['forwarded_for_headers']);
$request = new Request($configClass, $server);
self::assertEquals($assertion, $request->getRemoteAddress());
}
}

View file

@ -22,6 +22,7 @@
namespace Friendica\Test\src\Model\User;
use Friendica\App\BaseURL;
use Friendica\App\Request;
use Friendica\Core\Config\Capability\IManageConfigValues;
use Friendica\Model\User\Cookie;
use Friendica\Test\MockedTest;
@ -35,6 +36,8 @@ class CookieTest extends MockedTest
/** @var MockInterface|BaseURL */
private $baseUrl;
const SERVER_ARRAY = ['REMOTE_ADDR' => '1.2.3.4'];
protected function setUp(): void
{
StaticCookie::clearStatic();
@ -60,8 +63,11 @@ class CookieTest extends MockedTest
$this->baseUrl->shouldReceive('getSSLPolicy')->andReturn(true)->once();
$this->config->shouldReceive('get')->with('system', 'site_prvkey')->andReturn('1235')->once();
$this->config->shouldReceive('get')->with('system', 'auth_cookie_lifetime', Cookie::DEFAULT_EXPIRE)->andReturn('7')->once();
$this->config->shouldReceive('get')->with('proxy', 'trusted_proxies', '')->andReturn('')->once();
$cookie = new Cookie($this->config, $this->baseUrl);
$request = new Request($this->config,static::SERVER_ARRAY);
$cookie = new Cookie($request, $this->config, $this->baseUrl);
self::assertInstanceOf(Cookie::class, $cookie);
}
@ -124,8 +130,11 @@ class CookieTest extends MockedTest
$this->baseUrl->shouldReceive('getSSLPolicy')->andReturn(true)->once();
$this->config->shouldReceive('get')->with('system', 'site_prvkey')->andReturn('1235')->once();
$this->config->shouldReceive('get')->with('system', 'auth_cookie_lifetime', Cookie::DEFAULT_EXPIRE)->andReturn('7')->once();
$this->config->shouldReceive('get')->with('proxy', 'trusted_proxies', '')->andReturn('')->once();
$cookie = new Cookie($this->config, $this->baseUrl, [], $cookieData);
$request = new Request($this->config, static::SERVER_ARRAY);
$cookie = new Cookie($request, $this->config, $this->baseUrl, $cookieData);
self::assertInstanceOf(Cookie::class, $cookie);
if (isset($uid)) {
@ -182,8 +191,11 @@ class CookieTest extends MockedTest
$this->baseUrl->shouldReceive('getSSLPolicy')->andReturn(true)->once();
$this->config->shouldReceive('get')->with('system', 'site_prvkey')->andReturn($serverPrivateKey)->once();
$this->config->shouldReceive('get')->with('system', 'auth_cookie_lifetime', Cookie::DEFAULT_EXPIRE)->andReturn('7')->once();
$this->config->shouldReceive('get')->with('proxy', 'trusted_proxies', '')->andReturn('')->once();
$cookie = new Cookie($this->config, $this->baseUrl);
$request = new Request($this->config, static::SERVER_ARRAY);
$cookie = new Cookie($request, $this->config, $this->baseUrl);
self::assertInstanceOf(Cookie::class, $cookie);
self::assertEquals($assertTrue, $cookie->comparePrivateDataHash($assertHash, $password, $userPrivateKey));
@ -239,8 +251,13 @@ class CookieTest extends MockedTest
$this->baseUrl->shouldReceive('getSSLPolicy')->andReturn(true)->once();
$this->config->shouldReceive('get')->with('system', 'site_prvkey')->andReturn($serverKey)->once();
$this->config->shouldReceive('get')->with('system', 'auth_cookie_lifetime', Cookie::DEFAULT_EXPIRE)->andReturn(Cookie::DEFAULT_EXPIRE)->once();
$this->config->shouldReceive('get')->with('proxy', 'trusted_proxies', '')->andReturn('')->once();
$this->config->shouldReceive('get')->with('proxy', 'forwarded_for_headers')->andReturn(Request::ORDERED_FORWARD_FOR_HEADER);
$cookie = new StaticCookie($this->config, $this->baseUrl, $serverArray);
$request = new Request($this->config, $serverArray);
$cookie = new StaticCookie($request, $this->config, $this->baseUrl);
self::assertInstanceOf(Cookie::class, $cookie);
$cookie->setMultiple([
@ -261,8 +278,12 @@ class CookieTest extends MockedTest
$this->baseUrl->shouldReceive('getSSLPolicy')->andReturn(true)->once();
$this->config->shouldReceive('get')->with('system', 'site_prvkey')->andReturn($serverKey)->once();
$this->config->shouldReceive('get')->with('system', 'auth_cookie_lifetime', Cookie::DEFAULT_EXPIRE)->andReturn(Cookie::DEFAULT_EXPIRE)->once();
$this->config->shouldReceive('get')->with('proxy', 'trusted_proxies', '')->andReturn('')->once();
$this->config->shouldReceive('get')->with('proxy', 'forwarded_for_headers')->andReturn(Request::ORDERED_FORWARD_FOR_HEADER);
$cookie = new StaticCookie($this->config, $this->baseUrl, $serverArray);
$request = new Request($this->config, $serverArray);
$cookie = new StaticCookie($request, $this->config, $this->baseUrl, $serverArray);
self::assertInstanceOf(Cookie::class, $cookie);
$cookie->set('uid', $uid);
@ -283,8 +304,11 @@ class CookieTest extends MockedTest
$this->baseUrl->shouldReceive('getSSLPolicy')->andReturn(true)->once();
$this->config->shouldReceive('get')->with('system', 'site_prvkey')->andReturn(24)->once();
$this->config->shouldReceive('get')->with('system', 'auth_cookie_lifetime', Cookie::DEFAULT_EXPIRE)->andReturn(Cookie::DEFAULT_EXPIRE)->once();
$this->config->shouldReceive('get')->with('proxy', 'trusted_proxies', '')->andReturn('')->once();
$cookie = new StaticCookie($this->config, $this->baseUrl);
$request = new Request($this->config, static::SERVER_ARRAY);
$cookie = new StaticCookie($request, $this->config, $this->baseUrl);
self::assertInstanceOf(Cookie::class, $cookie);
self::assertEquals('test', StaticCookie::$_COOKIE[Cookie::NAME]);