From 25f5e4f18f9aed7272b3f3f9e8647c287b689b29 Mon Sep 17 00:00:00 2001 From: Tristan Darricau Date: Thu, 17 Sep 2015 15:36:04 +0200 Subject: [ticket/12577] Lazy initialize the password manager PHPBB3-12577 --- phpBB/phpbb/passwords/manager.php | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) (limited to 'phpBB/phpbb/passwords') diff --git a/phpBB/phpbb/passwords/manager.php b/phpBB/phpbb/passwords/manager.php index aa9147ecf4..03729815ba 100644 --- a/phpBB/phpbb/passwords/manager.php +++ b/phpBB/phpbb/passwords/manager.php @@ -49,6 +49,10 @@ class manager */ protected $config; + private $initialized = false; + private $hashing_algorithms; + private $defaults; + /** * Construct a passwords object * @@ -62,9 +66,18 @@ class manager { $this->config = $config; $this->helper = $helper; + $this->hashing_algorithms = $hashing_algorithms; + $this->defaults = $defaults; + } - $this->fill_type_map($hashing_algorithms); - $this->register_default_type($defaults); + protected function initialize() + { + if (!$this->initialized) + { + $this->initialized = true; + $this->fill_type_map($this->hashing_algorithms); + $this->register_default_type($this->defaults); + } } /** @@ -144,6 +157,8 @@ class manager return false; } + $this->initialize(); + // Be on the lookout for multiple hashing algorithms // 2 is correct: H\2a > 2, H\P > 2 if (strlen($match[1]) > 2) @@ -192,6 +207,8 @@ class manager return false; } + $this->initialize(); + // Try to retrieve algorithm by service name if type doesn't // start with dollar sign if (!is_array($type) && strpos($type, '$') !== 0 && isset($this->algorithms[$type])) @@ -242,6 +259,8 @@ class manager return false; } + $this->initialize(); + // First find out what kind of hash we're dealing with $stored_hash_type = $this->detect_algorithm($hash); if ($stored_hash_type == false) @@ -297,6 +316,8 @@ class manager */ public function combined_hash_password($password_hash, $type) { + $this->initialize(); + $data = array( 'prefix' => '$', 'settings' => '$', -- cgit v1.2.1 From f9f7f935b4fe2742cd84d100be1dc03fd66919ec Mon Sep 17 00:00:00 2001 From: Tristan Darricau Date: Fri, 18 Sep 2015 19:54:06 +0200 Subject: [ticket/12577] Docblock PHPBB3-12577 --- phpBB/phpbb/passwords/manager.php | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) (limited to 'phpBB/phpbb/passwords') diff --git a/phpBB/phpbb/passwords/manager.php b/phpBB/phpbb/passwords/manager.php index 03729815ba..b2caba81f2 100644 --- a/phpBB/phpbb/passwords/manager.php +++ b/phpBB/phpbb/passwords/manager.php @@ -49,18 +49,28 @@ class manager */ protected $config; + /** + * @var bool Whether or not initialized() has been called + */ private $initialized = false; + + /** + * @var array Hashing driver service collection + */ private $hashing_algorithms; + + /** + * @var array List of default driver types + */ private $defaults; /** * Construct a passwords object * - * @param \phpbb\config\config $config phpBB configuration - * @param array $hashing_algorithms Hashing driver - * service collection - * @param \phpbb\passwords\helper $helper Passwords helper object - * @param array $defaults List of default driver types + * @param \phpbb\config\config $config phpBB configuration + * @param array $hashing_algorithms Hashing driver service collection + * @param \phpbb\passwords\helper $helper Passwords helper object + * @param array $defaults List of default driver types */ public function __construct(\phpbb\config\config $config, $hashing_algorithms, helper $helper, $defaults) { @@ -70,6 +80,9 @@ class manager $this->defaults = $defaults; } + /** + * Initialize the internal state + */ protected function initialize() { if (!$this->initialized) -- cgit v1.2.1 From 1d40c0f43b366638de16a99a874ce1475249ade0 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Tue, 9 Aug 2016 21:07:49 +0200 Subject: [ticket/14733] Support increasing hashing cost factor PHPBB3-14733 --- phpBB/phpbb/passwords/driver/base.php | 8 ++++++ phpBB/phpbb/passwords/driver/bcrypt.php | 31 ++++++++++++++++++++++- phpBB/phpbb/passwords/driver/driver_interface.php | 8 ++++++ phpBB/phpbb/passwords/manager.php | 2 +- 4 files changed, 47 insertions(+), 2 deletions(-) (limited to 'phpBB/phpbb/passwords') diff --git a/phpBB/phpbb/passwords/driver/base.php b/phpBB/phpbb/passwords/driver/base.php index fd07a61bf4..49b96af372 100644 --- a/phpBB/phpbb/passwords/driver/base.php +++ b/phpBB/phpbb/passwords/driver/base.php @@ -52,6 +52,14 @@ abstract class base implements driver_interface return false; } + /** + * {@inheritdoc} + */ + public function needs_rehash($hash) + { + return false; + } + /** * {@inheritdoc} */ diff --git a/phpBB/phpbb/passwords/driver/bcrypt.php b/phpBB/phpbb/passwords/driver/bcrypt.php index eab1c3d569..39fb5e5cf1 100644 --- a/phpBB/phpbb/passwords/driver/bcrypt.php +++ b/phpBB/phpbb/passwords/driver/bcrypt.php @@ -17,6 +17,23 @@ class bcrypt extends base { const PREFIX = '$2a$'; + /** @var int Hashing cost factor */ + protected $cost_factor; + + /** + * Constructor of passwords driver object + * + * @param \phpbb\config\config $config phpBB config + * @param \phpbb\passwords\driver\helper $helper Password driver helper + */ + public function __construct(\phpbb\config\config $config, helper $helper, $cost_factor) + { + parent::__construct($config, $helper); + + // Don't allow cost factor to be below default setting + $this->cost_factor = max(10, $cost_factor); + } + /** * {@inheritdoc} */ @@ -25,6 +42,18 @@ class bcrypt extends base return self::PREFIX; } + /** + * {@inheritdoc} + */ + public function needs_rehash($hash) + { + preg_match('/^' . preg_quote($this->get_prefix()) . '([0-9]+)\$/', $hash, $matches); + + list(, $cost_factor) = $matches; + + return empty($cost_factor) || $this->cost_factor !== intval($cost_factor); + } + /** * {@inheritdoc} */ @@ -46,7 +75,7 @@ class bcrypt extends base if ($salt == '') { - $salt = $prefix . '10$' . $this->get_random_salt(); + $salt = $prefix . $this->cost_factor . '$' . $this->get_random_salt(); } $hash = crypt($password, $salt); diff --git a/phpBB/phpbb/passwords/driver/driver_interface.php b/phpBB/phpbb/passwords/driver/driver_interface.php index 3974484f13..6a660b80ea 100644 --- a/phpBB/phpbb/passwords/driver/driver_interface.php +++ b/phpBB/phpbb/passwords/driver/driver_interface.php @@ -29,6 +29,14 @@ interface driver_interface */ public function is_legacy(); + /** + * Check if password needs to be rehashed + * + * @param string $hash Hash to check for rehash + * @return bool True if password needs to be rehashed, false if not + */ + public function needs_rehash($hash); + /** * Returns the hash prefix * diff --git a/phpBB/phpbb/passwords/manager.php b/phpBB/phpbb/passwords/manager.php index b2caba81f2..38c12995b4 100644 --- a/phpBB/phpbb/passwords/manager.php +++ b/phpBB/phpbb/passwords/manager.php @@ -297,7 +297,7 @@ class manager } else { - $this->convert_flag = false; + $this->convert_flag = $stored_hash_type->needs_rehash($hash); } // Check all legacy hash types if prefix is $CP$ -- cgit v1.2.1 From 297376ee949f3afe6ad2bd6849be8b8115a1adbb Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Tue, 16 Aug 2016 21:08:00 +0200 Subject: [ticket/14733] Use default cost factor in bcrypt constructor PHPBB3-14733 --- phpBB/phpbb/passwords/driver/bcrypt.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'phpBB/phpbb/passwords') diff --git a/phpBB/phpbb/passwords/driver/bcrypt.php b/phpBB/phpbb/passwords/driver/bcrypt.php index 39fb5e5cf1..eb1aeeeb76 100644 --- a/phpBB/phpbb/passwords/driver/bcrypt.php +++ b/phpBB/phpbb/passwords/driver/bcrypt.php @@ -25,8 +25,9 @@ class bcrypt extends base * * @param \phpbb\config\config $config phpBB config * @param \phpbb\passwords\driver\helper $helper Password driver helper + * @param int $cost_factor Hashing cost factor (optional) */ - public function __construct(\phpbb\config\config $config, helper $helper, $cost_factor) + public function __construct(\phpbb\config\config $config, helper $helper, $cost_factor = 10) { parent::__construct($config, $helper); -- cgit v1.2.1 From d15269950d8f577a69f3359614d48087c84d4cec Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 25 Sep 2016 18:00:49 +0200 Subject: [ticket/14733] Use new interface to preserve backwards compatibility PHPBB3-14733 --- phpBB/phpbb/passwords/driver/base.php | 4 +- phpBB/phpbb/passwords/driver/driver_interface.php | 8 --- .../driver/rehashable_driver_interface.php | 77 ++++++++++++++++++++++ 3 files changed, 79 insertions(+), 10 deletions(-) create mode 100644 phpBB/phpbb/passwords/driver/rehashable_driver_interface.php (limited to 'phpBB/phpbb/passwords') diff --git a/phpBB/phpbb/passwords/driver/base.php b/phpBB/phpbb/passwords/driver/base.php index 49b96af372..0997b5b700 100644 --- a/phpBB/phpbb/passwords/driver/base.php +++ b/phpBB/phpbb/passwords/driver/base.php @@ -13,7 +13,7 @@ namespace phpbb\passwords\driver; -abstract class base implements driver_interface +abstract class base implements rehashable_driver_interface { /** @var \phpbb\config\config */ protected $config; @@ -21,7 +21,7 @@ abstract class base implements driver_interface /** @var \phpbb\passwords\driver\helper */ protected $helper; - /** @var driver name */ + /** @var string Driver name */ protected $name; /** diff --git a/phpBB/phpbb/passwords/driver/driver_interface.php b/phpBB/phpbb/passwords/driver/driver_interface.php index 6a660b80ea..3974484f13 100644 --- a/phpBB/phpbb/passwords/driver/driver_interface.php +++ b/phpBB/phpbb/passwords/driver/driver_interface.php @@ -29,14 +29,6 @@ interface driver_interface */ public function is_legacy(); - /** - * Check if password needs to be rehashed - * - * @param string $hash Hash to check for rehash - * @return bool True if password needs to be rehashed, false if not - */ - public function needs_rehash($hash); - /** * Returns the hash prefix * diff --git a/phpBB/phpbb/passwords/driver/rehashable_driver_interface.php b/phpBB/phpbb/passwords/driver/rehashable_driver_interface.php new file mode 100644 index 0000000000..c22f41cf6b --- /dev/null +++ b/phpBB/phpbb/passwords/driver/rehashable_driver_interface.php @@ -0,0 +1,77 @@ + +* @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\passwords\driver; + +interface rehashable_driver_interface +{ + /** + * Check if hash type is supported + * + * @return bool True if supported, false if not + */ + public function is_supported(); + + /** + * Check if hash type is a legacy hash type + * + * @return bool True if it's a legacy hash type, false if not + */ + public function is_legacy(); + + /** + * Check if password needs to be rehashed + * + * @param string $hash Hash to check for rehash + * @return bool True if password needs to be rehashed, false if not + */ + public function needs_rehash($hash); + + /** + * Returns the hash prefix + * + * @return string Hash prefix + */ + public function get_prefix(); + + /** + * Hash the password + * + * @param string $password The password that should be hashed + * + * @return bool|string Password hash or false if something went wrong + * during hashing + */ + public function hash($password); + + /** + * Check the password against the supplied hash + * + * @param string $password The password to check + * @param string $hash The password hash to check against + * @param array $user_row User's row in users table + * + * @return bool True if password is correct, else false + */ + public function check($password, $hash, $user_row = array()); + + /** + * Get only the settings of the specified hash + * + * @param string $hash Password hash + * @param bool $full Return full settings or only settings + * related to the salt + * @return string String containing the hash settings + */ + public function get_settings_only($hash, $full = false); +} -- cgit v1.2.1 From 722639a0e213e905cfb4a01aa54e638f7670ba63 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 25 Sep 2016 20:32:42 +0200 Subject: [ticket/14733] Extend passwords driver_interface in rehashable_driver_interface PHPBB3-14733 --- .../driver/rehashable_driver_interface.php | 54 +--------------------- phpBB/phpbb/passwords/manager.php | 9 +++- 2 files changed, 9 insertions(+), 54 deletions(-) (limited to 'phpBB/phpbb/passwords') diff --git a/phpBB/phpbb/passwords/driver/rehashable_driver_interface.php b/phpBB/phpbb/passwords/driver/rehashable_driver_interface.php index c22f41cf6b..ca30748502 100644 --- a/phpBB/phpbb/passwords/driver/rehashable_driver_interface.php +++ b/phpBB/phpbb/passwords/driver/rehashable_driver_interface.php @@ -13,22 +13,8 @@ namespace phpbb\passwords\driver; -interface rehashable_driver_interface +interface rehashable_driver_interface extends driver_interface { - /** - * Check if hash type is supported - * - * @return bool True if supported, false if not - */ - public function is_supported(); - - /** - * Check if hash type is a legacy hash type - * - * @return bool True if it's a legacy hash type, false if not - */ - public function is_legacy(); - /** * Check if password needs to be rehashed * @@ -36,42 +22,4 @@ interface rehashable_driver_interface * @return bool True if password needs to be rehashed, false if not */ public function needs_rehash($hash); - - /** - * Returns the hash prefix - * - * @return string Hash prefix - */ - public function get_prefix(); - - /** - * Hash the password - * - * @param string $password The password that should be hashed - * - * @return bool|string Password hash or false if something went wrong - * during hashing - */ - public function hash($password); - - /** - * Check the password against the supplied hash - * - * @param string $password The password to check - * @param string $hash The password hash to check against - * @param array $user_row User's row in users table - * - * @return bool True if password is correct, else false - */ - public function check($password, $hash, $user_row = array()); - - /** - * Get only the settings of the specified hash - * - * @param string $hash Password hash - * @param bool $full Return full settings or only settings - * related to the salt - * @return string String containing the hash settings - */ - public function get_settings_only($hash, $full = false); } diff --git a/phpBB/phpbb/passwords/manager.php b/phpBB/phpbb/passwords/manager.php index 38c12995b4..6c3ef4c477 100644 --- a/phpBB/phpbb/passwords/manager.php +++ b/phpBB/phpbb/passwords/manager.php @@ -297,7 +297,14 @@ class manager } else { - $this->convert_flag = $stored_hash_type->needs_rehash($hash); + if ($stored_hash_type instanceof driver\rehashable_driver_interface) + { + $this->convert_flag = $stored_hash_type->needs_rehash($hash); + } + else + { + $this->convert_flag = false; + } } // Check all legacy hash types if prefix is $CP$ -- cgit v1.2.1 From 380be9f1fd713dbcee91f12f18060d6b3ff4819e Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 25 Sep 2016 20:33:10 +0200 Subject: [ticket/14733] Make sure detect_algorithm() works correctly and add tests detect_algorithm() returned array() if an algorithm prefix was more than 2 characters long. This might have been invalid for other prefixes. In order to correctly cope with other prefixes, another check for a backslash in the prefix definitino has been added. This was discovered while writing the tests for the newly added interface. PHPBB3-14733 --- phpBB/phpbb/passwords/manager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'phpBB/phpbb/passwords') diff --git a/phpBB/phpbb/passwords/manager.php b/phpBB/phpbb/passwords/manager.php index 6c3ef4c477..fad76a9fe5 100644 --- a/phpBB/phpbb/passwords/manager.php +++ b/phpBB/phpbb/passwords/manager.php @@ -174,7 +174,7 @@ class manager // Be on the lookout for multiple hashing algorithms // 2 is correct: H\2a > 2, H\P > 2 - if (strlen($match[1]) > 2) + if (strlen($match[1]) > 2 && strpos($match[1], '\\') !== false) { $hash_types = explode('\\', $match[1]); $return_ary = array(); -- cgit v1.2.1 From 1d5f5ccffcfd30a652734485b51066950bbb8a76 Mon Sep 17 00:00:00 2001 From: javiexin Date: Sun, 21 May 2017 12:58:05 +0200 Subject: [ticket/15227] Remove STRIP, as always false PHPBB3-15227 --- phpBB/phpbb/passwords/driver/md5_phpbb2.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'phpBB/phpbb/passwords') diff --git a/phpBB/phpbb/passwords/driver/md5_phpbb2.php b/phpBB/phpbb/passwords/driver/md5_phpbb2.php index bd8cc51e5a..b38b041d6c 100644 --- a/phpBB/phpbb/passwords/driver/md5_phpbb2.php +++ b/phpBB/phpbb/passwords/driver/md5_phpbb2.php @@ -95,7 +95,7 @@ class md5_phpbb2 extends base // in phpBB2 passwords were used exactly as they were sent, with addslashes applied $password_old_format = isset($_REQUEST['password']) ? (string) $_REQUEST['password'] : ''; - $password_old_format = (!STRIP) ? addslashes($password_old_format) : $password_old_format; + $password_old_format = addslashes($password_old_format); $password_new_format = $this->request->variable('password', '', true); if ($super_globals_disabled) -- cgit v1.2.1