1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-10 17:02:41 +01:00

Update rate limiting for APCu and X-Forwarded-For

Summary:
Ref T12612. This updates the rate limiting code to:

  - Support a customizable token, like the client's X-Forwarded-For address, rather than always using `REMOTE_ADDR`.
  - Support APCu.
  - Report a little more rate limiting information.
  - Not reference nonexistent documentation (removed in D16403).

I'm planning to put this into production on `secure` for now and then we can deploy it more broadly if things work well.

Test Plan:
 - Enabled it locally, used `ab -n 100` to hit the limit, saw the limit enforced.
 - Waited a while, was allowed to browse again.

Reviewers: chad, amckinley

Reviewed By: amckinley

Maniphest Tasks: T12612

Differential Revision: https://secure.phabricator.com/D17758
This commit is contained in:
epriestley 2017-04-21 12:41:53 -07:00
parent 03d0e3fdbc
commit 3698e4a14f
3 changed files with 94 additions and 27 deletions

View file

@ -205,9 +205,8 @@ abstract class AphrontApplicationConfiguration extends Phobject {
DarkConsoleXHProfPluginAPI::saveProfilerSample($access_log); DarkConsoleXHProfPluginAPI::saveProfilerSample($access_log);
// Add points to the rate limits for this request. // Add points to the rate limits for this request.
if (isset($_SERVER['REMOTE_ADDR'])) { $rate_token = PhabricatorStartup::getRateLimitToken();
$user_ip = $_SERVER['REMOTE_ADDR']; if ($rate_token !== null) {
// The base score for a request allows users to make 30 requests per // The base score for a request allows users to make 30 requests per
// minute. // minute.
$score = (1000 / 30); $score = (1000 / 30);
@ -217,7 +216,7 @@ abstract class AphrontApplicationConfiguration extends Phobject {
$score = $score / 5; $score = $score / 5;
} }
PhabricatorStartup::addRateLimitScore($user_ip, $score); PhabricatorStartup::addRateLimitScore($rate_token, $score);
} }
if ($processing_exception) { if ($processing_exception) {

View file

@ -1,8 +1,7 @@
@title Configuring a Preamble Script @title Configuring a Preamble Script
@group config @group config
Adjust environmental settings (SSL, remote IP, rate limiting) using a preamble Adjust environmental settings (SSL, remote IPs) using a preamble script.
script.
Overview Overview
======== ========

View file

@ -50,6 +50,7 @@ final class PhabricatorStartup {
// iterate on it a bit for Conduit, some of the specific score levels, and // iterate on it a bit for Conduit, some of the specific score levels, and
// to deal with NAT'd offices. // to deal with NAT'd offices.
private static $maximumRate = 0; private static $maximumRate = 0;
private static $rateLimitToken;
/* -( Accessing Request Information )-------------------------------------- */ /* -( Accessing Request Information )-------------------------------------- */
@ -137,8 +138,9 @@ final class PhabricatorStartup {
// we can switch over to relying on our own exception recovery mechanisms. // we can switch over to relying on our own exception recovery mechanisms.
ini_set('display_errors', 0); ini_set('display_errors', 0);
if (isset($_SERVER['REMOTE_ADDR'])) { $rate_token = self::getRateLimitToken();
self::rateLimitRequest($_SERVER['REMOTE_ADDR']); if ($rate_token !== null) {
self::rateLimitRequest($rate_token);
} }
self::normalizeInput(); self::normalizeInput();
@ -680,6 +682,36 @@ final class PhabricatorStartup {
} }
/**
* 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 * 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. * requests recently. If they have, end the request with a 429 error code.
@ -699,13 +731,18 @@ final class PhabricatorStartup {
} }
$score = self::getRateLimitScore($user_identity); $score = self::getRateLimitScore($user_identity);
if ($score > (self::$maximumRate * self::getRateLimitBucketCount())) { $limit = self::$maximumRate * self::getRateLimitBucketCount();
if ($score > $limit) {
// Give the user some bonus points for getting rate limited. This keeps // Give the user some bonus points for getting rate limited. This keeps
// bad actors who keep slamming the 429 page locked out completely, // bad actors who keep slamming the 429 page locked out completely,
// instead of letting them get a burst of requests through every minute // instead of letting them get a burst of requests through every minute
// after a bucket expires. // after a bucket expires.
self::addRateLimitScore($user_identity, 50); $penalty = 50;
self::didRateLimit($user_identity);
self::addRateLimitScore($user_identity, $penalty);
$score += $penalty;
self::didRateLimit($user_identity, $score, $limit);
} }
} }
@ -729,15 +766,21 @@ final class PhabricatorStartup {
return; return;
} }
$is_apcu = (bool)function_exists('apcu_fetch');
$current = self::getRateLimitBucket(); $current = self::getRateLimitBucket();
// There's a bit of a race here, if a second process reads the bucket before // There's a bit of a race here, if a second process reads the bucket
// this one writes it, but it's fine if we occasionally fail to record a // before this one writes it, but it's fine if we occasionally fail to
// user's score. If they're making requests fast enough to hit rate // record a user's score. If they're making requests fast enough to hit
// limiting, we'll get them soon. // rate limiting, we'll get them soon enough.
$bucket_key = self::getRateLimitBucketKey($current); $bucket_key = self::getRateLimitBucketKey($current);
$bucket = apc_fetch($bucket_key); if ($is_apcu) {
$bucket = apcu_fetch($bucket_key);
} else {
$bucket = apc_fetch($bucket_key);
}
if (!is_array($bucket)) { if (!is_array($bucket)) {
$bucket = array(); $bucket = array();
} }
@ -747,7 +790,12 @@ final class PhabricatorStartup {
} }
$bucket[$user_identity] += $score; $bucket[$user_identity] += $score;
apc_store($bucket_key, $bucket);
if ($is_apcu) {
apcu_store($bucket_key, $bucket);
} else {
apc_store($bucket_key, $bucket);
}
} }
@ -761,11 +809,12 @@ final class PhabricatorStartup {
* @task ratelimit * @task ratelimit
*/ */
private static function canRateLimit() { private static function canRateLimit() {
if (!self::$maximumRate) { if (!self::$maximumRate) {
return false; return false;
} }
if (!function_exists('apc_fetch')) { if (!function_exists('apc_fetch') && !function_exists('apcu_fetch')) {
return false; return false;
} }
@ -826,16 +875,26 @@ final class PhabricatorStartup {
* @task ratelimit * @task ratelimit
*/ */
private static function getRateLimitScore($user_identity) { private static function getRateLimitScore($user_identity) {
$is_apcu = (bool)function_exists('apcu_fetch');
$min_key = self::getRateLimitMinKey(); $min_key = self::getRateLimitMinKey();
// Identify the oldest bucket stored in APC. // Identify the oldest bucket stored in APC.
$cur = self::getRateLimitBucket(); $cur = self::getRateLimitBucket();
$min = apc_fetch($min_key); 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 // If we don't have any buckets stored yet, store the current bucket as
// the oldest bucket. // the oldest bucket.
if (!$min) { if (!$min) {
apc_store($min_key, $cur); if ($is_apcu) {
apcu_store($min_key, $cur);
} else {
apc_store($min_key, $cur);
}
$min = $cur; $min = $cur;
} }
@ -844,14 +903,25 @@ final class PhabricatorStartup {
// up an old bucket once per minute. // up an old bucket once per minute.
$count = self::getRateLimitBucketCount(); $count = self::getRateLimitBucketCount();
for ($cursor = $min; $cursor < ($cur - $count); $cursor++) { for ($cursor = $min; $cursor < ($cur - $count); $cursor++) {
apc_delete(self::getRateLimitBucketKey($cursor)); $bucket_key = self::getRateLimitBucketKey($cursor);
apc_store($min_key, $cursor + 1); 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. // Now, sum up the user's scores in all of the active buckets.
$score = 0; $score = 0;
for (; $cursor <= $cur; $cursor++) { for (; $cursor <= $cur; $cursor++) {
$bucket = apc_fetch(self::getRateLimitBucketKey($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])) { if (isset($bucket[$user_identity])) {
$score += $bucket[$user_identity]; $score += $bucket[$user_identity];
} }
@ -868,12 +938,11 @@ final class PhabricatorStartup {
* @return exit This method **does not return**. * @return exit This method **does not return**.
* @task ratelimit * @task ratelimit
*/ */
private static function didRateLimit() { private static function didRateLimit($user_identity, $score, $limit) {
$message = $message =
"TOO MANY REQUESTS\n". "TOO MANY REQUESTS\n".
"You are issuing too many requests too quickly.\n". "You (\"{$user_identity}\") are issuing too many requests ".
"To adjust limits, see \"Configuring a Preamble Script\" in the ". "too quickly.\n";
"documentation.";
header( header(
'Content-Type: text/plain; charset=utf-8', 'Content-Type: text/plain; charset=utf-8',