revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-18 21:02:41 +01:00
Code Issues Releases Wiki Activity

Make CSRF salt per-user instead of per-request

Browse source 
Summary:
Fixes T8326. This removes calls to PhabricatorStartup from places that daemons may access.

This salt doesn't need to be global; it's embedded in the token we return. It's fine if we use a different salt every time. In practice, we always use the same viewer, so this change causes little or no behavioral change.

Ref T8424. For Spaces, I need a per-request cache for all spaces, because they have unusual access patterns and require repeated access, in some cases by multiple viewers.

We don't currently have a per-request in-process cache that we, e.g., clear in the daemons.

We do have a weak/theoretical/forward-looking attempt at this in `PhabricatorStartup::getGlobal()` but I'm going to throw that away (it's kind of junky, partly because of T8326) and replace it with a more formal mechanism.

Test Plan:
  - Submitted some forms.
  - Grepped for `csrf.salt`.
  - Viewed page source, saw nice CSRF tokens with salt.
  - All the salts are still the same on every page I checked, but it doesn't matter if this isn't true everywhere.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8326, T8424

Differential Revision: https://secure.phabricator.com/D13151
This commit is contained in:
epriestley 2015-06-04 17:26:23 -07:00
parent b9d004e9c4
commit e5b923743a
1 changed files with 6 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

13
src/applications/people/storage/PhabricatorUser.php
View file

@ -59,6 +59,7 @@ final class PhabricatorUser
private $authorities = array(); private $authorities = array();
private $handlePool; private $handlePool;
private $csrfSalt;
protected function readField($field) { protected function readField($field) {
switch ($field) { switch ($field) {
@ -342,16 +343,14 @@ final class PhabricatorUser
self::CSRF_TOKEN_LENGTH); self::CSRF_TOKEN_LENGTH);
} }
/**
* @phutil-external-symbol class PhabricatorStartup
*/
public function getCSRFToken() { public function getCSRFToken() {
$salt = PhabricatorStartup::getGlobal('csrf.salt'); if ($this->csrfSalt === null) {
if (!$salt) { $this->csrfSalt = Filesystem::readRandomCharacters(
$salt = Filesystem::readRandomCharacters(self::CSRF_SALT_LENGTH); self::CSRF_SALT_LENGTH);
PhabricatorStartup::setGlobal('csrf.salt', $salt);
} }
$salt = $this->csrfSalt;
// Generate a token hash to mitigate BREACH attacks against SSL. See // Generate a token hash to mitigate BREACH attacks against SSL. See
// discussion in T3684. // discussion in T3684.
$token = $this->getRawCSRFToken(); $token = $this->getRawCSRFToken();