mirror of
https://we.phorge.it/source/phorge.git
synced 2024-12-22 21:40:55 +01:00
Implement bcrypt hasher, transparent login upgrade, and explicit upgrade for passwords
Summary: Ref T4443. - Add a `password_hash()`-based bcrypt hasher if `password_hash()` is available. - When a user logs in using a password, upgrade their password to the strongest available hash format. - On the password settings page: - Warn the user if their password uses any algorithm other than the strongest one. - Show the algorithm the password uses. - Show the best available algorithm. Test Plan: As an md5 user, viewed password settings page and saw a warning. Logged out. Logged in, got upgraded, no more warning. Changed password, verified database rehash. Logged out, logged in. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T4443 Differential Revision: https://secure.phabricator.com/D8270
This commit is contained in:
parent
5778627e41
commit
580bcd0d2b
6 changed files with 137 additions and 6 deletions
|
@ -1244,6 +1244,7 @@ phutil_register_library_map(array(
|
|||
'PhabricatorBarePageView' => 'view/page/PhabricatorBarePageView.php',
|
||||
'PhabricatorBaseEnglishTranslation' => 'infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php',
|
||||
'PhabricatorBaseProtocolAdapter' => 'infrastructure/daemon/bot/adapter/PhabricatorBaseProtocolAdapter.php',
|
||||
'PhabricatorBcryptPasswordHasher' => 'infrastructure/util/password/PhabricatorBcryptPasswordHasher.php',
|
||||
'PhabricatorBot' => 'infrastructure/daemon/bot/PhabricatorBot.php',
|
||||
'PhabricatorBotBaseStreamingProtocolAdapter' => 'infrastructure/daemon/bot/adapter/PhabricatorBotBaseStreamingProtocolAdapter.php',
|
||||
'PhabricatorBotChannel' => 'infrastructure/daemon/bot/target/PhabricatorBotChannel.php',
|
||||
|
@ -3915,6 +3916,7 @@ phutil_register_library_map(array(
|
|||
'PhabricatorBarePageExample' => 'PhabricatorUIExample',
|
||||
'PhabricatorBarePageView' => 'AphrontPageView',
|
||||
'PhabricatorBaseEnglishTranslation' => 'PhabricatorTranslation',
|
||||
'PhabricatorBcryptPasswordHasher' => 'PhabricatorPasswordHasher',
|
||||
'PhabricatorBot' => 'PhabricatorDaemon',
|
||||
'PhabricatorBotBaseStreamingProtocolAdapter' => 'PhabricatorBaseProtocolAdapter',
|
||||
'PhabricatorBotChannel' => 'PhabricatorBotTarget',
|
||||
|
|
|
@ -264,6 +264,19 @@ final class PhabricatorAuthProviderPassword
|
|||
if ($user->comparePassword($envelope)) {
|
||||
$account = $this->loadOrCreateAccount($user->getPHID());
|
||||
$log_user = $user;
|
||||
|
||||
// If the user's password is stored using a less-than-optimal
|
||||
// hash, upgrade them to the strongest available hash.
|
||||
|
||||
$hash_envelope = new PhutilOpaqueEnvelope(
|
||||
$user->getPasswordHash());
|
||||
if (PhabricatorPasswordHasher::canUpgradeHash($hash_envelope)) {
|
||||
$user->setPassword($envelope);
|
||||
|
||||
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
|
||||
$user->save();
|
||||
unset($unguarded);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -112,6 +112,15 @@ final class PhabricatorSettingsPanelPassword
|
|||
}
|
||||
}
|
||||
|
||||
$hash_envelope = new PhutilOpaqueEnvelope($user->getPasswordHash());
|
||||
if (PhabricatorPasswordHasher::canUpgradeHash($hash_envelope)) {
|
||||
$best_hash = PhabricatorPasswordHasher::getBestHasher();
|
||||
$errors[] = pht(
|
||||
'The strength of your stored password hash can be upgraded. '.
|
||||
'To upgrade, either: log out and log in using your password; or '.
|
||||
'change your password.');
|
||||
}
|
||||
|
||||
$len_caption = null;
|
||||
if ($min_len) {
|
||||
$len_caption = pht('Minimum password length: %d characters.', $min_len);
|
||||
|
@ -146,7 +155,36 @@ final class PhabricatorSettingsPanelPassword
|
|||
$form
|
||||
->appendChild(
|
||||
id(new AphrontFormSubmitControl())
|
||||
->setValue(pht('Save')));
|
||||
->setValue(pht('Change Password')));
|
||||
|
||||
if (!strlen($user->getPasswordHash())) {
|
||||
$current_name = pht('None');
|
||||
} else {
|
||||
try {
|
||||
$current_hasher = PhabricatorPasswordHasher::getHasherForHash(
|
||||
new PhutilOpaqueEnvelope($user->getPasswordHash()));
|
||||
$current_name = $current_hasher->getHumanReadableName();
|
||||
} catch (Exception $ex) {
|
||||
$current_name = pht('Unknown');
|
||||
}
|
||||
}
|
||||
|
||||
$form->appendChild(
|
||||
id(new AphrontFormStaticControl())
|
||||
->setLabel(pht('Current Algorithm'))
|
||||
->setValue($current_name));
|
||||
|
||||
try {
|
||||
$best_hasher = PhabricatorPasswordHasher::getBestHasher();
|
||||
$best_name = $best_hasher->getHumanReadableName();
|
||||
} catch (Exception $ex) {
|
||||
$best_name = pht('Unknown');
|
||||
}
|
||||
|
||||
$form->appendChild(
|
||||
id(new AphrontFormStaticControl())
|
||||
->setLabel(pht('Best Available Algorithm'))
|
||||
->setValue($best_name));
|
||||
|
||||
$form_box = id(new PHUIObjectBoxView())
|
||||
->setHeaderText(pht('Change Password'))
|
||||
|
|
|
@ -0,0 +1,56 @@
|
|||
<?php
|
||||
|
||||
final class PhabricatorBcryptPasswordHasher
|
||||
extends PhabricatorPasswordHasher {
|
||||
|
||||
public function getHumanReadableName() {
|
||||
return pht('bcrypt');
|
||||
}
|
||||
|
||||
public function getHashName() {
|
||||
return 'bcrypt';
|
||||
}
|
||||
|
||||
public function getHashLength() {
|
||||
return 60;
|
||||
}
|
||||
|
||||
public function canHashPasswords() {
|
||||
return function_exists('password_hash');
|
||||
}
|
||||
|
||||
public function getInstallInstructions() {
|
||||
return pht('Upgrade to PHP 5.5.0 or newer.');
|
||||
}
|
||||
|
||||
public function getStrength() {
|
||||
return 2.0;
|
||||
}
|
||||
|
||||
public function getHumanReadableStrength() {
|
||||
return pht("Good");
|
||||
}
|
||||
|
||||
protected function getPasswordHash(PhutilOpaqueEnvelope $envelope) {
|
||||
$raw_input = $envelope->openEnvelope();
|
||||
|
||||
// NOTE: The default cost is "10", but my laptop can do a hash of cost
|
||||
// "12" in about 300ms. Since server hardware is often virtualized or old,
|
||||
// just split the difference.
|
||||
|
||||
$options = array(
|
||||
'cost' => 11,
|
||||
);
|
||||
|
||||
$raw_hash = password_hash($raw_input, CRYPT_BLOWFISH, $options);
|
||||
|
||||
return new PhutilOpaqueEnvelope($raw_hash);
|
||||
}
|
||||
|
||||
protected function verifyPassword(
|
||||
PhutilOpaqueEnvelope $password,
|
||||
PhutilOpaqueEnvelope $hash) {
|
||||
return password_verify($password->openEnvelope(), $hash->openEnvelope());
|
||||
}
|
||||
|
||||
}
|
|
@ -12,7 +12,7 @@ final class PhabricatorIteratedMD5PasswordHasher
|
|||
}
|
||||
|
||||
public function getHashLength() {
|
||||
return 40;
|
||||
return 32;
|
||||
}
|
||||
|
||||
public function canHashPasswords() {
|
||||
|
|
|
@ -108,6 +108,28 @@ abstract class PhabricatorPasswordHasher extends Phobject {
|
|||
abstract protected function getPasswordHash(PhutilOpaqueEnvelope $envelope);
|
||||
|
||||
|
||||
/**
|
||||
* Verify that a password matches a hash.
|
||||
*
|
||||
* The default implementation checks for equality; if a hasher embeds salt in
|
||||
* hashes it should override this method and perform a salt-aware comparison.
|
||||
*
|
||||
* @param PhutilOpaqueEnvelope Password to compare.
|
||||
* @param PhutilOpaqueEnvelope Bare password hash.
|
||||
* @return bool True if the passwords match.
|
||||
* @task hasher
|
||||
*/
|
||||
protected function verifyPassword(
|
||||
PhutilOpaqueEnvelope $password,
|
||||
PhutilOpaqueEnvelope $hash) {
|
||||
|
||||
$actual_hash = $this->getPasswordHash($password)->openEnvelope();
|
||||
$expect_hash = $hash->openEnvelope();
|
||||
|
||||
return ($actual_hash === $expect_hash);
|
||||
}
|
||||
|
||||
|
||||
/* -( Using Hashers )------------------------------------------------------ */
|
||||
|
||||
|
||||
|
@ -236,7 +258,7 @@ abstract class PhabricatorPasswordHasher extends Phobject {
|
|||
*/
|
||||
public static function getBestHasher() {
|
||||
$hashers = self::getAllUsableHashers();
|
||||
msort($hashers, 'getStrength');
|
||||
$hashers = msort($hashers, 'getStrength');
|
||||
|
||||
$hasher = last($hashers);
|
||||
if (!$hasher) {
|
||||
|
@ -292,7 +314,7 @@ abstract class PhabricatorPasswordHasher extends Phobject {
|
|||
* the hash strength.
|
||||
* @task hashing
|
||||
*/
|
||||
public static function canHashBeUpgraded(PhutilOpaqueEnvelope $hash) {
|
||||
public static function canUpgradeHash(PhutilOpaqueEnvelope $hash) {
|
||||
$current_hasher = self::getHasherForHash($hash);
|
||||
$best_hasher = self::getBestHasher();
|
||||
|
||||
|
@ -328,9 +350,9 @@ abstract class PhabricatorPasswordHasher extends Phobject {
|
|||
PhutilOpaqueEnvelope $hash) {
|
||||
|
||||
$hasher = self::getHasherForHash($hash);
|
||||
$password_hash = $hasher->getPasswordHashForStorage($password);
|
||||
$parts = self::parseHashFromStorage($hash);
|
||||
|
||||
return ($password_hash->openEnvelope() == $hash->openEnvelope());
|
||||
return $hasher->verifyPassword($password, $parts['hash']);
|
||||
}
|
||||
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue