Page MenuHomePhabricator

D18703.diff
No OneTemporary

D18703.diff

diff --git a/src/aphront/configuration/AphrontApplicationConfiguration.php b/src/aphront/configuration/AphrontApplicationConfiguration.php
--- a/src/aphront/configuration/AphrontApplicationConfiguration.php
+++ b/src/aphront/configuration/AphrontApplicationConfiguration.php
@@ -204,20 +204,10 @@
DarkConsoleXHProfPluginAPI::saveProfilerSample($access_log);
- // Add points to the rate limits for this request.
- $rate_token = PhabricatorStartup::getRateLimitToken();
- if ($rate_token !== null) {
- // The base score for a request allows users to make 30 requests per
- // minute.
- $score = (1000 / 30);
-
- // If the user was logged in, let them make more requests.
- if ($request->getUser() && $request->getUser()->getPHID()) {
- $score = $score / 5;
- }
-
- PhabricatorStartup::addRateLimitScore($rate_token, $score);
- }
+ PhabricatorStartup::disconnectRateLimits(
+ array(
+ 'viewer' => $request->getUser(),
+ ));
if ($processing_exception) {
throw $processing_exception;
diff --git a/src/applications/base/controller/__tests__/PhabricatorAccessControlTestCase.php b/src/applications/base/controller/__tests__/PhabricatorAccessControlTestCase.php
--- a/src/applications/base/controller/__tests__/PhabricatorAccessControlTestCase.php
+++ b/src/applications/base/controller/__tests__/PhabricatorAccessControlTestCase.php
@@ -10,7 +10,7 @@
public function testControllerAccessControls() {
$root = dirname(phutil_get_library_root('phabricator'));
- require_once $root.'/support/PhabricatorStartup.php';
+ require_once $root.'/support/startup/PhabricatorStartup.php';
$application_configuration = new AphrontDefaultApplicationConfiguration();
diff --git a/support/startup/PhabricatorClientConnectionLimit.php b/support/startup/PhabricatorClientConnectionLimit.php
new file mode 100644
--- /dev/null
+++ b/support/startup/PhabricatorClientConnectionLimit.php
@@ -0,0 +1,44 @@
+<?php
+
+final class PhabricatorClientConnectionLimit
+ extends PhabricatorClientLimit {
+
+ protected function getBucketDuration() {
+ return 60;
+ }
+
+ protected function getBucketCount() {
+ return 15;
+ }
+
+ protected function shouldRejectConnection($score) {
+ // Reject connections if the cumulative score across all buckets exceeds
+ // the limit.
+ return ($score > $this->getLimit());
+ }
+
+ protected function getConnectScore() {
+ return 1;
+ }
+
+ protected function getPenaltyScore() {
+ return 0;
+ }
+
+ protected function getDisconnectScore(array $request_state) {
+ return -1;
+ }
+
+ protected function getRateLimitReason($score) {
+ $client_key = $this->getClientKey();
+
+ // NOTE: This happens before we load libraries, so we can not use pht()
+ // here.
+
+ return
+ "TOO MANY CONCURRENT CONNECTIONS\n".
+ "You (\"{$client_key}\") have too many concurrent ".
+ "connections.\n";
+ }
+
+}
diff --git a/support/startup/PhabricatorClientLimit.php b/support/startup/PhabricatorClientLimit.php
new file mode 100644
--- /dev/null
+++ b/support/startup/PhabricatorClientLimit.php
@@ -0,0 +1,291 @@
+<?php
+
+abstract class PhabricatorClientLimit {
+
+ private $limitKey;
+ private $clientKey;
+ private $limit;
+
+ final public function setLimitKey($limit_key) {
+ $this->limitKey = $limit_key;
+ return $this;
+ }
+
+ final public function getLimitKey() {
+ return $this->limitKey;
+ }
+
+ final public function setClientKey($client_key) {
+ $this->clientKey = $client_key;
+ return $this;
+ }
+
+ final public function getClientKey() {
+ return $this->clientKey;
+ }
+
+ final public function setLimit($limit) {
+ $this->limit = $limit;
+ return $this;
+ }
+
+ final public function getLimit() {
+ return $this->limit;
+ }
+
+ final public function didConnect() {
+ // NOTE: We can not use pht() here because this runs before libraries
+ // load.
+
+ if (!function_exists('apc_fetch') && !function_exists('apcu_fetch')) {
+ throw new Exception(
+ 'You can not configure connection rate limits unless APC/APCu are '.
+ 'available. Rate limits rely on APC/APCu to track clients and '.
+ 'connections.');
+ }
+
+ if ($this->getClientKey() === null) {
+ throw new Exception(
+ 'You must configure a client key when defining a rate limit.');
+ }
+
+ if ($this->getLimitKey() === null) {
+ throw new Exception(
+ 'You must configure a limit key when defining a rate limit.');
+ }
+
+ if ($this->getLimit() === null) {
+ throw new Exception(
+ 'You must configure a limit when defining a rate limit.');
+ }
+
+ $points = $this->getConnectScore();
+ if ($points) {
+ $this->addScore($points);
+ }
+
+ $score = $this->getScore();
+ if (!$this->shouldRejectConnection($score)) {
+ // Client has not hit the limit, so continue processing the request.
+ return null;
+ }
+
+ $penalty = $this->getPenaltyScore();
+ if ($penalty) {
+ $this->addScore($penalty);
+ $score += $penalty;
+ }
+
+ return $this->getRateLimitReason($score);
+ }
+
+ final public function didDisconnect(array $request_state) {
+ $score = $this->getDisconnectScore($request_state);
+ if ($score) {
+ $this->addScore($score);
+ }
+ }
+
+
+ /**
+ * Get the number of seconds for each rate bucket.
+ *
+ * For example, a value of 60 will create one-minute buckets.
+ *
+ * @return int Number of seconds per bucket.
+ */
+ abstract protected function getBucketDuration();
+
+
+ /**
+ * Get the total number of rate limit buckets to retain.
+ *
+ * @return int Total number of rate limit buckets to retain.
+ */
+ abstract protected function getBucketCount();
+
+
+ /**
+ * Get the score to add when a client connects.
+ *
+ * @return double Connection score.
+ */
+ abstract protected function getConnectScore();
+
+
+ /**
+ * Get the number of penalty points to add when a client hits a rate limit.
+ *
+ * @return double Penalty score.
+ */
+ abstract protected function getPenaltyScore();
+
+
+ /**
+ * Get the score to add when a client disconnects.
+ *
+ * @return double Connection score.
+ */
+ abstract protected function getDisconnectScore(array $request_state);
+
+
+ /**
+ * Get a human-readable explanation of why the client is being rejected.
+ *
+ * @return string Brief rejection message.
+ */
+ abstract protected function getRateLimitReason($score);
+
+
+ /**
+ * Determine whether to reject a connection.
+ *
+ * @return bool True to reject the connection.
+ */
+ abstract protected function shouldRejectConnection($score);
+
+
+ /**
+ * Get the APC key for the smallest stored bucket.
+ *
+ * @return string APC key for the smallest stored bucket.
+ * @task ratelimit
+ */
+ private function getMinimumBucketCacheKey() {
+ $limit_key = $this->getLimitKey();
+ return "limit:min:{$limit_key}";
+ }
+
+
+ /**
+ * Get the current bucket ID for storing rate limit scores.
+ *
+ * @return int The current bucket ID.
+ */
+ private function getCurrentBucketID() {
+ return (int)(time() / $this->getBucketDuration());
+ }
+
+
+ /**
+ * Get the APC key for a given bucket.
+ *
+ * @param int Bucket to get the key for.
+ * @return string APC key for the bucket.
+ */
+ private function getBucketCacheKey($bucket_id) {
+ $limit_key = $this->getLimitKey();
+ return "limit:bucket:{$limit_key}:{$bucket_id}";
+ }
+
+
+ /**
+ * Add points to the rate limit score for some client.
+ *
+ * @param string Some key which identifies the client making the request.
+ * @param float The cost for this request; more points pushes them toward
+ * the limit faster.
+ * @return this
+ */
+ private function addScore($score) {
+ $is_apcu = (bool)function_exists('apcu_fetch');
+
+ $current = $this->getCurrentBucketID();
+ $bucket_key = $this->getBucketCacheKey($current);
+
+ // There's a bit of a race here, if a second process reads the bucket
+ // before this one writes it, but it's fine if we occasionally fail to
+ // record a client's score. If they're making requests fast enough to hit
+ // rate limiting, we'll get them soon enough.
+
+ if ($is_apcu) {
+ $bucket = apcu_fetch($bucket_key);
+ } else {
+ $bucket = apc_fetch($bucket_key);
+ }
+
+ if (!is_array($bucket)) {
+ $bucket = array();
+ }
+
+ $client_key = $this->getClientKey();
+ if (empty($bucket[$client_key])) {
+ $bucket[$client_key] = 0;
+ }
+
+ $bucket[$client_key] += $score;
+
+ if ($is_apcu) {
+ apcu_store($bucket_key, $bucket);
+ } else {
+ apc_store($bucket_key, $bucket);
+ }
+
+ return $this;
+ }
+
+
+ /**
+ * Get the current rate limit score for a given client.
+ *
+ * @return float The client's current score.
+ * @task ratelimit
+ */
+ private function getScore() {
+ $is_apcu = (bool)function_exists('apcu_fetch');
+
+ // Identify the oldest bucket stored in APC.
+ $min_key = $this->getMinimumBucketCacheKey();
+ if ($is_apcu) {
+ $min = apcu_fetch($min_key);
+ } else {
+ $min = apc_fetch($min_key);
+ }
+
+ // If we don't have any buckets stored yet, store the current bucket as
+ // the oldest bucket.
+ $cur = $this->getCurrentBucketID();
+ if (!$min) {
+ if ($is_apcu) {
+ apcu_store($min_key, $cur);
+ } else {
+ apc_store($min_key, $cur);
+ }
+ $min = $cur;
+ }
+
+ // Destroy any buckets that are older than the minimum bucket we're keeping
+ // track of. Under load this normally shouldn't do anything, but will clean
+ // up an old bucket once per minute.
+ $count = $this->getBucketCount();
+ for ($cursor = $min; $cursor < ($cur - $count); $cursor++) {
+ $bucket_key = $this->getBucketCacheKey($cursor);
+ if ($is_apcu) {
+ apcu_delete($bucket_key);
+ apcu_store($min_key, $cursor + 1);
+ } else {
+ apc_delete($bucket_key);
+ apc_store($min_key, $cursor + 1);
+ }
+ }
+
+ $client_key = $this->getClientKey();
+
+ // Now, sum up the client's scores in all of the active buckets.
+ $score = 0;
+ for (; $cursor <= $cur; $cursor++) {
+ $bucket_key = $this->getBucketCacheKey($cursor);
+ if ($is_apcu) {
+ $bucket = apcu_fetch($bucket_key);
+ } else {
+ $bucket = apc_fetch($bucket_key);
+ }
+ if (isset($bucket[$client_key])) {
+ $score += $bucket[$client_key];
+ }
+ }
+
+ return $score;
+ }
+
+}
diff --git a/support/startup/PhabricatorClientRateLimit.php b/support/startup/PhabricatorClientRateLimit.php
new file mode 100644
--- /dev/null
+++ b/support/startup/PhabricatorClientRateLimit.php
@@ -0,0 +1,58 @@
+<?php
+
+final class PhabricatorClientRateLimit
+ extends PhabricatorClientLimit {
+
+ protected function getBucketDuration() {
+ return 60;
+ }
+
+ protected function getBucketCount() {
+ return 5;
+ }
+
+ protected function shouldRejectConnection($score) {
+ $limit = $this->getLimit();
+
+ // Reject connections if the average score across all buckets exceeds the
+ // limit.
+ $average_score = $score / $this->getBucketCount();
+
+ return ($average_score > $limit);
+ }
+
+ protected function getConnectScore() {
+ return 0;
+ }
+
+ protected function getPenaltyScore() {
+ return 1;
+ }
+
+ protected function getDisconnectScore(array $request_state) {
+ $score = 1;
+
+ // If the user was logged in, let them make more requests.
+ if (isset($request_state['viewer'])) {
+ $viewer = $request_state['viewer'];
+ if ($viewer->isLoggedIn()) {
+ $score = 0.25;
+ }
+ }
+
+ return $score;
+ }
+
+ protected function getRateLimitReason($score) {
+ $client_key = $this->getClientKey();
+
+ // NOTE: This happens before we load libraries, so we can not use pht()
+ // here.
+
+ return
+ "TOO MANY REQUESTS\n".
+ "You (\"{$client_key}\") are issuing too many requests ".
+ "too quickly.\n";
+ }
+
+}
diff --git a/support/PhabricatorStartup.php b/support/startup/PhabricatorStartup.php
rename from support/PhabricatorStartup.php
rename to support/startup/PhabricatorStartup.php
--- a/support/PhabricatorStartup.php
+++ b/support/startup/PhabricatorStartup.php
@@ -46,11 +46,7 @@
private static $oldMemoryLimit;
private static $phases;
- // TODO: For now, disable rate limiting entirely by default. We need to
- // iterate on it a bit for Conduit, some of the specific score levels, and
- // to deal with NAT'd offices.
- private static $maximumRate = 0;
- private static $rateLimitToken;
+ private static $limits = array();
/* -( Accessing Request Information )-------------------------------------- */
@@ -138,10 +134,7 @@
// we can switch over to relying on our own exception recovery mechanisms.
ini_set('display_errors', 0);
- $rate_token = self::getRateLimitToken();
- if ($rate_token !== null) {
- self::rateLimitRequest($rate_token);
- }
+ self::connectRateLimits();
self::normalizeInput();
@@ -193,7 +186,7 @@
}
public static function loadCoreLibraries() {
- $phabricator_root = dirname(dirname(__FILE__));
+ $phabricator_root = dirname(dirname(dirname(__FILE__)));
$libraries_root = dirname($phabricator_root);
$root = null;
@@ -683,269 +676,66 @@
/**
- * Adjust the permissible rate limit score.
- *
- * By default, the limit is `1000`. You can use this method to set it to
- * a larger or smaller value. If you set it to `2000`, users may make twice
- * as many requests before rate limiting.
- *
- * @param int Maximum score before rate limiting.
- * @return void
- * @task ratelimit
- */
- public static function setMaximumRate($rate) {
- self::$maximumRate = $rate;
- }
-
-
- /**
- * Set a token to identify the client for purposes of rate limiting.
- *
- * By default, the `REMOTE_ADDR` is used. If your install is behind a load
- * balancer, you may want to parse `X-Forwarded-For` and use that address
- * instead.
- *
- * @param string Client identity for rate limiting.
- */
- public static function setRateLimitToken($token) {
- self::$rateLimitToken = $token;
- }
-
-
- /**
- * Get the current client identity for rate limiting.
- */
- public static function getRateLimitToken() {
- if (self::$rateLimitToken !== null) {
- return self::$rateLimitToken;
- }
-
- if (isset($_SERVER['REMOTE_ADDR'])) {
- return $_SERVER['REMOTE_ADDR'];
- }
-
- return null;
- }
-
-
- /**
- * Check if the user (identified by `$user_identity`) has issued too many
- * requests recently. If they have, end the request with a 429 error code.
- *
- * The key just needs to identify the user. Phabricator uses both user PHIDs
- * and user IPs as keys, tracking logged-in and logged-out users separately
- * and enforcing different limits.
+ * Add a new client limits.
*
- * @param string Some key which identifies the user making the request.
- * @return void If the user has exceeded the rate limit, this method
- * does not return.
- * @task ratelimit
+ * @param PhabricatorClientLimit New limit.
+ * @return PhabricatorClientLimit The limit.
*/
- public static function rateLimitRequest($user_identity) {
- if (!self::canRateLimit()) {
- return;
- }
-
- $score = self::getRateLimitScore($user_identity);
- $limit = self::$maximumRate * self::getRateLimitBucketCount();
- if ($score > $limit) {
- // Give the user some bonus points for getting rate limited. This keeps
- // bad actors who keep slamming the 429 page locked out completely,
- // instead of letting them get a burst of requests through every minute
- // after a bucket expires.
- $penalty = 50;
-
- self::addRateLimitScore($user_identity, $penalty);
- $score += $penalty;
-
- self::didRateLimit($user_identity, $score, $limit);
- }
+ public static function addRateLimit(PhabricatorClientLimit $limit) {
+ self::$limits[] = $limit;
+ return $limit;
}
/**
- * Add points to the rate limit score for some user.
+ * Apply configured rate limits.
*
- * If users have earned more than 1000 points per minute across all the
- * buckets they'll be locked out of the application, so awarding 1 point per
- * request roughly corresponds to allowing 1000 requests per second, while
- * awarding 50 points roughly corresponds to allowing 20 requests per second.
+ * If any limit is exceeded, this method terminates the request.
*
- * @param string Some key which identifies the user making the request.
- * @param float The cost for this request; more points pushes them toward
- * the limit faster.
* @return void
* @task ratelimit
*/
- public static function addRateLimitScore($user_identity, $score) {
- if (!self::canRateLimit()) {
- return;
- }
-
- $is_apcu = (bool)function_exists('apcu_fetch');
- $current = self::getRateLimitBucket();
-
- // There's a bit of a race here, if a second process reads the bucket
- // before this one writes it, but it's fine if we occasionally fail to
- // record a user's score. If they're making requests fast enough to hit
- // rate limiting, we'll get them soon enough.
+ private static function connectRateLimits() {
+ $limits = self::$limits;
- $bucket_key = self::getRateLimitBucketKey($current);
- if ($is_apcu) {
- $bucket = apcu_fetch($bucket_key);
- } else {
- $bucket = apc_fetch($bucket_key);
- }
-
- if (!is_array($bucket)) {
- $bucket = array();
- }
-
- if (empty($bucket[$user_identity])) {
- $bucket[$user_identity] = 0;
+ $reason = null;
+ $connected = array();
+ foreach ($limits as $limit) {
+ $reason = $limit->didConnect();
+ $connected[] = $limit;
+ if ($reason !== null) {
+ break;
+ }
}
- $bucket[$user_identity] += $score;
+ // If we're killing the request here, disconnect any limits that we
+ // connected to try to keep the accounting straight.
+ if ($reason !== null) {
+ foreach ($connected as $limit) {
+ $limit->didDisconnect(array());
+ }
- if ($is_apcu) {
- apcu_store($bucket_key, $bucket);
- } else {
- apc_store($bucket_key, $bucket);
+ self::didRateLimit($reason);
}
}
/**
- * Determine if rate limiting is available.
- *
- * Rate limiting depends on APC, and isn't available unless the APC user
- * cache is available.
+ * Tear down rate limiting and allow limits to score the request.
*
- * @return bool True if rate limiting is available.
+ * @param map<string, wild> Additional, freeform request state.
+ * @return void
* @task ratelimit
*/
- private static function canRateLimit() {
+ public static function disconnectRateLimits(array $request_state) {
+ $limits = self::$limits;
- if (!self::$maximumRate) {
- return false;
+ foreach ($limits as $limit) {
+ $limit->didDisconnect($request_state);
}
-
- if (!function_exists('apc_fetch') && !function_exists('apcu_fetch')) {
- return false;
- }
-
- return true;
}
- /**
- * Get the current bucket for storing rate limit scores.
- *
- * @return int The current bucket.
- * @task ratelimit
- */
- private static function getRateLimitBucket() {
- return (int)(time() / 60);
- }
-
-
- /**
- * Get the total number of rate limit buckets to retain.
- *
- * @return int Total number of rate limit buckets to retain.
- * @task ratelimit
- */
- private static function getRateLimitBucketCount() {
- return 5;
- }
-
-
- /**
- * Get the APC key for a given bucket.
- *
- * @param int Bucket to get the key for.
- * @return string APC key for the bucket.
- * @task ratelimit
- */
- private static function getRateLimitBucketKey($bucket) {
- return 'rate:bucket:'.$bucket;
- }
-
-
- /**
- * Get the APC key for the smallest stored bucket.
- *
- * @return string APC key for the smallest stored bucket.
- * @task ratelimit
- */
- private static function getRateLimitMinKey() {
- return 'rate:min';
- }
-
-
- /**
- * Get the current rate limit score for a given user.
- *
- * @param string Unique key identifying the user.
- * @return float The user's current score.
- * @task ratelimit
- */
- private static function getRateLimitScore($user_identity) {
- $is_apcu = (bool)function_exists('apcu_fetch');
-
- $min_key = self::getRateLimitMinKey();
-
- // Identify the oldest bucket stored in APC.
- $cur = self::getRateLimitBucket();
- if ($is_apcu) {
- $min = apcu_fetch($min_key);
- } else {
- $min = apc_fetch($min_key);
- }
-
- // If we don't have any buckets stored yet, store the current bucket as
- // the oldest bucket.
- if (!$min) {
- if ($is_apcu) {
- apcu_store($min_key, $cur);
- } else {
- apc_store($min_key, $cur);
- }
- $min = $cur;
- }
-
- // Destroy any buckets that are older than the minimum bucket we're keeping
- // track of. Under load this normally shouldn't do anything, but will clean
- // up an old bucket once per minute.
- $count = self::getRateLimitBucketCount();
- for ($cursor = $min; $cursor < ($cur - $count); $cursor++) {
- $bucket_key = self::getRateLimitBucketKey($cursor);
- if ($is_apcu) {
- apcu_delete($bucket_key);
- apcu_store($min_key, $cursor + 1);
- } else {
- apc_delete($bucket_key);
- apc_store($min_key, $cursor + 1);
- }
- }
-
- // Now, sum up the user's scores in all of the active buckets.
- $score = 0;
- for (; $cursor <= $cur; $cursor++) {
- $bucket_key = self::getRateLimitBucketKey($cursor);
- if ($is_apcu) {
- $bucket = apcu_fetch($bucket_key);
- } else {
- $bucket = apc_fetch($bucket_key);
- }
- if (isset($bucket[$user_identity])) {
- $score += $bucket[$user_identity];
- }
- }
-
- return $score;
- }
-
/**
* Emit an HTTP 429 "Too Many Requests" response (indicating that the user
@@ -954,18 +744,13 @@
* @return exit This method **does not return**.
* @task ratelimit
*/
- private static function didRateLimit($user_identity, $score, $limit) {
- $message =
- "TOO MANY REQUESTS\n".
- "You (\"{$user_identity}\") are issuing too many requests ".
- "too quickly.\n";
-
+ private static function didRateLimit($reason) {
header(
'Content-Type: text/plain; charset=utf-8',
$replace = true,
$http_error = 429);
- echo $message;
+ echo $reason;
exit(1);
}
diff --git a/webroot/index.php b/webroot/index.php
--- a/webroot/index.php
+++ b/webroot/index.php
@@ -37,7 +37,12 @@
// Load the PhabricatorStartup class itself.
$t_startup = microtime(true);
$root = dirname(dirname(__FILE__));
- require_once $root.'/support/PhabricatorStartup.php';
+ require_once $root.'/support/startup/PhabricatorStartup.php';
+
+ // Load client limit classes so the preamble can configure limits.
+ require_once $root.'/support/startup/PhabricatorClientLimit.php';
+ require_once $root.'/support/startup/PhabricatorClientRateLimit.php';
+ require_once $root.'/support/startup/PhabricatorClientConnectionLimit.php';
// If the preamble script exists, load it.
$t_preamble = microtime(true);

File Metadata

Mime Type
text/plain
Expires
Tue, May 21, 2:53 PM (1 w, 5 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6287413
Default Alt Text
D18703.diff (23 KB)

Event Timeline