aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--phpBB/config/default/container/services_password.yml5
-rw-r--r--phpBB/phpbb/passwords/driver/base.php12
-rw-r--r--phpBB/phpbb/passwords/driver/bcrypt.php32
-rw-r--r--phpBB/phpbb/passwords/driver/rehashable_driver_interface.php25
-rw-r--r--phpBB/phpbb/passwords/manager.php11
-rw-r--r--tests/passwords/drivers_test.php23
-rw-r--r--tests/passwords/manager_test.php54
7 files changed, 153 insertions, 9 deletions
diff --git a/phpBB/config/default/container/services_password.yml b/phpBB/config/default/container/services_password.yml
index 82850dc2a7..c276365c56 100644
--- a/phpBB/config/default/container/services_password.yml
+++ b/phpBB/config/default/container/services_password.yml
@@ -1,3 +1,6 @@
+parameters:
+ passwords.driver.bcrypt_cost: 10
+
services:
# ----- Password management -----
passwords.manager:
@@ -29,6 +32,7 @@ services:
arguments:
- '@config'
- '@passwords.driver_helper'
+ - '%passwords.driver.bcrypt_cost%'
tags:
- { name: passwords.driver }
@@ -37,6 +41,7 @@ services:
arguments:
- '@config'
- '@passwords.driver_helper'
+ - '%passwords.driver.bcrypt_cost%'
tags:
- { name: passwords.driver }
diff --git a/phpBB/phpbb/passwords/driver/base.php b/phpBB/phpbb/passwords/driver/base.php
index fd07a61bf4..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;
/**
@@ -53,6 +53,14 @@ abstract class base implements driver_interface
}
/**
+ * {@inheritdoc}
+ */
+ public function needs_rehash($hash)
+ {
+ return false;
+ }
+
+ /**
* {@inheritdoc}
*/
public function get_settings_only($hash, $full = false)
diff --git a/phpBB/phpbb/passwords/driver/bcrypt.php b/phpBB/phpbb/passwords/driver/bcrypt.php
index eab1c3d569..eb1aeeeb76 100644
--- a/phpBB/phpbb/passwords/driver/bcrypt.php
+++ b/phpBB/phpbb/passwords/driver/bcrypt.php
@@ -17,6 +17,24 @@ 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
+ * @param int $cost_factor Hashing cost factor (optional)
+ */
+ public function __construct(\phpbb\config\config $config, helper $helper, $cost_factor = 10)
+ {
+ parent::__construct($config, $helper);
+
+ // Don't allow cost factor to be below default setting
+ $this->cost_factor = max(10, $cost_factor);
+ }
+
/**
* {@inheritdoc}
*/
@@ -26,6 +44,18 @@ class bcrypt extends base
}
/**
+ * {@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}
*/
public function hash($password, $salt = '')
@@ -46,7 +76,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/rehashable_driver_interface.php b/phpBB/phpbb/passwords/driver/rehashable_driver_interface.php
new file mode 100644
index 0000000000..ca30748502
--- /dev/null
+++ b/phpBB/phpbb/passwords/driver/rehashable_driver_interface.php
@@ -0,0 +1,25 @@
+<?php
+/**
+*
+* This file is part of the phpBB Forum Software package.
+*
+* @copyright (c) phpBB Limited <https://www.phpbb.com>
+* @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 extends driver_interface
+{
+ /**
+ * 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);
+}
diff --git a/phpBB/phpbb/passwords/manager.php b/phpBB/phpbb/passwords/manager.php
index b2caba81f2..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();
@@ -297,7 +297,14 @@ class manager
}
else
{
- $this->convert_flag = false;
+ 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$
diff --git a/tests/passwords/drivers_test.php b/tests/passwords/drivers_test.php
index 5f9fd523c9..01c69a38bb 100644
--- a/tests/passwords/drivers_test.php
+++ b/tests/passwords/drivers_test.php
@@ -23,8 +23,8 @@ class phpbb_passwords_helper_test extends \phpbb_test_case
$php_ext = 'php';
$this->passwords_drivers = array(
- 'passwords.driver.bcrypt_2y' => new \phpbb\passwords\driver\bcrypt_2y($config, $this->driver_helper),
- 'passwords.driver.bcrypt' => new \phpbb\passwords\driver\bcrypt($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),
'passwords.driver.phpass' => new \phpbb\passwords\driver\phpass($config, $this->driver_helper),
'passwords.driver.sha1_smf' => new \phpbb\passwords\driver\sha1_smf($config, $this->driver_helper),
@@ -413,4 +413,23 @@ class phpbb_passwords_helper_test extends \phpbb_test_case
);
return strtr($string, $transform);
}
+
+ public function data_needs_rehash()
+ {
+ return 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),
+ );
+ }
+
+ /**
+ * @dataProvider data_needs_rehash
+ */
+ public function test_needs_rehash($driver, $hash, $expected)
+ {
+ $this->assertSame($this->passwords_drivers[$driver]->needs_rehash($hash), $expected);
+ }
}
diff --git a/tests/passwords/manager_test.php b/tests/passwords/manager_test.php
index 333834ee07..0410d7035f 100644
--- a/tests/passwords/manager_test.php
+++ b/tests/passwords/manager_test.php
@@ -29,8 +29,8 @@ class phpbb_passwords_manager_test extends \phpbb_test_case
$php_ext = 'php';
$this->passwords_drivers = array(
- 'passwords.driver.bcrypt_2y' => new \phpbb\passwords\driver\bcrypt_2y($config, $this->driver_helper),
- 'passwords.driver.bcrypt' => new \phpbb\passwords\driver\bcrypt($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),
'passwords.driver.phpass' => new \phpbb\passwords\driver\phpass($config, $this->driver_helper),
'passwords.driver.convert_password' => new \phpbb\passwords\driver\convert_password($config, $this->driver_helper),
@@ -344,4 +344,54 @@ class phpbb_passwords_manager_test extends \phpbb_test_case
{
$this->assertSame($expected, $this->driver_helper->string_compare($a, $b));
}
+
+ public function data_driver_interface_driver()
+ {
+ return array(
+ array(false, false, false),
+ array(true, false, false),
+ array(true, true, true),
+ );
+ }
+
+ /**
+ * @dataProvider data_driver_interface_driver
+ */
+ public function test_driver_interface_driver($use_new_interface, $needs_rehash, $expected)
+ {
+ if ($use_new_interface)
+ {
+ $test_driver = $this->getMock('\phpbb\passwords\driver\rehashable_driver_interface', array('needs_rehash', 'get_prefix', 'check', 'is_supported', 'is_legacy', 'hash', 'get_settings_only'));
+ $test_driver->method('needs_rehash')
+ ->willReturn($needs_rehash);
+ }
+ else
+ {
+ $test_driver = $this->getMock('\phpbb\passwords\driver\driver_interface', array('get_prefix', 'check', 'is_supported', 'is_legacy', 'hash', 'get_settings_only'));
+ }
+ $config = new \phpbb\config\config(array());
+
+ $test_driver->method('is_supported')
+ ->willReturn(true);
+ $test_driver->method('get_prefix')
+ ->willReturn('$test$');
+ $test_driver->method('check')
+ ->with($this->anything())
+ ->willReturn(true);
+ $passwords_drivers = array(
+ 'passwords.driver.foobar' => $test_driver,
+ 'passwords.driver.bcrypt_2y' => new \phpbb\passwords\driver\bcrypt_2y($config, $this->driver_helper, 10),
+ );
+ // Set up another manager
+ $foobar_manager = new \phpbb\passwords\manager($config, $passwords_drivers, $this->helper, array('passwords.driver.foobar'));
+
+ $this->assertTrue($foobar_manager->check('foobar', '$test$somerandomstuff'));
+ $this->assertEquals($expected, $foobar_manager->convert_flag);
+
+ // Should always return true in case a different driver is default
+ $foobar_manager = new \phpbb\passwords\manager($config, $passwords_drivers, $this->helper, array('passwords.driver.bcrypt_2y', 'passwords.driver.foobar'));
+
+ $this->assertTrue($foobar_manager->check('foobar', '$test$somerandomstuff'));
+ $this->assertTrue($foobar_manager->convert_flag);
+ }
}