diff --git a/lib/private/Security/Hasher.php b/lib/private/Security/Hasher.php index 882f80ea2b..21271fffbd 100644 --- a/lib/private/Security/Hasher.php +++ b/lib/private/Security/Hasher.php @@ -94,6 +94,10 @@ class Hasher implements IHasher { public function hash(string $message): string { $alg = $this->getPrefferedAlgorithm(); + if (\defined('PASSWORD_ARGON2ID') && $alg === PASSWORD_ARGON2ID) { + return 3 . '|' . password_hash($message, PASSWORD_ARGON2ID, $this->options); + } + if (\defined('PASSWORD_ARGON2I') && $alg === PASSWORD_ARGON2I) { return 2 . '|' . password_hash($message, PASSWORD_ARGON2I, $this->options); } @@ -142,12 +146,14 @@ class Hasher implements IHasher { /** * Verify V1 (blowfish) hashes + * Verify V2 (argon2i) hashes + * Verify V3 (argon2id) hashes * @param string $message Message to verify * @param string $hash Assumed hash of the message * @param null|string &$newHash Reference will contain the updated hash if necessary. Update the existing hash with this one. * @return bool Whether $hash is a valid hash of $message */ - protected function verifyHashV1(string $message, string $hash, &$newHash = null): bool { + protected function verifyHash(string $message, string $hash, &$newHash = null): bool { if(password_verify($message, $hash)) { if ($this->needsRehash($hash)) { $newHash = $this->hash($message); @@ -158,24 +164,6 @@ class Hasher implements IHasher { return false; } - /** - * Verify V2 (argon2i) hashes - * @param string $message Message to verify - * @param string $hash Assumed hash of the message - * @param null|string &$newHash Reference will contain the updated hash if necessary. Update the existing hash with this one. - * @return bool Whether $hash is a valid hash of $message - */ - protected function verifyHashV2(string $message, string $hash, &$newHash = null) : bool { - if(password_verify($message, $hash)) { - if($this->needsRehash($hash)) { - $newHash = $this->hash($message); - } - return true; - } - - return false; - } - /** * @param string $message Message to verify * @param string $hash Assumed hash of the message @@ -187,10 +175,10 @@ class Hasher implements IHasher { if(isset($splittedHash['version'])) { switch ($splittedHash['version']) { + case 3: case 2: - return $this->verifyHashV2($message, $splittedHash['hash'], $newHash); case 1: - return $this->verifyHashV1($message, $splittedHash['hash'], $newHash); + return $this->verifyHash($message, $splittedHash['hash'], $newHash); } } else { return $this->legacyHashVerify($message, $hash, $newHash); @@ -211,6 +199,10 @@ class Hasher implements IHasher { $default = PASSWORD_ARGON2I; } + if (\defined('PASSWORD_ARGON2ID')) { + $default = PASSWORD_ARGON2ID; + } + // Check if we should use PASSWORD_DEFAULT if ($this->config->getSystemValue('hashing_default_password', false) === true) { $default = PASSWORD_DEFAULT; diff --git a/tests/lib/Security/HasherTest.php b/tests/lib/Security/HasherTest.php index e680efb19b..76f880c1c1 100644 --- a/tests/lib/Security/HasherTest.php +++ b/tests/lib/Security/HasherTest.php @@ -30,10 +30,7 @@ class HasherTest extends \Test\TestCase { ]; } - /** - * @return array - */ - public function hashProviders70_71() + public function hashProviders70_71(): array { return [ // Valid SHA1 strings @@ -70,11 +67,7 @@ class HasherTest extends \Test\TestCase { ]; } - - /** - * @return array - */ - public function hashProviders72() { + public function hashProviders72(): array { return [ // Valid ARGON2 hashes ['password', '2|$argon2i$v=19$m=1024,t=2,p=2$T3JGcEkxVFNOVktNSjZUcg$4/hyLtSejxNgAuzSFFV/HLM3qRQKBwEtKw61qPN4zWA', true], @@ -91,6 +84,26 @@ class HasherTest extends \Test\TestCase { ]; } + public function hashProviders73(): array { + return [ + // Valid ARGON2ID hashes + ['password', '2|$argon2id$v=19$m=65536,t=4,p=1$TEtIMnhUczliQzI0Y01WeA$BpMUDrApy25iagIogUAnlc0rNTPJmGs8lOEeVHujJ9Q', true], + ['password', '2|$argon2id$v=19$m=65536,t=4,p=1$RzdUdDNvbHhZalVQa2VIcQ$Wo8CGasVCBcSe69ldPdoVKTWEDQkET2cgQJSUiKcIzs', true], + ['password', '2|$argon2id$v=19$m=65536,t=4,p=1$djlDMTVkL3VnMlNZNWZPeg$PCMpdAjB+OtwGpM75IGWmYHh1h2I7l5P8YabYtKubWg', true], + ['nextcloud.com', '2|$argon2id$v=19$m=65536,t=4,p=1$VGhGL05rcUI3d3k3WVhibQ$CSy0ShUnamZQhu8oeZfUTTd/S3z966zuQ/uz1Y80Rss', true], + ['nextcloud.com', '2|$argon2id$v=19$m=65536,t=4,p=1$ZVlZTVlCaTZhRlZHOGFpYQ$xd1TtMz1Mi0SuZrP+VWB3v/hwoC7HfSVsUYmzOo2DUU', true], + ['nextcloud.com', '2|$argon2id$v=19$m=65536,t=4,p=1$OG1wZUtzZ0tnLjF2MUZVMA$CBluq8W8ISmZ9QumeWsVhaVREP0Zcq8rwk2NrA9d4YE', true], + + //Invalid ARGON2ID hashes + ['password', '2|$argon2id$v=19$m=65536,t=4,p=1$V3ovTHlvc0Eyb24xenVRNQ$iY/A0Yf24c2DToedj2rj9+KeoJBGsJYQOlJMoa0SFXk', false], + ['password', '2|$argon2id$v=19$m=65536,t=4,p=1$NlYuMlQ0ODIudTRkZDhYUw$/Z71ckOIuydujedUGK73iXC9vbLzlH/iXkG9+gGgn+c', false], + ['password', '2|$argon2id$v=19$m=65536,t=4,p=1$b09kNFZTZWFjS05aTkl6ZA$llE4TnIYYrC0H7wkTL1JsIwAAgoMJERlqtFcHHQcXTs', false], + + ]; + } + + + /** @var Hasher */ protected $hasher; @@ -149,7 +162,19 @@ class HasherTest extends \Test\TestCase { $this->assertSame($expected, $result); } - public function testUpgradeHashBlowFishToArgon2i() { + /** + * @dataProvider hashProviders73 + */ + public function testVerifyArgon2id(string $password, string $hash, bool $expected) { + if (!\defined('PASSWORD_ARGON2ID')) { + $this->markTestSkipped('Need ARGON2ID support to test ARGON2ID hashes'); + } + + $result = $this->hasher->verify($password, $hash); + $this->assertSame($expected, $result); + } + + public function testUpgradeHashBlowFishToArgon2() { if (!\defined('PASSWORD_ARGON2I')) { $this->markTestSkipped('Need ARGON2 support to test ARGON2 hashes'); } @@ -157,14 +182,21 @@ class HasherTest extends \Test\TestCase { $message = 'mysecret'; $blowfish = 1 . '|' . password_hash($message, PASSWORD_BCRYPT, []); - $argon2i = 2 . '|' . password_hash($message, PASSWORD_ARGON2I, []); + $argon2 = 2 . '|' . password_hash($message, PASSWORD_ARGON2I, []); + + $newAlg = PASSWORD_ARGON2I; + if (\defined('PASSWORD_ARGON2ID')) { + $newAlg = PASSWORD_ARGON2ID; + $argon2 = 2 . '|' . password_hash($message, PASSWORD_ARGON2ID, []); + } + $this->assertTrue($this->hasher->verify($message, $blowfish,$newHash)); - $this->assertTrue($this->hasher->verify($message, $argon2i)); + $this->assertTrue($this->hasher->verify($message, $argon2)); $relativePath = self::invokePrivate($this->hasher, 'splitHash', [$newHash]); - $this->assertFalse(password_needs_rehash($relativePath['hash'], PASSWORD_ARGON2I, [])); + $this->assertFalse(password_needs_rehash($relativePath['hash'], $newAlg, [])); } public function testUsePasswordDefaultArgon2iVerify() { @@ -183,11 +215,17 @@ class HasherTest extends \Test\TestCase { $newHash = null; $this->assertTrue($this->hasher->verify($message, $argon2i, $newHash)); $this->assertNotNull($newHash); + + $relativePath = self::invokePrivate($this->hasher, 'splitHash', [$newHash]); + $this->assertEquals(1, $relativePath['version']); + $this->assertEquals(PASSWORD_BCRYPT, password_get_info($relativePath['hash'])['algo']); + $this->assertFalse(password_needs_rehash($relativePath['hash'], PASSWORD_BCRYPT)); + $this->assertTrue(password_verify($message, $relativePath['hash'])); } - public function testDoNotUserPasswordDefaultArgon2iVerify() { - if (!\defined('PASSWORD_ARGON2I')) { - $this->markTestSkipped('Need ARGON2 support to test ARGON2 hashes'); + public function testDoNotUsePasswordDefaultArgon2idVerify() { + if (!\defined('PASSWORD_ARGON2ID')) { + $this->markTestSkipped('Need ARGON2ID support to test ARGON2ID hashes'); } $this->config->method('getSystemValue') @@ -196,10 +234,10 @@ class HasherTest extends \Test\TestCase { $message = 'mysecret'; - $argon2i = 2 . '|' . password_hash($message, PASSWORD_ARGON2I, []); + $argon2id = 3 . '|' . password_hash($message, PASSWORD_ARGON2ID, []); $newHash = null; - $this->assertTrue($this->hasher->verify($message, $argon2i, $newHash)); + $this->assertTrue($this->hasher->verify($message, $argon2id, $newHash)); $this->assertNull($newHash); }