From b96ab5aadf5377bd426ff7406fe635763e8ee8a4 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 18 Feb 2014 12:18:04 -0800 Subject: [PATCH] Modernize VCS password storage to use shared hash infrastructure Summary: Fixes T4443. Plug VCS passwords into the shared key stretching. They don't use any real stretching now (I anticipated doing something like T4443 eventually) so we can just migrate them into stretching all at once. Test Plan: - Viewed VCS settings. - Used VCS password after migration. - Set VCS password. - Upgraded VCS password by using it. - Used VCS password some more. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T4443 Differential Revision: https://secure.phabricator.com/D8272 --- .../20140218.passwords.3.vcsextend.sql | 2 ++ .../autopatches/20140218.passwords.4.vcs.php | 27 ++++++++++++++ .../controller/DiffusionServeController.php | 12 +++++++ .../panel/DiffusionSetPasswordPanel.php | 20 +++++++++++ .../PhabricatorRepositoryVCSPassword.php | 26 ++++++++++---- .../PhabricatorSettingsPanelPassword.php | 25 ++----------- .../password/PhabricatorPasswordHasher.php | 36 +++++++++++++++++++ 7 files changed, 119 insertions(+), 29 deletions(-) create mode 100644 resources/sql/autopatches/20140218.passwords.3.vcsextend.sql create mode 100644 resources/sql/autopatches/20140218.passwords.4.vcs.php diff --git a/resources/sql/autopatches/20140218.passwords.3.vcsextend.sql b/resources/sql/autopatches/20140218.passwords.3.vcsextend.sql new file mode 100644 index 0000000000..38753cc6d3 --- /dev/null +++ b/resources/sql/autopatches/20140218.passwords.3.vcsextend.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_repository.repository_vcspassword + CHANGE passwordHash passwordHash VARCHAR(128) COLLATE utf8_bin NOT NULL; diff --git a/resources/sql/autopatches/20140218.passwords.4.vcs.php b/resources/sql/autopatches/20140218.passwords.4.vcs.php new file mode 100644 index 0000000000..5e58c63e86 --- /dev/null +++ b/resources/sql/autopatches/20140218.passwords.4.vcs.php @@ -0,0 +1,27 @@ +establishConnection('w'); + +echo "Upgrading password hashing for VCS passwords.\n"; + +$best_hasher = PhabricatorPasswordHasher::getBestHasher(); +foreach (new LiskMigrationIterator($table) as $password) { + $id = $password->getID(); + + echo "Migrating VCS password {$id}...\n"; + + $input_hash = $password->getPasswordHash(); + $input_envelope = new PhutilOpaqueEnvelope($input_hash); + + $storage_hash = $best_hasher->getPasswordHashForStorage($input_envelope); + + queryfx( + $conn_w, + 'UPDATE %T SET passwordHash = %s WHERE id = %d', + $table->getTableName(), + $storage_hash->openEnvelope(), + $id); +} + +echo "Done.\n"; diff --git a/src/applications/diffusion/controller/DiffusionServeController.php b/src/applications/diffusion/controller/DiffusionServeController.php index 9cd7b2aee0..403522a056 100644 --- a/src/applications/diffusion/controller/DiffusionServeController.php +++ b/src/applications/diffusion/controller/DiffusionServeController.php @@ -426,6 +426,18 @@ final class DiffusionServeController extends DiffusionController { return null; } + // If the user's password is stored using a less-than-optimal hash, upgrade + // them to the strongest available hash. + + $hash_envelope = new PhutilOpaqueEnvelope( + $password_entry->getPasswordHash()); + if (PhabricatorPasswordHasher::canUpgradeHash($hash_envelope)) { + $password_entry->setPassword($password, $user); + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); + $password_entry->save(); + unset($unguarded); + } + return $user; } diff --git a/src/applications/diffusion/panel/DiffusionSetPasswordPanel.php b/src/applications/diffusion/panel/DiffusionSetPasswordPanel.php index 25684c1c2e..0f4829d2ec 100644 --- a/src/applications/diffusion/panel/DiffusionSetPasswordPanel.php +++ b/src/applications/diffusion/panel/DiffusionSetPasswordPanel.php @@ -165,6 +165,26 @@ final class DiffusionSetPasswordPanel extends PhabricatorSettingsPanel { } } + $hash_envelope = new PhutilOpaqueEnvelope($vcspassword->getPasswordHash()); + + $form->appendChild( + id(new AphrontFormStaticControl()) + ->setLabel(pht('Current Algorithm')) + ->setValue( + PhabricatorPasswordHasher::getCurrentAlgorithmName($hash_envelope))); + + $form->appendChild( + id(new AphrontFormStaticControl()) + ->setLabel(pht('Best Available Algorithm')) + ->setValue(PhabricatorPasswordHasher::getBestAlgorithmName())); + + if (PhabricatorPasswordHasher::canUpgradeHash($hash_envelope)) { + $errors[] = pht( + 'The strength of your stored VCS password hash can be upgraded. '. + 'To upgrade, either: use the password to authenticate with a '. + 'repository; or change your password.'); + } + $object_box = id(new PHUIObjectBoxView()) ->setHeaderText($title) ->setForm($form) diff --git a/src/applications/repository/storage/PhabricatorRepositoryVCSPassword.php b/src/applications/repository/storage/PhabricatorRepositoryVCSPassword.php index ffdd65b958..9230bd5911 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryVCSPassword.php +++ b/src/applications/repository/storage/PhabricatorRepositoryVCSPassword.php @@ -9,26 +9,38 @@ final class PhabricatorRepositoryVCSPassword extends PhabricatorRepositoryDAO { public function setPassword( PhutilOpaqueEnvelope $password, PhabricatorUser $user) { - return $this->setPasswordHash($this->hashPassword($password, $user)); + $hash_envelope = $this->hashPassword($password, $user); + return $this->setPasswordHash($hash_envelope->openEnvelope()); } public function comparePassword( PhutilOpaqueEnvelope $password, PhabricatorUser $user) { - $hash = $this->hashPassword($password, $user); - return ($hash == $this->getPasswordHash()); + return PhabricatorPasswordHasher::comparePassword( + $this->getPasswordHashInput($password, $user), + new PhutilOpaqueEnvelope($this->getPasswordHash())); + } + + private function getPasswordHashInput( + PhutilOpaqueEnvelope $password, + PhabricatorUser $user) { + if ($user->getPHID() != $this->getUserPHID()) { + throw new Exception("User does not match password user PHID!"); + } + + $raw_input = PhabricatorHash::digestPassword($password, $user->getPHID()); + return new PhutilOpaqueEnvelope($raw_input); } private function hashPassword( PhutilOpaqueEnvelope $password, PhabricatorUser $user) { - if ($user->getPHID() != $this->getUserPHID()) { - throw new Exception("User does not match password user PHID!"); - } + $input_envelope = $this->getPasswordHashInput($password, $user); - return PhabricatorHash::digestPassword($password, $user->getPHID()); + $best_hasher = PhabricatorPasswordHasher::getBestHasher(); + return $best_hasher->getPasswordHashForStorage($input_envelope); } } diff --git a/src/applications/settings/panel/PhabricatorSettingsPanelPassword.php b/src/applications/settings/panel/PhabricatorSettingsPanelPassword.php index 9db5e08880..769ab03a90 100644 --- a/src/applications/settings/panel/PhabricatorSettingsPanelPassword.php +++ b/src/applications/settings/panel/PhabricatorSettingsPanelPassword.php @@ -114,7 +114,6 @@ 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 '. @@ -157,34 +156,16 @@ final class PhabricatorSettingsPanelPassword id(new AphrontFormSubmitControl()) ->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'); - } + ->setValue(PhabricatorPasswordHasher::getCurrentAlgorithmName( + new PhutilOpaqueEnvelope($user->getPasswordHash())))); $form->appendChild( id(new AphrontFormStaticControl()) ->setLabel(pht('Best Available Algorithm')) - ->setValue($best_name)); + ->setValue(PhabricatorPasswordHasher::getBestAlgorithmName())); $form_box = id(new PHUIObjectBoxView()) ->setHeaderText(pht('Change Password')) diff --git a/src/infrastructure/util/password/PhabricatorPasswordHasher.php b/src/infrastructure/util/password/PhabricatorPasswordHasher.php index 0dcdb0e7c8..b344893689 100644 --- a/src/infrastructure/util/password/PhabricatorPasswordHasher.php +++ b/src/infrastructure/util/password/PhabricatorPasswordHasher.php @@ -385,4 +385,40 @@ abstract class PhabricatorPasswordHasher extends Phobject { return $hasher->verifyPassword($password, $parts['hash']); } + + /** + * Get the human-readable algorithm name for a given hash. + * + * @param PhutilOpaqueEnvelope Storage hash. + * @return string Human-readable algorithm name. + */ + public static function getCurrentAlgorithmName(PhutilOpaqueEnvelope $hash) { + $raw_hash = $hash->openEnvelope(); + if (!strlen($raw_hash)) { + return pht('None'); + } + + try { + $current_hasher = PhabricatorPasswordHasher::getHasherForHash($hash); + return $current_hasher->getHumanReadableName(); + } catch (Exception $ex) { + return pht('Unknown'); + } + } + + + /** + * Get the human-readable algorithm name for the best available hash. + * + * @return string Human-readable name for best hash. + */ + public static function getBestAlgorithmName() { + try { + $best_hasher = PhabricatorPasswordHasher::getBestHasher(); + return $best_hasher->getHumanReadableName(); + } catch (Exception $ex) { + return pht('Unknown'); + } + } + }