diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 5d68a5ae60..eb76217a3d 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -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', diff --git a/src/applications/auth/provider/PhabricatorAuthProviderPassword.php b/src/applications/auth/provider/PhabricatorAuthProviderPassword.php index 70216561f7..c7005e8bc6 100644 --- a/src/applications/auth/provider/PhabricatorAuthProviderPassword.php +++ b/src/applications/auth/provider/PhabricatorAuthProviderPassword.php @@ -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); + } } } } diff --git a/src/applications/settings/panel/PhabricatorSettingsPanelPassword.php b/src/applications/settings/panel/PhabricatorSettingsPanelPassword.php index dcc4d293a9..9db5e08880 100644 --- a/src/applications/settings/panel/PhabricatorSettingsPanelPassword.php +++ b/src/applications/settings/panel/PhabricatorSettingsPanelPassword.php @@ -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')) diff --git a/src/infrastructure/util/password/PhabricatorBcryptPasswordHasher.php b/src/infrastructure/util/password/PhabricatorBcryptPasswordHasher.php new file mode 100644 index 0000000000..11e5a9389d --- /dev/null +++ b/src/infrastructure/util/password/PhabricatorBcryptPasswordHasher.php @@ -0,0 +1,56 @@ +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()); + } + +} diff --git a/src/infrastructure/util/password/PhabricatorIteratedMD5PasswordHasher.php b/src/infrastructure/util/password/PhabricatorIteratedMD5PasswordHasher.php index 8897d15fd2..2847352f09 100644 --- a/src/infrastructure/util/password/PhabricatorIteratedMD5PasswordHasher.php +++ b/src/infrastructure/util/password/PhabricatorIteratedMD5PasswordHasher.php @@ -12,7 +12,7 @@ final class PhabricatorIteratedMD5PasswordHasher } public function getHashLength() { - return 40; + return 32; } public function canHashPasswords() { diff --git a/src/infrastructure/util/password/PhabricatorPasswordHasher.php b/src/infrastructure/util/password/PhabricatorPasswordHasher.php index d901013f32..12a341289f 100644 --- a/src/infrastructure/util/password/PhabricatorPasswordHasher.php +++ b/src/infrastructure/util/password/PhabricatorPasswordHasher.php @@ -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']); } }