diff --git a/conf/default.conf.php b/conf/default.conf.php index 078c4e128d..bbc0a31906 100644 --- a/conf/default.conf.php +++ b/conf/default.conf.php @@ -65,6 +65,12 @@ return array( // attack difficult, but it is viable unless you isolate the file domain. 'security.alternate-file-domain' => null, + // Default key for HMAC digests where the key is not important (i.e., the + // hash itself is secret). You can change this if you want (to any other + // string), but doing so will break existing sessions and CSRF tokens. + 'security.hmac-key' => '[D\t~Y7eNmnQGJ;rnH6aF;m2!vJ8@v8C=Cs:aQS\.Qw', + + // -- DarkConsole ----------------------------------------------------------- // // DarkConsole is a administrative debugging/profiling tool built into diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index bab5bcf691..f6aa7a6666 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -471,6 +471,7 @@ phutil_register_library_map(array( 'PhabricatorGarbageCollectorDaemon' => 'infrastructure/daemon/garbagecollector', 'PhabricatorGoodForNothingWorker' => 'infrastructure/daemon/workers/worker/goodfornothing', 'PhabricatorHandleObjectSelectorDataView' => 'applications/phid/handle/view/selector', + 'PhabricatorHash' => 'infrastructure/util/hash', 'PhabricatorHelpController' => 'applications/help/controller/base', 'PhabricatorHelpKeyboardShortcutController' => 'applications/help/controller/keyboardshortcut', 'PhabricatorIRCBot' => 'infrastructure/daemon/irc/bot', diff --git a/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php b/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php index 860994c6c6..7f83330bd9 100644 --- a/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php +++ b/src/applications/metamta/storage/receivedmail/PhabricatorMetaMTAReceivedMail.php @@ -176,7 +176,11 @@ class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO { } $expect_hash = self::computeMailHash($receiver->getMailKey(), $check_phid); - if ($expect_hash != $hash) { + + // See note at computeOldMailHash(). + $old_hash = self::computeOldMailHash($receiver->getMailKey(), $check_phid); + + if ($expect_hash != $hash && $old_hash != $hash) { return $this->setMessage("Invalid mail hash!")->save(); } @@ -228,6 +232,20 @@ class PhabricatorMetaMTAReceivedMail extends PhabricatorMetaMTADAO { public static function computeMailHash($mail_key, $phid) { $global_mail_key = PhabricatorEnv::getEnvConfig('phabricator.mail-key'); + $hash = PhabricatorHash::digest($mail_key.$global_mail_key.$phid); + return substr($hash, 0, 16); + } + + public static function computeOldMailHash($mail_key, $phid) { + + // TODO: Remove this method entirely in a couple of months. We've moved from + // plain sha1 to sha1+hmac to make the codebase more auditable for good uses + // of hash functions, but still accept the old hashes on email replies to + // avoid breaking things. Once we've been sending only hmac hashes for a + // while, remove this and start rejecting old hashes. See T547. + + $global_mail_key = PhabricatorEnv::getEnvConfig('phabricator.mail-key'); + $hash = sha1($mail_key.$global_mail_key.$phid); return substr($hash, 0, 16); } diff --git a/src/applications/metamta/storage/receivedmail/__init__.php b/src/applications/metamta/storage/receivedmail/__init__.php index 6254939d3f..37d4e56863 100644 --- a/src/applications/metamta/storage/receivedmail/__init__.php +++ b/src/applications/metamta/storage/receivedmail/__init__.php @@ -14,6 +14,7 @@ phutil_require_module('phabricator', 'applications/metamta/parser'); phutil_require_module('phabricator', 'applications/metamta/storage/base'); phutil_require_module('phabricator', 'applications/people/storage/user'); phutil_require_module('phabricator', 'infrastructure/env'); +phutil_require_module('phabricator', 'infrastructure/util/hash'); phutil_require_module('phutil', 'utils'); diff --git a/src/applications/people/storage/log/PhabricatorUserLog.php b/src/applications/people/storage/log/PhabricatorUserLog.php index abc0355b1f..339877e4bb 100644 --- a/src/applications/people/storage/log/PhabricatorUserLog.php +++ b/src/applications/people/storage/log/PhabricatorUserLog.php @@ -89,7 +89,7 @@ class PhabricatorUserLog extends PhabricatorUserDAO { // seeing the logs doesn't compromise all the sessions which appear in // them. This just prevents casual leaks, like in a screenshot. if (strlen($session)) { - $this->session = sha1($session); + $this->session = PhabricatorHash::digest($session); } return $this; } diff --git a/src/applications/people/storage/log/__init__.php b/src/applications/people/storage/log/__init__.php index 90f82b5ed8..3392d47eae 100644 --- a/src/applications/people/storage/log/__init__.php +++ b/src/applications/people/storage/log/__init__.php @@ -7,6 +7,7 @@ phutil_require_module('phabricator', 'applications/people/storage/base'); +phutil_require_module('phabricator', 'infrastructure/util/hash'); phutil_require_module('phutil', 'utils'); diff --git a/src/applications/people/storage/user/PhabricatorUser.php b/src/applications/people/storage/user/PhabricatorUser.php index ca5eb1b5cd..0cb6990ad1 100644 --- a/src/applications/people/storage/user/PhabricatorUser.php +++ b/src/applications/people/storage/user/PhabricatorUser.php @@ -191,7 +191,7 @@ class PhabricatorUser extends PhabricatorUserDAO { private function generateToken($epoch, $frequency, $key, $len) { $time_block = floor($epoch / $frequency); $vec = $this->getPHID().$this->getPasswordHash().$key.$time_block; - return substr(sha1($vec), 0, $len); + return substr(PhabricatorHash::digest($vec), 0, $len); } /** diff --git a/src/applications/people/storage/user/__init__.php b/src/applications/people/storage/user/__init__.php index c93a02dfda..8cdfe8db6e 100644 --- a/src/applications/people/storage/user/__init__.php +++ b/src/applications/people/storage/user/__init__.php @@ -14,6 +14,7 @@ phutil_require_module('phabricator', 'applications/phid/constants'); phutil_require_module('phabricator', 'applications/phid/storage/phid'); phutil_require_module('phabricator', 'applications/search/index/indexer/user'); phutil_require_module('phabricator', 'infrastructure/env'); +phutil_require_module('phabricator', 'infrastructure/util/hash'); phutil_require_module('phabricator', 'storage/qsprintf'); phutil_require_module('phabricator', 'storage/queryfx'); diff --git a/src/docs/userguide/diffusion.diviner b/src/docs/userguide/diffusion.diviner index d59960ced5..e4a33dae7d 100644 --- a/src/docs/userguide/diffusion.diviner +++ b/src/docs/userguide/diffusion.diviner @@ -58,7 +58,7 @@ of 7-character hashes: Because 7-character hashes are likely to collide for even moderately large repositories, Diffusion generally uses either a 16-character prefix (which makes -collisions very unlikely) or the full 40-character SHA1 (which makes collisions +collisions very unlikely) or the full 40-character hash (which makes collisions astronomically unlikely). = Adding Repositories = diff --git a/src/infrastructure/util/hash/PhabricatorHash.php b/src/infrastructure/util/hash/PhabricatorHash.php new file mode 100644 index 0000000000..9a54d29c10 --- /dev/null +++ b/src/infrastructure/util/hash/PhabricatorHash.php @@ -0,0 +1,31 @@ +