diff --git a/application/Espo/Core/Utils/PasswordHash.php b/application/Espo/Core/Utils/PasswordHash.php index 0996d66075..d127161cbd 100644 --- a/application/Espo/Core/Utils/PasswordHash.php +++ b/application/Espo/Core/Utils/PasswordHash.php @@ -32,10 +32,12 @@ namespace Espo\Core\Utils; use RuntimeException; use SensitiveParameter; +use const PASSWORD_BCRYPT; + class PasswordHash { /** - * SHA-512 salt format. + * Legacy. */ private string $saltFormat = '$6${0}$'; @@ -47,26 +49,45 @@ class PasswordHash */ public function hash(#[SensitiveParameter] string $password): string { - $salt = $this->getSalt(); - - $password = md5($password); - - $hash = crypt($password, $salt); - - return str_replace($salt, '', $hash); + return password_hash($password, PASSWORD_BCRYPT); } + /** + * Verify a password against a hash. + */ public function verify( #[SensitiveParameter] string $password, #[SensitiveParameter] string $hash ): bool { - return $this->hash($password) === $hash; + if (password_verify($password, $hash)) { + return true; + } + + return $this->legacyVerify($password, $hash); + } + + private function legacyVerify( + #[SensitiveParameter] string $password, + #[SensitiveParameter] string $hash + ): bool { + + if (!$this->config->get('passwordSalt')) { + return false; + } + + return $this->legacyHash($password) === $hash; + } + + private function legacyHash(#[SensitiveParameter] string $password): string + { + $salt = $this->getSalt(); + + $hash = crypt(md5($password), $salt); + + return str_replace($salt, '', $hash); } - /** - * Get a salt from the config and normalize it. - */ private function getSalt(): string { $salt = $this->config->get('passwordSalt'); @@ -78,19 +99,8 @@ class PasswordHash return $this->normalizeSalt($salt); } - /** - * Convert salt in format in accordance to $saltFormat. - */ private function normalizeSalt(string $salt): string { return str_replace("{0}", $salt, $this->saltFormat); } - - /** - * Generate a new salt. - */ - public function generateSalt(): string - { - return substr(md5(uniqid()), 0, 16); - } } diff --git a/install/core/Installer.php b/install/core/Installer.php index e866c095e4..ab2e547428 100644 --- a/install/core/Installer.php +++ b/install/core/Installer.php @@ -394,7 +394,6 @@ class Installer 'database' => array_merge($databaseDefaults, $saveData['database']), 'language' => $saveData['language'] ?? 'en_US', 'siteUrl' => $siteUrl, - 'passwordSalt' => $this->getPasswordHash()->generateSalt(), 'cryptKey' => Util::generateSecretKey(), 'hashSecretKey' => Util::generateSecretKey(), 'theme' => $saveData['theme'] ?? 'Light', diff --git a/tests/unit/Espo/Core/Utils/PasswordHashTest.php b/tests/unit/Espo/Core/Utils/PasswordHashTest.php deleted file mode 100644 index 9421cb6a09..0000000000 --- a/tests/unit/Espo/Core/Utils/PasswordHashTest.php +++ /dev/null @@ -1,103 +0,0 @@ -. - * - * The interactive user interfaces in modified source and object code versions - * of this program must display Appropriate Legal Notices, as required under - * Section 5 of the GNU Affero General Public License version 3. - * - * In accordance with Section 7(b) of the GNU Affero General Public License version 3, - * these Appropriate Legal Notices must retain the display of the "EspoCRM" word. - ************************************************************************/ - -namespace tests\unit\Espo\Core\Utils; - -use tests\unit\ReflectionHelper; - -class PasswordHashTest extends \PHPUnit\Framework\TestCase -{ - protected $object; - - protected $objects; - - protected $reflection; - - protected $salt = 'bdaff81c7b8db54d'; - - protected function setUp() : void - { - $this->objects['config'] = $this->getMockBuilder('\Espo\Core\Utils\Config')->disableOriginalConstructor()->getMock(); - - $this->object = new \Espo\Core\Utils\PasswordHash($this->objects['config']); - - $this->reflection = new ReflectionHelper($this->object); - } - - protected function tearDown() : void - { - $this->object = NULL; - } - - public function testGenerateSalt() - { - $salt = $this->object->generateSalt(); - - $this->assertEquals(16, strlen($salt)); - } - - public function testNormalizeSalt() - { - $salt = $this->object->generateSalt(); - - $result = '$6$' . $salt . '$'; - $this->assertEquals($result, $this->reflection->invokeMethod('normalizeSalt', array($salt))); - } - - public function testGetSalt() - { - $this->objects['config'] - ->expects($this->once()) - ->method('get') - ->will($this->returnValue($this->salt)); - - $result = '$6$' . $this->salt . '$'; - $this->assertEquals($result, $this->reflection->invokeMethod('getSalt')); - } - - public function testGetSaltException() - { - $this->expectException(\RuntimeException::class); - - $this->reflection->invokeMethod('getSalt'); - } - - public function testHash() - { - $password = 'test-password'; - - $this->objects['config'] - ->expects($this->once()) - ->method('get') - ->will($this->returnValue($this->salt)); - - $result = '4gDlJKdkj/MMo2axSwvvWUv0ktSUeGpis/wLcpL8aEBUxXTVa.rxFb1cfKzTiSE4ookBdNpLMheJmtZqzDSRA0'; - $this->assertEquals($result, $this->object->hash($password)); - } -}