diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index d9f65bbdce..5d68a5ae60 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1595,6 +1595,7 @@ phutil_register_library_map(array( 'PhabricatorInlineSummaryView' => 'infrastructure/diff/view/PhabricatorInlineSummaryView.php', 'PhabricatorInternationalizationManagementExtractWorkflow' => 'infrastructure/internationalization/management/PhabricatorInternationalizationManagementExtractWorkflow.php', 'PhabricatorInternationalizationManagementWorkflow' => 'infrastructure/internationalization/management/PhabricatorInternationalizationManagementWorkflow.php', + 'PhabricatorIteratedMD5PasswordHasher' => 'infrastructure/util/password/PhabricatorIteratedMD5PasswordHasher.php', 'PhabricatorJavelinLinter' => 'infrastructure/lint/linter/PhabricatorJavelinLinter.php', 'PhabricatorJumpNavHandler' => 'applications/search/engine/PhabricatorJumpNavHandler.php', 'PhabricatorKeyValueDatabaseCache' => 'applications/cache/PhabricatorKeyValueDatabaseCache.php', @@ -1764,6 +1765,9 @@ phutil_register_library_map(array( 'PhabricatorPHIDType' => 'applications/phid/type/PhabricatorPHIDType.php', 'PhabricatorPHPMailerConfigOptions' => 'applications/config/option/PhabricatorPHPMailerConfigOptions.php', 'PhabricatorPagedFormExample' => 'applications/uiexample/examples/PhabricatorPagedFormExample.php', + 'PhabricatorPasswordHasher' => 'infrastructure/util/password/PhabricatorPasswordHasher.php', + 'PhabricatorPasswordHasherTestCase' => 'infrastructure/util/password/__tests__/PhabricatorPasswordHasherTestCase.php', + 'PhabricatorPasswordHasherUnavailableException' => 'infrastructure/util/password/PhabricatorPasswordHasherUnavailableException.php', 'PhabricatorPaste' => 'applications/paste/storage/PhabricatorPaste.php', 'PhabricatorPasteCommentController' => 'applications/paste/controller/PhabricatorPasteCommentController.php', 'PhabricatorPasteConfigOptions' => 'applications/paste/config/PhabricatorPasteConfigOptions.php', @@ -4327,6 +4331,7 @@ phutil_register_library_map(array( 'PhabricatorInlineSummaryView' => 'AphrontView', 'PhabricatorInternationalizationManagementExtractWorkflow' => 'PhabricatorInternationalizationManagementWorkflow', 'PhabricatorInternationalizationManagementWorkflow' => 'PhabricatorManagementWorkflow', + 'PhabricatorIteratedMD5PasswordHasher' => 'PhabricatorPasswordHasher', 'PhabricatorJavelinLinter' => 'ArcanistLinter', 'PhabricatorKeyValueDatabaseCache' => 'PhutilKeyValueCache', 'PhabricatorLegalpadConfigOptions' => 'PhabricatorApplicationConfigOptions', @@ -4491,6 +4496,9 @@ phutil_register_library_map(array( 'PhabricatorPHDConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorPHPMailerConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorPagedFormExample' => 'PhabricatorUIExample', + 'PhabricatorPasswordHasher' => 'Phobject', + 'PhabricatorPasswordHasherTestCase' => 'PhabricatorTestCase', + 'PhabricatorPasswordHasherUnavailableException' => 'Exception', 'PhabricatorPaste' => array( 0 => 'PhabricatorPasteDAO', diff --git a/src/applications/auth/controller/config/PhabricatorAuthEditController.php b/src/applications/auth/controller/config/PhabricatorAuthEditController.php index 5ed434c1b6..99be761000 100644 --- a/src/applications/auth/controller/config/PhabricatorAuthEditController.php +++ b/src/applications/auth/controller/config/PhabricatorAuthEditController.php @@ -276,6 +276,8 @@ final class PhabricatorAuthEditController $form->appendRemarkupInstructions($help); } + $footer = $provider->renderConfigurationFooter(); + $crumbs = $this->buildApplicationCrumbs(); $crumbs->addTextCrumb($crumb); @@ -305,6 +307,7 @@ final class PhabricatorAuthEditController array( $crumbs, $form_box, + $footer, $xaction_view, ), array( diff --git a/src/applications/auth/provider/PhabricatorAuthProvider.php b/src/applications/auth/provider/PhabricatorAuthProvider.php index ac56803e1b..56bc61c99b 100644 --- a/src/applications/auth/provider/PhabricatorAuthProvider.php +++ b/src/applications/auth/provider/PhabricatorAuthProvider.php @@ -437,4 +437,8 @@ abstract class PhabricatorAuthProvider { $content); } + public function renderConfigurationFooter() { + return null; + } + } diff --git a/src/applications/auth/provider/PhabricatorAuthProviderPassword.php b/src/applications/auth/provider/PhabricatorAuthProviderPassword.php index a779df0f2d..70216561f7 100644 --- a/src/applications/auth/provider/PhabricatorAuthProviderPassword.php +++ b/src/applications/auth/provider/PhabricatorAuthProviderPassword.php @@ -11,8 +11,91 @@ final class PhabricatorAuthProviderPassword public function getConfigurationHelp() { return pht( - 'You can select a minimum password length by setting '. - '`account.minimum-password-length` in configuration.'); + "(WARNING) Examine the table below for information on how password ". + "hashes will be stored in the database.\n\n". + "(NOTE) You can select a minimum password length by setting ". + "`account.minimum-password-length` in configuration."); + } + + public function renderConfigurationFooter() { + $hashers = PhabricatorPasswordHasher::getAllHashers(); + $hashers = msort($hashers, 'getStrength'); + $hashers = array_reverse($hashers); + + $yes = phutil_tag( + 'strong', + array( + 'style' => 'color: #009900', + ), + pht('Yes')); + + $no = phutil_tag( + 'strong', + array( + 'style' => 'color: #990000', + ), + pht('Not Installed')); + + $best_hasher_name = null; + try { + $best_hasher = PhabricatorPasswordHasher::getBestHasher(); + $best_hasher_name = $best_hasher->getHashName(); + } catch (PhabricatorPasswordHasherUnavailableException $ex) { + // There are no suitable hashers. The user might be able to enable some, + // so we don't want to fatal here. We'll fatal when users try to actually + // use this stuff if it isn't fixed before then. Until then, we just + // don't highlight a row. In practice, at least one hasher should always + // be available. + } + + $rows = array(); + $rowc = array(); + foreach ($hashers as $hasher) { + $is_installed = $hasher->canHashPasswords(); + + $rows[] = array( + $hasher->getHumanReadableName(), + $hasher->getHashName(), + $hasher->getHumanReadableStrength(), + ($is_installed ? $yes : $no), + ($is_installed ? null : $hasher->getInstallInstructions()), + ); + $rowc[] = ($best_hasher_name == $hasher->getHashName()) + ? 'highlighted' + : null; + } + + $table = new AphrontTableView($rows); + $table->setRowClasses($rowc); + $table->setHeaders( + array( + pht('Algorithm'), + pht('Name'), + pht('Strength'), + pht('Installed'), + pht('Install Instructions'), + )); + + $table->setColumnClasses( + array( + '', + '', + '', + '', + 'wide', + )); + + $header = id(new PHUIHeaderView()) + ->setHeader(pht('Password Hash Algorithms')) + ->setSubheader( + pht( + 'Stronger algorithms are listed first. The highlighted algorithm '. + 'will be used when storing new hashes. Older hashes will be '. + 'upgraded to the best algorithm over time.')); + + return id(new PHUIObjectBoxView()) + ->setHeader($header) + ->appendChild($table); } public function getDescriptionForCreate() { diff --git a/src/applications/people/storage/PhabricatorUser.php b/src/applications/people/storage/PhabricatorUser.php index d7df6f7065..322cff1a1b 100644 --- a/src/applications/people/storage/PhabricatorUser.php +++ b/src/applications/people/storage/PhabricatorUser.php @@ -112,9 +112,9 @@ final class PhabricatorUser if (!strlen($envelope->openEnvelope())) { $this->setPasswordHash(''); } else { - $this->setPasswordSalt(md5(mt_rand())); + $this->setPasswordSalt(md5(Filesystem::readRandomBytes(32))); $hash = $this->hashPassword($envelope); - $this->setPasswordHash($hash); + $this->setPasswordHash($hash->openEnvelope()); } return $this; } @@ -170,19 +170,37 @@ final class PhabricatorUser if (!strlen($this->getPasswordHash())) { return false; } - $password_hash = $this->hashPassword($envelope); - return ($password_hash === $this->getPasswordHash()); + + return PhabricatorPasswordHasher::comparePassword( + $this->getPasswordHashInput($envelope), + // TODO: For now, we need to add a prefix. + new PhutilOpaqueEnvelope('md5:'.$this->getPasswordHash())); } - private function hashPassword(PhutilOpaqueEnvelope $envelope) { - $hash = $this->getUsername(). - $envelope->openEnvelope(). - $this->getPHID(). - $this->getPasswordSalt(); - for ($ii = 0; $ii < 1000; $ii++) { - $hash = md5($hash); - } - return $hash; + private function getPasswordHashInput(PhutilOpaqueEnvelope $password) { + $input = + $this->getUsername(). + $password->openEnvelope(). + $this->getPHID(). + $this->getPasswordSalt(); + + return new PhutilOpaqueEnvelope($input); + } + + private function hashPassword(PhutilOpaqueEnvelope $password) { + + $hasher = PhabricatorPasswordHasher::getBestHasher(); + + $input_envelope = $this->getPasswordHashInput($password); + $output_envelope = $hasher->getPasswordHashForStorage($input_envelope); + + // TODO: For now, we need to strip the type prefix until we can upgrade + // the storage. + + $raw_output = $output_envelope->openEnvelope(); + $raw_output = substr($raw_output, strlen('md5:')); + + return new PhutilOpaqueEnvelope($raw_output); } const CSRF_CYCLE_FREQUENCY = 3600; diff --git a/src/infrastructure/util/password/PhabricatorIteratedMD5PasswordHasher.php b/src/infrastructure/util/password/PhabricatorIteratedMD5PasswordHasher.php new file mode 100644 index 0000000000..8897d15fd2 --- /dev/null +++ b/src/infrastructure/util/password/PhabricatorIteratedMD5PasswordHasher.php @@ -0,0 +1,46 @@ +openEnvelope(); + + $hash = $raw_input; + for ($ii = 0; $ii < 1000; $ii++) { + $hash = md5($hash); + } + + return new PhutilOpaqueEnvelope($hash); + } + +} diff --git a/src/infrastructure/util/password/PhabricatorPasswordHasher.php b/src/infrastructure/util/password/PhabricatorPasswordHasher.php new file mode 100644 index 0000000000..d901013f32 --- /dev/null +++ b/src/infrastructure/util/password/PhabricatorPasswordHasher.php @@ -0,0 +1,336 @@ +getHashName(); + $hash = $this->getPasswordHash($envelope); + + $actual_len = strlen($hash->openEnvelope()); + $expect_len = $this->getHashLength(); + if ($actual_len > $expect_len) { + throw new Exception( + pht( + "Password hash '%s' produced a hash of length %d, but a ". + "maximum length of %d was expected.", + $name, + new PhutilNumber($actual_len), + new PhutilNumber($expect_len))); + } + + return new PhutilOpaqueEnvelope($name.':'.$hash->openEnvelope()); + } + + + /** + * Parse a storage hash into its components, like the hash type and hash + * data. + * + * @return map Dictionary of information about the hash. + * @task hashing + */ + private static function parseHashFromStorage(PhutilOpaqueEnvelope $hash) { + $raw_hash = $hash->openEnvelope(); + if (strpos($raw_hash, ':') === false) { + throw new Exception( + pht( + 'Malformed password hash, expected "name:hash".')); + } + + list($name, $hash) = explode(':', $raw_hash); + + return array( + 'name' => $name, + 'hash' => new PhutilOpaqueEnvelope($hash), + ); + } + + + /** + * Get all available password hashers. This may include hashers which can not + * actually be used (for example, a required extension is missing). + * + * @return list Hasher objects. + * @task hashing + */ + public static function getAllHashers() { + $objects = id(new PhutilSymbolLoader()) + ->setAncestorClass('PhabricatorPasswordHasher') + ->loadObjects(); + + $map = array(); + foreach ($objects as $object) { + $name = $object->getHashName(); + + $potential_length = strlen($name) + $object->getHashLength() + 1; + $maximum_length = self::MAXIMUM_STORAGE_SIZE; + + if ($potential_length > $maximum_length) { + throw new Exception( + pht( + 'Hasher "%s" may produce hashes which are too long to fit in '. + 'storage. %d characters are available, but its hashes may be '. + 'up to %d characters in length.', + $name, + $maximum_length, + $potential_length)); + } + + if (isset($map[$name])) { + throw new Exception( + pht( + 'Two hashers use the same hash name ("%s"), "%s" and "%s". Each '. + 'hasher must have a unique name.', + $name, + get_class($object), + get_class($map[$name]))); + } + $map[$name] = $object; + } + + return $map; + } + + + /** + * Get all usable password hashers. This may include hashers which are + * not desirable or advisable. + * + * @return list Hasher objects. + * @task hashing + */ + public static function getAllUsableHashers() { + $hashers = self::getAllHashers(); + foreach ($hashers as $key => $hasher) { + if (!$hasher->canHashPasswords()) { + unset($hashers[$key]); + } + } + return $hashers; + } + + + /** + * Get the best (strongest) available hasher. + * + * @return PhabicatorPasswordHasher Best hasher. + * @task hashing + */ + public static function getBestHasher() { + $hashers = self::getAllUsableHashers(); + msort($hashers, 'getStrength'); + + $hasher = last($hashers); + if (!$hasher) { + throw new PhabricatorPasswordHasherUnavailableException( + pht( + 'There are no password hashers available which are usable for '. + 'new passwords.')); + } + + return $hasher; + } + + + /** + * Get the hashser for a given stored hash. + * + * @return PhabicatorPasswordHasher Corresponding hasher. + * @task hashing + */ + public static function getHasherForHash(PhutilOpaqueEnvelope $hash) { + $info = self::parseHashFromStorage($hash); + $name = $info['name']; + + $usable = self::getAllUsableHashers(); + if (isset($usable[$name])) { + return $usable[$name]; + } + + $all = self::getAllHashers(); + if (isset($all[$name])) { + throw new PhabricatorPasswordHasherUnavailableException( + pht( + 'Attempting to compare a password saved with the "%s" hash. The '. + 'hasher exists, but is not currently usable. %s', + $name, + $all[$name]->getInstallInstructions())); + } + + throw new PhabricatorPasswordHasherUnavailableException( + pht( + 'Attempting to compare a password saved with the "%s" hash. No such '. + 'hasher is known to Phabricator.', + $name)); + } + + + /** + * Test if a password is using an weaker hash than the strongest available + * hash. This can be used to prompt users to upgrade, or automatically upgrade + * on login. + * + * @return bool True to indicate that rehashing this password will improve + * the hash strength. + * @task hashing + */ + public static function canHashBeUpgraded(PhutilOpaqueEnvelope $hash) { + $current_hasher = self::getHasherForHash($hash); + $best_hasher = self::getBestHasher(); + + return ($current_hasher->getHashName() != $best_hasher->getHashName()); + } + + + /** + * Generate a new hash for a password, using the best available hasher. + * + * @param PhutilOpaqueEnvelope Password to hash. + * @return PhutilOpaqueEnvelope Hashed password, using best available + * hasher. + * @task hashing + */ + public static function generateNewPasswordHash( + PhutilOpaqueEnvelope $password) { + $hasher = self::getBestHasher(); + return $hasher->getPasswordHashForStorage($password); + } + + + /** + * Compare a password to a stored hash. + * + * @param PhutilOpaqueEnvelope Password to compare. + * @param PhutilOpaqueEnvelope Stored password hash. + * @return bool True if the passwords match. + * @task hashing + */ + public static function comparePassword( + PhutilOpaqueEnvelope $password, + PhutilOpaqueEnvelope $hash) { + + $hasher = self::getHasherForHash($hash); + $password_hash = $hasher->getPasswordHashForStorage($password); + + return ($password_hash->openEnvelope() == $hash->openEnvelope()); + } + +} diff --git a/src/infrastructure/util/password/PhabricatorPasswordHasherUnavailableException.php b/src/infrastructure/util/password/PhabricatorPasswordHasherUnavailableException.php new file mode 100644 index 0000000000..a25ddd73b2 --- /dev/null +++ b/src/infrastructure/util/password/PhabricatorPasswordHasherUnavailableException.php @@ -0,0 +1,5 @@ +assertEqual( + true, + ($caught instanceof Exception), + pht('Exception on unparseable hash format.')); + + $caught = null; + try { + PhabricatorPasswordHasher::getHasherForHash( + new PhutilOpaqueEnvelope('__test__:yyy')); + } catch (Exception $ex) { + $caught = $ex; + } + + $this->assertEqual( + true, + ($caught instanceof PhabricatorPasswordHasherUnavailableException), + pht('Fictional hasher unavailable.')); + } + + public function testMD5Hasher() { + $hasher = new PhabricatorIteratedMD5PasswordHasher(); + + $this->assertEqual( + 'md5:4824a35493d8b5dceab36f017d68425f', + $hasher->getPasswordHashForStorage( + new PhutilOpaqueEnvelope('quack'))->openEnvelope()); + } + +}