Add semi-generic rate limiting infrastructure
Summary:
This adds a system which basically keeps a record of recent actions, who took them, and how many "points" they were worth, like:
  epriestley email.add 1 1233989813
  epriestley email.add 1 1234298239
  epriestley email.add 1 1238293981
We can use this to rate-limit actions by examining how many actions the user has taken in the past hour (i.e., their total score) and comparing that to an allowed limit.
One major thing I want to use this for is to limit the amount of error email we'll send to an email address. A big concern I have with sending more error email is that we'll end up in loops. We have some protections against this in headers already, but hard-limiting the system so it won't send more than a few errors to a particular address per hour should provide a reasonable secondary layer of protection.
This use case (where the "actor" needs to be an email address) is why the table uses strings + hashes instead of PHIDs. For external users, it might be appropriate to rate limit by cookies or IPs, too.
To prove it works, I rate limited adding email addresses. This is a very, very low-risk security thing where a user with an account can enumerate addresses (by checking if they get an error) and sort of spam/annoy people (by adding their address over and over again). Limiting them to 6 actions / hour should satisfy all real users while preventing these behaviors.
Test Plan:
This dialog is uggos but I'll fix that in a sec:
{F137406}
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D8683
			
			
This commit is contained in:
		
							
								
								
									
										12
									
								
								resources/sql/autopatches/20140402.actionlog.sql
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										12
									
								
								resources/sql/autopatches/20140402.actionlog.sql
									
									
									
									
									
										Normal file
									
								
							@@ -0,0 +1,12 @@
 | 
			
		||||
CREATE TABLE {$NAMESPACE}_system.system_actionlog (
 | 
			
		||||
  id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY,
 | 
			
		||||
  actorHash CHAR(12) NOT NULL COLLATE latin1_bin,
 | 
			
		||||
  actorIdentity VARCHAR(255) NOT NULL COLLATE utf8_bin,
 | 
			
		||||
  action CHAR(32) NOT NULL COLLATE utf8_bin,
 | 
			
		||||
  score DOUBLE NOT NULL,
 | 
			
		||||
  epoch INT UNSIGNED NOT NULL,
 | 
			
		||||
 | 
			
		||||
  KEY `key_epoch` (epoch),
 | 
			
		||||
  KEY `key_action` (actorHash, action, epoch)
 | 
			
		||||
 | 
			
		||||
) ENGINE=InnoDB, COLLATE utf8_general_ci;
 | 
			
		||||
@@ -2043,6 +2043,7 @@ phutil_register_library_map(array(
 | 
			
		||||
    'PhabricatorSearchWorker' => 'applications/search/worker/PhabricatorSearchWorker.php',
 | 
			
		||||
    'PhabricatorSecurityConfigOptions' => 'applications/config/option/PhabricatorSecurityConfigOptions.php',
 | 
			
		||||
    'PhabricatorSendGridConfigOptions' => 'applications/config/option/PhabricatorSendGridConfigOptions.php',
 | 
			
		||||
    'PhabricatorSettingsAddEmailAction' => 'applications/settings/action/PhabricatorSettingsAddEmailAction.php',
 | 
			
		||||
    'PhabricatorSettingsAdjustController' => 'applications/settings/controller/PhabricatorSettingsAdjustController.php',
 | 
			
		||||
    'PhabricatorSettingsMainController' => 'applications/settings/controller/PhabricatorSettingsMainController.php',
 | 
			
		||||
    'PhabricatorSettingsPanel' => 'applications/settings/panel/PhabricatorSettingsPanel.php',
 | 
			
		||||
@@ -2142,6 +2143,12 @@ phutil_register_library_map(array(
 | 
			
		||||
    'PhabricatorSymbolNameLinter' => 'infrastructure/lint/hook/PhabricatorSymbolNameLinter.php',
 | 
			
		||||
    'PhabricatorSyntaxHighlighter' => 'infrastructure/markup/PhabricatorSyntaxHighlighter.php',
 | 
			
		||||
    'PhabricatorSyntaxHighlightingConfigOptions' => 'applications/config/option/PhabricatorSyntaxHighlightingConfigOptions.php',
 | 
			
		||||
    'PhabricatorSystemAction' => 'applications/system/action/PhabricatorSystemAction.php',
 | 
			
		||||
    'PhabricatorSystemActionEngine' => 'applications/system/engine/PhabricatorSystemActionEngine.php',
 | 
			
		||||
    'PhabricatorSystemActionGarbageCollector' => 'applications/system/garbagecollector/PhabricatorSystemActionGarbageCollector.php',
 | 
			
		||||
    'PhabricatorSystemActionLog' => 'applications/system/storage/PhabricatorSystemActionLog.php',
 | 
			
		||||
    'PhabricatorSystemActionRateLimitException' => 'applications/system/exception/PhabricatorSystemActionRateLimitException.php',
 | 
			
		||||
    'PhabricatorSystemDAO' => 'applications/system/storage/PhabricatorSystemDAO.php',
 | 
			
		||||
    'PhabricatorTaskmasterDaemon' => 'infrastructure/daemon/workers/PhabricatorTaskmasterDaemon.php',
 | 
			
		||||
    'PhabricatorTestCase' => 'infrastructure/testing/PhabricatorTestCase.php',
 | 
			
		||||
    'PhabricatorTestController' => 'applications/base/controller/__tests__/PhabricatorTestController.php',
 | 
			
		||||
@@ -4906,6 +4913,7 @@ phutil_register_library_map(array(
 | 
			
		||||
    'PhabricatorSearchWorker' => 'PhabricatorWorker',
 | 
			
		||||
    'PhabricatorSecurityConfigOptions' => 'PhabricatorApplicationConfigOptions',
 | 
			
		||||
    'PhabricatorSendGridConfigOptions' => 'PhabricatorApplicationConfigOptions',
 | 
			
		||||
    'PhabricatorSettingsAddEmailAction' => 'PhabricatorSystemAction',
 | 
			
		||||
    'PhabricatorSettingsAdjustController' => 'PhabricatorController',
 | 
			
		||||
    'PhabricatorSettingsMainController' => 'PhabricatorController',
 | 
			
		||||
    'PhabricatorSettingsPanelAccount' => 'PhabricatorSettingsPanel',
 | 
			
		||||
@@ -5006,6 +5014,11 @@ phutil_register_library_map(array(
 | 
			
		||||
    'PhabricatorSubscriptionsUIEventListener' => 'PhabricatorEventListener',
 | 
			
		||||
    'PhabricatorSymbolNameLinter' => 'ArcanistXHPASTLintNamingHook',
 | 
			
		||||
    'PhabricatorSyntaxHighlightingConfigOptions' => 'PhabricatorApplicationConfigOptions',
 | 
			
		||||
    'PhabricatorSystemActionEngine' => 'Phobject',
 | 
			
		||||
    'PhabricatorSystemActionGarbageCollector' => 'PhabricatorGarbageCollector',
 | 
			
		||||
    'PhabricatorSystemActionLog' => 'PhabricatorSystemDAO',
 | 
			
		||||
    'PhabricatorSystemActionRateLimitException' => 'Exception',
 | 
			
		||||
    'PhabricatorSystemDAO' => 'PhabricatorLiskDAO',
 | 
			
		||||
    'PhabricatorTaskmasterDaemon' => 'PhabricatorDaemon',
 | 
			
		||||
    'PhabricatorTestCase' => 'ArcanistPhutilTestCase',
 | 
			
		||||
    'PhabricatorTestController' => 'PhabricatorController',
 | 
			
		||||
 
 | 
			
		||||
@@ -111,6 +111,23 @@ class AphrontDefaultApplicationConfiguration
 | 
			
		||||
      $user = new PhabricatorUser();
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    if ($ex instanceof PhabricatorSystemActionRateLimitException) {
 | 
			
		||||
      $error_view = id(new AphrontErrorView())
 | 
			
		||||
        ->setErrors(array(pht('You are being rate limited.')));
 | 
			
		||||
 | 
			
		||||
      $dialog = id(new AphrontDialogView())
 | 
			
		||||
        ->setTitle(pht('Slow Down!'))
 | 
			
		||||
        ->setUser($user)
 | 
			
		||||
        ->appendChild($error_view)
 | 
			
		||||
        ->appendParagraph($ex->getMessage())
 | 
			
		||||
        ->appendParagraph($ex->getRateExplanation())
 | 
			
		||||
        ->addCancelButton('/', pht('Okaaaaaaaaaaaaaay...'));
 | 
			
		||||
 | 
			
		||||
      $response = new AphrontDialogResponse();
 | 
			
		||||
      $response->setDialog($dialog);
 | 
			
		||||
      return $response;
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    if ($ex instanceof PhabricatorPolicyException) {
 | 
			
		||||
 | 
			
		||||
      if (!$user->isLoggedIn()) {
 | 
			
		||||
 
 | 
			
		||||
@@ -0,0 +1,20 @@
 | 
			
		||||
<?php
 | 
			
		||||
 | 
			
		||||
final class PhabricatorSettingsAddEmailAction extends PhabricatorSystemAction {
 | 
			
		||||
 | 
			
		||||
  const TYPECONST = 'email.add';
 | 
			
		||||
 | 
			
		||||
  public function getActionConstant() {
 | 
			
		||||
    return self::TYPECONST;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public function getScoreThreshold() {
 | 
			
		||||
    return 6 / phutil_units('1 hour in seconds');
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public function getLimitExplanation() {
 | 
			
		||||
    return pht(
 | 
			
		||||
      'You are adding too many email addresses to your account too quickly.');
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
}
 | 
			
		||||
@@ -171,6 +171,11 @@ final class PhabricatorSettingsPanelEmailAddresses
 | 
			
		||||
        return id(new AphrontReloadResponse())->setURI($uri);
 | 
			
		||||
      }
 | 
			
		||||
 | 
			
		||||
      PhabricatorSystemActionEngine::willTakeAction(
 | 
			
		||||
        array($user->getPHID()),
 | 
			
		||||
        new PhabricatorSettingsAddEmailAction(),
 | 
			
		||||
        1);
 | 
			
		||||
 | 
			
		||||
      if (!strlen($email)) {
 | 
			
		||||
        $e_email = pht('Required');
 | 
			
		||||
        $errors[] = pht('Email is required.');
 | 
			
		||||
 
 | 
			
		||||
							
								
								
									
										40
									
								
								src/applications/system/action/PhabricatorSystemAction.php
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										40
									
								
								src/applications/system/action/PhabricatorSystemAction.php
									
									
									
									
									
										Normal file
									
								
							@@ -0,0 +1,40 @@
 | 
			
		||||
<?php
 | 
			
		||||
 | 
			
		||||
abstract class PhabricatorSystemAction {
 | 
			
		||||
 | 
			
		||||
  abstract public function getActionConstant();
 | 
			
		||||
  abstract public function getScoreThreshold();
 | 
			
		||||
 | 
			
		||||
  public function shouldBlockActor($actor, $score) {
 | 
			
		||||
    return ($score > $this->getScoreThreshold());
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public function getLimitExplanation() {
 | 
			
		||||
    return pht('You are performing too many actions too quickly.');
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public function getRateExplanation($score) {
 | 
			
		||||
    return pht(
 | 
			
		||||
      'The maximum allowed rate for this action is %s. You are taking '.
 | 
			
		||||
      'actions at a rate of %s.',
 | 
			
		||||
      $this->formatRate($this->getScoreThreshold()),
 | 
			
		||||
      $this->formatRate($score));
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  protected function formatRate($rate) {
 | 
			
		||||
    if ($rate > 10) {
 | 
			
		||||
      $str = pht('%d / second', $rate);
 | 
			
		||||
    } else {
 | 
			
		||||
      $rate *= 60;
 | 
			
		||||
      if ($rate > 10) {
 | 
			
		||||
        $str = pht('%d / minute', $rate);
 | 
			
		||||
      } else {
 | 
			
		||||
        $rate *= 60;
 | 
			
		||||
        $str = pht('%d / hour', $rate);
 | 
			
		||||
      }
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    return phutil_tag('strong', array(), $str);
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
}
 | 
			
		||||
							
								
								
									
										119
									
								
								src/applications/system/engine/PhabricatorSystemActionEngine.php
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										119
									
								
								src/applications/system/engine/PhabricatorSystemActionEngine.php
									
									
									
									
									
										Normal file
									
								
							@@ -0,0 +1,119 @@
 | 
			
		||||
<?php
 | 
			
		||||
 | 
			
		||||
final class PhabricatorSystemActionEngine extends Phobject {
 | 
			
		||||
 | 
			
		||||
  public static function willTakeAction(
 | 
			
		||||
    array $actors,
 | 
			
		||||
    PhabricatorSystemAction $action,
 | 
			
		||||
    $score) {
 | 
			
		||||
 | 
			
		||||
    // If the score for this action is negative, we're giving the user a credit,
 | 
			
		||||
    // so don't bother checking if they're blocked or not.
 | 
			
		||||
    if ($score >= 0) {
 | 
			
		||||
      $blocked = self::loadBlockedActors($actors, $action, $score);
 | 
			
		||||
      if ($blocked) {
 | 
			
		||||
        foreach ($blocked as $actor => $actor_score) {
 | 
			
		||||
          throw new PhabricatorSystemActionRateLimitException(
 | 
			
		||||
            $action,
 | 
			
		||||
            $actor_score + ($score / self::getWindow()));
 | 
			
		||||
        }
 | 
			
		||||
      }
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    self::recordAction($actors, $action, $score);
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public static function loadBlockedActors(
 | 
			
		||||
    array $actors,
 | 
			
		||||
    PhabricatorSystemAction $action) {
 | 
			
		||||
 | 
			
		||||
    $scores = self::loadScores($actors, $action);
 | 
			
		||||
 | 
			
		||||
    $blocked = array();
 | 
			
		||||
    foreach ($scores as $actor => $score) {
 | 
			
		||||
      if ($action->shouldBlockActor($actor, $score)) {
 | 
			
		||||
        $blocked[$actor] = $score;
 | 
			
		||||
      }
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    return $blocked;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public static function loadScores(
 | 
			
		||||
    array $actors,
 | 
			
		||||
    PhabricatorSystemAction $action) {
 | 
			
		||||
 | 
			
		||||
    if (!$actors) {
 | 
			
		||||
      return array();
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    $actor_hashes = array();
 | 
			
		||||
    foreach ($actors as $actor) {
 | 
			
		||||
      $actor_hashes[] = PhabricatorHash::digestForIndex($actor);
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    $log = new PhabricatorSystemActionLog();
 | 
			
		||||
 | 
			
		||||
    $window = self::getWindow();
 | 
			
		||||
 | 
			
		||||
    $conn_r = $log->establishConnection('r');
 | 
			
		||||
    $scores = queryfx_all(
 | 
			
		||||
      $conn_r,
 | 
			
		||||
      'SELECT actorIdentity, SUM(score) totalScore FROM %T
 | 
			
		||||
        WHERE action = %s AND actorHash IN (%Ls)
 | 
			
		||||
          AND epoch >= %d GROUP BY actorHash',
 | 
			
		||||
      $log->getTableName(),
 | 
			
		||||
      $action->getActionConstant(),
 | 
			
		||||
      $actor_hashes,
 | 
			
		||||
      (time() - $window));
 | 
			
		||||
 | 
			
		||||
    $scores = ipull($scores, 'totalScore', 'actorIdentity');
 | 
			
		||||
 | 
			
		||||
    foreach ($scores as $key => $score) {
 | 
			
		||||
      $scores[$key] = $score / $window;
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    $scores = $scores + array_fill_keys($actors, 0);
 | 
			
		||||
 | 
			
		||||
    return $scores;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  private static function recordAction(
 | 
			
		||||
    array $actors,
 | 
			
		||||
    PhabricatorSystemAction $action,
 | 
			
		||||
    $score) {
 | 
			
		||||
 | 
			
		||||
    $log = new PhabricatorSystemActionLog();
 | 
			
		||||
    $conn_w = $log->establishConnection('w');
 | 
			
		||||
 | 
			
		||||
    $sql = array();
 | 
			
		||||
    foreach ($actors as $actor) {
 | 
			
		||||
      $sql[] = qsprintf(
 | 
			
		||||
        $conn_w,
 | 
			
		||||
        '(%s, %s, %s, %f, %d)',
 | 
			
		||||
        PhabricatorHash::digestForIndex($actor),
 | 
			
		||||
        $actor,
 | 
			
		||||
        $action->getActionConstant(),
 | 
			
		||||
        $score,
 | 
			
		||||
        time());
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    foreach (PhabricatorLiskDAO::chunkSQL($sql) as $chunk) {
 | 
			
		||||
      queryfx(
 | 
			
		||||
        $conn_w,
 | 
			
		||||
        'INSERT INTO %T (actorHash, actorIdentity, action, score, epoch)
 | 
			
		||||
          VALUES %Q',
 | 
			
		||||
        $log->getTableName(),
 | 
			
		||||
        $chunk);
 | 
			
		||||
    }
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  private static function getWindow() {
 | 
			
		||||
    // Limit queries to the last hour of data so we don't need to look at as
 | 
			
		||||
    // many rows. We can use an arbitrarily larger window instead (we normalize
 | 
			
		||||
    // scores to actions per second) but all the actions we care about limiting
 | 
			
		||||
    // have a limit much higher than one action per hour.
 | 
			
		||||
    return phutil_units('1 hour in seconds');
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
}
 | 
			
		||||
@@ -0,0 +1,18 @@
 | 
			
		||||
<?php
 | 
			
		||||
 | 
			
		||||
final class PhabricatorSystemActionRateLimitException extends Exception {
 | 
			
		||||
 | 
			
		||||
  private $action;
 | 
			
		||||
  private $score;
 | 
			
		||||
 | 
			
		||||
  public function __construct(PhabricatorSystemAction $action, $score) {
 | 
			
		||||
    $this->action = $action;
 | 
			
		||||
    $this->score = $score;
 | 
			
		||||
    parent::__construct($action->getLimitExplanation());
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public function getRateExplanation() {
 | 
			
		||||
    return $this->action->getRateExplanation($this->score);
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
}
 | 
			
		||||
@@ -0,0 +1,21 @@
 | 
			
		||||
<?php
 | 
			
		||||
 | 
			
		||||
final class PhabricatorSystemActionGarbageCollector
 | 
			
		||||
  extends PhabricatorGarbageCollector {
 | 
			
		||||
 | 
			
		||||
  public function collectGarbage() {
 | 
			
		||||
    $ttl = phutil_units('3 days in seconds');
 | 
			
		||||
 | 
			
		||||
    $table = new PhabricatorSystemActionLog();
 | 
			
		||||
    $conn_w = $table->establishConnection('w');
 | 
			
		||||
 | 
			
		||||
    queryfx(
 | 
			
		||||
      $conn_w,
 | 
			
		||||
      'DELETE FROM %T WHERE epoch < %d LIMIT 100',
 | 
			
		||||
      $table->getTableName(),
 | 
			
		||||
      time() - $ttl);
 | 
			
		||||
 | 
			
		||||
    return ($conn_w->getAffectedRows() == 100);
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
}
 | 
			
		||||
@@ -0,0 +1,22 @@
 | 
			
		||||
<?php
 | 
			
		||||
 | 
			
		||||
final class PhabricatorSystemActionLog extends PhabricatorSystemDAO {
 | 
			
		||||
 | 
			
		||||
  protected $actorHash;
 | 
			
		||||
  protected $actorIdentity;
 | 
			
		||||
  protected $action;
 | 
			
		||||
  protected $score;
 | 
			
		||||
  protected $epoch;
 | 
			
		||||
 | 
			
		||||
  public function getConfiguration() {
 | 
			
		||||
    return array(
 | 
			
		||||
      self::CONFIG_TIMESTAMPS => false,
 | 
			
		||||
    ) + parent::getConfiguration();
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public function setActorIdentity($identity) {
 | 
			
		||||
    $this->setActorHash(PhabricatorHash::digestForIndex($identity));
 | 
			
		||||
    return parent::setActorIdentity($identity);
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
}
 | 
			
		||||
							
								
								
									
										9
									
								
								src/applications/system/storage/PhabricatorSystemDAO.php
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										9
									
								
								src/applications/system/storage/PhabricatorSystemDAO.php
									
									
									
									
									
										Normal file
									
								
							@@ -0,0 +1,9 @@
 | 
			
		||||
<?php
 | 
			
		||||
 | 
			
		||||
abstract class PhabricatorSystemDAO extends PhabricatorLiskDAO {
 | 
			
		||||
 | 
			
		||||
  public function getApplicationName() {
 | 
			
		||||
    return 'system';
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
}
 | 
			
		||||
@@ -118,6 +118,7 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList {
 | 
			
		||||
      'db.passphrase' => array(),
 | 
			
		||||
      'db.phragment' => array(),
 | 
			
		||||
      'db.dashboard' => array(),
 | 
			
		||||
      'db.system' => array(),
 | 
			
		||||
      '0000.legacy.sql' => array(
 | 
			
		||||
        'legacy' => 0,
 | 
			
		||||
      ),
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user