From 0d651f106c3bd317835c15cc82f3689d71432d48 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 29 Jan 2020 21:39:58 +0100 Subject: [PATCH] Allow selecting the hashing algorithm Signed-off-by: Roeland Jago Douma --- config/config.sample.php | 10 +++++ lib/private/Security/Hasher.php | 38 ++++++++++++------ tests/lib/Security/HasherTest.php | 65 ++++++++++++++++++++++++++++++- 3 files changed, 100 insertions(+), 13 deletions(-) diff --git a/config/config.sample.php b/config/config.sample.php index e1c6208710..2894bc5deb 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -1435,6 +1435,16 @@ $CONFIG = array( /** * Hashing + */ + +/** + * By default Nextcloud will use the Argon2 password hashing if available. + * However if for whatever reason you want to stick with the PASSWORD_DEFAULT + * of your php version. Then set the setting to true. + */ +'hashing_default_password' => false, + +/** * * Nextcloud uses the Argon2 algorithm (with PHP >= 7.2) to create hashes by its * own and exposes its configuration options as following. More information can diff --git a/lib/private/Security/Hasher.php b/lib/private/Security/Hasher.php index 5f8529c782..882f80ea2b 100644 --- a/lib/private/Security/Hasher.php +++ b/lib/private/Security/Hasher.php @@ -92,11 +92,13 @@ class Hasher implements IHasher { * @return string Hash of the message with appended version parameter */ public function hash(string $message): string { - if (\defined('PASSWORD_ARGON2I')) { + $alg = $this->getPrefferedAlgorithm(); + + if (\defined('PASSWORD_ARGON2I') && $alg === PASSWORD_ARGON2I) { return 2 . '|' . password_hash($message, PASSWORD_ARGON2I, $this->options); - } else { - return 1 . '|' . password_hash($message, PASSWORD_BCRYPT, $this->options); } + + return 1 . '|' . password_hash($message, PASSWORD_BCRYPT, $this->options); } /** @@ -147,12 +149,7 @@ class Hasher implements IHasher { */ protected function verifyHashV1(string $message, string $hash, &$newHash = null): bool { if(password_verify($message, $hash)) { - $algo = PASSWORD_BCRYPT; - if (\defined('PASSWORD_ARGON2I')) { - $algo = PASSWORD_ARGON2I; - } - - if(password_needs_rehash($hash, $algo, $this->options)) { + if ($this->needsRehash($hash)) { $newHash = $this->hash($message); } return true; @@ -170,7 +167,7 @@ class Hasher implements IHasher { */ protected function verifyHashV2(string $message, string $hash, &$newHash = null) : bool { if(password_verify($message, $hash)) { - if(password_needs_rehash($hash, PASSWORD_ARGON2I, $this->options)) { + if($this->needsRehash($hash)) { $newHash = $this->hash($message); } return true; @@ -199,8 +196,27 @@ class Hasher implements IHasher { return $this->legacyHashVerify($message, $hash, $newHash); } - return false; } + private function needsRehash(string $hash): bool { + $algorithm = $this->getPrefferedAlgorithm(); + + return password_needs_rehash($hash, $algorithm, $this->options); + } + + private function getPrefferedAlgorithm() { + $default = PASSWORD_BCRYPT; + if (\defined('PASSWORD_ARGON2I')) { + $default = PASSWORD_ARGON2I; + } + + // Check if we should use PASSWORD_DEFAULT + if ($this->config->getSystemValue('hashing_default_password', false) === true) { + $default = PASSWORD_DEFAULT; + } + + return $default; + } + } diff --git a/tests/lib/Security/HasherTest.php b/tests/lib/Security/HasherTest.php index 3222b5d098..e680efb19b 100644 --- a/tests/lib/Security/HasherTest.php +++ b/tests/lib/Security/HasherTest.php @@ -126,8 +126,12 @@ class HasherTest extends \Test\TestCase { $this->config ->expects($this->any()) ->method('getSystemValue') - ->with('passwordsalt', null) - ->will($this->returnValue('6Wow67q1wZQZpUUeI6G2LsWUu4XKx')); + ->willReturnCallback(function ($key, $default) { + if($key === 'passwordsalt') { + return '6Wow67q1wZQZpUUeI6G2LsWUu4XKx'; + } + return $default; + }); $result = $this->hasher->verify($password, $hash); $this->assertSame($expected, $result); @@ -162,4 +166,61 @@ class HasherTest extends \Test\TestCase { $this->assertFalse(password_needs_rehash($relativePath['hash'], PASSWORD_ARGON2I, [])); } + + public function testUsePasswordDefaultArgon2iVerify() { + if (!\defined('PASSWORD_ARGON2I')) { + $this->markTestSkipped('Need ARGON2 support to test ARGON2 hashes'); + } + + $this->config->method('getSystemValue') + ->with('hashing_default_password') + ->willReturn(true); + + $message = 'mysecret'; + + $argon2i = 2 . '|' . password_hash($message, PASSWORD_ARGON2I, []); + + $newHash = null; + $this->assertTrue($this->hasher->verify($message, $argon2i, $newHash)); + $this->assertNotNull($newHash); + } + + public function testDoNotUserPasswordDefaultArgon2iVerify() { + if (!\defined('PASSWORD_ARGON2I')) { + $this->markTestSkipped('Need ARGON2 support to test ARGON2 hashes'); + } + + $this->config->method('getSystemValue') + ->with('hashing_default_password') + ->willReturn(false); + + $message = 'mysecret'; + + $argon2i = 2 . '|' . password_hash($message, PASSWORD_ARGON2I, []); + + $newHash = null; + $this->assertTrue($this->hasher->verify($message, $argon2i, $newHash)); + $this->assertNull($newHash); + } + + public function testHashUsePasswordDefault() { + if (!\defined('PASSWORD_ARGON2I')) { + $this->markTestSkipped('Need ARGON2 support to test ARGON2 hashes'); + } + + $this->config->method('getSystemValue') + ->with('hashing_default_password') + ->willReturn(true); + + $message = 'mysecret'; + + $hash = $this->hasher->hash($message); + $relativePath = self::invokePrivate($this->hasher, 'splitHash', [$hash]); + + $this->assertSame(1, $relativePath['version']); + + $info = password_get_info($relativePath['hash']); + $this->assertEquals(PASSWORD_BCRYPT, $info['algo']); + + } }