Merge pull request #19307 from nextcloud/enh/argon2id

Add Argon2id support

2 files changed, 69 insertions, 39 deletions

diff --git a/lib/private/Security/Hasher.php b/lib/private/Security/Hasher.php
index 882f80ea2bf..21271fffbd7 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,32 +146,16 @@ class Hasher implements IHasher {
 
 	/**
 	 * Verify V1 (blowfish) 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 {
-		if(password_verify($message, $hash)) {
-			if ($this->needsRehash($hash)) {
-				$newHash = $this->hash($message);
-			}
-			return true;
-		}
-
-		return false;
-	}
-
-	/**
 	 * 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 verifyHashV2(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)) {
+			if ($this->needsRehash($hash)) {
 				$newHash = $this->hash($message);
 			}
 			return true;
@@ -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 e680efb19b6..76f880c1c12 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);
 	}