From 18fe3b28330a234ef8a2c1a244b61ec2af1bb294 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 5 May 2017 20:52:21 -0400 Subject: [PATCH 1/3] [ticket/15219] Update hashes to bcrypt with cron PHPBB3-15219 --- phpBB/config/cron.yml | 14 ++ phpBB/config/password.yml | 7 + phpBB/phpbb/cron/task/core/update_hashes.php | 130 ++++++++++++++++++ .../db/migration/data/v31x/update_hashes.php | 33 +++++ 4 files changed, 184 insertions(+) create mode 100644 phpBB/phpbb/cron/task/core/update_hashes.php create mode 100644 phpBB/phpbb/db/migration/data/v31x/update_hashes.php diff --git a/phpBB/config/cron.yml b/phpBB/config/cron.yml index c5b88df181..dc628b43ff 100644 --- a/phpBB/config/cron.yml +++ b/phpBB/config/cron.yml @@ -146,3 +146,17 @@ services: - [set_name, [cron.task.core.tidy_warnings]] tags: - { name: cron.task } + + cron.task.core.update_hashes: + class: phpbb\cron\task\core\update_hashes + arguments: + - @config + - @dbal.conn + - @passwords.update.lock + - @passwords.manager + - @passwords.driver_collection + - %passwords.algorithms% + calls: + - [set_name, [cron.task.core.update_hashes]] + tags: + - { name: cron.task } diff --git a/phpBB/config/password.yml b/phpBB/config/password.yml index cb45ec3d42..938cef7e16 100644 --- a/phpBB/config/password.yml +++ b/phpBB/config/password.yml @@ -122,3 +122,10 @@ services: - @passwords.driver_helper tags: - { name: passwords.driver } + + passwords.update.lock: + class: phpbb\lock\db + arguments: + - update_hashes_lock + - '@config' + - '@dbal.conn' diff --git a/phpBB/phpbb/cron/task/core/update_hashes.php b/phpBB/phpbb/cron/task/core/update_hashes.php new file mode 100644 index 0000000000..458853f2fd --- /dev/null +++ b/phpBB/phpbb/cron/task/core/update_hashes.php @@ -0,0 +1,130 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +namespace phpbb\cron\task\core; + +/** + * Update old hashes to the current default hashing algorithm + * + * It is intended to gradually update all "old" style hashes to the + * current default hashing algorithm. + */ +class update_hashes extends \phpbb\cron\task\base +{ + /** @var \phpbb\config\config */ + protected $config; + + /** @var \phpbb\db\driver\driver_interface */ + protected $db; + + /** @var \phpbb\lock\db */ + protected $update_lock; + + /** @var \phpbb\passwords\manager */ + protected $passwords_manager; + + /** @var string Default hashing type */ + protected $default_type; + + /** + * Constructor. + * + * @param \phpbb\config\config $config + * @param \phpbb\db\driver\driver_interface $db + * @param \phpbb\lock\db $update_lock + * @param \phpbb\passwords\manager $passwords_manager + * @param array $hashing_algorithms Hashing driver + * service collection + * @param array $defaults Default password types + */ + public function __construct(\phpbb\config\config $config, \phpbb\db\driver\driver_interface $db, \phpbb\lock\db $update_lock, \phpbb\passwords\manager $passwords_manager, $hashing_algorithms, $defaults) + { + $this->config = $config; + $this->db = $db; + $this->passwords_manager = $passwords_manager; + $this->update_lock = $update_lock; + + foreach ($defaults as $type) + { + if ($hashing_algorithms[$type]->is_supported()) + { + $this->default_type = $type; + break; + } + } + } + + /** + * {@inheritdoc} + */ + public function is_runnable() + { + return !$this->config['use_system_cron']; + } + + /** + * {@inheritdoc} + */ + public function should_run() + { + if (!empty($this->config['update_hashes_lock'])) + { + $last_run = explode(' ', $this->config['update_hashes_lock']); + if ($last_run[0] + 60 >= time()) + { + return false; + } + } + + return $this->config['enable_update_hashes'] && $this->config['update_hashes_last_cron'] < (time() - 60); + } + + /** + * {@inheritdoc} + */ + public function run() + { + if ($this->update_lock->acquire()) + { + $sql = 'SELECT user_id, user_password + FROM ' . USERS_TABLE . ' + WHERE user_password ' . $this->db->sql_like_expression('$H$' . $this->db->get_any_char()) . ' + OR user_password ' . $this->db->sql_like_expression('$CP$' . $this->db->get_any_char()); + $result = $this->db->sql_query_limit($sql, 20); + + $affected_rows = 0; + + while ($row = $this->db->sql_fetchrow($result)) + { + $new_hash = $this->passwords_manager->hash($row['user_password'], array($this->default_type)); + + // Increase number so we know that users were selected from the database + $affected_rows++; + + $sql = 'UPDATE ' . USERS_TABLE . ' + SET user_password = "' . $this->db->sql_escape($new_hash) . '" + WHERE user_id = ' . (int)$row['user_id']; + $this->db->sql_query($sql); + } + + $this->config->set('update_hashes_last_cron', time()); + $this->update_lock->release(); + + // Stop cron for good once all hashes are converted + if ($affected_rows === 0) + { + $this->config->set('enable_update_hashes', '0'); + } + } + } +} diff --git a/phpBB/phpbb/db/migration/data/v31x/update_hashes.php b/phpBB/phpbb/db/migration/data/v31x/update_hashes.php new file mode 100644 index 0000000000..aa83c3ffbf --- /dev/null +++ b/phpBB/phpbb/db/migration/data/v31x/update_hashes.php @@ -0,0 +1,33 @@ + + * @license GNU General Public License, version 2 (GPL-2.0) + * + * For full copyright and license information, please see + * the docs/CREDITS.txt file. + * + */ + +namespace phpbb\db\migration\data\v31x; + +class update_hashes extends \phpbb\db\migration\migration +{ + static public function depends_on() + { + return array( + '\phpbb\db\migration\data\v31x\v3110', + ); + } + + public function update_data() + { + return array( + array('config.add', array('enable_update_hashes', '1')), + array('config.add', array('update_hashes_lock', '')), + array('config.add', array('update_hashes_last_cron', '0')) + ); + } +} From 70349864273eae4ee689cb1302bc416f5c535d25 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 5 May 2017 21:25:29 -0400 Subject: [PATCH 2/3] [ticket/15219] Add missing space PHPBB3-15219 --- phpBB/phpbb/cron/task/core/update_hashes.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/phpbb/cron/task/core/update_hashes.php b/phpBB/phpbb/cron/task/core/update_hashes.php index 458853f2fd..a4fe477d99 100644 --- a/phpBB/phpbb/cron/task/core/update_hashes.php +++ b/phpBB/phpbb/cron/task/core/update_hashes.php @@ -113,7 +113,7 @@ class update_hashes extends \phpbb\cron\task\base $sql = 'UPDATE ' . USERS_TABLE . ' SET user_password = "' . $this->db->sql_escape($new_hash) . '" - WHERE user_id = ' . (int)$row['user_id']; + WHERE user_id = ' . (int) $row['user_id']; $this->db->sql_query($sql); } From 170613848ad86cea1f140ed1cfdb55d581324ffb Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sat, 3 Jun 2017 11:07:34 +0200 Subject: [PATCH 3/3] [ticket/15219] Add console command for updating hashes to bcrypt PHPBB3-15219 --- phpBB/config/console.yml | 12 ++ phpBB/language/en/cli.php | 2 + .../console/command/fixup/update_hashes.php | 117 ++++++++++++++++++ 3 files changed, 131 insertions(+) create mode 100644 phpBB/phpbb/console/command/fixup/update_hashes.php diff --git a/phpBB/config/console.yml b/phpBB/config/console.yml index 1e18a7dd37..4118803663 100644 --- a/phpBB/config/console.yml +++ b/phpBB/config/console.yml @@ -139,3 +139,15 @@ services: - @dbal.conn tags: - { name: console.command } + + console.command.fixup.update_hashes: + class: phpbb\console\command\fixup\update_hashes + arguments: + - @config + - @user + - @dbal.conn + - @passwords.manager + - @passwords.driver_collection + - %passwords.algorithms% + tags: + - { name: console.command } diff --git a/phpBB/language/en/cli.php b/phpBB/language/en/cli.php index 6989f26f72..27e72d5ccd 100644 --- a/phpBB/language/en/cli.php +++ b/phpBB/language/en/cli.php @@ -64,6 +64,7 @@ $lang = array_merge($lang, array( 'CLI_DESCRIPTION_RECALCULATE_EMAIL_HASH' => 'Recalculates the user_email_hash column of the users table.', 'CLI_DESCRIPTION_SET_ATOMIC_CONFIG' => 'Sets a configuration option’s value only if the old matches the current value', 'CLI_DESCRIPTION_SET_CONFIG' => 'Sets a configuration option’s value', + 'CLI_DESCRIPTION_UPDATE_HASH_BCRYPT' => 'Updates outdated password hashes to be hashed with bcrypt.', 'CLI_EXTENSION_DISABLE_FAILURE' => 'Could not disable extension %s', 'CLI_EXTENSION_DISABLE_SUCCESS' => 'Successfully disabled extension %s', @@ -78,6 +79,7 @@ $lang = array_merge($lang, array( 'CLI_EXTENSIONS_ENABLED' => 'Enabled', 'CLI_FIXUP_RECALCULATE_EMAIL_HASH_SUCCESS' => 'Successfully recalculated all email hashes.', + 'CLI_FIXUP_UPDATE_HASH_BCRYPT_SUCCESS' => 'Successfully updated outdated password hashes to bcrypt.' )); // Additional help for commands. diff --git a/phpBB/phpbb/console/command/fixup/update_hashes.php b/phpBB/phpbb/console/command/fixup/update_hashes.php new file mode 100644 index 0000000000..4bcc3b5d19 --- /dev/null +++ b/phpBB/phpbb/console/command/fixup/update_hashes.php @@ -0,0 +1,117 @@ + +* @license GNU General Public License, version 2 (GPL-2.0) +* +* For full copyright and license information, please see +* the docs/CREDITS.txt file. +* +*/ +namespace phpbb\console\command\fixup; + +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Output\OutputInterface; +use Symfony\Component\Console\Helper\ProgressBar; + +class update_hashes extends \phpbb\console\command\command +{ + /** @var \phpbb\config\config */ + protected $config; + + /** @var \phpbb\db\driver\driver_interface */ + protected $db; + + /** @var \phpbb\passwords\manager */ + protected $passwords_manager; + + /** @var string Default hashing type */ + protected $default_type; + + /** + * Update_hashes constructor + * + * @param \phpbb\config\config $config + * @param \phpbb\user $user + * @param \phpbb\db\driver\driver_interface $db + * @param \phpbb\passwords\manager $passwords_manager + * @param array $hashing_algorithms Hashing driver + * service collection + * @param array $defaults Default password types + */ + public function __construct(\phpbb\config\config $config, \phpbb\user $user, + \phpbb\db\driver\driver_interface $db, \phpbb\passwords\manager $passwords_manager, + $hashing_algorithms, $defaults) + { + $this->config = $config; + $this->db = $db; + + $this->passwords_manager = $passwords_manager; + + foreach ($defaults as $type) + { + if ($hashing_algorithms[$type]->is_supported()) + { + $this->default_type = $type; + break; + } + } + + parent::__construct($user); + } + + /** + * {@inheritdoc} + */ + protected function configure() + { + $this + ->setName('fixup:update-hashes') + ->setDescription($this->user->lang('CLI_DESCRIPTION_UPDATE_HASH_BCRYPT')) + ; + } + + /** + * {@inheritdoc} + */ + protected function execute(InputInterface $input, OutputInterface $output) + { + // Get count to be able to display progress + $sql = 'SELECT COUNT(user_id) AS count + FROM ' . USERS_TABLE . ' + WHERE user_password ' . $this->db->sql_like_expression('$H$' . $this->db->get_any_char()) . ' + OR user_password ' . $this->db->sql_like_expression('$CP$' . $this->db->get_any_char()); + $result = $this->db->sql_query($sql); + $total_update_passwords = $this->db->sql_fetchfield('count'); + $this->db->sql_freeresult($result); + + // Create progress bar + $progress_bar = new ProgressBar($output, $total_update_passwords); + $progress_bar->start(); + + $sql = 'SELECT user_id, user_password + FROM ' . USERS_TABLE . ' + WHERE user_password ' . $this->db->sql_like_expression('$H$' . $this->db->get_any_char()) . ' + OR user_password ' . $this->db->sql_like_expression('$CP$' . $this->db->get_any_char()); + $result = $this->db->sql_query($sql); + + while ($row = $this->db->sql_fetchrow($result)) + { + $new_hash = $this->passwords_manager->hash($row['user_password'], array($this->default_type)); + + $sql = 'UPDATE ' . USERS_TABLE . ' + SET user_password = "' . $this->db->sql_escape($new_hash) . '" + WHERE user_id = ' . (int) $row['user_id']; + $this->db->sql_query($sql); + $progress_bar->advance(); + } + + $this->config->set('update_hashes_last_cron', time()); + + $progress_bar->finish(); + + $output->writeln('' . $this->user->lang('CLI_FIXUP_UPDATE_HASH_BCRYPT_SUCCESS') . ''); + } +}