From d122d9ec86a8cfb41575bfa556b0eb1ea4427c8e Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 21 Aug 2014 11:30:05 -0700 Subject: [PATCH] Allow users to recover from a missing password hasher Summary: Fixes T5934. If you hash a password with, e.g., bcrypt, and then lose the bcrypt hasher for some reason, we currently fatal when trying to figure out if we can upgrade. Instead, detect that the current hasher implementation has vanished and let the user reset their password (for account passwords) or choose a new one (for VCS passwords)> Test Plan: Account password: - Artifically disabled bcrypt hasher. - Viewed password panel, saw warnings about missing hasher. - Used password reset workflow to change password, saw iterated MD5 hashed password get set. - Enabled bcrypt hasher again. - Saw upgrade warning. - Upgraded password to bcrypt. VCS password: - Artificially disabled bcrypt hasher. - Viewed password panel, saw warnings about missing hasher. - Reset password. - Saw iterated md5 password. - Reenabled bcrypt. - Upgraded to bcrypt. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T5934 Differential Revision: https://secure.phabricator.com/D10325 --- .../panel/DiffusionSetPasswordPanel.php | 36 +++++++++++++++---- .../PhabricatorSettingsPanelPassword.php | 23 +++++++++++- .../password/PhabricatorPasswordHasher.php | 4 ++- 3 files changed, 54 insertions(+), 9 deletions(-) diff --git a/src/applications/diffusion/panel/DiffusionSetPasswordPanel.php b/src/applications/diffusion/panel/DiffusionSetPasswordPanel.php index c45acb7185..21e00c7076 100644 --- a/src/applications/diffusion/panel/DiffusionSetPasswordPanel.php +++ b/src/applications/diffusion/panel/DiffusionSetPasswordPanel.php @@ -74,16 +74,23 @@ final class DiffusionSetPasswordPanel extends PhabricatorSettingsPanel { if (!$errors) { $envelope = new PhutilOpaqueEnvelope($new_password); + try { + // NOTE: This test is against $viewer (not $user), so that the error + // message below makes sense in the case that the two are different, + // and because an admin reusing their own password is bad, while + // system agents generally do not have passwords anyway. + + $same_password = $viewer->comparePassword($envelope); + } catch (PhabricatorPasswordHasherUnavailableException $ex) { + // If we're missing the hasher, just let the user continue. + $same_password = false; + } + if ($new_password !== $confirm) { $e_password = pht('Does Not Match'); $e_confirm = pht('Does Not Match'); $errors[] = pht('Password and confirmation do not match.'); - } else if ($viewer->comparePassword($envelope)) { - // NOTE: The above test is against $viewer (not $user), so that the - // error message below makes sense in the case that the two are - // different, and because an admin reusing their own password is bad, - // while system agents generally do not have passwords anyway. - + } else if ($same_password) { $e_password = pht('Not Unique'); $e_confirm = pht('Not Unique'); $errors[] = pht( @@ -197,7 +204,22 @@ final class DiffusionSetPasswordPanel extends PhabricatorSettingsPanel { ->setValue(PhabricatorPasswordHasher::getBestAlgorithmName())); if (strlen($hash_envelope->openEnvelope())) { - if (PhabricatorPasswordHasher::canUpgradeHash($hash_envelope)) { + try { + $can_upgrade = PhabricatorPasswordHasher::canUpgradeHash( + $hash_envelope); + } catch (PhabricatorPasswordHasherUnavailableException $ex) { + $can_upgrade = false; + $errors[] = pht( + 'Your VCS password is currently hashed using an algorithm which is '. + 'no longer available on this install.'); + $errors[] = pht( + 'Because the algorithm implementation is missing, your password '. + 'can not be used.'); + $errors[] = pht( + 'You can set a new password to replace the old password.'); + } + + if ($can_upgrade) { $errors[] = pht( 'The strength of your stored VCS password hash can be upgraded. '. 'To upgrade, either: use the password to authenticate with a '. diff --git a/src/applications/settings/panel/PhabricatorSettingsPanelPassword.php b/src/applications/settings/panel/PhabricatorSettingsPanelPassword.php index 2aef196b96..eeb9b3410e 100644 --- a/src/applications/settings/panel/PhabricatorSettingsPanelPassword.php +++ b/src/applications/settings/panel/PhabricatorSettingsPanelPassword.php @@ -126,7 +126,28 @@ final class PhabricatorSettingsPanelPassword $hash_envelope = new PhutilOpaqueEnvelope($user->getPasswordHash()); if (strlen($hash_envelope->openEnvelope())) { - if (PhabricatorPasswordHasher::canUpgradeHash($hash_envelope)) { + try { + $can_upgrade = PhabricatorPasswordHasher::canUpgradeHash( + $hash_envelope); + } catch (PhabricatorPasswordHasherUnavailableException $ex) { + $can_upgrade = false; + + // Only show this stuff if we aren't on the reset workflow. We can + // do resets regardless of the old hasher's availability. + if (!$token) { + $errors[] = pht( + 'Your password is currently hashed using an algorithm which is '. + 'no longer available on this install.'); + $errors[] = pht( + 'Because the algorithm implementation is missing, your password '. + 'can not be used or updated.'); + $errors[] = pht( + 'To set a new password, request a password reset link from the '. + 'login screen and then follow the instructions.'); + } + } + + if ($can_upgrade) { $errors[] = pht( 'The strength of your stored password hash can be upgraded. '. 'To upgrade, either: log out and log in using your password; or '. diff --git a/src/infrastructure/util/password/PhabricatorPasswordHasher.php b/src/infrastructure/util/password/PhabricatorPasswordHasher.php index 338a7615ee..4409163509 100644 --- a/src/infrastructure/util/password/PhabricatorPasswordHasher.php +++ b/src/infrastructure/util/password/PhabricatorPasswordHasher.php @@ -407,7 +407,9 @@ abstract class PhabricatorPasswordHasher extends Phobject { $current_hasher = PhabricatorPasswordHasher::getHasherForHash($hash); return $current_hasher->getHumanReadableName(); } catch (Exception $ex) { - return pht('Unknown'); + $info = self::parseHashFromStorage($hash); + $name = $info['name']; + return pht('Unknown ("%s")', $name); } }