From 186a3d40c60b4d5f11e6f399737557ef08913078 Mon Sep 17 00:00:00 2001 From: rxu Date: Thu, 26 Dec 2019 19:44:22 +0700 Subject: [ticket/16266] Fix argon2 driver issue for Sodium implementation PHPBB3-16266 --- phpBB/phpbb/passwords/driver/argon2i.php | 19 +++++++++++++++---- phpBB/phpbb/passwords/driver/base_native.php | 12 ++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/phpBB/phpbb/passwords/driver/argon2i.php b/phpBB/phpbb/passwords/driver/argon2i.php index 49d7d6393e..3babbaa780 100644 --- a/phpBB/phpbb/passwords/driver/argon2i.php +++ b/phpBB/phpbb/passwords/driver/argon2i.php @@ -37,10 +37,21 @@ class argon2i extends base_native { parent::__construct($config, $helper); - // Don't allow cost factors to be below default settings - $this->memory_cost = max($memory_cost, 1024); - $this->threads = max($threads, 2); - $this->time_cost = max($time_cost, 2); + if ($this->is_sodium()) + { + // For Sodium implementation, set special cost factor values (since PHP 7.4) + // See https://wiki.php.net/rfc/sodium.argon.hash and PHPBB3-16266 + $this->memory_cost = max($memory_cost, 256*1024); + $this->threads = 1; + $this->time_cost = max($time_cost, 3); + } + else + { + // Otherwise don't allow cost factors to be below default settings + $this->memory_cost = max($memory_cost, 1024); + $this->threads = max($threads, 2); + $this->time_cost = max($time_cost, 2); + } } /** diff --git a/phpBB/phpbb/passwords/driver/base_native.php b/phpBB/phpbb/passwords/driver/base_native.php index 87498327f9..31d3465165 100644 --- a/phpBB/phpbb/passwords/driver/base_native.php +++ b/phpBB/phpbb/passwords/driver/base_native.php @@ -57,6 +57,18 @@ abstract class base_native extends base return password_hash($password, $this->get_algo_value(), $this->get_options()); } + /** + * Check if Sodium implementation for argon2 algorithm is being used + * + * @link https://wiki.php.net/rfc/sodium.argon.hash + * + * @return bool + */ + public function is_sodium() + { + return defined('PASSWORD_ARGON2_PROVIDER') && PASSWORD_ARGON2_PROVIDER == 'sodium'; + } + /** * {@inheritdoc} */ -- cgit v1.2.1 From 5dfba1b06473ecb0298d9b61fd9dec28ac60f884 Mon Sep 17 00:00:00 2001 From: rxu Date: Sat, 28 Dec 2019 00:15:08 +0700 Subject: [ticket/16266] Optimize code PHPBB3-16266 --- phpBB/phpbb/passwords/driver/argon2i.php | 21 ++++++--------------- phpBB/phpbb/passwords/driver/base_native.php | 9 ++++++++- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/phpBB/phpbb/passwords/driver/argon2i.php b/phpBB/phpbb/passwords/driver/argon2i.php index 3babbaa780..9aa2b6f14c 100644 --- a/phpBB/phpbb/passwords/driver/argon2i.php +++ b/phpBB/phpbb/passwords/driver/argon2i.php @@ -37,21 +37,12 @@ class argon2i extends base_native { parent::__construct($config, $helper); - if ($this->is_sodium()) - { - // For Sodium implementation, set special cost factor values (since PHP 7.4) - // See https://wiki.php.net/rfc/sodium.argon.hash and PHPBB3-16266 - $this->memory_cost = max($memory_cost, 256*1024); - $this->threads = 1; - $this->time_cost = max($time_cost, 3); - } - else - { - // Otherwise don't allow cost factors to be below default settings - $this->memory_cost = max($memory_cost, 1024); - $this->threads = max($threads, 2); - $this->time_cost = max($time_cost, 2); - } + // For Sodium implementation, set special cost factor values (since PHP 7.4) + // See https://wiki.php.net/rfc/sodium.argon.hash and PHPBB3-16266 + // Otherwise don't allow cost factors to be below default settings + $this->memory_cost = ($this->is_sodium()) ? max($memory_cost, 256*1024) : max($memory_cost, 1024); + $this->threads = ($this->is_sodium()) ? 1 : max($threads, 2); + $this->time_cost = ($this->is_sodium()) ? max($time_cost, 3) : max($time_cost, 2); } /** diff --git a/phpBB/phpbb/passwords/driver/base_native.php b/phpBB/phpbb/passwords/driver/base_native.php index 31d3465165..fa4f0995a5 100644 --- a/phpBB/phpbb/passwords/driver/base_native.php +++ b/phpBB/phpbb/passwords/driver/base_native.php @@ -66,7 +66,14 @@ abstract class base_native extends base */ public function is_sodium() { - return defined('PASSWORD_ARGON2_PROVIDER') && PASSWORD_ARGON2_PROVIDER == 'sodium'; + static $is_sodium; + + if (empty($is_sodium)) + { + $is_sodium = defined('PASSWORD_ARGON2_PROVIDER') && PASSWORD_ARGON2_PROVIDER == 'sodium'; + } + + return $is_sodium; } /** -- cgit v1.2.1 From a750372a030c343b4f158be23d8aa3901c6094f3 Mon Sep 17 00:00:00 2001 From: rxu Date: Sat, 28 Dec 2019 01:04:13 +0700 Subject: [ticket/16266] More code optimizing PHPBB3-16266 --- phpBB/phpbb/passwords/driver/argon2i.php | 8 +++++--- phpBB/phpbb/passwords/driver/base_native.php | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/phpBB/phpbb/passwords/driver/argon2i.php b/phpBB/phpbb/passwords/driver/argon2i.php index 9aa2b6f14c..575fbf05bd 100644 --- a/phpBB/phpbb/passwords/driver/argon2i.php +++ b/phpBB/phpbb/passwords/driver/argon2i.php @@ -37,9 +37,11 @@ class argon2i extends base_native { parent::__construct($config, $helper); - // For Sodium implementation, set special cost factor values (since PHP 7.4) - // See https://wiki.php.net/rfc/sodium.argon.hash and PHPBB3-16266 - // Otherwise don't allow cost factors to be below default settings + /** + * For Sodium implementation of argon2 algorithm, set special cost factor values (since PHP 7.4) + * See https://wiki.php.net/rfc/sodium.argon.hash and PHPBB3-16266 + * Don't allow cost factors to be below default settings where possible + */ $this->memory_cost = ($this->is_sodium()) ? max($memory_cost, 256*1024) : max($memory_cost, 1024); $this->threads = ($this->is_sodium()) ? 1 : max($threads, 2); $this->time_cost = ($this->is_sodium()) ? max($time_cost, 3) : max($time_cost, 2); diff --git a/phpBB/phpbb/passwords/driver/base_native.php b/phpBB/phpbb/passwords/driver/base_native.php index fa4f0995a5..ab2e9f83a4 100644 --- a/phpBB/phpbb/passwords/driver/base_native.php +++ b/phpBB/phpbb/passwords/driver/base_native.php @@ -68,7 +68,7 @@ abstract class base_native extends base { static $is_sodium; - if (empty($is_sodium)) + if (!isset($is_sodium)) { $is_sodium = defined('PASSWORD_ARGON2_PROVIDER') && PASSWORD_ARGON2_PROVIDER == 'sodium'; } -- cgit v1.2.1 From d000717d341a2c12099b0fba3ab677bbb0f2340c Mon Sep 17 00:00:00 2001 From: rxu Date: Sat, 28 Dec 2019 02:11:58 +0700 Subject: [ticket/16266] More code optimizing PHPBB3-16266 --- phpBB/phpbb/passwords/driver/argon2i.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/phpBB/phpbb/passwords/driver/argon2i.php b/phpBB/phpbb/passwords/driver/argon2i.php index 575fbf05bd..f4a6e3e644 100644 --- a/phpBB/phpbb/passwords/driver/argon2i.php +++ b/phpBB/phpbb/passwords/driver/argon2i.php @@ -42,9 +42,9 @@ class argon2i extends base_native * See https://wiki.php.net/rfc/sodium.argon.hash and PHPBB3-16266 * Don't allow cost factors to be below default settings where possible */ - $this->memory_cost = ($this->is_sodium()) ? max($memory_cost, 256*1024) : max($memory_cost, 1024); - $this->threads = ($this->is_sodium()) ? 1 : max($threads, 2); - $this->time_cost = ($this->is_sodium()) ? max($time_cost, 3) : max($time_cost, 2); + $this->memory_cost = $this->is_sodium() ? max($memory_cost, 256 * 1024) : max($memory_cost, 1024); + $this->threads = $this->is_sodium() ? 1 : max($threads, 2); + $this->time_cost = $this->is_sodium() ? max($time_cost, 3) : max($time_cost, 2); } /** -- cgit v1.2.1 From 3669849368b8b39d661e08c2476c510cd4fc7445 Mon Sep 17 00:00:00 2001 From: rxu Date: Sat, 28 Dec 2019 12:20:51 +0700 Subject: [ticket/16266] Refactor patch using argon2 predefined constants PHPBB3-16266 --- phpBB/phpbb/passwords/driver/argon2i.php | 9 +++++---- phpBB/phpbb/passwords/driver/base_native.php | 19 ------------------- 2 files changed, 5 insertions(+), 23 deletions(-) diff --git a/phpBB/phpbb/passwords/driver/argon2i.php b/phpBB/phpbb/passwords/driver/argon2i.php index f4a6e3e644..f622ad889b 100644 --- a/phpBB/phpbb/passwords/driver/argon2i.php +++ b/phpBB/phpbb/passwords/driver/argon2i.php @@ -38,13 +38,14 @@ class argon2i extends base_native parent::__construct($config, $helper); /** - * For Sodium implementation of argon2 algorithm, set special cost factor values (since PHP 7.4) + * For Sodium implementation of argon2 algorithm (since PHP 7.4), set special value of 1 for "threads" cost factor * See https://wiki.php.net/rfc/sodium.argon.hash and PHPBB3-16266 * Don't allow cost factors to be below default settings where possible */ - $this->memory_cost = $this->is_sodium() ? max($memory_cost, 256 * 1024) : max($memory_cost, 1024); - $this->threads = $this->is_sodium() ? 1 : max($threads, 2); - $this->time_cost = $this->is_sodium() ? max($time_cost, 3) : max($time_cost, 2); + $this->memory_cost = max($memory_cost, PASSWORD_ARGON2_DEFAULT_MEMORY_COST); + $this->time_cost = max($time_cost, PASSWORD_ARGON2_DEFAULT_TIME_COST); + $this->threads = (defined('PASSWORD_ARGON2_PROVIDER') && PASSWORD_ARGON2_PROVIDER == 'sodium') ? + PASSWORD_ARGON2_DEFAULT_THREADS : max($threads, PASSWORD_ARGON2_DEFAULT_THREADS); } /** diff --git a/phpBB/phpbb/passwords/driver/base_native.php b/phpBB/phpbb/passwords/driver/base_native.php index ab2e9f83a4..87498327f9 100644 --- a/phpBB/phpbb/passwords/driver/base_native.php +++ b/phpBB/phpbb/passwords/driver/base_native.php @@ -57,25 +57,6 @@ abstract class base_native extends base return password_hash($password, $this->get_algo_value(), $this->get_options()); } - /** - * Check if Sodium implementation for argon2 algorithm is being used - * - * @link https://wiki.php.net/rfc/sodium.argon.hash - * - * @return bool - */ - public function is_sodium() - { - static $is_sodium; - - if (!isset($is_sodium)) - { - $is_sodium = defined('PASSWORD_ARGON2_PROVIDER') && PASSWORD_ARGON2_PROVIDER == 'sodium'; - } - - return $is_sodium; - } - /** * {@inheritdoc} */ -- cgit v1.2.1 From 37fddf8eef49f8fa185c7f0e808e2c6f767c28fd Mon Sep 17 00:00:00 2001 From: rxu Date: Sat, 28 Dec 2019 21:10:58 +0700 Subject: [ticket/16266] Fix tests PHPBB3-16266 --- tests/passwords/drivers_test.php | 54 ++++++++++++++++++++++++++++++++-------- 1 file changed, 43 insertions(+), 11 deletions(-) diff --git a/tests/passwords/drivers_test.php b/tests/passwords/drivers_test.php index 8300431dfe..e5343010bc 100644 --- a/tests/passwords/drivers_test.php +++ b/tests/passwords/drivers_test.php @@ -21,10 +21,15 @@ class phpbb_passwords_helper_test extends \phpbb_test_case $this->driver_helper = new \phpbb\passwords\driver\helper($config); $phpbb_root_path = dirname(__FILE__) . '/../../phpBB/'; $php_ext = 'php'; + + // Initialize argon2 default options + $this->argon2_default_cost_options = [ + 'memory_cost' => 1024, + 'time_cost' => 2, + 'threads' => 2 + ]; $this->passwords_drivers = array( - 'passwords.driver.argon2i' => new \phpbb\passwords\driver\argon2i($config, $this->driver_helper), - 'passwords.driver.argon2id' => new \phpbb\passwords\driver\argon2id($config, $this->driver_helper), 'passwords.driver.bcrypt_2y' => new \phpbb\passwords\driver\bcrypt_2y($config, $this->driver_helper, 10), 'passwords.driver.bcrypt' => new \phpbb\passwords\driver\bcrypt($config, $this->driver_helper, 10), 'passwords.driver.salted_md5' => new \phpbb\passwords\driver\salted_md5($config, $this->driver_helper), @@ -39,6 +44,19 @@ class phpbb_passwords_helper_test extends \phpbb_test_case ); $this->passwords_drivers['passwords.driver.md5_phpbb2'] = new \phpbb\passwords\driver\md5_phpbb2($request, $this->passwords_drivers['passwords.driver.salted_md5'], $this->driver_helper, $phpbb_root_path, $php_ext); $this->passwords_drivers['passwords.driver.bcrypt_wcf2'] = new \phpbb\passwords\driver\bcrypt_wcf2($this->passwords_drivers['passwords.driver.bcrypt'], $this->driver_helper); + + $pwhash_supported = function_exists('password_hash') && function_exists('password_needs_rehash') && function_exists('password_verify'); + if (defined('PASSWORD_ARGON2I') && $pwhash_supported) + { + $this->passwords_drivers['passwords.driver.argon2i'] = new \phpbb\passwords\driver\argon2i($config, $this->driver_helper); + $this->argon2_default_cost_options = $this->passwords_drivers['passwords.driver.argon2i']->get_options(); + } + + if (defined('PASSWORD_ARGON2ID') && $pwhash_supported) + { + $this->passwords_drivers['passwords.driver.argon2id'] = new \phpbb\passwords\driver\argon2id($config, $this->driver_helper); + $this->argon2_default_cost_options = $this->passwords_drivers['passwords.driver.argon2id']->get_options(); + } } public function data_helper_encode64() @@ -418,20 +436,34 @@ class phpbb_passwords_helper_test extends \phpbb_test_case public function data_needs_rehash() { - return array( + $data_array = [ array('passwords.driver.bcrypt_2y', '$2y$10$somerandomhash', false), array('passwords.driver.bcrypt', '$2a$10$somerandomhash', false), array('passwords.driver.salted_md5', 'foobar', false), array('passwords.driver.bcrypt_2y', '$2y$9$somerandomhash', true), array('passwords.driver.bcrypt', '$2a$04$somerandomhash', true), - array('passwords.driver.argon2i', '$argon2i$v=19$m=1024,t=2,p=2$NEF0S1JSN04yNGQ1UVRKdA$KYGNI9CbjoKh1UEu1PpdlqbuLbveGwkMcwcT2Un9pPM', false), - array('passwords.driver.argon2i', '$argon2i$v=19$m=128,t=2,p=2$M29GUi51QjdKLjIzbC9scQ$6h1gZDqn7JTmVdQ0lJh1x5nyvgO/DaJWUKOFJ0itCJ0', true), - array('passwords.driver.argon2i', '$argon2i$v=19$m=1024,t=1,p=2$UnFHb2F4NER3M0xWWmxMUQ$u3javvoAZJeIyR1P3eg0tb8VjEeXvQPagqwetonq1NA', true), - array('passwords.driver.argon2i', '$argon2i$v=19$m=1024,t=2,p=1$bm5SeGJ3R3ZRY1A0YXJPNg$v1A9m4sJW+ge0RBtpJ4w9861+J9xkguKBAsZHrG8LQU', true), - array('passwords.driver.argon2id', '$argon2id$v=19$m=1024,t=2,p=2$MXB4OW5sczE5TnFPYkEuYQ$2bxaMIp8+9x37O6v8zkqpBU72ohCibUrtgVZw7vyr5Q', false), - array('passwords.driver.argon2id', '$argon2id$v=19$m=128,t=2,p=2$RWV2VFAuWXk5bTVjbktOLg$Nt7Z7koa25SVRSKr3RKqjwKz26FENDuU+aL1DfMcWRo', true), - array('passwords.driver.argon2id', '$argon2id$v=19$m=1024,t=1,p=2$Rmw5M21IUFZDVEltYU0uTA$GIObGbHV6sOw5OQEtF8z+2ESztT96OWhCk17sUlwLAY', true), - ); + ]; + + if (isset($this->passwords_drivers['passwords.driver.argon2i'])) + { + $data_array = array_merge($data_array, [ + array('passwords.driver.argon2i', '$argon2i$v=19$m=' . $this->argon2_default_cost_options['memory_cost'] . ',t=' . $this->argon2_default_cost_options['time_cost'] . ',p=' . $this->argon2_default_cost_options['threads'] . '$NEF0S1JSN04yNGQ1UVRKdA$KYGNI9CbjoKh1UEu1PpdlqbuLbveGwkMcwcT2Un9pPM', false), + array('passwords.driver.argon2i', '$argon2i$v=19$m=128,t=2,p=2$M29GUi51QjdKLjIzbC9scQ$6h1gZDqn7JTmVdQ0lJh1x5nyvgO/DaJWUKOFJ0itCJ0', true), + array('passwords.driver.argon2i', '$argon2i$v=19$m=1024,t=1,p=2$UnFHb2F4NER3M0xWWmxMUQ$u3javvoAZJeIyR1P3eg0tb8VjEeXvQPagqwetonq1NA', true), + array('passwords.driver.argon2i', '$argon2i$v=19$m=1024,t=2,p=1$bm5SeGJ3R3ZRY1A0YXJPNg$v1A9m4sJW+ge0RBtpJ4w9861+J9xkguKBAsZHrG8LQU', true), + ]); + } + + if (isset($this->passwords_drivers['passwords.driver.argon2id'])) + { + $data_array = array_merge($data_array, [ + array('passwords.driver.argon2id', '$argon2id$v=19$m=' . $this->argon2_default_cost_options['memory_cost'] . ',t=' . $this->argon2_default_cost_options['time_cost'] . ',p=' . $this->argon2_default_cost_options['threads'] . '$MXB4OW5sczE5TnFPYkEuYQ$2bxaMIp8+9x37O6v8zkqpBU72ohCibUrtgVZw7vyr5Q', false), + array('passwords.driver.argon2id', '$argon2id$v=19$m=128,t=2,p=2$RWV2VFAuWXk5bTVjbktOLg$Nt7Z7koa25SVRSKr3RKqjwKz26FENDuU+aL1DfMcWRo', true), + array('passwords.driver.argon2id', '$argon2id$v=19$m=1024,t=1,p=2$Rmw5M21IUFZDVEltYU0uTA$GIObGbHV6sOw5OQEtF8z+2ESztT96OWhCk17sUlwLAY', true), + ]); + } + + return $data_array; } /** -- cgit v1.2.1 From c71d4c364adc27dfecd60b47857968f1050f6df6 Mon Sep 17 00:00:00 2001 From: rxu Date: Sun, 29 Dec 2019 16:09:17 +0700 Subject: [ticket/16266] Prevent "Use of undefined constant" warning in tests PHPBB3-16266 --- phpBB/phpbb/passwords/driver/argon2i.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/phpBB/phpbb/passwords/driver/argon2i.php b/phpBB/phpbb/passwords/driver/argon2i.php index f622ad889b..bf4d6ec33a 100644 --- a/phpBB/phpbb/passwords/driver/argon2i.php +++ b/phpBB/phpbb/passwords/driver/argon2i.php @@ -37,6 +37,14 @@ class argon2i extends base_native { parent::__construct($config, $helper); + // Workaround to prevent "Use of undefined constant" warning on some unsupported PHP installations + if (!defined('PASSWORD_ARGON2I')) + { + define('PASSWORD_ARGON2_DEFAULT_MEMORY_COST', 1024); + define('PASSWORD_ARGON2_DEFAULT_TIME_COST', 2); + define('PASSWORD_ARGON2_DEFAULT_THREADS', 1); + } + /** * For Sodium implementation of argon2 algorithm (since PHP 7.4), set special value of 1 for "threads" cost factor * See https://wiki.php.net/rfc/sodium.argon.hash and PHPBB3-16266 -- cgit v1.2.1