diff --git a/lib/Controller.php b/lib/Controller.php index 8ead8848..56f424c0 100644 --- a/lib/Controller.php +++ b/lib/Controller.php @@ -195,35 +195,14 @@ class Controller */ private function _create() { - // Check if whitelist feature is enabled - if (($option = $this->_conf->getKey('creators', 'traffic')) !== '') { - // Parse whitelist into array - $whitelist = explode(',', $option); - // Check for source IP in HTTP header - if (($option = $this->_conf->getKey('header', 'traffic')) !== null) { - $httpHeader = 'HTTP_' . $option; - // Grab source IP from HTTP header (if it exists) - if (array_key_exists($httpHeader, $_SERVER) && !empty($_SERVER[$httpHeader])) { - // Check if source IP reported from HTTP header is in whitelist array - if (!in_array($_SERVER[$httpHeader], $whitelist)) { - $this->_return_message(1, I18n::_('Your IP is not authorized to create pastes.')); - return; - } - } - } - } - // Ensure last paste from visitors IP address was more than configured amount of seconds ago. ServerSalt::setStore($this->_model->getStore()); TrafficLimiter::setConfiguration($this->_conf); TrafficLimiter::setStore($this->_model->getStore()); - if (!TrafficLimiter::canPass()) { - $this->_return_message( - 1, I18n::_( - 'Please wait %d seconds between each post.', - $this->_conf->getKey('limit', 'traffic') - ) - ); + try { + TrafficLimiter::canPass(); + } catch (Exception $e) { + $this->_return_message(1, $e->getMessage()); return; } diff --git a/lib/Persistence/TrafficLimiter.php b/lib/Persistence/TrafficLimiter.php index f5f8e7a8..fab33c41 100644 --- a/lib/Persistence/TrafficLimiter.php +++ b/lib/Persistence/TrafficLimiter.php @@ -13,8 +13,10 @@ namespace PrivateBin\Persistence; +use Exception; use IPLib\Factory; use PrivateBin\Configuration; +use PrivateBin\I18n; /** * TrafficLimiter @@ -24,13 +26,13 @@ use PrivateBin\Configuration; class TrafficLimiter extends AbstractPersistence { /** - * time limit in seconds, defaults to 10s + * listed IPs are the only ones allowed to create, defaults to null * * @access private * @static - * @var int + * @var string|null */ - private static $_limit = 10; + private static $_creators = null; /** * listed IPs are exempted from limits, defaults to null @@ -51,19 +53,49 @@ class TrafficLimiter extends AbstractPersistence private static $_ipKey = 'REMOTE_ADDR'; /** - * set the time limit in seconds + * time limit in seconds, defaults to 10s + * + * @access private + * @static + * @var int + */ + private static $_limit = 10; + + /** + * set configuration options of the traffic limiter * * @access public * @static - * @param int $limit + * @param Configuration $conf */ - public static function setLimit($limit) + public static function setConfiguration(Configuration $conf) { - self::$_limit = $limit; + self::setCreators($conf->getKey('creators', 'traffic')); + self::setExempted($conf->getKey('exempted', 'traffic')); + self::setLimit($conf->getKey('limit', 'traffic')); + + if (($option = $conf->getKey('header', 'traffic')) !== '') { + $httpHeader = 'HTTP_' . $option; + if (array_key_exists($httpHeader, $_SERVER) && !empty($_SERVER[$httpHeader])) { + self::$_ipKey = $httpHeader; + } + } } /** - * set a list of IP(-ranges) as string + * set a list of creator IP(-ranges) as string + * + * @access public + * @static + * @param string $creators + */ + public static function setCreators($creators) + { + self::$_creators = $creators; + } + + /** + * set a list of exempted IP(-ranges) as string * * @access public * @static @@ -75,23 +107,15 @@ class TrafficLimiter extends AbstractPersistence } /** - * set configuration options of the traffic limiter + * set the time limit in seconds * * @access public * @static - * @param Configuration $conf + * @param int $limit */ - public static function setConfiguration(Configuration $conf) + public static function setLimit($limit) { - self::setLimit($conf->getKey('limit', 'traffic')); - self::setExempted($conf->getKey('exempted', 'traffic')); - - if (($option = $conf->getKey('header', 'traffic')) !== '') { - $httpHeader = 'HTTP_' . $option; - if (array_key_exists($httpHeader, $_SERVER) && !empty($_SERVER[$httpHeader])) { - self::$_ipKey = $httpHeader; - } - } + self::$_limit = $limit; } /** @@ -108,7 +132,7 @@ class TrafficLimiter extends AbstractPersistence } /** - * Validate $_ipKey against configured ipranges. If matched we will ignore the ip + * validate $_ipKey against configured ipranges. If matched we will ignore the ip * * @access private * @static @@ -136,22 +160,33 @@ class TrafficLimiter extends AbstractPersistence } /** - * traffic limiter - * - * Make sure the IP address makes at most 1 request every 10 seconds. + * make sure the IP address is allowed to perfom a request * * @access public * @static - * @return bool + * @throws Exception + * @return true */ public static function canPass() { + // if creators are defined, the traffic limiter will only allow creation + // for these, with no limits, and skip any other rules + if (!empty(self::$_creators)) { + $creatorIps = explode(',', self::$_creators); + foreach ($creatorIps as $ipRange) { + if (self::matchIp($ipRange) === true) { + return true; + } + } + throw new Exception(I18n::_('Your IP is not authorized to create pastes.')); + } + // disable limits if set to less then 1 if (self::$_limit < 1) { return true; } - // Check if $_ipKey is exempted from ratelimiting + // check if $_ipKey is exempted from ratelimiting if (!empty(self::$_exempted)) { $exIp_array = explode(',', self::$_exempted); foreach ($exIp_array as $ipRange) { @@ -175,6 +210,10 @@ class TrafficLimiter extends AbstractPersistence if (!self::$_store->setValue((string) $tl, 'traffic_limiter', $hash)) { error_log('failed to store the traffic limiter, it probably contains outdated information'); } - return $result; + if ($result) return true; + throw new Exception(I18n::_( + 'Please wait %d seconds between each post.', + self::$_limit + )); } } diff --git a/tst/Persistence/TrafficLimiterTest.php b/tst/Persistence/TrafficLimiterTest.php index 4e56b8a3..e5256c74 100644 --- a/tst/Persistence/TrafficLimiterTest.php +++ b/tst/Persistence/TrafficLimiterTest.php @@ -38,13 +38,21 @@ class TrafficLimiterTest extends PHPUnit_Framework_TestCase $_SERVER['REMOTE_ADDR'] = '127.0.0.1'; $this->assertTrue(TrafficLimiter::canPass(), 'first request may pass'); sleep(1); - $this->assertFalse(TrafficLimiter::canPass(), 'second request is to fast, may not pass'); + try { + $this->assertFalse(TrafficLimiter::canPass(), 'expected an exception'); + } catch (Exception $e) { + $this->assertEquals($e->getMessage(), 'Please wait 4 seconds between each post.', 'second request is to fast, may not pass'); + } sleep(4); $this->assertTrue(TrafficLimiter::canPass(), 'third request waited long enough and may pass'); $_SERVER['REMOTE_ADDR'] = '2001:1620:2057:dead:beef::cafe:babe'; $this->assertTrue(TrafficLimiter::canPass(), 'fourth request has different ip and may pass'); $_SERVER['REMOTE_ADDR'] = '127.0.0.1'; - $this->assertFalse(TrafficLimiter::canPass(), 'fifth request is to fast, may not pass'); + try { + $this->assertFalse(TrafficLimiter::canPass(), 'expected an exception'); + } catch (Exception $e) { + $this->assertEquals($e->getMessage(), 'Please wait 4 seconds between each post.', 'fifth request is to fast, may not pass'); + } } public function testTrafficLimitExempted() @@ -52,7 +60,11 @@ class TrafficLimiterTest extends PHPUnit_Framework_TestCase TrafficLimiter::setExempted('1.2.3.4,10.10.10.0/24,2001:1620:2057::/48'); $_SERVER['REMOTE_ADDR'] = '127.0.0.1'; $this->assertTrue(TrafficLimiter::canPass(), 'first request may pass'); - $this->assertFalse(TrafficLimiter::canPass(), 'not exempted'); + try { + $this->assertFalse(TrafficLimiter::canPass(), 'expected an exception'); + } catch (Exception $e) { + $this->assertEquals($e->getMessage(), 'Please wait 4 seconds between each post.', 'not exempted'); + } $_SERVER['REMOTE_ADDR'] = '10.10.10.10'; $this->assertTrue(TrafficLimiter::canPass(), 'IPv4 in exempted range'); $this->assertTrue(TrafficLimiter::canPass(), 'request is to fast, but IPv4 in exempted range'); @@ -61,7 +73,11 @@ class TrafficLimiterTest extends PHPUnit_Framework_TestCase $this->assertTrue(TrafficLimiter::canPass(), 'request is to fast, but IPv6 in exempted range'); TrafficLimiter::setExempted('127.*,foobar'); $this->assertTrue(TrafficLimiter::canPass(), 'first cached request may pass'); - $this->assertFalse(TrafficLimiter::canPass(), 'request is to fast, invalid range'); + try { + $this->assertFalse(TrafficLimiter::canPass(), 'expected an exception'); + } catch (Exception $e) { + $this->assertEquals($e->getMessage(), 'Please wait 4 seconds between each post.', 'request is to fast, invalid range'); + } $_SERVER['REMOTE_ADDR'] = 'foobar'; $this->assertTrue(TrafficLimiter::canPass(), 'non-IP address'); $this->assertTrue(TrafficLimiter::canPass(), 'request is to fast, but non-IP address matches exempted range'); @@ -71,16 +87,23 @@ class TrafficLimiterTest extends PHPUnit_Framework_TestCase { TrafficLimiter::setCreators('1.2.3.4,10.10.10.0/24,2001:1620:2057::/48'); $_SERVER['REMOTE_ADDR'] = '127.0.0.1'; - $this->assertFalse(TrafficLimiter::canPass(), 'not a creator'); + try { + $this->assertFalse(TrafficLimiter::canPass(), 'expected an exception'); + } catch (Exception $e) { + $this->assertEquals($e->getMessage(), 'Your IP is not authorized to create pastes.', 'not a creator'); + } $_SERVER['REMOTE_ADDR'] = '10.10.10.10'; $this->assertTrue(TrafficLimiter::canPass(), 'IPv4 in creator range'); $this->assertTrue(TrafficLimiter::canPass(), 'request is to fast, but IPv4 in creator range'); $_SERVER['REMOTE_ADDR'] = '2001:1620:2057:dead:beef::cafe:babe'; $this->assertTrue(TrafficLimiter::canPass(), 'IPv6 in creator range'); $this->assertTrue(TrafficLimiter::canPass(), 'request is to fast, but IPv6 in creator range'); - TrafficLimiter::setExempted('127.*,foobar'); - $this->assertTrue(TrafficLimiter::canPass(), 'first cached request may pass'); - $this->assertFalse(TrafficLimiter::canPass(), 'request is to fast, not a creator'); + TrafficLimiter::setCreators('127.*,foobar'); + try { + $this->assertFalse(TrafficLimiter::canPass(), 'expected an exception'); + } catch (Exception $e) { + $this->assertEquals($e->getMessage(), 'Your IP is not authorized to create pastes.', 'request is to fast, not a creator'); + } $_SERVER['REMOTE_ADDR'] = 'foobar'; $this->assertTrue(TrafficLimiter::canPass(), 'non-IP address'); $this->assertTrue(TrafficLimiter::canPass(), 'request is to fast, but non-IP address matches creator');