1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-23 22:10:55 +01:00

Proof of concept mitigation of BREACH

Summary: Ref T3684 for discussion. This could be cleaned up a bit (it would be nice to draw entropy once per request, for instance, and maybe respect CSRF_TOKEN_LENGTH more closely) but should effectively mitigate BREACH.

Test Plan: Submitted forms; submitted forms after mucking with CSRF and observed CSRF error. Verified that source now has "B@..." tokens.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T3684

Differential Revision: https://secure.phabricator.com/D6686
This commit is contained in:
epriestley 2013-08-07 16:09:05 -07:00
parent ab7a091212
commit 7298589c86
3 changed files with 58 additions and 7 deletions

View file

@ -152,12 +152,14 @@ final class PhabricatorUser
}
const CSRF_CYCLE_FREQUENCY = 3600;
const CSRF_SALT_LENGTH = 8;
const CSRF_TOKEN_LENGTH = 16;
const CSRF_BREACH_PREFIX = 'B@';
const EMAIL_CYCLE_FREQUENCY = 86400;
const EMAIL_TOKEN_LENGTH = 24;
public function getCSRFToken($offset = 0) {
private function getRawCSRFToken($offset = 0) {
return $this->generateToken(
time() + (self::CSRF_CYCLE_FREQUENCY * $offset),
self::CSRF_CYCLE_FREQUENCY,
@ -165,12 +167,42 @@ final class PhabricatorUser
self::CSRF_TOKEN_LENGTH);
}
public function validateCSRFToken($token) {
/**
* @phutil-external-symbol class PhabricatorStartup
*/
public function getCSRFToken() {
$salt = PhabricatorStartup::getGlobal('csrf.salt');
if (!$salt) {
$salt = Filesystem::readRandomCharacters(self::CSRF_SALT_LENGTH);
PhabricatorStartup::setGlobal('csrf.salt', $salt);
}
// Generate a token hash to mitigate BREACH attacks against SSL. See
// discussion in T3684.
$token = $this->getRawCSRFToken();
$hash = PhabricatorHash::digest($token, $salt);
return 'B@'.$salt.substr($hash, 0, self::CSRF_TOKEN_LENGTH);
}
public function validateCSRFToken($token) {
if (!$this->getPHID()) {
return true;
}
$salt = null;
$version = 'plain';
// This is a BREACH-mitigating token. See T3684.
$breach_prefix = self::CSRF_BREACH_PREFIX;
$breach_prelen = strlen($breach_prefix);
if (!strncmp($token, $breach_prefix, $breach_prelen)) {
$version = 'breach';
$salt = substr($token, $breach_prelen, self::CSRF_SALT_LENGTH);
$token = substr($token, $breach_prelen + self::CSRF_SALT_LENGTH);
}
// When the user posts a form, we check that it contains a valid CSRF token.
// Tokens cycle each hour (every CSRF_CYLCE_FREQUENCY seconds) and we accept
// either the current token, the next token (users can submit a "future"
@ -199,9 +231,23 @@ final class PhabricatorUser
$csrf_window = 6;
for ($ii = -$csrf_window; $ii <= 1; $ii++) {
$valid = $this->getCSRFToken($ii);
if ($token == $valid) {
return true;
$valid = $this->getRawCSRFToken($ii);
switch ($version) {
// TODO: We can remove this after the BREACH version has been in the
// wild for a while.
case 'plain':
if ($token == $valid) {
return true;
}
break;
case 'breach':
$digest = PhabricatorHash::digest($valid, $salt);
if (substr($digest, 0, self::CSRF_TOKEN_LENGTH) == $token) {
return true;
}
break;
default:
throw new Exception("Unknown CSRF token format!");
}
}

View file

@ -9,8 +9,11 @@ final class PhabricatorHash {
* @param string Input string.
* @return string 32-byte hexidecimal SHA1+HMAC hash.
*/
public static function digest($string) {
$key = PhabricatorEnv::getEnvConfig('security.hmac-key');
public static function digest($string, $key = null) {
if ($key === null) {
$key = PhabricatorEnv::getEnvConfig('security.hmac-key');
}
if (!$key) {
throw new Exception(
"Set a 'security.hmac-key' in your Phabricator configuration!");

View file

@ -59,6 +59,7 @@ final class PhabricatorStartup {
if (!array_key_exists($key, self::$globals)) {
return $default;
}
return self::$globals[$key];
}
@ -370,6 +371,7 @@ final class PhabricatorStartup {
private static function validateGlobal($key) {
static $globals = array(
'log.access' => true,
'csrf.salt' => true,
);
if (empty($globals[$key])) {